alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH v2 0/2] Update FE/BE trigger order based on the command
@ 2019-11-04 22:48 Ranjani Sridharan
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 1/2] ASoC: pcm: update " Ranjani Sridharan
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
  0 siblings, 2 replies; 8+ messages in thread
From: Ranjani Sridharan @ 2019-11-04 22:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, broonie, pierre-louis.bossart

Currently, the trigger orders SND_SOC_DPCM_TRIGGER_PRE/POST
determine the order in which FE DAI and BE DAI are triggered.
In the case of SND_SOC_DPCM_TRIGGER_PRE, the FE DAI is
triggered before the BE DAI and in the case of
SND_SOC_DPCM_TRIGGER_POST, the BE DAI is triggered before
the FE DAI. And this order remains the same irrespective of the
trigger command.
  
In the case of the SOF driver, during playback, the FW
expects the BE DAI to be triggered before the FE DAI during
the START trigger. The BE DAI trigger handles the starting of
Link DMA and so it must be started before the FE DAI is started
to prevent xruns during pause/release. This can be addressed
by setting the trigger order for the FE dai link to
SND_SOC_DPCM_TRIGGER_POST. But during the STOP trigger,
the FW expects the FE DAI to be triggered before the BE DAI.
Retaining the same order during the START and STOP commands,
results in FW error as the DAI component in the FW is still
active.
    
The issue can be fixed by mirroring the trigger order of
FE and BE DAI's during the START and STOP trigger. So, with the
trigger order set to SND_SOC_DPCM_TRIGGER_PRE, the FE DAI will be
trigger first during SNDRV_PCM_TRIGGER_START/STOP/RESUME
and the BE DAI will be triggered first during the
STOP/SUSPEND/PAUSE commands. Conversely, with the trigger order
set to SND_SOC_DPCM_TRIGGER_POST, the BE DAI will be triggered
first during the SNDRV_PCM_TRIGGER_START/STOP/RESUME commands
and the FE DAI will be triggered first during the
SNDRV_PCM_TRIGGER_STOP/SUSPEND/PAUSE commands.

The above change has been tested with the Skylake driver
on the Kabylake based chromebook and legacy HDA driver
on Kabylake based Dell XPS13. It has also been tested
on all platforms with SOF CI without any regressions
for the past 3 weeks.

More details on the issue can be found here:
https://github.com/thesofproject/linux/issues/1160

Changes in v2:
Fixed comments on function name and consolidating the switch
cases.

Ranjani Sridharan (2):
  ASoC: pcm: update FE/BE trigger order based on the command
  ASoC: SOF: topology: set trigger order for FE DAI link

 sound/soc/soc-pcm.c      | 95 ++++++++++++++++++++++++++++------------
 sound/soc/sof/topology.c |  4 ++
 2 files changed, 72 insertions(+), 27 deletions(-)

-- 
2.17.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH v2 1/2] ASoC: pcm: update FE/BE trigger order based on the command
  2019-11-04 22:48 [alsa-devel] [PATCH v2 0/2] Update FE/BE trigger order based on the command Ranjani Sridharan
@ 2019-11-04 22:48 ` Ranjani Sridharan
  2019-11-05 18:55   ` [alsa-devel] Applied "ASoC: pcm: update FE/BE trigger order based on the command" to the asoc tree Mark Brown
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
  1 sibling, 1 reply; 8+ messages in thread
From: Ranjani Sridharan @ 2019-11-04 22:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, broonie, pierre-louis.bossart

Currently, the trigger orders SND_SOC_DPCM_TRIGGER_PRE/POST
determine the order in which FE DAI and BE DAI are triggered.
In the case of SND_SOC_DPCM_TRIGGER_PRE, the FE DAI is
triggered before the BE DAI and in the case of
SND_SOC_DPCM_TRIGGER_POST, the BE DAI is triggered before
the FE DAI. And this order remains the same irrespective of the
trigger command.

In the case of the SOF driver, during playback, the FW
expects the BE DAI to be triggered before the FE DAI during
the START trigger. The BE DAI trigger handles the starting of
Link DMA and so it must be started before the FE DAI is started
to prevent xruns during pause/release. This can be addressed
by setting the trigger order for the FE dai link to
SND_SOC_DPCM_TRIGGER_POST. But during the STOP trigger,
the FW expects the FE DAI to be triggered before the BE DAI.
Retaining the same order during the START and STOP commands,
results in FW error as the DAI component in the FW is still
active.

