Re: [PATCH V3] ASoC: add a flag to reverse the stop sequence

["Mukunda,Vijendar" <vijendar.mukunda@amd.com>]

On 7/9/21 12:21 AM, Deucher, Alexander wrote:
> [AMD Public Use]
> 
>> -----Original Message-----
>> From: Vijendar Mukunda <vijendar.mukunda@amd.com>
>> Sent: Thursday, July 8, 2021 1:55 PM
>> To: broonie@kernel.org; alsa-devel@alsa-project.org
>> Cc: amistry@google.com; nartemiev@google.com; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Hiregoudar, Basavaraj
>> <Basavaraj.Hiregoudar@amd.com>; Dommati, Sunil-kumar <Sunil-
>> kumar.Dommati@amd.com>; Mukunda, Vijendar
>> <Vijendar.Mukunda@amd.com>; Liam Girdwood <lgirdwood@gmail.com>;
>> Jaroslav Kysela <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>;
>> Chuhong Yuan <hslester96@gmail.com>; Kuninori Morimoto
>> <kuninori.morimoto.gx@renesas.com>; open list <linux-
>> kernel@vger.kernel.org>
>> Subject: [PATCH V3] ASoC: add a flag to reverse the stop sequence
>>
>> From: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>>
>> On stream stop, currently CPU DAI stop sequence invoked first followed by
>> DMA. For Few platforms, it is required to stop the DMA first before stopping
>> CPU DAI.
>>
>> For Stoneyridge platform, it is required to invoke DMA driver stop first rather
>> than invoking DWC I2S controller stop.
>> Introduced new flag in dai_link structure for reordering stop sequence.
>> Based on flag check, ASoC core will re-order the stop sequence.
>>
> 
> You should add a fixes tag as well for stable:
> Fixes: 4378f1fbe92405 ("ASoC: soc-pcm: Use different sequence for start/stop trigger")
> 
> Alex

