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

On Tue, 2020-05-12 at 08:53 +0900, Kuninori Morimoto wrote:

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.

I am wondering what the original intention for having separate stream_active and active members for snd_soc_dai was. With your proposal there wouldnt be a need for "active" anymore, isnt it?

Thanks, Ranjani