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

From: Mukunda,Vijendar
Date: Fri Jul 09 2021 - 00:54:24 EST

On 7/9/21 12:21 AM, Deucher, Alexander wrote:
> [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

will add fixes tag and post the new version.
>
>> 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