[PATCH v2 0/3] ASoC: SOF: Fixes for suspend after firmware crash
Hi,
This is the followup series for the v1 sent out by Ranjani [1]. Unfortunately Ranjani was dragged away to another issue and could not send the update herself.
Changes since v1: - In patch 2, move the tear_down_all_pipelines call instead of duplicating it
Amadeusz: I have kept the check as it is: if (tplg_ops && tplg_ops->tear_down_all_pipelines) I'm preparing the ops optionality change series which would require this change.
[1] https://lore.kernel.org/alsa-devel/20221215185347.1457541-1-ranjani.sridhara...
Regards, Peter --- Curtis Malainey (1): ASoC: SOF: Add FW state to debugfs
Ranjani Sridharan (2): ASoC: SOF: pm: Set target state earlier ASoC: SOF: pm: Always tear down pipelines before DSP suspend
sound/soc/sof/debug.c | 4 +++- sound/soc/sof/pm.c | 9 ++++----- 2 files changed, 7 insertions(+), 6 deletions(-)
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
If the DSP crashes before the system suspends, the setting of target state will be skipped because the firmware state will no longer be SOF_FW_BOOT_COMPLETE. This leads to the incorrect assumption that the DSP should suspend to D0I3 instead of suspending to D3. To fix this, set the target_state before we skip to DSP suspend even when the DSP has crashed.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Curtis Malainey cujomalainey@chromium.org Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/pm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index df740be645e8..5f88c4a01fa3 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -182,7 +182,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) const struct sof_ipc_pm_ops *pm_ops = sdev->ipc->ops->pm; const struct sof_ipc_tplg_ops *tplg_ops = sdev->ipc->ops->tplg; pm_message_t pm_state; - u32 target_state = 0; + u32 target_state = snd_sof_dsp_power_target(sdev); int ret;
/* do nothing if dsp suspend callback is not set */ @@ -206,7 +206,6 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) } }
- target_state = snd_sof_dsp_power_target(sdev); pm_state.event = target_state;
/* Skip to platform-specific suspend if DSP is entering D0 */
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
When the DSP is suspended while the firmware is in the crashed state, we skip tearing down the pipelines. This means that the widget reference counts will not get to reset to 0 before suspend. This will lead to errors with resuming audio after system resume. To fix this, invoke the tear_down_all_pipelines op before skipping to DSP suspend.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Curtis Malainey cujomalainey@chromium.org Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/pm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 5f88c4a01fa3..8722bbd7fd3d 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -192,6 +192,9 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) if (runtime_suspend && !sof_ops(sdev)->runtime_suspend) return 0;
+ if (tplg_ops && tplg_ops->tear_down_all_pipelines) + tplg_ops->tear_down_all_pipelines(sdev, false); + if (sdev->fw_state != SOF_FW_BOOT_COMPLETE) goto suspend;
@@ -216,9 +219,6 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) goto suspend; }
- if (tplg_ops->tear_down_all_pipelines) - tplg_ops->tear_down_all_pipelines(sdev, false); - /* suspend DMA trace */ sof_fw_trace_suspend(sdev, pm_state);
From: Curtis Malainey cujomalainey@chromium.org
Allow system health detection mechanisms to check the FW state, this will allow them to check if the FW is in its "crashed" state going forward to help automatically diagnose driver state.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/debug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index d9a3ce7b69e1..ade0507328af 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -353,7 +353,9 @@ int snd_sof_dbg_init(struct snd_sof_dev *sdev) return err; }
- return 0; + return snd_sof_debugfs_buf_item(sdev, &sdev->fw_state, + sizeof(sdev->fw_state), + "fw_state", 0444); } EXPORT_SYMBOL_GPL(snd_sof_dbg_init);
On Tue, 20 Dec 2022 14:56:26 +0200, Peter Ujfalusi wrote:
This is the followup series for the v1 sent out by Ranjani [1]. Unfortunately Ranjani was dragged away to another issue and could not send the update herself.
Changes since v1:
- In patch 2, move the tear_down_all_pipelines call instead of duplicating it
Amadeusz: I have kept the check as it is: if (tplg_ops && tplg_ops->tear_down_all_pipelines) I'm preparing the ops optionality change series which would require this change.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ASoC: SOF: pm: Set target state earlier commit: 6f95eec6fb89e195dbdf30de65553c7fc57d9372 [2/3] ASoC: SOF: pm: Always tear down pipelines before DSP suspend commit: d185e0689abc98ef55fb7a7d75aa0c48a0ed5838 [3/3] ASoC: SOF: Add FW state to debugfs commit: 9a9134fd56f6ba614ff7b2b3b0bac0bf1d0dc0c9
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
participants (2)
-
Mark Brown
-
Peter Ujfalusi