diff mbox series

ASoC: add dai_reoder flag to reverse the stop sequence

Message ID 20210705155830.24693-1-vijendar.mukunda@amd.com
State New
Headers show
Series ASoC: add dai_reoder flag to reverse the stop sequence | expand

Commit Message

Vijendar Mukunda July 5, 2021, 3:58 p.m. UTC 
From: Vijendar Mukunda <Vijendar.Mukunda@amd.com>

On stream stop, currently CPU DAI stop sequence is 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 sound card structure for reordering stop sequence.
Based on flag check, ASoC core will re-order the stop requence.

Signed-off-by: Vijendar Mukunda <vijendar.mukunda@amd.com>
---
 include/sound/soc.h                  |  1 +
 sound/soc/amd/acp-da7219-max98357a.c |  1 +
 sound/soc/soc-pcm.c                  | 23 +++++++++++++++++------
 3 files changed, 19 insertions(+), 6 deletions(-)

Comments

kernel test robot July 5, 2021, 6:06 p.m. UTC | #1 
Hi Vijendar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on asoc/for-next]
[also build test WARNING on v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vijendar-Mukunda/ASoC-add-dai_reoder-flag-to-reverse-the-stop-sequence/20210705-234319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: i386-randconfig-r031-20210705 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/a132ec819f3d15c103f3afb5f3e8154355fafdfc
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vijendar-Mukunda/ASoC-add-dai_reoder-flag-to-reverse-the-stop-sequence/20210705-234319
        git checkout a132ec819f3d15c103f3afb5f3e8154355fafdfc
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   sound/soc/soc-pcm.c: In function 'soc_pcm_trigger':
   sound/soc/soc-pcm.c:1019:3: error: expected expression before 'struct'
    1019 | + struct snd_soc_card *card = rtd->card;
         |   ^~~~~~
>> sound/soc/soc-pcm.c:1020:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    1020 |  int ret = -EINVAL, _ret = 0;
         |  ^~~
   sound/soc/soc-pcm.c:1060:7: error: 'card' undeclared (first use in this function)
    1060 |   if (card->dai_reorder) {
         |       ^~~~
   sound/soc/soc-pcm.c:1060:7: note: each undeclared identifier is reported only once for each function it appears in
   sound/soc/soc-pcm.c:1018:30: warning: unused variable 'rtd' [-Wunused-variable]
    1018 |  struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
         |                              ^~~


vim +1020 sound/soc/soc-pcm.c

ddee627cf6bb60 Liam Girdwood     2011-06-09  1015  
836367be289d5b Kuninori Morimoto 2020-06-04  1016  static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
ddee627cf6bb60 Liam Girdwood     2011-06-09  1017  {
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1018  	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1019  +	struct snd_soc_card *card = rtd->card;
6374f493d93b22 Kuninori Morimoto 2020-12-01 @1020  	int ret = -EINVAL, _ret = 0;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1021  	int rollback = 0;
ddee627cf6bb60 Liam Girdwood     2011-06-09  1022  
836367be289d5b Kuninori Morimoto 2020-06-04  1023  	switch (cmd) {
836367be289d5b Kuninori Morimoto 2020-06-04  1024  	case SNDRV_PCM_TRIGGER_START:
836367be289d5b Kuninori Morimoto 2020-06-04  1025  	case SNDRV_PCM_TRIGGER_RESUME:
836367be289d5b Kuninori Morimoto 2020-06-04  1026  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
6374f493d93b22 Kuninori Morimoto 2020-12-01  1027  		ret = snd_soc_link_trigger(substream, cmd, 0);
ddee627cf6bb60 Liam Girdwood     2011-06-09  1028  		if (ret < 0)
6374f493d93b22 Kuninori Morimoto 2020-12-01  1029  			goto start_err;
ddee627cf6bb60 Liam Girdwood     2011-06-09  1030  
6374f493d93b22 Kuninori Morimoto 2020-12-01  1031  		ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
b8135864d4d33d Kuninori Morimoto 2017-10-11  1032  		if (ret < 0)
6374f493d93b22 Kuninori Morimoto 2020-12-01  1033  			goto start_err;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1034  
6374f493d93b22 Kuninori Morimoto 2020-12-01  1035  		ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
6374f493d93b22 Kuninori Morimoto 2020-12-01  1036  start_err:
6374f493d93b22 Kuninori Morimoto 2020-12-01  1037  		if (ret < 0)
6374f493d93b22 Kuninori Morimoto 2020-12-01  1038  			rollback = 1;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1039  	}
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1040  
6374f493d93b22 Kuninori Morimoto 2020-12-01  1041  	if (rollback) {
6374f493d93b22 Kuninori Morimoto 2020-12-01  1042  		_ret = ret;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1043  		switch (cmd) {
6374f493d93b22 Kuninori Morimoto 2020-12-01  1044  		case SNDRV_PCM_TRIGGER_START:
6374f493d93b22 Kuninori Morimoto 2020-12-01  1045  			cmd = SNDRV_PCM_TRIGGER_STOP;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1046  			break;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1047  		case SNDRV_PCM_TRIGGER_RESUME:
6374f493d93b22 Kuninori Morimoto 2020-12-01  1048  			cmd = SNDRV_PCM_TRIGGER_SUSPEND;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1049  			break;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1050  		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
6374f493d93b22 Kuninori Morimoto 2020-12-01  1051  			cmd = SNDRV_PCM_TRIGGER_PAUSE_PUSH;
836367be289d5b Kuninori Morimoto 2020-06-04  1052  			break;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1053  		}
6374f493d93b22 Kuninori Morimoto 2020-12-01  1054  	}
6374f493d93b22 Kuninori Morimoto 2020-12-01  1055  
6374f493d93b22 Kuninori Morimoto 2020-12-01  1056  	switch (cmd) {
836367be289d5b Kuninori Morimoto 2020-06-04  1057  	case SNDRV_PCM_TRIGGER_STOP:
836367be289d5b Kuninori Morimoto 2020-06-04  1058  	case SNDRV_PCM_TRIGGER_SUSPEND:
836367be289d5b Kuninori Morimoto 2020-06-04  1059  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1060  		if (card->dai_reorder) {
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1061  			ret = snd_soc_pcm_component_trigger(substream, cmd, rollback);
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1062  			if (ret < 0)
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1063  				break;
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1064  
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1065  			ret = snd_soc_pcm_dai_trigger(substream, cmd, rollback);
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1066  			if (ret < 0)
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1067  				break;
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1068  		} else {
6374f493d93b22 Kuninori Morimoto 2020-12-01  1069  			ret = snd_soc_pcm_dai_trigger(substream, cmd, rollback);
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1070  			if (ret < 0)
836367be289d5b Kuninori Morimoto 2020-06-04  1071  				break;
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1072  
6374f493d93b22 Kuninori Morimoto 2020-12-01  1073  			ret = snd_soc_pcm_component_trigger(substream, cmd, rollback);
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1074  			if (ret < 0)
836367be289d5b Kuninori Morimoto 2020-06-04  1075  				break;
a132ec819f3d15 Vijendar Mukunda  2021-07-05  1076  		}
6374f493d93b22 Kuninori Morimoto 2020-12-01  1077  		ret = snd_soc_link_trigger(substream, cmd, rollback);
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1078  		break;
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1079  	}
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1080  
6374f493d93b22 Kuninori Morimoto 2020-12-01  1081  	if (_ret)
6374f493d93b22 Kuninori Morimoto 2020-12-01  1082  		ret = _ret;
6374f493d93b22 Kuninori Morimoto 2020-12-01  1083  
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1084  	return ret;
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1085  }
4378f1fbe92405 Peter Ujfalusi    2019-09-27  1086  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot July 5, 2021, 7:01 p.m. UTC | #2 
Hi Vijendar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vijendar-Mukunda/ASoC-add-dai_reoder-flag-to-reverse-the-stop-sequence/20210705-234319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: i386-randconfig-r031-20210705 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/a132ec819f3d15c103f3afb5f3e8154355fafdfc
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vijendar-Mukunda/ASoC-add-dai_reoder-flag-to-reverse-the-stop-sequence/20210705-234319
        git checkout a132ec819f3d15c103f3afb5f3e8154355fafdfc
        # save the attached .config to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   sound/soc/soc-pcm.c: In function 'soc_pcm_trigger':
>> sound/soc/soc-pcm.c:1019:3: error: expected expression before 'struct'
    1019 | + struct snd_soc_card *card = rtd->card;
         |   ^~~~~~
   sound/soc/soc-pcm.c:1020:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    1020 |  int ret = -EINVAL, _ret = 0;
         |  ^~~
>> sound/soc/soc-pcm.c:1060:7: error: 'card' undeclared (first use in this function)
    1060 |   if (card->dai_reorder) {
         |       ^~~~
   sound/soc/soc-pcm.c:1060:7: note: each undeclared identifier is reported only once for each function it appears in
   sound/soc/soc-pcm.c:1018:30: warning: unused variable 'rtd' [-Wunused-variable]
    1018 |  struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
         |                              ^~~


vim +/struct +1019 sound/soc/soc-pcm.c

  1015	
  1016	static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
  1017	{
  1018		struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> 1019	+	struct snd_soc_card *card = rtd->card;
  1020		int ret = -EINVAL, _ret = 0;
  1021		int rollback = 0;
  1022	
  1023		switch (cmd) {
  1024		case SNDRV_PCM_TRIGGER_START:
  1025		case SNDRV_PCM_TRIGGER_RESUME:
  1026		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
  1027			ret = snd_soc_link_trigger(substream, cmd, 0);
  1028			if (ret < 0)
  1029				goto start_err;
  1030	
  1031			ret = snd_soc_pcm_component_trigger(substream, cmd, 0);
  1032			if (ret < 0)
  1033				goto start_err;
  1034	
  1035			ret = snd_soc_pcm_dai_trigger(substream, cmd, 0);
  1036	start_err:
  1037			if (ret < 0)
  1038				rollback = 1;
  1039		}
  1040	
  1041		if (rollback) {
  1042			_ret = ret;
  1043			switch (cmd) {
  1044			case SNDRV_PCM_TRIGGER_START:
  1045				cmd = SNDRV_PCM_TRIGGER_STOP;
  1046				break;
  1047			case SNDRV_PCM_TRIGGER_RESUME:
  1048				cmd = SNDRV_PCM_TRIGGER_SUSPEND;
  1049				break;
  1050			case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
  1051				cmd = SNDRV_PCM_TRIGGER_PAUSE_PUSH;
  1052				break;
  1053			}
  1054		}
  1055	
  1056		switch (cmd) {
  1057		case SNDRV_PCM_TRIGGER_STOP:
  1058		case SNDRV_PCM_TRIGGER_SUSPEND:
  1059		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> 1060			if (card->dai_reorder) {
  1061				ret = snd_soc_pcm_component_trigger(substream, cmd, rollback);
  1062				if (ret < 0)
  1063					break;
  1064	
  1065				ret = snd_soc_pcm_dai_trigger(substream, cmd, rollback);
  1066				if (ret < 0)
  1067					break;
  1068			} else {
  1069				ret = snd_soc_pcm_dai_trigger(substream, cmd, rollback);
  1070				if (ret < 0)
  1071					break;
  1072	
  1073				ret = snd_soc_pcm_component_trigger(substream, cmd, rollback);
  1074				if (ret < 0)
  1075					break;
  1076			}
  1077			ret = snd_soc_link_trigger(substream, cmd, rollback);
  1078			break;
  1079		}
  1080	
  1081		if (_ret)
  1082			ret = _ret;
  1083	
  1084		return ret;
  1085	}
  1086	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Mark Brown July 5, 2021, 7:29 p.m. UTC | #3 
On Tue, Jul 06, 2021 at 12:30:10AM +0530, Mukunda,Vijendar wrote:

> As per our understanding by going with card wide option is easier rather
> than checking dai link name for re-ordering the stop sequence for
> specific platforms.
> We will rename the flag as "stop_dma_fist" and will post the new version.

Why would we need to check the name for the link?  Presumably all
affected AMD cards would just unconditionally set the flag, and other
systems could do what they like?
Mark Brown July 6, 2021, 12:30 p.m. UTC | #4 
On Tue, Jul 06, 2021 at 01:40:59AM +0530, Mukunda,Vijendar wrote:

> To make AMD specific platform change(which uses ACP 2.x IP), As per our
> understanding we should only update the flag in ACP DMA driver by adding
> flag in snd_pcm_substream structure rather than adding flag in card
> structure.
> Please suggest us, if there is any better place holder to add
> "stop_dma_first" flag.

I'd expect this to be configured by the machine driver in the dai_link.
It might need copying over to the substream for runtime use but the core
should do that.
diff mbox series

Patch

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 675849d07284..12f79cb70600 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -982,6 +982,7 @@  struct snd_soc_card {
 	unsigned int disable_route_checks:1;
 	unsigned int probed:1;
 	unsigned int component_chaining:1;
+	unsigned int dai_reorder:1;
 
 	void *drvdata;
 };
diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index 84e3906abd4f..b0533b4b7b18 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -742,6 +742,7 @@  static int cz_probe(struct platform_device *pdev)
 	if (!machine)
 		return -ENOMEM;
 	card->dev = &pdev->dev;
+	cz_card.dai_reorder = true;
 	platform_set_drvdata(pdev, card);
 	snd_soc_card_set_drvdata(card, machine);
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 46513bb97904..4a9a51e6c988 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1015,6 +1015,8 @@  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);
++	struct snd_soc_card *card = rtd->card;
 	int ret = -EINVAL, _ret = 0;
 	int rollback = 0;
 
@@ -1055,14 +1057,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 (card->dai_reorder) {
+			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;
 	}