From 5a2443b4900b1e2734d6560d6b31398e96ea6fed Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 10 Jun 2024 14:16:36 +0300 Subject: [PATCH 1/8] ASoC: SOF: Intel: hda-dai: Be really consistent with TRIGGER_SUSPEND in dai_suspend The TRIGGER_SUSPEND sequence is: pre_trigger() trigger() post_trigger() hda_link_dma_cleanup() In hda_dai_suspend() we only call the post_trigger() and the link cleanup, do it in the same order as it would have been during a trigger. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-dai.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index c61d298ea6b3a1..1c823f9eea5700 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -617,12 +617,6 @@ static int hda_dai_suspend(struct hdac_bus *bus) sdai = swidget->private; ops = sdai->platform_private; - ret = hda_link_dma_cleanup(hext_stream->link_substream, - hext_stream, - cpu_dai); - if (ret < 0) - return ret; - /* for consistency with TRIGGER_SUSPEND */ if (ops->post_trigger) { ret = ops->post_trigger(sdev, cpu_dai, @@ -631,6 +625,12 @@ static int hda_dai_suspend(struct hdac_bus *bus) if (ret < 0) return ret; } + + ret = hda_link_dma_cleanup(hext_stream->link_substream, + hext_stream, + cpu_dai); + if (ret < 0) + return ret; } } From 9a7c7ab02a93eebd71c027f03321514b56194814 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Tue, 11 Jun 2024 09:57:55 +0300 Subject: [PATCH 2/8] ASoC: SOF: Intel: hda-dai-ops: ipc4: Reset the paused counter on STOP/SUSPEND The same rule applies to the paused_count as for the started_count: on STOP/SUSPEND both needs to be reset to 0. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-dai-ops.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c index 484c761478853f..3fd6a3eb14c30c 100644 --- a/sound/soc/sof/intel/hda-dai-ops.c +++ b/sound/soc/sof/intel/hda-dai-ops.c @@ -416,10 +416,12 @@ static int hda_ipc4_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: /* - * STOP/SUSPEND trigger is invoked only once when all users of this pipeline have - * been stopped. So, clear the started_count so that the pipeline can be reset + * STOP/SUSPEND trigger is invoked only once when all users of + * this pipeline have been stopped. So, clear the started and + * paused count so that the pipeline can be reset */ swidget->spipe->started_count = 0; + swidget->spipe->paused_count = 0; break; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: break; From 365cd783b70bbc848be95fa07472b16122baacf7 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 10 Jun 2024 17:23:46 +0300 Subject: [PATCH 3/8] ASoC: SOF: Add new optional suspend_early() dsp_ops callback function The new suspend_early() can be used by platform code to prepare for the system suspend and execute tasks needed to be ready to handle the suspend call. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/ops.h | 8 ++++++++ sound/soc/sof/pm.c | 10 ++++++++++ sound/soc/sof/sof-priv.h | 1 + 3 files changed, 19 insertions(+) diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index 2584621c3b2d48..7f192e22d3a841 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -235,6 +235,14 @@ static inline int snd_sof_dsp_resume(struct snd_sof_dev *sdev) return 0; } +static inline int snd_sof_dsp_suspend_early(struct snd_sof_dev *sdev) +{ + if (sof_ops(sdev)->suspend_early) + return sof_ops(sdev)->suspend_early(sdev); + + return 0; +} + static inline int snd_sof_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state) { diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 8e3bcf602beb31..cfc707acc65454 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -210,6 +210,16 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) if (runtime_suspend && !sof_ops(sdev)->runtime_suspend) return 0; + /* Prepare the DSP for system suspend */ + if (!runtime_suspend) { + ret = snd_sof_dsp_suspend_early(sdev); + if (ret) { + dev_err(sdev->dev, "%s: DSP early suspend failed: %d\n", + __func__, ret); + return ret; + } + } + /* we need to tear down pipelines only if the DSP hardware is * active, which happens for PCI devices. if the device is * suspended, it is brought back to full power and then diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 4d6a1517f9b351..28a353054bcb63 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -301,6 +301,7 @@ struct snd_sof_dsp_ops { const struct sof_ext_man_elem_header *hdr); /* DSP PM */ + int (*suspend_early)(struct snd_sof_dev *sof_dev); /* optional */ int (*suspend)(struct snd_sof_dev *sof_dev, u32 target_state); /* optional */ int (*resume)(struct snd_sof_dev *sof_dev); /* optional */ From 5b5897aba377360bfa040c27d99fda723a56aeea Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Tue, 11 Jun 2024 08:15:36 +0300 Subject: [PATCH 4/8] ASoC: SOF: Intel: hda: Switch to use the suspend_early callback from set_hw_params_upon_resume The function 'hda_dsp_set_hw_params_upon_resume' has lost it's purpose as we are not remotely doing anything resembling what the function implies. The only thing we do is to stop any paused streams (paused streams will not receive suspend trigger and would block the suspend in hardware). The call sequence in pm.c is also wrong for the set_hw_params_upon_resume() since we would need to make sure that the link DMA is stopped _before_ we tear down the pipelines belonging to the paused stream. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-common-ops.c | 2 +- sound/soc/sof/intel/hda-dsp.c | 26 +++++++++++++------------- sound/soc/sof/intel/hda.h | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/sound/soc/sof/intel/hda-common-ops.c b/sound/soc/sof/intel/hda-common-ops.c index 5fc28039a8e825..1e9e2706f9240a 100644 --- a/sound/soc/sof/intel/hda-common-ops.c +++ b/sound/soc/sof/intel/hda-common-ops.c @@ -89,12 +89,12 @@ const struct snd_sof_dsp_ops sof_hda_common_ops = { .is_chain_dma_supported = hda_is_chain_dma_supported, /* PM */ + .suspend_early = hda_dsp_suspend_early, .suspend = hda_dsp_suspend, .resume = hda_dsp_resume, .runtime_suspend = hda_dsp_runtime_suspend, .runtime_resume = hda_dsp_runtime_resume, .runtime_idle = hda_dsp_runtime_idle, - .set_hw_params_upon_resume = hda_dsp_set_hw_params_upon_resume, /* ALSA HW info flags */ .hw_info = SNDRV_PCM_INFO_MMAP | diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 4c88522d404847..4cff6d895cada8 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -1007,6 +1007,19 @@ int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev) } EXPORT_SYMBOL_NS(hda_dsp_runtime_suspend, SND_SOC_SOF_INTEL_HDA_COMMON); +int hda_dsp_suspend_early(struct snd_sof_dev *sdev) +{ + int ret; + + /* make sure all DAI resources are freed */ + ret = hda_dsp_dais_suspend(sdev); + if (ret < 0) + dev_warn(sdev->dev, "%s: failure in hda_dsp_dais_suspend\n", __func__); + + return ret; +} +EXPORT_SYMBOL_NS(hda_dsp_suspend_early, SND_SOC_SOF_INTEL_HDA_COMMON); + int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; @@ -1148,19 +1161,6 @@ int hda_dsp_shutdown(struct snd_sof_dev *sdev) } EXPORT_SYMBOL_NS(hda_dsp_shutdown, SND_SOC_SOF_INTEL_HDA_COMMON); -int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) -{ - int ret; - - /* make sure all DAI resources are freed */ - ret = hda_dsp_dais_suspend(sdev); - if (ret < 0) - dev_warn(sdev->dev, "%s: failure in hda_dsp_dais_suspend\n", __func__); - - return ret; -} -EXPORT_SYMBOL_NS(hda_dsp_set_hw_params_upon_resume, SND_SOC_SOF_INTEL_HDA_COMMON); - void hda_dsp_d0i3_work(struct work_struct *work) { struct sof_intel_hda_dev *hdev = container_of(work, diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index b74a472435b5d2..a77164a8286116 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -605,6 +605,7 @@ int hda_dsp_set_power_state_ipc3(struct snd_sof_dev *sdev, int hda_dsp_set_power_state_ipc4(struct snd_sof_dev *sdev, const struct sof_dsp_power_state *target_state); +int hda_dsp_suspend_early(struct snd_sof_dev *sdev); int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state); int hda_dsp_resume(struct snd_sof_dev *sdev); int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev); @@ -612,7 +613,6 @@ int hda_dsp_runtime_resume(struct snd_sof_dev *sdev); int hda_dsp_runtime_idle(struct snd_sof_dev *sdev); int hda_dsp_shutdown_dma_flush(struct snd_sof_dev *sdev); int hda_dsp_shutdown(struct snd_sof_dev *sdev); -int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev); void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags); void hda_ipc4_dsp_dump(struct snd_sof_dev *sdev, u32 flags); void hda_ipc_dump(struct snd_sof_dev *sdev); From d897da66f8ed5bd75f02f41ae82029068535206e Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Tue, 11 Jun 2024 08:25:33 +0300 Subject: [PATCH 5/8] ASoC: SOF: Remove unused set_hw_params_upon_resume callback The set_hw_params_upon_resume() was used by Intel platforms. Over time it's purpose got changed and finally the Intel code is converted to use the suspend_early() callback instead. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/ops.h | 7 ------- sound/soc/sof/pm.c | 11 ----------- sound/soc/sof/sof-priv.h | 1 - 3 files changed, 19 deletions(-) diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index 7f192e22d3a841..66a66b9f96b1a6 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -276,13 +276,6 @@ static inline int snd_sof_dsp_runtime_idle(struct snd_sof_dev *sdev) return 0; } -static inline int snd_sof_dsp_hw_params_upon_resume(struct snd_sof_dev *sdev) -{ - if (sof_ops(sdev)->set_hw_params_upon_resume) - return sof_ops(sdev)->set_hw_params_upon_resume(sdev); - return 0; -} - static inline int snd_sof_dsp_set_clk(struct snd_sof_dev *sdev, u32 freq) { if (sof_ops(sdev)->set_clk) diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index cfc707acc65454..072d02484de232 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -231,17 +231,6 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) if (sdev->fw_state != SOF_FW_BOOT_COMPLETE) goto suspend; - /* prepare for streams to be resumed properly upon resume */ - if (!runtime_suspend) { - ret = snd_sof_dsp_hw_params_upon_resume(sdev); - if (ret < 0) { - dev_err(sdev->dev, - "error: setting hw_params flag during suspend %d\n", - ret); - return ret; - } - } - pm_state.event = target_state; /* suspend DMA trace */ diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 28a353054bcb63..0aff8cc6215eca 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -308,7 +308,6 @@ struct snd_sof_dsp_ops { int (*runtime_suspend)(struct snd_sof_dev *sof_dev); /* optional */ int (*runtime_resume)(struct snd_sof_dev *sof_dev); /* optional */ int (*runtime_idle)(struct snd_sof_dev *sof_dev); /* optional */ - int (*set_hw_params_upon_resume)(struct snd_sof_dev *sdev); /* optional */ int (*set_power_state)(struct snd_sof_dev *sdev, const struct sof_dsp_power_state *target_state); /* optional */ From 09d8c8a6b6de877afaeb7cd1b800048f2d4d20dc Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 10 Jun 2024 16:55:15 +0300 Subject: [PATCH 6/8] ASoC: SOF: sof-audio: Send STOP trigger to platforms during system suspend Introduce a new flag to mark a stream during system suspend and if it is set, send the STOP trigger for the platform driver to stop the DMA. This is needed in case the platform_stop_during_hw_free is not true (IPC3) and the suspend happens when a stream is in paused state. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/sof-audio.c | 3 ++- sound/soc/sof/sof-audio.h | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 9a52781bf8d8b9..4559adfd19c37d 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -838,7 +838,8 @@ int sof_pcm_stream_free(struct snd_sof_dev *sdev, struct snd_pcm_substream *subs if (spcm->prepared[substream->stream]) { /* stop DMA first if needed */ - if (pcm_ops && pcm_ops->platform_stop_during_hw_free) + if (spcm->stream[dir].suspending || + (pcm_ops && pcm_ops->platform_stop_during_hw_free)) snd_sof_pcm_platform_trigger(sdev, substream, SNDRV_PCM_TRIGGER_STOP); /* free PCM in the DSP */ diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 49be02234fc386..b9b063fc9deaa3 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -340,6 +340,9 @@ struct snd_sof_pcm_stream { bool suspend_ignored; struct snd_sof_pcm_stream_pipeline_list pipeline_list; + /* Flag to indicate that the stream is prepared for system suspend */ + bool suspending; + /* used by IPC implementation and core does not touch it */ void *private; }; From c3aa4bda91fd6c03fb9b806cc09f8201e025098c Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Tue, 11 Jun 2024 09:44:56 +0300 Subject: [PATCH 7/8] ASoC: SOF: ipc3-topology: Set the suspending flag for paused streams on system suspend The suspending flag will force the generic code to stop the platform DMA of the paused stream. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/ipc3-topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index be61e377e59e03..679e6a5cd5af8f 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -2379,7 +2379,9 @@ static int sof_tear_down_left_over_pipelines(struct snd_sof_dev *sdev) continue; if (spcm->stream[dir].list) { + spcm->stream[dir].suspending = true; ret = sof_pcm_stream_free(sdev, substream, spcm, dir, true); + spcm->stream[dir].suspending = false; if (ret < 0) return ret; } From 5c6fa89abec03cd5a0a15121945a0b8461d7d1a5 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Tue, 11 Jun 2024 09:53:50 +0300 Subject: [PATCH 8/8] ASoC: SOF: ipc4: Use the suspending stream flag to handle paused streams correctly During system suspend while we have paused streams we need to make sure that the pipelines are properly reset. Since the stream is in paused state, the pipelines are also in paused state, which is correct for the RESET transition. Reset the started/paused counters to indicate that the pipeline will need to be initialized after resume and PAUSE_RELEASE. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/ipc4-pcm.c | 27 +++++++++++++++++++++++---- sound/soc/sof/ipc4-topology.c | 2 ++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 4df2be3d39eba0..c17054d507970f 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -131,6 +131,7 @@ static void sof_ipc4_add_pipeline_by_priority(struct ipc4_pipeline_set_state_dat static void sof_ipc4_add_pipeline_to_trigger_list(struct snd_sof_dev *sdev, int state, + bool suspending, struct snd_sof_pipeline *spipe, struct ipc4_pipeline_set_state_data *trigger_list, s8 *pipe_priority) @@ -152,6 +153,18 @@ sof_ipc4_add_pipeline_to_trigger_list(struct snd_sof_dev *sdev, int state, false); break; case SOF_IPC4_PIPE_RESET: + if (suspending) { + if (pipeline->state != SOF_IPC4_PIPE_PAUSED) + dev_warn(sdev->dev, + "%s: %s is not in PAUSED state" + "(state: %d, started/paused count: %d/%d)\n", + __func__, pipe_widget->widget->name, + pipeline->state, spipe->started_count, spipe->paused_count); + + spipe->started_count = 0; + spipe->paused_count = 0; + } + /* RESET if the pipeline is neither running nor paused */ if (!spipe->started_count && !spipe->paused_count) sof_ipc4_add_pipeline_by_priority(trigger_list, pipe_widget, pipe_priority, @@ -436,18 +449,24 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, * indeterministic. But the sink->source trigger order sink->source would still be * guaranteed for each fork independently. */ - if (state == SOF_IPC4_PIPE_RUNNING || state == SOF_IPC4_PIPE_RESET) + if (state == SOF_IPC4_PIPE_RUNNING || state == SOF_IPC4_PIPE_RESET) { + bool suspending = spcm->stream[substream->stream].suspending; + for (i = pipeline_list->count - 1; i >= 0; i--) { spipe = pipeline_list->pipelines[i]; - sof_ipc4_add_pipeline_to_trigger_list(sdev, state, spipe, trigger_list, + sof_ipc4_add_pipeline_to_trigger_list(sdev, state, + suspending, spipe, + trigger_list, pipe_priority); } - else + } else { for (i = 0; i < pipeline_list->count; i++) { spipe = pipeline_list->pipelines[i]; - sof_ipc4_add_pipeline_to_trigger_list(sdev, state, spipe, trigger_list, + sof_ipc4_add_pipeline_to_trigger_list(sdev, state, false, + spipe, trigger_list, pipe_priority); } + } /* return if all pipelines are in the requested state already */ if (!trigger_list->count) { diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index d123edfa3bae59..ac8dcb891490b4 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -3278,7 +3278,9 @@ static int sof_ipc4_tear_down_all_pipelines(struct snd_sof_dev *sdev, bool verif continue; if (spcm->stream[dir].list) { + spcm->stream[dir].suspending = true; ret = sof_pcm_stream_free(sdev, substream, spcm, dir, true); + spcm->stream[dir].suspending = false; if (ret < 0) return ret; }