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

[Deucher, Alexander]

[AMD Public Use]

> -----Original Message-----
> From: Vijendar Mukunda <vijendar.mukunda@xxxxxxx>
> Sent: Thursday, July 8, 2021 1:55 PM
> To: broonie@xxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx
> Cc: amistry@xxxxxxxxxx; nartemiev@xxxxxxxxxx; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Hiregoudar, Basavaraj
> <Basavaraj.Hiregoudar@xxxxxxx>; Dommati, Sunil-kumar <Sunil-
> kumar.Dommati@xxxxxxx>; Mukunda, Vijendar
> <Vijendar.Mukunda@xxxxxxx>; Liam Girdwood <lgirdwood@xxxxxxxxx>;
> Jaroslav Kysela <perex@xxxxxxxx>; Takashi Iwai <tiwai@xxxxxxxx>;
> Chuhong Yuan <hslester96@xxxxxxxxx>; Kuninori Morimoto
> <kuninori.morimoto.gx@xxxxxxxxxxx>; open list <linux-
> kernel@xxxxxxxxxxxxxxx>
> Subject: [PATCH V3] ASoC: add a flag to reverse the stop sequence
> 
> From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
> 
> 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

> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
> ---
> 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