The issue can be fixed by mirroring the trigger order of
FE and BE DAI's during the START and STOP trigger. So, with the
trigger order set to SND_SOC_DPCM_TRIGGER_PRE, the FE DAI will be
trigger first during SNDRV_PCM_TRIGGER_START/STOP/RESUME
and the BE DAI will be triggered first during the
STOP/SUSPEND/PAUSE commands. Conversely, with the trigger order
set to SND_SOC_DPCM_TRIGGER_POST, the BE DAI will be triggered
first during the SNDRV_PCM_TRIGGER_START/STOP/RESUME commands
and the FE DAI will be triggered first during the
SNDRV_PCM_TRIGGER_STOP/SUSPEND/PAUSE commands.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-pcm.c | 95 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 27 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index b890481cffcb..493a2e80e893 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2329,42 +2329,81 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 }
 EXPORT_SYMBOL_GPL(dpcm_be_dai_trigger);
 
+static int dpcm_dai_trigger_fe_be(struct snd_pcm_substream *substream,
+				  int cmd, bool fe_first)
+{
+	struct snd_soc_pcm_runtime *fe = substream->private_data;
+	int ret;
+
+	/* call trigger on the frontend before the backend. */
+	if (fe_first) {
+		dev_dbg(fe->dev, "ASoC: pre trigger FE %s cmd %d\n",
+			fe->dai_link->name, cmd);
+
+		ret = soc_pcm_trigger(substream, cmd);
+		if (ret < 0)
+			return ret;
+
+		ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
+		return ret;
+	}
+
+	/* call trigger on the frontend after the backend. */
+	ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
+	if (ret < 0)
+		return ret;
+
+	dev_dbg(fe->dev, "ASoC: post trigger FE %s cmd %d\n",
+		fe->dai_link->name, cmd);
+
+	ret = soc_pcm_trigger(substream, cmd);
+
+	return ret;
+}
+
 static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *fe = substream->private_data;
-	int stream = substream->stream, ret;
+	int stream = substream->stream;
+	int ret = 0;
 	enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream];
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
 	switch (trigger) {
 	case SND_SOC_DPCM_TRIGGER_PRE:
-		/* call trigger on the frontend before the backend. */
-
-		dev_dbg(fe->dev, "ASoC: pre trigger FE %s cmd %d\n",
-				fe->dai_link->name, cmd);
-
-		ret = soc_pcm_trigger(substream, cmd);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
-			goto out;
+		switch (cmd) {
+		case SNDRV_PCM_TRIGGER_START:
+		case SNDRV_PCM_TRIGGER_RESUME:
+		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, true);
+			break;
+		case SNDRV_PCM_TRIGGER_STOP:
+		case SNDRV_PCM_TRIGGER_SUSPEND:
+		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, false);
+			break;
+		default:
+			ret = -EINVAL;
+			break;
 		}
-
-		ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
 		break;
 	case SND_SOC_DPCM_TRIGGER_POST:
-		/* call trigger on the frontend after the backend. */
-
-		ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
-			goto out;
+		switch (cmd) {
+		case SNDRV_PCM_TRIGGER_START:
+		case SNDRV_PCM_TRIGGER_RESUME:
+		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, false);
+			break;
+		case SNDRV_PCM_TRIGGER_STOP:
+		case SNDRV_PCM_TRIGGER_SUSPEND:
+		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, true);
+			break;
+		default:
+			ret = -EINVAL;
+			break;
 		}
-
-		dev_dbg(fe->dev, "ASoC: post trigger FE %s cmd %d\n",
-				fe->dai_link->name, cmd);
-
-		ret = soc_pcm_trigger(substream, cmd);
 		break;
 	case SND_SOC_DPCM_TRIGGER_BESPOKE:
 		/* bespoke trigger() - handles both FE and BEs */
@@ -2373,10 +2412,6 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 				fe->dai_link->name, cmd);
 
 		ret = soc_pcm_bespoke_trigger(substream, cmd);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
