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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 9 Jul 2021 10:41:42 +0530
From:   "Mukunda,Vijendar" <vijendar.mukunda@....com>
To:     "Deucher, Alexander" <Alexander.Deucher@....com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>
Cc:     "amistry@...gle.com" <amistry@...gle.com>,
        "nartemiev@...gle.com" <nartemiev@...gle.com>,
        "Hiregoudar, Basavaraj" <Basavaraj.Hiregoudar@....com>,
        "Dommati, Sunil-kumar" <Sunil-kumar.Dommati@....com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Chuhong Yuan <hslester96@...il.com>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3] ASoC: add a flag to reverse the stop sequence

On 7/9/21 12:21 AM, Deucher, Alexander wrote:
> [AMD Public Use]
> 
>> -----Original Message-----
>> From: Vijendar Mukunda <vijendar.mukunda@....com>
>> Sent: Thursday, July 8, 2021 1:55 PM
>> To: broonie@...nel.org; alsa-devel@...a-project.org
>> Cc: amistry@...gle.com; nartemiev@...gle.com; Deucher, Alexander
>> <Alexander.Deucher@....com>; Hiregoudar, Basavaraj
>> <Basavaraj.Hiregoudar@....com>; Dommati, Sunil-kumar <Sunil-
>> kumar.Dommati@....com>; Mukunda, Vijendar
>> <Vijendar.Mukunda@....com>; Liam Girdwood <lgirdwood@...il.com>;
>> Jaroslav Kysela <perex@...ex.cz>; Takashi Iwai <tiwai@...e.com>;
>> Chuhong Yuan <hslester96@...il.com>; Kuninori Morimoto
>> <kuninori.morimoto.gx@...esas.com>; open list <linux-
>> kernel@...r.kernel.org>
>> Subject: [PATCH V3] ASoC: add a flag to reverse the stop sequence
>>
>> From: Vijendar Mukunda <Vijendar.Mukunda@....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@....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