* [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command
@ 2019-10-04 15:41 Ranjani Sridharan
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command Ranjani Sridharan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ranjani Sridharan @ 2019-10-04 15:41 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, tiwai, 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.
My first thought was to use a BESPOKE trigger for the SOF driver
but looking more into the implementation of the bespoke trigger
in soc_pcm_bespoke_trigger() didnt indicate it had much to do
with the ordering of the FE/BE DAI's but rather just use the
bespoke trigger callbacks in the DAI driver.
More details on the SOF issue can be found in:
Github Issue: https://github.com/thesofproject/linux/issues/1160
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 | 99 +++++++++++++++++++++++++++++-----------
sound/soc/sof/topology.c | 4 ++
2 files changed, 76 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] 6+ messages in thread
* [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command
2019-10-04 15:41 [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Ranjani Sridharan
@ 2019-10-04 15:41 ` Ranjani Sridharan
2019-10-04 21:32 ` Pierre-Louis Bossart
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
2019-10-04 21:40 ` [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Pierre-Louis Bossart
2 siblings, 1 reply; 6+ messages in thread
From: Ranjani Sridharan @ 2019-10-04 15:41 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, tiwai, 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.
Github Issue: https://github.com/thesofproject/linux/issues/1160
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
sound/soc/soc-pcm.c | 99 ++++++++++++++++++++++++++++++++-------------
1 file changed, 72 insertions(+), 27 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 66910500e3b6..8e5097eead27 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2340,42 +2340,85 @@ 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_first(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_first(substream, cmd, true);
+ break;
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ ret = dpcm_dai_trigger_fe_first(substream, cmd, false);
+ break;
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ ret = dpcm_dai_trigger_fe_first(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_first(substream, cmd, false);
+ break;
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ ret = dpcm_dai_trigger_fe_first(substream, cmd, true);
+ break;
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ ret = dpcm_dai_trigger_fe_first(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 */
@@ -2384,10 +2427,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,
@@ -2396,6 +2435,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] 6+ messages in thread
* [alsa-devel] [RFC PATCH 2/2] ASoC: SOF: topology: set trigger order for FE DAI link
2019-10-04 15:41 [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Ranjani Sridharan
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command Ranjani Sridharan
@ 2019-10-04 15:41 ` Ranjani Sridharan
2019-10-04 21:40 ` [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Pierre-Louis Bossart
2 siblings, 0 replies; 6+ messages in thread
From: Ranjani Sridharan @ 2019-10-04 15:41 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, tiwai, 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>
---
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 0aabb3190ddc..f00ecbc38c6c 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -2827,6 +2827,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] 6+ messages in thread
* Re: [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command Ranjani Sridharan
@ 2019-10-04 21:32 ` Pierre-Louis Bossart
2019-10-04 21:37 ` Sridharan, Ranjani
0 siblings, 1 reply; 6+ messages in thread
From: Pierre-Louis Bossart @ 2019-10-04 21:32 UTC (permalink / raw)
To: Ranjani Sridharan, alsa-devel; +Cc: broonie, tiwai
On 10/4/19 8:41 AM, Ranjani Sridharan wrote:
> 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.
>
> Github Issue: https://github.com/thesofproject/linux/issues/1160
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
> sound/soc/soc-pcm.c | 99 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 72 insertions(+), 27 deletions(-)
>
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 66910500e3b6..8e5097eead27 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -2340,42 +2340,85 @@ 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_first(struct snd_pcm_substream *substream,
> + int cmd, bool fe_first)
the function name is odd with the fe_first repeat
maybe 'dpcm_dai_trigger_fe_be' ?
> +{
> + 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_first(substream, cmd, true);
> + break;
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + ret = dpcm_dai_trigger_fe_first(substream, cmd, false);
> + break;
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + ret = dpcm_dai_trigger_fe_first(substream, cmd, false);
> + break;
can we group these 3 cases together? The last two are identical.
> + 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_first(substream, cmd, false);
> + break;
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + ret = dpcm_dai_trigger_fe_first(substream, cmd, true);
> + break;
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + ret = dpcm_dai_trigger_fe_first(substream, cmd, true);
> + break;
can we group these 3 cases together? The last two are identical.
> + default:
> + ret = -EINVAL;
> + break;
> }
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command
2019-10-04 21:32 ` Pierre-Louis Bossart
@ 2019-10-04 21:37 ` Sridharan, Ranjani
0 siblings, 0 replies; 6+ messages in thread
From: Sridharan, Ranjani @ 2019-10-04 21:37 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan, tiwai
On Fri, Oct 4, 2019 at 2:34 PM Pierre-Louis Bossart <
pierre-louis.bossart@linux.intel.com> wrote:
> On 10/4/19 8:41 AM, Ranjani Sridharan wrote:
> > 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.
> >
> > Github Issue: https://github.com/thesofproject/linux/issues/1160
> > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > ---
> > sound/soc/soc-pcm.c | 99 ++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 72 insertions(+), 27 deletions(-)
> >
> > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> > index 66910500e3b6..8e5097eead27 100644
> > --- a/sound/soc/soc-pcm.c
> > +++ b/sound/soc/soc-pcm.c
> > @@ -2340,42 +2340,85 @@ 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_first(struct snd_pcm_substream
> *substream,
> > + int cmd, bool fe_first)
>
> the function name is odd with the fe_first repeat
>
> maybe 'dpcm_dai_trigger_fe_be' ?
>
Sure, will change in v2.
>
> > +{
> > + 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_first(substream, cmd,
> true);
> > + break;
> > + case SNDRV_PCM_TRIGGER_STOP:
> > + case SNDRV_PCM_TRIGGER_SUSPEND:
> > + ret = dpcm_dai_trigger_fe_first(substream, cmd,
> false);
> > + break;
> > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > + ret = dpcm_dai_trigger_fe_first(substream, cmd,
> false);
> > + break;
>
> can we group these 3 cases together? The last two are identical.
>
Ahh yes, I originally intended to set the state here as well but changed my
mind later on . Will fix in v2.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command
2019-10-04 15:41 [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Ranjani Sridharan
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command Ranjani Sridharan
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
@ 2019-10-04 21:40 ` Pierre-Louis Bossart
2 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2019-10-04 21:40 UTC (permalink / raw)
To: Ranjani Sridharan, alsa-devel; +Cc: broonie, tiwai
On 10/4/19 8:41 AM, Ranjani Sridharan wrote:
> 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.
>
> My first thought was to use a BESPOKE trigger for the SOF driver
> but looking more into the implementation of the bespoke trigger
> in soc_pcm_bespoke_trigger() didnt indicate it had much to do
> with the ordering of the FE/BE DAI's but rather just use the
> bespoke trigger callbacks in the DAI driver.
>
> More details on the SOF issue can be found in:
> Github Issue: https://github.com/thesofproject/linux/issues/1160
I am a bit confused because that GitHub link does not provide any test
results, and the PR1277 CI results show only half of the platforms
tested. not sure why. I also don't get if this is an issue specific to
the HDaudio Link DMA or if this is a across-the-board issue.
The other comment is that I see quite a few legacy Intel machine drivers
with the POST trigger hard-coded for front-ends, and we'd need to retest
some of these platforms to see if this change broke them.
>
> 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 | 99 +++++++++++++++++++++++++++++-----------
> sound/soc/sof/topology.c | 4 ++
> 2 files changed, 76 insertions(+), 27 deletions(-)
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-04 21:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 15:41 [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Ranjani Sridharan
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 1/2] ASoC: pcm: update FE/BE trigger order based on the command Ranjani Sridharan
2019-10-04 21:32 ` Pierre-Louis Bossart
2019-10-04 21:37 ` Sridharan, Ranjani
2019-10-04 15:41 ` [alsa-devel] [RFC PATCH 2/2] ASoC: SOF: topology: set trigger order for FE DAI link Ranjani Sridharan
2019-10-04 21:40 ` [alsa-devel] [RFC PATCH 0/2] Alter the trigger order for FE/BE DAI's based on command Pierre-Louis Bossart
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).