-			goto out;
-		}
 		break;
 	default:
 		dev_err(fe->dev, "ASoC: invalid trigger cmd %d for %s\n", cmd,
@@ -2385,6 +2420,12 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 		goto out;
 	}
 
+	if (ret < 0) {
+		dev_err(fe->dev, "ASoC: trigger FE cmd: %d failed: %d\n",
+			cmd, ret);
+		goto out;
+	}
+
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
-- 
2.17.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link
  2019-11-04 22:48 [alsa-devel] [PATCH v2 0/2] Update FE/BE trigger order based on the command Ranjani Sridharan
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 1/2] ASoC: pcm: update " Ranjani Sridharan
@ 2019-11-04 22:48 ` Ranjani Sridharan
  2019-11-05 18:55   ` [alsa-devel] Applied "ASoC: SOF: topology: set trigger order for FE DAI link" to the asoc tree Mark Brown
  2019-11-18 22:44   ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Jaroslav Kysela
  1 sibling, 2 replies; 8+ messages in thread
From: Ranjani Sridharan @ 2019-11-04 22:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, broonie, pierre-louis.bossart

Set trigger order for FE DAI links to SND_SOC_DPCM_TRIGGER_POST
to trigger the BE DAI's before the FE DAI's. This prevents the
xruns seen on playback pipelines using the link DMA.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/topology.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index e0e2ae734632..e7076692119b 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -2951,6 +2951,10 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
 	if (!link->no_pcm) {
 		link->nonatomic = true;
 
+		/* set trigger order */
+		link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
+		link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
+
 		/* nothing more to do for FE dai links */
 		return 0;
 	}
-- 
2.17.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] Applied "ASoC: SOF: topology: set trigger order for FE DAI link" to the asoc tree
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
@ 2019-11-05 18:55   ` Mark Brown
  2019-11-18 22:44   ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Jaroslav Kysela
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Brown @ 2019-11-05 18:55 UTC (permalink / raw)
  To: Ranjani Sridharan; +Cc: tiwai, alsa-devel, Mark Brown, Pierre-Louis Bossart

The patch

   ASoC: SOF: topology: set trigger order for FE DAI link

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 5eee2b3f60065a2530d13f28e771be48b989eb4c Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Mon, 4 Nov 2019 14:48:12 -0800
Subject: [PATCH] ASoC: SOF: topology: set trigger order for FE DAI link

Set trigger order for FE DAI links to SND_SOC_DPCM_TRIGGER_POST
to trigger the BE DAI's before the FE DAI's. This prevents the
xruns seen on playback pipelines using the link DMA.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20191104224812.3393-3-ranjani.sridharan@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/topology.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index e0e2ae734632..e7076692119b 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -2951,6 +2951,10 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
 	if (!link->no_pcm) {
 		link->nonatomic = true;
 
+		/* set trigger order */
+		link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
+		link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
+
 		/* nothing more to do for FE dai links */
 		return 0;
 	}
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] Applied "ASoC: pcm: update FE/BE trigger order based on the command" to the asoc tree
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 1/2] ASoC: pcm: update " Ranjani Sridharan
@ 2019-11-05 18:55   ` Mark Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2019-11-05 18:55 UTC (permalink / raw)
  To: Ranjani Sridharan; +Cc: tiwai, alsa-devel, Mark Brown, Pierre-Louis Bossart

The patch

   ASoC: pcm: update FE/BE trigger order based on the command

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From acbf27746ecfa96b290b54cc7f05273482ea128a Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Mon, 4 Nov 2019 14:48:11 -0800
Subject: [PATCH] ASoC: pcm: update FE/BE trigger order based on the command

Currently, the trigger orders SND_SOC_DPCM_TRIGGER_PRE/POST
determine the order in which FE DAI and BE DAI are triggered.
In the case of SND_SOC_DPCM_TRIGGER_PRE, the FE DAI is
triggered before the BE DAI and in the case of
SND_SOC_DPCM_TRIGGER_POST, the BE DAI is triggered before
the FE DAI. And this order remains the same irrespective of the
trigger command.

