| Message ID | 20221215185347.1457541-3-ranjani.sridharan@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: SOF: Fixes for suspend after firmware crash | expand |
On 12/15/2022 7:53 PM, Ranjani Sridharan wrote: > 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> > --- > sound/soc/sof/pm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c > index 5f88c4a01fa3..f153881db189 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; > Can tplg_ops even be null? Rest of SOF code seems to skip this check and only check for callback presence. Also won't tearing down pipelines few lines later become unnecessary then? https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/sof/pm.c?id=a12a383e59ce486abd719b6bda33c353a3b385e7#n220
On 16/12/2022 11:06, Amadeusz Sławiński wrote: > On 12/15/2022 7:53 PM, Ranjani Sridharan wrote: >> 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> >> --- >> sound/soc/sof/pm.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c >> index 5f88c4a01fa3..f153881db189 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; >> > > Can tplg_ops even be null? Rest of SOF code seems to skip this check and > only check for callback presence. We have another series not yet sent which will re-visit these ops and their optionality. This patch was created on top of that work in SOF's sof-dev branch. > Also won't tearing down pipelines few lines later become unnecessary then? > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/sof/pm.c?id=a12a383e59ce486abd719b6bda33c353a3b385e7#n220 Good catch. The original patch in sof-dev did the move: https://github.com/thesofproject/linux/commit/62c1ccce6c6514a3ff8590156fbd7fff9d24586f
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 5f88c4a01fa3..f153881db189 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;