ASoC: SOF: ipc3-topology: Fix pipeline tear down logic
Subsystems (3)
Peter Ujfalusi
maintainerPATCH 1/1 ASoC: SOF: ipc3-topology: Fix pipeline tear down logic
From: Ranjani Sridharan <[email protected]>
With the change in the widget free logic to power down the cores only when the scheduler widgets are freed, we need to ensure that the scheduler widget is freed only after all the widgets associated with the scheduler are freed. This is to ensure that the secondary core that the scheduler is scheduled to run on is kept powered on until all widgets that need them are in use. While this works well for dynamic pipelines, in the case of static pipelines the current logic does not take this into account and frees all widgets in the order they occur in the widget_list. So, modify this to ensure that the scheduler widgets are freed only after all other types of widgets in the widget_list are freed.
---
Hi Mark,
can you send this for 6.8 as a regression fix?
Thank you,
Peter
sound/soc/sof/ipc3-topology.c | 55 ++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 14 deletions(-)
sound/soc/sof/ipc3-topology.c +41 -14
@@ -2400,27 +2400,16 @@ static int sof_tear_down_left_over_pipelines(struct snd_sof_dev *sdev)return 0 ; } -/* -* For older firmware, this function doesn't free widgets for static pipelines during suspend. -* It only resets use_count for all widgets. -*/ -static int sof_ipc3_tear_down_all_pipelines (struct snd_sof_dev *sdev,bool verify) +static int sof_ipc3_free_widgets_in_list (struct snd_sof_dev *sdev,bool include_scheduler, +bool *dyn_widgets,bool verify) {struct sof_ipc_fw_version *v = &sdev->fw_ready.version;struct snd_sof_widget *swidget; -struct snd_sof_route *sroute; -bool dyn_widgets = false;int ret; -/* -* 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 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) { - dyn_widgets = true; + *dyn_widgets = true;continue ; }
@@ -2435,11 +2424,49 @@ static int sof_ipc3_tear_down_all_pipelines(struct snd_sof_dev *sdev, bool verifcontinue ; } +if (include_scheduler && swidget->id != snd_soc_dapm_scheduler) +continue ; + +if (!include_scheduler && swidget->id == snd_soc_dapm_scheduler) +continue ; + 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. +*/ +static int sof_ipc3_tear_down_all_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; +bool dyn_widgets = false; +int ret; + +/* +* 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 running streams are suspended and during +* topology loading the sound card unavailable to open PCMs. Do not free the scheduler +* widgets yet so that the secondary cores do not get powered down before all the widgets +* associated with the scheduler are freed. +*/ + ret =sof_ipc3_free_widgets_in_list (sdev, false, &dyn_widgets, verify); +if (ret <0 ) +return ret; + +/* free all the scheduler widgets now */ + ret =sof_ipc3_free_widgets_in_list (sdev, true, &dyn_widgets, verify); +if (ret <0 ) +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.
Signature · 1 line
2.43.0
Mark Brown
maintainerQuoted 11 lines · Peter Ujfalusi
With the change in the widget free logic to power down the cores only when the scheduler widgets are freed, we need to ensure that the scheduler widget is freed only after all the widgets associated with the scheduler are freed. This is to ensure that the secondary core that the scheduler is scheduled to run on is kept powered on until all widgets that need them are in use. While this works well for dynamic pipelines, in the case of static pipelines the current logic does not take this into account and frees all widgets in the order they occur in the
widget_list. So, modify this to ensure that the scheduler widgets are freed only after all other types of widgets in thewidget_listare freed.
Quoted 1 line
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: SOF: ipc3-topology: Fix pipeline tear down logic
commit: d7332c4a4f1a7d16f054c6357fb65c597b6a86a7
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