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

[Keyon Jie]

On 2019/7/17 下午11:48, Pierre-Louis Bossart wrote:
> 
> 
> On 7/3/19 10:10 AM, Keyon Jie wrote:
>> From: Marcin Rajwa <marcin.rajwa at linux.intel.com>
>>
>> In some cases, FW might need use the host_period_bytes even no position
>> update ipc reqiured from driver, here add another flag for position 
>> update,
>> and preserve host_period_bytes for FW to use.
> 
> please fix the commit message, e.g. with the suggested edit below
> 
> In some cases, FW might need to use the host_period_bytes field to fetch 
> data over DMA 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.

Good, thanks for the detail suggestion.

> 
>>
>> This might require corresponding FW change and ABI alignment.
> 
> remove this statement, this is already handled in backwards compatible 
> mode.

OK.

> 
>>
>> Signed-off-by: Marcin Rajwa <marcin.rajwa at linux.intel.com>
>> Signed-off-by: Keyon Jie <yang.jie 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..44acfa62fa69 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;
>> +    uint16_t no_period_irq; /* 1 means period IRQ mode OFF */
> 
> I'd like this field to be renamed as 'no_position_update'. This really 
> has nothing to do with no period_irq in general, even when you do use 
> the no_irq mode you can still retrieve the position information from the 
> HDaudio DMA registers.

Agree, that's actually my original version, will change in next version, 
thanks.

Thanks,
~Keyon

> 
>> -    uint32_t reserved[2];
>> +    uint16_t reserved[3];
>>       uint16_t chmap[SOF_IPC_MAX_CHANNELS];    /**< channel map - 
>> SOF_CHMAP_ */
>>   } __packed;
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>