[alsa-devel] [PATCH v3 1/2] ASoC: SOF: add flag for position update ipc

[Keyon Jie]

On 2019/7/18 上午11:35, Pierre-Louis Bossart wrote:
> 
> 
> On 7/17/19 10:11 PM, Keyon Jie wrote:
>> From: Marcin Rajwa <marcin.rajwa at linux.intel.com>
>>
>> In some cases, FW might need to use the host_period_bytes field to
>> synchronize the DMA copying (with host side) but the driver does not
> 
> it's your right to edit my suggested wording, but the notion of 
> 'synchronization' is far from clear. it's my understanding that the 
> host_period_bytes defines the DMA transfer size requested by the 
> firmware, which isn't a value that matters to the host except for rewind 
> usages.

Hi Pierre, here the host_period_bytes is requested by host, FW has its 
own period size, and DMA will transfer data in FW buffer period size. It 
works like this:

FW buffer[period 0, period 1, ...] <==> DMA <==> host/alsa 
buffer[host_period 0, host_priod 1, ...]

We need this host_preiod_bytes information in FW to do fast 
draining(e.g. record 2 seconds data within 10ms) in mmap capture, we are 
slowing down the draining in smaller host_period_bytes cases, otherwise, 
arecord can't read the buffer in time and overrun will happen.

Maybe the wording "synchronize" here is inaccurate, how about something 
like this:

"FW might need to use the host_period_bytes field to configure and 
control the DMA copying speed but the driver does not..."

> 
>> need any position information returned over the IPC channel by the
>> firmware. The current IPC definition prevents this capability, so add
>> new field.
>>
>> Signed-off-by: Marcin Rajwa <marcin.rajwa at linux.intel.com>
>> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
>> Reviewed-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
>> ---
>>   include/sound/sof/stream.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/sound/sof/stream.h b/include/sound/sof/stream.h
>> index 643f175cb479..06af4ecb2584 100644
>> --- a/include/sound/sof/stream.h
>> +++ b/include/sound/sof/stream.h
>> @@ -83,10 +83,10 @@ struct sof_ipc_stream_params {
>>       uint16_t sample_valid_bytes;
>>       uint16_t sample_container_bytes;
>> -    /* for notifying host period has completed - 0 means no period 
>> IRQ */
>>       uint32_t host_period_bytes;
>> +    uint32_t no_position_ipc; /* 1 means no IPC for position upadte */
> 
> typo: update

OK, thanks, another update version needed for it.

Thanks,
~Keyon

> 
>> -    uint32_t reserved[2];
>> +    uint16_t reserved[3];
>>       uint16_t chmap[SOF_IPC_MAX_CHANNELS];    /**< channel map - 
>> SOF_CHMAP_ */
>>   } __packed;
>>
>