In the case of the SOF driver, during playback, the FW
expects the BE DAI to be triggered before the FE DAI during
the START trigger. The BE DAI trigger handles the starting of
Link DMA and so it must be started before the FE DAI is started
to prevent xruns during pause/release. This can be addressed
by setting the trigger order for the FE dai link to
SND_SOC_DPCM_TRIGGER_POST. But during the STOP trigger,
the FW expects the FE DAI to be triggered before the BE DAI.
Retaining the same order during the START and STOP commands,
results in FW error as the DAI component in the FW is still
active.

The issue can be fixed by mirroring the trigger order of
FE and BE DAI's during the START and STOP trigger. So, with the
trigger order set to SND_SOC_DPCM_TRIGGER_PRE, the FE DAI will be
trigger first during SNDRV_PCM_TRIGGER_START/STOP/RESUME
and the BE DAI will be triggered first during the
STOP/SUSPEND/PAUSE commands. Conversely, with the trigger order
set to SND_SOC_DPCM_TRIGGER_POST, the BE DAI will be triggered
first during the SNDRV_PCM_TRIGGER_START/STOP/RESUME commands
and the FE DAI will be triggered first during the
SNDRV_PCM_TRIGGER_STOP/SUSPEND/PAUSE commands.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20191104224812.3393-2-ranjani.sridharan@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-pcm.c | 95 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 27 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 8655df6a6089..1c00119b72e3 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2322,42 +2322,81 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 }
 EXPORT_SYMBOL_GPL(dpcm_be_dai_trigger);
 
+static int dpcm_dai_trigger_fe_be(struct snd_pcm_substream *substream,
+				  int cmd, bool fe_first)
+{
+	struct snd_soc_pcm_runtime *fe = substream->private_data;
+	int ret;
+
+	/* call trigger on the frontend before the backend. */
+	if (fe_first) {
+		dev_dbg(fe->dev, "ASoC: pre trigger FE %s cmd %d\n",
+			fe->dai_link->name, cmd);
+
+		ret = soc_pcm_trigger(substream, cmd);
+		if (ret < 0)
+			return ret;
+
+		ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
+		return ret;
+	}
+
+	/* call trigger on the frontend after the backend. */
+	ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
+	if (ret < 0)
+		return ret;
+
+	dev_dbg(fe->dev, "ASoC: post trigger FE %s cmd %d\n",
+		fe->dai_link->name, cmd);
+
+	ret = soc_pcm_trigger(substream, cmd);
+
+	return ret;
+}
+
 static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *fe = substream->private_data;
-	int stream = substream->stream, ret;
+	int stream = substream->stream;
+	int ret = 0;
 	enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream];
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
 	switch (trigger) {
 	case SND_SOC_DPCM_TRIGGER_PRE:
-		/* call trigger on the frontend before the backend. */
-
-		dev_dbg(fe->dev, "ASoC: pre trigger FE %s cmd %d\n",
-				fe->dai_link->name, cmd);
-
-		ret = soc_pcm_trigger(substream, cmd);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
-			goto out;
+		switch (cmd) {
+		case SNDRV_PCM_TRIGGER_START:
+		case SNDRV_PCM_TRIGGER_RESUME:
+		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, true);
+			break;
+		case SNDRV_PCM_TRIGGER_STOP:
+		case SNDRV_PCM_TRIGGER_SUSPEND:
+		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, false);
+			break;
+		default:
+			ret = -EINVAL;
+			break;
 		}
-
-		ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
 		break;
 	case SND_SOC_DPCM_TRIGGER_POST:
-		/* call trigger on the frontend after the backend. */
-
-		ret = dpcm_be_dai_trigger(fe, substream->stream, cmd);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
-			goto out;
+		switch (cmd) {
+		case SNDRV_PCM_TRIGGER_START:
+		case SNDRV_PCM_TRIGGER_RESUME:
+		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, false);
+			break;
+		case SNDRV_PCM_TRIGGER_STOP:
+		case SNDRV_PCM_TRIGGER_SUSPEND:
+		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+			ret = dpcm_dai_trigger_fe_be(substream, cmd, true);
+			break;
+		default:
+			ret = -EINVAL;
+			break;
 		}
