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

[Vinod Koul]

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 at 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 at 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 at 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