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@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
> > 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
