| Message ID | 20210708175529.13313-1-vijendar.mukunda@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V3] ASoC: add a flag to reverse the stop sequence | expand |
[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 > 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
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
On Thu, Jul 08, 2021 at 11:25:27PM +0530, Vijendar Mukunda wrote: > 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(-) This should be two patches, one adding a new feature and the other using it.
On 7/15/21 7:47 PM, Mark Brown wrote: > On Thu, Jul 08, 2021 at 11:25:27PM +0530, Vijendar Mukunda wrote: > >> 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(-) > > This should be two patches, one adding a new feature and the other using > it. > Will split the patch and post it for review.
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; }