-
-		dev_dbg(fe->dev, "ASoC: post trigger FE %s cmd %d\n",
-				fe->dai_link->name, cmd);
-
-		ret = soc_pcm_trigger(substream, cmd);
 		break;
 	case SND_SOC_DPCM_TRIGGER_BESPOKE:
 		/* bespoke trigger() - handles both FE and BEs */
@@ -2366,10 +2405,6 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 				fe->dai_link->name, cmd);
 
 		ret = soc_pcm_bespoke_trigger(substream, cmd);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
-			goto out;
-		}
 		break;
 	default:
 		dev_err(fe->dev, "ASoC: invalid trigger cmd %d for %s\n", cmd,
@@ -2378,6 +2413,12 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 		goto out;
 	}
 
+	if (ret < 0) {
+		dev_err(fe->dev, "ASoC: trigger FE cmd: %d failed: %d\n",
+			cmd, ret);
+		goto out;
+	}
+
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link
  2019-11-04 22:48 ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
  2019-11-05 18:55   ` [alsa-devel] Applied "ASoC: SOF: topology: set trigger order for FE DAI link" to the asoc tree Mark Brown
@ 2019-11-18 22:44   ` Jaroslav Kysela
  2020-02-09  9:31     ` Hui Wang
  1 sibling, 1 reply; 8+ messages in thread
From: Jaroslav Kysela @ 2019-11-18 22:44 UTC (permalink / raw)
  To: Ranjani Sridharan, alsa-devel; +Cc: tiwai, broonie, pierre-louis.bossart

Dne 04. 11. 19 v 23:48 Ranjani Sridharan napsal(a):
> Set trigger order for FE DAI links to SND_SOC_DPCM_TRIGGER_POST
> to trigger the BE DAI's before the FE DAI's. This prevents the
> xruns seen on playback pipelines using the link DMA.
> 
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>   sound/soc/sof/topology.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> index e0e2ae734632..e7076692119b 100644
> --- a/sound/soc/sof/topology.c
> +++ b/sound/soc/sof/topology.c
> @@ -2951,6 +2951,10 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
>   	if (!link->no_pcm) {
>   		link->nonatomic = true;
>   
> +		/* set trigger order */
> +		link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
> +		link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
> +
>   		/* nothing more to do for FE dai links */
>   		return 0;
>   	}
> 

It seems that this patch breaks the signed 1.3 firmware. Tested on Lenovo 
Carbon X1 7th gen.

https://github.com/thesofproject/sof/issues/2102

					Jaroslav


-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link
  2019-11-18 22:44   ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Jaroslav Kysela
@ 2020-02-09  9:31     ` Hui Wang
  2020-02-09 12:45       ` Hui Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Hui Wang @ 2020-02-09  9:31 UTC (permalink / raw)
  To: Jaroslav Kysela, Ranjani Sridharan, alsa-devel
  Cc: tiwai, broonie, pierre-louis.bossart


On 2019/11/19 上午6:44, Jaroslav Kysela wrote:
> Dne 04. 11. 19 v 23:48 Ranjani Sridharan napsal(a):
>> Set trigger order for FE DAI links to SND_SOC_DPCM_TRIGGER_POST
>> to trigger the BE DAI's before the FE DAI's. This prevents the
>> xruns seen on playback pipelines using the link DMA.
>>
>> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>> Signed-off-by: Pierre-Louis Bossart 
>> <pierre-louis.bossart@linux.intel.com>
>> ---
>>   sound/soc/sof/topology.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
>> index e0e2ae734632..e7076692119b 100644
>> --- a/sound/soc/sof/topology.c
>> +++ b/sound/soc/sof/topology.c
>> @@ -2951,6 +2951,10 @@ static int sof_link_load(struct 
>> snd_soc_component *scomp, int index,
>>       if (!link->no_pcm) {
>>           link->nonatomic = true;
>>   +        /* set trigger order */
>> +        link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
>> +        link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
>> +
>>           /* nothing more to do for FE dai links */
>>           return 0;
>>       }
>>
>
> It seems that this patch breaks the signed 1.3 firmware. Tested on 
> Lenovo Carbon X1 7th gen.
>
> https://github.com/thesofproject/sof/issues/2102
>
>                     Jaroslav
>
Also met this problem, this patch is merged to 5.4 stable kernel, when I 
run the 5.4 stable kernel on the Lenovo Carbon x1 7th gen or Dell Dmic 
machines,  the sof driver will print error logs repeatedly and the sof 
fails to work. I tried with both 1.3 and 1.4 firmware. If I reverted 
this patch from 5.4 stable kernel, the sof driver will work with both 
1.3 and 1.4 firmware.

I also tried the 5.5-rc1 kernel, this  kernel already includes this 
patch, but there is no issue for this kernel, the sof driver works well. 
So looks like only backport this patch to 5.4 stable kernel is not 
enough, either we revert this patch or we backport more patches to 5.4 
stable kernel.


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link
  2020-02-09  9:31     ` Hui Wang
