From 1bf83fa6654ce8959948878aad14a6db586125b8 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Wed, 22 Mar 2023 11:43:44 +0200 Subject: [PATCH 1/3] ASoC: SOF: Intel: hda-dai: Do not perform DMA cleanup during stop In the case of repeated start/stop without involving hw_free, the stream tag needs to be preserved for the subsequent starts. So, skip performing the DMA clean up during stop and handle it only during suspend or hw_free. Signed-off-by: Ranjani Sridharan Reviewed-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Reviewed-by: Bard Liao Signed-off-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20230322094346.6019-2-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-dai.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 9637f0f44b01..46a17afdd1ea 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -305,7 +305,6 @@ static int hda_dai_trigger(struct snd_pcm_substream *substream, int cmd, struct switch (cmd) { case SNDRV_PCM_TRIGGER_SUSPEND: - case SNDRV_PCM_TRIGGER_STOP: ret = hda_link_dma_cleanup(substream, hext_stream, dai, codec_dai); if (ret < 0) { dev_err(sdev->dev, "%s: failed to clean up link DMA\n", __func__); From 7d6f623c6a9d05195d1b19120383d4f42a1747db Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Wed, 22 Mar 2023 11:43:45 +0200 Subject: [PATCH 2/3] ASoC: SOF: pcm: Make hw_params reset conditional for IPC3 In the case of IPC4, since there is no PCM_PARAMS IPC to send the new stream tag when restarting a stream without a hw_free, the original stream tag needs to be preserved. So, add new a flag as part of struct sof_ipc_pcm_ops, reset_hw_params_during_stop and set it only for IPC3. This will ensure that the host DMA stream tag will not be given up during the STOP trigger for IPC4. Signed-off-by: Ranjani Sridharan Reviewed-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Reviewed-by: Bard Liao Signed-off-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20230322094346.6019-3-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc3-pcm.c | 1 + sound/soc/sof/pcm.c | 3 ++- sound/soc/sof/sof-audio.h | 3 +++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/ipc3-pcm.c b/sound/soc/sof/ipc3-pcm.c index b29d93e0d216..b7f1eb21ca26 100644 --- a/sound/soc/sof/ipc3-pcm.c +++ b/sound/soc/sof/ipc3-pcm.c @@ -382,4 +382,5 @@ const struct sof_ipc_pcm_ops ipc3_pcm_ops = { .hw_free = sof_ipc3_pcm_hw_free, .trigger = sof_ipc3_pcm_trigger, .dai_link_fixup = sof_ipc3_pcm_dai_link_fixup, + .reset_hw_params_during_stop = true, }; diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 445acb5c3a21..f75b161125fa 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -328,7 +328,8 @@ static int sof_pcm_trigger(struct snd_soc_component *component, fallthrough; case SNDRV_PCM_TRIGGER_STOP: ipc_first = true; - reset_hw_params = true; + if (pcm_ops && pcm_ops->reset_hw_params_during_stop) + reset_hw_params = true; break; default: dev_err(component->dev, "Unhandled trigger cmd %d\n", cmd); diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index d220af5f08fb..81685e778ad6 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -104,6 +104,8 @@ struct snd_sof_dai_config_data { * @pcm_free: Function pointer for PCM free that can be used for freeing any * additional memory in the SOF PCM stream structure * @delay: Function pointer for pcm delay calculation + * @reset_hw_params_during_stop: Flag indicating whether the hw_params should be reset during the + * STOP pcm trigger */ struct sof_ipc_pcm_ops { int (*hw_params)(struct snd_soc_component *component, struct snd_pcm_substream *substream, @@ -117,6 +119,7 @@ struct sof_ipc_pcm_ops { void (*pcm_free)(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm); snd_pcm_sframes_t (*delay)(struct snd_soc_component *component, struct snd_pcm_substream *substream); + bool reset_hw_params_during_stop; }; /** From 51ce3e6effab4fd4e13a3f187f4e256259f6e5a4 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Wed, 22 Mar 2023 11:43:46 +0200 Subject: [PATCH 3/3] ASoC: SOF: pcm: Improve the pcm trigger sequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recommended sequence for triggering the host DMA is to first program the DMA in the FW before setting the RUN bit to start the stream in the host. With IPC3, this sequence is honored because the FW programs the DMA when the HW_PARAMS IPC is sent during PCM hw_params and then the host sets the RUN bit during sof_pcm_trigger(). But with IPC4, sof_pcm_trigger() sends the SET_PIPELINE_STATE IPC to program the DMA in the FW after the DMA RUN bit is set. In order to minimize the impact for IPC3, introduce a new flag as part of struct sof_ipc_pcm_ops, ipc_first_on_start, which will be set for IPC4 only. With this flag set, the SET_PIPELINE_STATE IPC will be sent before the DMA RUN bit is set by the host during the START/PAUSE_RELEASE triggers. Signed-off-by: Ranjani Sridharan Reviewed-by: Péter Ujfalusi Reviewed-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Signed-off-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20230322094346.6019-4-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-pcm.c | 3 ++- sound/soc/sof/pcm.c | 26 ++++++++++++++++++++------ sound/soc/sof/sof-audio.h | 3 +++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index db64200ba1e5..a2cd21256e44 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -835,5 +835,6 @@ const struct sof_ipc_pcm_ops ipc4_pcm_ops = { .dai_link_fixup = sof_ipc4_pcm_dai_link_fixup, .pcm_setup = sof_ipc4_pcm_setup, .pcm_free = sof_ipc4_pcm_free, - .delay = sof_ipc4_pcm_delay + .delay = sof_ipc4_pcm_delay, + .ipc_first_on_start = true }; diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index f75b161125fa..d9b4633bba7a 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -301,6 +301,8 @@ static int sof_pcm_trigger(struct snd_soc_component *component, ipc_first = true; break; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + if (pcm_ops && pcm_ops->ipc_first_on_start) + ipc_first = true; break; case SNDRV_PCM_TRIGGER_START: if (spcm->stream[substream->stream].suspend_ignored) { @@ -312,6 +314,9 @@ static int sof_pcm_trigger(struct snd_soc_component *component, spcm->stream[substream->stream].suspend_ignored = false; return 0; } + + if (pcm_ops && pcm_ops->ipc_first_on_start) + ipc_first = true; break; case SNDRV_PCM_TRIGGER_SUSPEND: if (sdev->system_suspend_target == SOF_SUSPEND_S0IX && @@ -336,19 +341,28 @@ static int sof_pcm_trigger(struct snd_soc_component *component, return -EINVAL; } - /* - * DMA and IPC sequence is different for start and stop. Need to send - * STOP IPC before stop DMA - */ if (!ipc_first) snd_sof_pcm_platform_trigger(sdev, substream, cmd); if (pcm_ops && pcm_ops->trigger) ret = pcm_ops->trigger(component, substream, cmd); - /* need to STOP DMA even if trigger IPC failed */ - if (ipc_first) + switch (cmd) { + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + case SNDRV_PCM_TRIGGER_START: + /* invoke platform trigger to start DMA only if pcm_ops is successful */ + if (ipc_first && !ret) + snd_sof_pcm_platform_trigger(sdev, substream, cmd); + break; + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + case SNDRV_PCM_TRIGGER_STOP: + /* invoke platform trigger to stop DMA even if pcm_ops failed */ snd_sof_pcm_platform_trigger(sdev, substream, cmd); + break; + default: + break; + } /* free PCM if reset_hw_params is set and the STOP IPC is successful */ if (!ret && reset_hw_params) diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 81685e778ad6..6c64376858b3 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -106,6 +106,8 @@ struct snd_sof_dai_config_data { * @delay: Function pointer for pcm delay calculation * @reset_hw_params_during_stop: Flag indicating whether the hw_params should be reset during the * STOP pcm trigger + * @ipc_first_on_start: Send IPC before invoking platform trigger during + * START/PAUSE_RELEASE triggers */ struct sof_ipc_pcm_ops { int (*hw_params)(struct snd_soc_component *component, struct snd_pcm_substream *substream, @@ -120,6 +122,7 @@ struct sof_ipc_pcm_ops { snd_pcm_sframes_t (*delay)(struct snd_soc_component *component, struct snd_pcm_substream *substream); bool reset_hw_params_during_stop; + bool ipc_first_on_start; }; /**