[RFC TEST] ASoC: soc-dai: revert all changes to DAI startup/shutdown sequence

Pierre-Louis Bossart pierre-louis.bossart at linux.intel.com
Wed Apr 15 21:05:02 CEST 2020



On 4/15/20 12:28 PM, Mark Brown wrote:
> On Tue, Apr 14, 2020 at 10:04:37PM -0500, Pierre-Louis Bossart wrote:
> 
>> Sending as RFC since I don't have a good understanding of the
>> root-cause and for others to confirm my findings. Tested on top of
>> v5.7-rc1.
> 
> Hans?  Morimoto-san?  I'm fine with this as a fix, it's not ideal but
> I'm guessing anything more substantial is going to be unsuitable for a
> -rc series.

I looked up the code for about an hour and can't really find a better 
solution.

My only theory is that the Atom/sst driver directly uses the dai->active 
state in its .startup and .shutdown DAI callbacks, and will e.g. 
initialize the DSP with:

static int sst_enable_ssp(struct snd_pcm_substream *substream,
			struct snd_soc_dai *dai)
{
	int ret = 0;

	if (!dai->active) {
		ret = sst_handle_vb_timer(dai, true);
		sst_fill_ssp_defaults(dai);
	}
	return ret;
}

No other Intel driver uses this dai->active status. This is legacy 
2013/14 stuff from the phone days, I only have a vague memory of why 
this was used - everyone else involved at the time moved on, I am 
probably the last person standing, a scary thought in itself.

Anyways, since this .startup routine is invoked from two locations:

sound/soc/soc-dapm.c:           ret = snd_soc_dai_startup(source, 
substream);
sound/soc/soc-dapm.c:           ret = snd_soc_dai_startup(sink, substream);
sound/soc/soc-pcm.c:            ret = snd_soc_dai_startup(dai, substream);

we end-up using three booleans (dai->active, dai->started[0], 
dai->started[1]) to control what happens in the .startup callback, so we 
probably end-up in an undefined state.

I don't see an alternative to a revert here, we should probably clarify 
why we startup the dai in multiple locations and do something better for 
5.8.

-Pierre



More information about the Alsa-devel mailing list