Re: [PATCH 02/17] ASoC: use snd_soc_dai/component_activity()

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH 02/17] ASoC: use snd_soc_dai/component_activity()
Date: 12 May 2020 08:53:50 +0900	[thread overview]
Message-ID: <87ftc681v5.wl-kuninori.morimoto.gx@renesas.com> (raw)
In-Reply-To: <d0850429f144f345864776afb59410693abfd33b.camel@linux.intel.com>

Hi Ranjani

Thank you for reviewing

> > -	if (!cpu_dai->active)
> > +	if (!snd_soc_dai_activity(cpu_dai))
> I have a feeling this is probably incorrect. snd_soc_dai_activity()
> checks for stream_active count which is different from snd_soc_dai's
> active member, isnt it?

OK, let me check.
The original code is like this

	static void snd_soc_runtime_action(struct snd_soc_pcm_runtime *rtd,
					   int stream, int action)
	{
		...
		for_each_rtd_dais(rtd, i, dai) {
			dai->stream_active[stream] += action;
			dai->active += action;
			...
		}
	}

	void snd_soc_runtime_activate(...)
	{
		snd_soc_runtime_action(rtd, stream, 1);
	}

	void snd_soc_runtime_deactivate(...)
	{
		snd_soc_runtime_action(rtd, stream, -1);
	}

Basically, ASoC calls
snd_soc_runtime_activate()   for activate count up, and,
snd_soc_runtime_deactivate() for activate count down.

I think snd_soc_dai_activity() is correct, *so far*.

Exceptions are
soc-dapm.c :: snd_soc_dai_link_event_pre_pmu()
soc-dapm.c :: snd_soc_dai_link_event()

	snd_soc_dai_link_event_pre_pmu(...)
	{
		...
		snd_soc_dapm_widget_for_each_source_path(w, path) {
			...
			source->active++;
		}
		...
		snd_soc_dapm_widget_for_each_sink_path(w, path) {
			...
			sink->active++;
		}
		...
	}

	snd_soc_dai_link_event(...)
	{
		...
		snd_soc_dapm_widget_for_each_source_path(w, path) {
			...
			source->active--;
			...
		}

		snd_soc_dapm_widget_for_each_sink_path(w, path) {
			...
			sink->active--;
			...
		}
		...
	}

Only these directly access to dai->active, *without* thinking stream_active.
I think it should use snd_soc_runtime_activate() / snd_soc_runtime_deactivate().
My patch cares it... Oops !!
I thought my patch cares it, but not enough (= [02/17]).

Can you agree below ?
	1) use runtime_activate()/deactivate() at above functions.
	2) apply my original patches on top of 1)
	   then, update git-log to explain above

Thank you for your help !!

Best regards
---
Kuninori Morimoto