[PATCH] ASoC: soc-dai: pull out be_hw_params_fixup from snd_soc_dai_hw_params

[Gyeongtaek Lee]

On 5/12/20 03:47 AM, Pierre-Louis Bossart wrote:
>On 5/10/20 10:31 PM, Gyeongtaek Lee wrote:
>> When dpcm_be_dai_hw_params() called, be_hw_params_fixup() callback is
>> called with same arguments 3times.
>> It is called in be_hw_params_fixup() itself and in soc_pcm_hw_params()
>> for cpu dai and codec dai.
>> Tested in 5.4.
>> 
>> Signed-off-by: Gyeongtaek Lee <gt82.lee at samsung.com>
>> Cc: stable at vger.kernel.org
>> ---
>>   sound/soc/soc-dai.c  | 12 ------------
>>   sound/soc/soc-dapm.c | 11 +++++++++++
>>   2 files changed, 11 insertions(+), 12 deletions(-)
>> 
>> diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
>> index 31c41559034b..4785cb6b336f 100644
>> --- a/sound/soc/soc-dai.c
>> +++ b/sound/soc/soc-dai.c
>> @@ -257,20 +257,8 @@ int snd_soc_dai_hw_params(struct snd_soc_dai *dai,
>>   			  struct snd_pcm_substream *substream,
>>   			  struct snd_pcm_hw_params *params)
>>   {
>> -	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>   	int ret;
>>   
>> -	/* perform any topology hw_params fixups before DAI  */
>> -	if (rtd->dai_link->be_hw_params_fixup) {
>> -		ret = rtd->dai_link->be_hw_params_fixup(rtd, params);
>> -		if (ret < 0) {
>> -			dev_err(rtd->dev,
>> -				"ASoC: hw_params topology fixup failed %d\n",
>> -				ret);
>> -			return ret;
>> -		}
>> -	}
>> -
>
>Sorry I don't get this change.
>
>If the be_hw_params_fixup() callback is called three times, it's because 
>the soc_soc_dai_hw_params() routine is called three times, so what is 
>the problem here?
>
>Also the comment is explicit about doing fixups before calling the dai 
>hw_params() callback, so that is not longer the case with this change? 
>Even if the change was legit, the comment is no longer relevant and 
>should be updated.
>
Sorry, the comment was too short and inexact to explain the intention of the patch.
When dpcm_be_dai_hw_params() called, be_hw_params_fixup() is called three times
with same substream and params in dpcm_be_dai_hw_params() and
snd_soc_dai_hw_params() in soc_pcm_hw_params() for cpu dai and codec dai.
Calling same code three times may not be a problem in most systems, but in some
system which has limited computing power and changes audio routing frequently,
couple of milliseconds are consumed and make the system a little bit slower to
audio response.
If the be_hw_params_fixup() could be pull out from soc_soc_dai_hw_params(),
and make it call once at pcm start or routing change, response time can be increased.

In my search, the only point that calls snd_soc_dai_hw_params() without 
calling be_hw_params_fixup() callback directly is snd_soc_dai_link_event_pre_pmu().
So, I proposed pulling out be_hw_params_fixup() from snd_soc_dai_hw_params() and then
add direct call to be_hw_params_fixup() callback in snd_soc_dai_link_event_pre_pmu()
not to harm current working process. 
>>   	if (dai->driver->ops->hw_params) {
>>   		ret = dai->driver->ops->hw_params(substream, params, dai);
>>   		if (ret < 0) {
>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
>> index e2632841b321..d86c1cd4e8fa 100644
>> --- a/sound/soc/soc-dapm.c
>> +++ b/sound/soc/soc-dapm.c
>> @@ -3886,6 +3886,17 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
>>   	hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max
>>   		= config->channels_max;
>>   
>> +	/* perform any topology hw_params fixups before DAI  */
>> +	if (rtd->dai_link->be_hw_params_fixup) {
>> +		ret = rtd->dai_link->be_hw_params_fixup(rtd, params);
>> +		if (ret < 0) {
>> +			dev_err(rtd->dev,
>> +				"ASoC: hw_params topology fixup failed %d\n",
>> +				ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>>   	substream->stream = SNDRV_PCM_STREAM_CAPTURE;
>>   	snd_soc_dapm_widget_for_each_source_path(w, path) {
>>   		source = path->source->priv;
>> 
>> base-commit: f3643491bd079c973ac6c693da7966cd17506ca3
>> 
>