[PATCH v2 1/3] ASoC: SOF: sof-audio: setup sched widgets during pipeline complete step
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Older firmware prior to ABI 3.19 has a dependency where the scheduler widgets need to be setup last. Moving the call to sof_widget_setup() before the pipeline_complete() call also helps remove the need for the 'reverse' direction when walking through the widget list - this was only working because of the topology macros but the topology does not require any order.
Fixes: 5fcdbb2d45df ("ASoC: SOF: Add support for dynamic pipelines") Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/sof-audio.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 0f2566f7c094..f4e142ec0fbd 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -637,16 +637,25 @@ const struct sof_ipc_pipe_new *snd_sof_pipeline_find(struct snd_sof_dev *sdev,
int sof_set_up_pipelines(struct snd_sof_dev *sdev, bool verify) { + struct sof_ipc_fw_version *v = &sdev->fw_ready.version; struct snd_sof_widget *swidget; struct snd_sof_route *sroute; int ret;
/* restore pipeline components */ - list_for_each_entry_reverse(swidget, &sdev->widget_list, list) { + list_for_each_entry(swidget, &sdev->widget_list, list) { /* only set up the widgets belonging to static pipelines */ if (!verify && swidget->dynamic_pipeline_widget) continue;
+ /* + * For older firmware, skip scheduler widgets in this loop, + * sof_widget_setup() will be called in the 'complete pipeline' loop + */ + if (v->abi_version < SOF_ABI_VER(3, 19, 0) && + swidget->id == snd_soc_dapm_scheduler) + continue; + /* update DAI config. The IPC will be sent in sof_widget_setup() */ if (WIDGET_IS_DAI(swidget->id)) { struct snd_sof_dai *dai = swidget->private; @@ -694,6 +703,12 @@ int sof_set_up_pipelines(struct snd_sof_dev *sdev, bool verify) if (!verify && swidget->dynamic_pipeline_widget) continue;
+ if (v->abi_version < SOF_ABI_VER(3, 19, 0)) { + ret = sof_widget_setup(sdev, swidget); + if (ret < 0) + return ret; + } + swidget->complete = snd_sof_complete_pipeline(sdev, swidget); break; @@ -722,7 +737,7 @@ int sof_tear_down_pipelines(struct snd_sof_dev *sdev, bool verify) * sroute->setup because during suspend all streams are suspended and during topology * loading the sound card unavailable to open PCMs. */ - list_for_each_entry_reverse(swidget, &sdev->widget_list, list) { + list_for_each_entry(swidget, &sdev->widget_list, list) { if (swidget->dynamic_pipeline_widget) continue;
base-commit: 65c16dd2942f4476a3f96a2625b1475c6137c09a
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
It's not clear why we would walk the list backwards. That makes no difference.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 63948bb30710..b3ad3a604918 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -3529,7 +3529,7 @@ static int sof_complete(struct snd_soc_component *scomp) * Apply the dynamic_pipeline_widget flag and set the pipe_widget field * for all widgets that have the same pipeline ID as the scheduler widget */ - list_for_each_entry_reverse(comp_swidget, &sdev->widget_list, list) + list_for_each_entry(comp_swidget, &sdev->widget_list, list) if (comp_swidget->pipeline_id == swidget->pipeline_id) { ret = sof_set_pipe_widget(sdev, swidget, comp_swidget); if (ret < 0)
On Tue, Nov 23, 2021 at 07:16:05PM +0200, Kai Vehmanen wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
It's not clear why we would walk the list backwards. That makes no difference.
Cleanup patches like this should come after fixes rather than in the middle of them to ensure that the fixes can be applied as such.
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
During system suspend, paused streams do not get suspended. Therefore, we need to explicitly free these PCMs in the DSP and free the associated DAPM widgets so that they can be set up again during resume.
Fixes: 5fcdbb2d45df ("ASoC: SOF: Add support for dynamic pipelines") Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Paul Olaru paul.olaru@oss.nxp.com Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/pcm.c | 5 ++- sound/soc/sof/sof-audio.c | 74 +++++++++++++++++++++++++++++++++++++-- sound/soc/sof/sof-audio.h | 2 ++ 3 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 31dd79b794f1..0ceb1a9cbf73 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -100,9 +100,8 @@ void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream) } EXPORT_SYMBOL(snd_sof_pcm_period_elapsed);
-static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, - struct snd_sof_dev *sdev, - struct snd_sof_pcm *spcm) +int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, struct snd_sof_dev *sdev, + struct snd_sof_pcm *spcm) { struct sof_ipc_stream stream; struct sof_ipc_reply reply; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index f4e142ec0fbd..e00ce275052f 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -129,6 +129,14 @@ int sof_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget) case snd_soc_dapm_buffer: ipc_free.hdr.cmd |= SOF_IPC_TPLG_BUFFER_FREE; break; + case snd_soc_dapm_dai_in: + case snd_soc_dapm_dai_out: + { + struct snd_sof_dai *dai = swidget->private; + + dai->configured = false; + fallthrough; + } default: ipc_free.hdr.cmd |= SOF_IPC_TPLG_COMP_FREE; break; @@ -720,6 +728,55 @@ int sof_set_up_pipelines(struct snd_sof_dev *sdev, bool verify) return 0; }
+/* + * Free the PCM, its associated widgets and set the prepared flag to false for all PCMs that + * did not get suspended(ex: paused streams) so the widgets can be set up again during resume. + */ +static int sof_tear_down_left_over_pipelines(struct snd_sof_dev *sdev) +{ + struct snd_sof_widget *swidget; + struct snd_sof_pcm *spcm; + int dir, ret; + + /* + * free all PCMs and their associated DAPM widgets if their connected DAPM widget + * list is not NULL. This should only be true for paused streams at this point. + * This is equivalent to the handling of FE DAI suspend trigger for running streams. + */ + list_for_each_entry(spcm, &sdev->pcm_list, list) + for_each_pcm_streams(dir) { + struct snd_pcm_substream *substream = spcm->stream[dir].substream; + + if (!substream || !substream->runtime) + continue; + + if (spcm->stream[dir].list) { + ret = sof_pcm_dsp_pcm_free(substream, sdev, spcm); + if (ret < 0) + return ret; + + ret = sof_widget_list_free(sdev, spcm, dir); + if (ret < 0) { + dev_err(sdev->dev, "failed to free widgets during suspend\n"); + return ret; + } + } + } + + /* + * free any left over DAI widgets. This is equivalent to the handling of suspend trigger + * for the BE DAI for running streams. + */ + list_for_each_entry(swidget, &sdev->widget_list, list) + if (WIDGET_IS_DAI(swidget->id) && swidget->use_count == 1) { + ret = sof_widget_free(sdev, swidget); + if (ret < 0) + return ret; + } + + return 0; +} + /* * For older firmware, this function doesn't free widgets for static pipelines during suspend. * It only resets use_count for all widgets. @@ -734,8 +791,8 @@ int sof_tear_down_pipelines(struct snd_sof_dev *sdev, bool verify) /* * This function is called during suspend and for one-time topology verification during * first boot. In both cases, there is no need to protect swidget->use_count and - * sroute->setup because during suspend all streams are suspended and during topology - * loading the sound card unavailable to open PCMs. + * sroute->setup because during suspend all running streams are suspended and during + * topology loading the sound card unavailable to open PCMs. */ list_for_each_entry(swidget, &sdev->widget_list, list) { if (swidget->dynamic_pipeline_widget) @@ -754,6 +811,19 @@ int sof_tear_down_pipelines(struct snd_sof_dev *sdev, bool verify) return ret; }
+ /* + * Tear down all pipelines associated with PCMs that did not get suspended + * and unset the prepare flag so that they can be set up again during resume. + * Skip this step for older firmware. + */ + if (!verify && v->abi_version >= SOF_ABI_VER(3, 19, 0)) { + ret = sof_tear_down_left_over_pipelines(sdev); + if (ret < 0) { + dev_err(sdev->dev, "failed to tear down paused pipelines\n"); + return ret; + } + } + list_for_each_entry(sroute, &sdev->route_list, list) sroute->setup = false;
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 389d56ac3aba..1c4f59d34717 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -265,4 +265,6 @@ int sof_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget); /* PCM */ int sof_widget_list_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, int dir); int sof_widget_list_free(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, int dir); +int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, struct snd_sof_dev *sdev, + struct snd_sof_pcm *spcm); #endif
On Tue, Nov 23, 2021 at 07:16:04PM +0200, Kai Vehmanen wrote:
Fixes: 5fcdbb2d45df ("ASoC: SOF: Add support for dynamic pipelines") Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This doesn't actually apply as a fix FWIW, I've queued it for -next.
Hi,
On Wed, 24 Nov 2021, Mark Brown wrote:
On Tue, Nov 23, 2021 at 07:16:04PM +0200, Kai Vehmanen wrote:
Fixes: 5fcdbb2d45df ("ASoC: SOF: Add support for dynamic pipelines") Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This doesn't actually apply as a fix FWIW, I've queued it for -next.
ack, this won't apply on top of for-5.16. The dependencies around the SOF dynamic pipelines (and related code) are unfortunately rather complicated with many recent changes. I now sent these on top of for-5.17/65c16dd2942 as baseline.
Given the limited use of dynamic pipelines in current SOF topologies, we should be good for 5.16 even if these are left out. The Fixes tags should still be useful for anyone backporting patches to older kernels.
Br, Kai
On Tue, 23 Nov 2021 19:16:04 +0200, Kai Vehmanen wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Older firmware prior to ABI 3.19 has a dependency where the scheduler widgets need to be setup last. Moving the call to sof_widget_setup() before the pipeline_complete() call also helps remove the need for the 'reverse' direction when walking through the widget list - this was only working because of the topology macros but the topology does not require any order.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ASoC: SOF: sof-audio: setup sched widgets during pipeline complete step commit: 01429183f479c54c1b5d15453a8ce574ea43e525 [2/3] ASoC: SOF: topology: don't use list_for_each_entry_reverse() commit: fb71d03b29bcbd8c03798d36e7b2a2297b6dea45 [3/3] ASoC: SOF: handle paused streams during system suspend commit: 96da174024b9c63bd5d3358668d0bc12677be877
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)
-
Kai Vehmanen -
Mark Brown