Re: [RFC PATCH v2] ASoC: soc-pcm: Optimize hw_params() BE DAI call

["Sheetal ." <sheetal@nvidia.com>]

On 23-05-2025 15:25, Pierre-Louis Bossart wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 5/13/25 13:10, Péter Ujfalusi wrote:
>>
>>
>> On 08/04/2025 11:30, Sheetal . wrote:
>>> From: Sheetal <sheetal@nvidia.com>
>>>
>>> The hw_params() function for BE DAI was being called multiple times due
>>> to an unnecessary SND_SOC_DPCM_STATE_HW_PARAMS state check.
>>>
>>> Remove the redundant state check to ensure hw_params() is called only once
>>> per BE DAI configuration.
>>
>> The first sentence tells that the hw_params() of the BE is called
>> multiple times.
>>
>> The second sentence states that the check is redundant then tells that
>> it is removed to not call the hw_params() of the BE, so the check was
>> not redundant, it got exercised.
>>
>> Which one is true?
>>
>> Under what circumstance the __soc_pcm_hw_params() got called multiple
>> times? Was it normal or was it error? What causes it?
> 
> I share Peter's question. The code has been around since 2016, in what case would the existing logic need to be updated?

As such it is not causing any issue. But I think if the BE DAI state is 
already SND_SOC_DPCM_STATE_HW_PARAMS, there is no need to call it again. 
Similar to how dpcm_be_dai_prepare() and dpcm_be_dai_startup() won't 
call BE DAI prepare() / open() callbacks if the state is already 
SND_SOC_DPCM_STATE_PREPARE or SND_SOC_DPCM_STATE_OPEN respectively.

For example:

When two or more streams sharing a common BE DAI are started 
consecutively, the following sequence can cause multiple unnecessary 
hw_params() calls for the BE DAI:
(1) Stream1 hw_params() called for BE DAI and updates state to
    SND_SOC_DPCM_STATE_HW_PARAMS
(2) Before Stream1 BE DAI prepare() call triggered,
    Stream2 dpcm_be_dai_hw_params() call happened and now BE DAI 
hw_params() callback happen again unnecessarily as the state of BE DAI 
is still SND_SOC_DPCM_STATE_HW_PARAMS.


> 
> One key point is that hw_params() may be called multiple times with different parameters, it's a feature and not a bug. If a call to hw_params() changes an internal state, a follow-up call to hw_params() shall undo the initial state change and rerun the appropriate configuration.

As per my understanding, after hw_params() callback, prepare() will be 
called and the BE DAI state will be updated to PREPARE now, then 
hw_params() callback for same BE DAI will not occur as condition will be 
unmet. Not sure how the different parameters will be configured in this 
case? Please clarify.

> 
>>> Signed-off-by: Sheetal <sheetal@nvidia.com>
>>> ---
>>> Changes in v2:
>>> - Update commit message as its not a fix.
>>> - Marked as RFC patch as it requires feedback from other users
>>>    perspective as well.
>>> - The patch is being sent separately as other patch is not RFC.
>>>
>>>   sound/soc/soc-pcm.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
>>> index d7f6d3a6d312..c73be27c4ecb 100644
>>> --- a/sound/soc/soc-pcm.c
>>> +++ b/sound/soc/soc-pcm.c
>>> @@ -2123,7 +2123,6 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
>>>                       continue;
>>>
>>>               if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN) &&
>>> -                (be->dpcm[stream].state != SND_SOC_DPCM_STATE_HW_PARAMS) &&
>>>                   (be->dpcm[stream].state != SND_SOC_DPCM_STATE_HW_FREE))
>>>                       continue;
>>>
>>
>