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@xxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> ---
>> 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
>>
>
