Re: [alsa-devel] [PATCH][RFC] ASoC: soc-dpm: fixup DAI active unbalance

17 May 2019

On 16-05-19, 22:21, Pierre-Louis Bossart wrote:
...
On 5/16/19 8:21 PM, Kuninori Morimoto wrote:
...
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_dai_link_event() is updating snd_soc_dai :: active,
but it is unbalance.
It counts up if it has startup callback.
case SND_SOC_DAPM_PRE_PMU:
   	...
   	snd_soc_dapm_widget_for_each_source_path(w, path) {
   		...
   		if (source->driver->ops->startup) {
   			...
=>				source->active++;
   		}
   		...
   	}
   	...
But, always counts down
case SND_SOC_DAPM_PRE_PMD:
   	...
   	snd_soc_dapm_widget_for_each_source_path(w, path) {
   		...
=>			source->active--;
   		...
   	}
This patch always counts up when SND_SOC_DAPM_PRE_PMD.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Mark, Liam
I think this is bug, but I can't confirm it,
because my driver need to have .startup.
Thus, I added [RFC] on this patch.
I'm happy if someone can confirm it.
Right, seems a bug to me!
...
This looks like a bug since the initial Intel contribution in 2015.
9b8ef9f6b3fc ('ASoC: dapm: Add startup & shutdown for dai_links') already
has this imbalance.
Well yes this would be seen if someone has a dai_link without the
startup and shutdown calls (which is not mandatory). Agreed that it was
a miss to not put counters in right places.
...
I don't have a clue why this is not symmetric or not done as suggested by
Morimoto-san. Vinod, any idea?
Since while testing we always had the calls and never run in imbalance,
but we should have done better!
I think you should be able to test it on Intel platforms with dai_links
and removing the optional calls.
The change looks fine to be
Reviewed-by: Vinod Koul vkoul@kernel.org
...
...
sound/soc/soc-dapm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c864502..147ad9d 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3828,8 +3828,8 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
   					ret);
   				goto out;
   			}

		source->active++;
}




	source->active++;
ret = soc_dai_hw_params(&substream, params, source);
if (ret < 0)
	goto out;



@@ -3850,8 +3850,8 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
   					ret);
   				goto out;
   			}

		sink->active++;
}




	sink->active++;
ret = soc_dai_hw_params(&substream, params, sink);
if (ret < 0)
	goto out;




-- 
~Vinod

    