will add fixes tag and post the new version.
> 
>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>> ---
>> v2 -> v3: moved "stop_dma_first" flag from card structure
>> 	  to dai_link structure and modified code to use
>>           "stop_dma_first" flag.
>> v1 -> v2: renamed flag as "stop_dma_fist"
>>           fixed build error by removing extra + symbol
>>           sound/soc/soc-pcm.c:1019:3: error: expected expression before 'struct'
>>           1019 | + struct snd_soc_card *card = rtd->card;
>>
>>  include/sound/soc.h                  |  6 ++++++
>>  sound/soc/amd/acp-da7219-max98357a.c |  5 +++++
>>  sound/soc/soc-pcm.c                  | 22 ++++++++++++++++------
>>  3 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/sound/soc.h b/include/sound/soc.h index
>> 675849d07284..8e6dd8a257c5 100644
>> --- a/include/sound/soc.h
>> +++ b/include/sound/soc.h
>> @@ -712,6 +712,12 @@ struct snd_soc_dai_link {
>>  	/* Do not create a PCM for this DAI link (Backend link) */
>>  	unsigned int ignore:1;
>>
>> +	/* This flag will reorder stop sequence. By enabling this flag
>> +	 * DMA controller stop sequence will be invoked first followed by
>> +	 * CPU DAI driver stop sequence
>> +	 */
>> +	unsigned int stop_dma_first:1;
>> +
>>  #ifdef CONFIG_SND_SOC_TOPOLOGY
>>  	struct snd_soc_dobj dobj; /* For topology */  #endif diff --git
>> a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-
>> max98357a.c
>> index 84e3906abd4f..9449fb40a956 100644
>> --- a/sound/soc/amd/acp-da7219-max98357a.c
>> +++ b/sound/soc/amd/acp-da7219-max98357a.c
>> @@ -576,6 +576,7 @@ static struct snd_soc_dai_link cz_dai_5682_98357[] = {
>>  				| SND_SOC_DAIFMT_CBM_CFM,
>>  		.init = cz_rt5682_init,
>>  		.dpcm_playback = 1,
>> +		.stop_dma_first = 1,
>>  		.ops = &cz_rt5682_play_ops,
>>  		SND_SOC_DAILINK_REG(designware1, rt5682, platform),
>>  	},
>> @@ -585,6 +586,7 @@ static struct snd_soc_dai_link cz_dai_5682_98357[] = {
>>  		.dai_fmt = SND_SOC_DAIFMT_I2S |
>> SND_SOC_DAIFMT_NB_NF
>>  				| SND_SOC_DAIFMT_CBM_CFM,
>>  		.dpcm_capture = 1,
>> +		.stop_dma_first = 1,
>>  		.ops = &cz_rt5682_cap_ops,
>>  		SND_SOC_DAILINK_REG(designware2, rt5682, platform),
>>  	},
>> @@ -594,6 +596,7 @@ static struct snd_soc_dai_link cz_dai_5682_98357[] = {
>>  		.dai_fmt = SND_SOC_DAIFMT_I2S |
>> SND_SOC_DAIFMT_NB_NF
>>  				| SND_SOC_DAIFMT_CBM_CFM,
>>  		.dpcm_playback = 1,
>> +		.stop_dma_first = 1,
>>  		.ops = &cz_rt5682_max_play_ops,
>>  		SND_SOC_DAILINK_REG(designware3, mx, platform),
>>  	},
>> @@ -604,6 +607,7 @@ static struct snd_soc_dai_link cz_dai_5682_98357[] = {
>>  		.dai_fmt = SND_SOC_DAIFMT_I2S |
>> SND_SOC_DAIFMT_NB_NF
>>  				| SND_SOC_DAIFMT_CBM_CFM,
>>  		.dpcm_capture = 1,
>> +		.stop_dma_first = 1,
>>  		.ops = &cz_rt5682_dmic0_cap_ops,
>>  		SND_SOC_DAILINK_REG(designware3, adau, platform),
>>  	},
>> @@ -614,6 +618,7 @@ static struct snd_soc_dai_link cz_dai_5682_98357[] = {
>>  		.dai_fmt = SND_SOC_DAIFMT_I2S |
>> SND_SOC_DAIFMT_NB_NF
>>  				| SND_SOC_DAIFMT_CBM_CFM,
>>  		.dpcm_capture = 1,
>> +		.stop_dma_first = 1,
>>  		.ops = &cz_rt5682_dmic1_cap_ops,
>>  		SND_SOC_DAILINK_REG(designware2, adau, platform),
>>  	},
>> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index
>> 46513bb97904..d1c570ca21ea 100644
>> --- a/sound/soc/soc-pcm.c
>> +++ b/sound/soc/soc-pcm.c
>> @@ -1015,6 +1015,7 @@ static int soc_pcm_hw_params(struct
>> snd_pcm_substream *substream,
>>
>>  static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
>> {
>> +	struct snd_soc_pcm_runtime *rtd =
>> asoc_substream_to_rtd(substream);
>>  	int ret = -EINVAL, _ret = 0;
>>  	int rollback = 0;
>>
>> @@ -1055,14 +1056,23 @@ static int soc_pcm_trigger(struct
>> snd_pcm_substream *substream, int cmd)
>>  	case SNDRV_PCM_TRIGGER_STOP:
>>  	case SNDRV_PCM_TRIGGER_SUSPEND:
>>  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>> -		ret = snd_soc_pcm_dai_trigger(substream, cmd, rollback);
>> -		if (ret < 0)
>> -			break;
>> +		if (rtd->dai_link->stop_dma_first) {
>> +			ret = snd_soc_pcm_component_trigger(substream,
>> cmd, rollback);
>> +			if (ret < 0)
>> +				break;
>>
>> -		ret = snd_soc_pcm_component_trigger(substream, cmd,
>> rollback);
>> -		if (ret < 0)
>> -			break;
>> +			ret = snd_soc_pcm_dai_trigger(substream, cmd,
>> rollback);
>> +			if (ret < 0)
>> +				break;
>> +		} else {
>> +			ret = snd_soc_pcm_dai_trigger(substream, cmd,
>> rollback);
>> +			if (ret < 0)
>> +				break;
>>
>> +			ret = snd_soc_pcm_component_trigger(substream,
>> cmd, rollback);
>> +			if (ret < 0)
>> +				break;
>> +		}
>>  		ret = snd_soc_link_trigger(substream, cmd, rollback);
>>  		break;
>>  	}
>> --
>> 2.17.1