@ 2020-02-09 12:45       ` Hui Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Hui Wang @ 2020-02-09 12:45 UTC (permalink / raw)
  To: Jaroslav Kysela, Ranjani Sridharan, alsa-devel
  Cc: tiwai, broonie, pierre-louis.bossart

After cherry-pick this commit (acbf27746ecfa96b290b54cc7f05273482ea128a 
ASoC: pcm: update FE/BE trigger order based on the command) to 5.4 
stable kernel, the sof driver works.

On 2020/2/9 下午5:31, Hui Wang wrote:
>
> On 2019/11/19 上午6:44, Jaroslav Kysela wrote:
>> Dne 04. 11. 19 v 23:48 Ranjani Sridharan napsal(a):
>>> Set trigger order for FE DAI links to SND_SOC_DPCM_TRIGGER_POST
>>> to trigger the BE DAI's before the FE DAI's. This prevents the
>>> xruns seen on playback pipelines using the link DMA.
>>>
>>> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>>> Signed-off-by: Pierre-Louis Bossart 
>>> <pierre-louis.bossart@linux.intel.com>
>>> ---
>>>   sound/soc/sof/topology.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
>>> index e0e2ae734632..e7076692119b 100644
>>> --- a/sound/soc/sof/topology.c
>>> +++ b/sound/soc/sof/topology.c
>>> @@ -2951,6 +2951,10 @@ static int sof_link_load(struct 
>>> snd_soc_component *scomp, int index,
>>>       if (!link->no_pcm) {
>>>           link->nonatomic = true;
>>>   +        /* set trigger order */
>>> +        link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST;
>>> +        link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
>>> +
>>>           /* nothing more to do for FE dai links */
>>>           return 0;
>>>       }
>>>
>>
>> It seems that this patch breaks the signed 1.3 firmware. Tested on 
>> Lenovo Carbon X1 7th gen.
>>
>> https://github.com/thesofproject/sof/issues/2102
>>
>>                     Jaroslav
>>
> Also met this problem, this patch is merged to 5.4 stable kernel, when 
> I run the 5.4 stable kernel on the Lenovo Carbon x1 7th gen or Dell 
> Dmic machines,  the sof driver will print error logs repeatedly and 
> the sof fails to work. I tried with both 1.3 and 1.4 firmware. If I 
> reverted this patch from 5.4 stable kernel, the sof driver will work 
> with both 1.3 and 1.4 firmware.
>
> I also tried the 5.5-rc1 kernel, this  kernel already includes this 
> patch, but there is no issue for this kernel, the sof driver works 
> well. So looks like only backport this patch to 5.4 stable kernel is 
> not enough, either we revert this patch or we backport more patches to 
> 5.4 stable kernel.
>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-02-09 12:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 22:48 [alsa-devel] [PATCH v2 0/2] Update FE/BE trigger order based on the command Ranjani Sridharan
2019-11-04 22:48 ` [alsa-devel] [PATCH v2 1/2] ASoC: pcm: update " Ranjani Sridharan
2019-11-05 18:55   ` [alsa-devel] Applied "ASoC: pcm: update FE/BE trigger order based on the command" to the asoc tree Mark Brown
2019-11-04 22:48 ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
2019-11-05 18:55   ` [alsa-devel] Applied "ASoC: SOF: topology: set trigger order for FE DAI link" to the asoc tree Mark Brown
2019-11-18 22:44   ` [alsa-devel] [PATCH v2 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Jaroslav Kysela
2020-02-09  9:31     ` Hui Wang
2020-02-09 12:45       ` Hui Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).