[BUG] refactoring breaks sof-rt5682 audio on chromebook; successfully bisected #4807

135e2 · 2024-02-06T08:33:10Z

As described in the kernel mailing list, commit 31ed8da (ASoC: SOF: sof-audio:
Modify logic for enabling/disabling topology cores) seems to break audio on tigerlake chromebooks with soundcard sof-rt5682.

alsa-info and dmesg log pasted below (w/ dynamic debugging enabled), for both the working version and the broken version:

revert 31ed8da, working

alsa-info
full dmesg

vanilla linux-git kernel, no sound

alsa-info
full-dmesg

changes suggested in the email, still no sound

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index e693dcb475e4..a16dee8ee704 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -221,7 +221,7 @@ static int sof_widget_setup_unlocked(struct
snd_sof_dev *sdev,
 pipe_widget_free:
        if (swidget->id != snd_soc_dapm_scheduler) {
                sof_widget_free_unlocked(sdev, swidget->spipe->pipe_widget);
-       } else {
+       } else if (!use_count_decremented) {
                int j;

                /* decrement ref count for all cores that were updated previously */

alsa-info
full-dmesg

Guess I'll also leave a link to the original issue here, maybe the ucm confs wasn't configured correctly either:

WeirdTreeThing/chromebook-linux-audio#70
https://github.com/WeirdTreeThing/chromebook-ucm-conf/tree/main/sof-rt5682

I don't really know much about the ALSA nor the linux sound subsystem, so apologize in advance if I did something wrong or if there're things can help further debugging, please let me know.

The text was updated successfully, but these errors were encountered:

ujfalusi · 2024-02-06T13:52:09Z

@135e2, thank you for the logs, I see where the failure is:
working:

snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PGA5.0 freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PCM2P freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30110000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PIPELINE.4.DMIC0.IN freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget DMIC0.IN freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30210000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget BUF4.0 freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x40070000
snd_sof:snd_sof_dsp_core_put: sof-audio-pci-intel-tgl 0000:00:1f.3: Core 1 powered down
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PCM99C freed

fail:

snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PGA5.0 freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PCM2P freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30110000
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x40070000
snd_sof:snd_sof_dsp_core_put: sof-audio-pci-intel-tgl 0000:00:1f.3: Core 1 powered down
snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget PIPELINE.4.DMIC0.IN freed
snd_sof:ipc3_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x30020000
sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx error for 0x30020000 (msg/reply size: 12/0): -13
sof-audio-pci-intel-tgl 0000:00:1f.3: failed to free widget DMIC0.IN

Core1 is shut down too early while we still have widgets on the pipeline and the scheduler widget is in the middle of the path.
@ranj063, @plbossart, any quick idea?

ujfalusi · 2024-02-06T14:53:17Z

The topology used is sof-tgl-max98373-rt5682.tplg

ujfalusi · 2024-02-06T14:54:08Z

@135e2, can this patch help in your setup:
0001-ASoC-SOF-sof-audio-Manage-core-refcount-per-widget-a.patch.txt

plbossart · 2024-02-06T14:54:15Z

where does this topology come from? sof-bin or the Chrome overlays?

ranj063 · 2024-02-06T16:41:05Z

@135e2, can this patch help in your setup: 0001-ASoC-SOF-sof-audio-Manage-core-refcount-per-widget-a.patch.txt

@ujfalusi I think I know the problem. This problem arises in the case of static pipelines where the widgets are freed in the order they're in the widget_list and in this case it so happens that the pipeline widget gets freed before the widgets in the pipeline themselves. Let me think a bit more about how to fix it gracefully.

ranj063 · 2024-02-06T16:45:05Z

@135e2 can you please try this diff and see if it fixes your issue?

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index 0970dbdfa78a..b1bb287a20b2 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2435,9 +2435,40 @@ static int sof_ipc3_tear_down_all_pipelines(struct snd_sof_dev *sdev, bool verif
                        continue;
                }
 
-               ret = sof_widget_free(sdev, swidget);
-               if (ret < 0)
-                       return ret;
+               /*
+                * do not free the scheduler widgets yet so that the secondary cores do not get
+                * powered down before all the widgets associated with the widgets are freed.
+                */
+               if (swidget->id != snd_soc_dapm_scheduler) {
+                       ret = sof_widget_free(sdev, swidget);
+                       if (ret < 0)
+                               return ret;
+               }
+       }
+
+       /* free the scheduler widgets now */
+       list_for_each_entry(swidget, &sdev->widget_list, list) {
+               if (swidget->dynamic_pipeline_widget) {
+                       dyn_widgets = true;
+                       continue;
+               }
+
+               /* Do not free widgets for static pipelines with FW older than SOF2.2 */
+               if (!verify && !swidget->dynamic_pipeline_widget &&
+                   SOF_FW_VER(v->major, v->minor, v->micro) < SOF_FW_VER(2, 2, 0)) {
+                       mutex_lock(&swidget->setup_mutex);
+                       swidget->use_count = 0;
+                       mutex_unlock(&swidget->setup_mutex);
+                       if (swidget->spipe)
+                               swidget->spipe->complete = 0;
+                       continue;
+               }
+
+               if (swidget->id == snd_soc_dapm_scheduler) {
+                       ret = sof_widget_free(sdev, swidget);
+                       if (ret < 0)
+                               return ret;
+               }
        }
 
        /*

plbossart · 2024-02-06T18:58:32Z

@ranj063 why would we have a static pipeline? Haven't we moved all topologies to dynamic stuff?

ranj063 · 2024-02-06T21:37:56Z

@ranj063 why would we have a static pipeline? Haven't we moved all topologies to dynamic stuff?

@plbossart this is a TGL chromebook and those still use static pipelines

135e2 · 2024-02-07T03:18:02Z

where does this topology come from? sof-bin or the Chrome overlays?

this is a TGL chromebook and those still use static pipelines

Yep, it's taken directly from sof-bin.

@135e2 can you please try this diff and see if it fixes your issue?

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index 0970dbdfa78a..b1bb287a20b2 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2435,9 +2435,40 @@ static int sof_ipc3_tear_down_all_pipelines(struct snd_sof_dev *sdev, bool verif
                        continue;
                }
 
-               ret = sof_widget_free(sdev, swidget);
-               if (ret < 0)
-                       return ret;
+               /*
+                * do not free the scheduler widgets yet so that the secondary cores do not get
+                * powered down before all the widgets associated with the widgets are freed.
+                */
+               if (swidget->id != snd_soc_dapm_scheduler) {
+                       ret = sof_widget_free(sdev, swidget);
+                       if (ret < 0)
+                               return ret;
+               }
+       }
+
+       /* free the scheduler widgets now */
+       list_for_each_entry(swidget, &sdev->widget_list, list) {
+               if (swidget->dynamic_pipeline_widget) {
+                       dyn_widgets = true;
+                       continue;
+               }
+
+               /* Do not free widgets for static pipelines with FW older than SOF2.2 */
+               if (!verify && !swidget->dynamic_pipeline_widget &&
+                   SOF_FW_VER(v->major, v->minor, v->micro) < SOF_FW_VER(2, 2, 0)) {
+                       mutex_lock(&swidget->setup_mutex);
+                       swidget->use_count = 0;
+                       mutex_unlock(&swidget->setup_mutex);
+                       if (swidget->spipe)
+                               swidget->spipe->complete = 0;
+                       continue;
+               }
+
+               if (swidget->id == snd_soc_dapm_scheduler) {
+                       ret = sof_widget_free(sdev, swidget);
+                       if (ret < 0)
+                               return ret;
+               }
        }
 
        /*

Can confirm this patch fixes the issue. I'll be pasting full dmesg log here just in case.

patch-3-dmesg.txt

ranj063 · 2024-02-07T05:13:00Z

where does this topology come from? sof-bin or the Chrome overlays?

this is a TGL chromebook and those still use static pipelines

Yep, it's taken directly from sof-bin.

@135e2 can you please try this diff and see if it fixes your issue?

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index 0970dbdfa78a..b1bb287a20b2 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2435,9 +2435,40 @@ static int sof_ipc3_tear_down_all_pipelines(struct snd_sof_dev *sdev, bool verif
                        continue;
                }
 
-               ret = sof_widget_free(sdev, swidget);
-               if (ret < 0)
-                       return ret;
+               /*
+                * do not free the scheduler widgets yet so that the secondary cores do not get
+                * powered down before all the widgets associated with the widgets are freed.
+                */
+               if (swidget->id != snd_soc_dapm_scheduler) {
+                       ret = sof_widget_free(sdev, swidget);
+                       if (ret < 0)
+                               return ret;
+               }
+       }
+
+       /* free the scheduler widgets now */
+       list_for_each_entry(swidget, &sdev->widget_list, list) {
+               if (swidget->dynamic_pipeline_widget) {
+                       dyn_widgets = true;
+                       continue;
+               }
+
+               /* Do not free widgets for static pipelines with FW older than SOF2.2 */
+               if (!verify && !swidget->dynamic_pipeline_widget &&
+                   SOF_FW_VER(v->major, v->minor, v->micro) < SOF_FW_VER(2, 2, 0)) {
+                       mutex_lock(&swidget->setup_mutex);
+                       swidget->use_count = 0;
+                       mutex_unlock(&swidget->setup_mutex);
+                       if (swidget->spipe)
+                               swidget->spipe->complete = 0;
+                       continue;
+               }
+
+               if (swidget->id == snd_soc_dapm_scheduler) {
+                       ret = sof_widget_free(sdev, swidget);
+                       if (ret < 0)
+                               return ret;
+               }
        }
 
        /*

Can confirm this patch fixes the issue. I'll be pasting full dmesg log here just in case.

patch-3-dmesg.txt

@135e2 thanks for testing. I will post a pull request with the fix soon.

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. Link: thesofproject#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2024-02-07T17:16:38Z

@135e2 here's the PR: #4808

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. Link: thesofproject#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

plbossart · 2024-02-07T18:48:22Z

Thanks indeed for @135e2 for the bug report and bisect results, much appreciated!

ujfalusi · 2024-02-08T06:56:40Z

@135e2, can you please try the patch from #4808 to make sure that we have the right fix

135e2 · 2024-02-08T11:00:49Z

@135e2, can you please try the patch from #4808 to make sure that we have the right fix

Yes, just tested and it works like a charm.

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. Link: #4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ujfalusi · 2024-02-08T13:36:06Z

patch upstream: https://lore.kernel.org/linux-sound/20240208133432.1688-1-peter.ujfalusi@linux.intel.com/

ujfalusi · 2024-02-08T13:36:35Z

@135e2, thank you for the detailed report!

ujfalusi · 2024-02-08T13:38:05Z

@ranj063, thank you for the quick fix!

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. Link: thesofproject/linux#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>

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. Link: #4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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>

[ Upstream commit d7332c4 ] 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. Link: thesofproject/linux#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>

[ Upstream commit d7332c4 ] 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. Link: thesofproject#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>

[ Upstream commit d7332c4 ] 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. Link: thesofproject/linux#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>

[ Upstream commit d7332c4 ] 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. Link: thesofproject#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>

[ Upstream commit d7332c4 ] 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. Link: thesofproject/linux#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>

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. Link: thesofproject/linux#4807 Fixes: 31ed8da1c8e5 ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> (cherry picked from commit d7332c4a4f1a7d16f054c6357fb65c597b6a86a7) BUG=b:266064196 TEST=Test Audio use cases Signed-off-by: Linux Patches Robot <linux-patches-robot@chromeos-missing-patches.google.com.iam.gserviceaccount.com> Change-Id: Ief7f9acc7c77d8beaf7f68639aaf9b6f4f7ba295 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/5304333 Reviewed-by: Sean Paul <sean@poorly.run> Reviewed-by: Terry Cheong <htcheong@chromium.org> Commit-Queue: Terry Cheong <htcheong@chromium.org>

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. Link: thesofproject/linux#4807 Fixes: 31ed8da1c8e5 ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> (cherry picked from commit d7332c4a4f1a7d16f054c6357fb65c597b6a86a7) BUG=None TEST=None Signed-off-by: Linux Patches Robot <linux-patches-robot@chromeos-missing-patches.google.com.iam.gserviceaccount.com> Change-Id: Ief7f9acc7c77d8beaf7f68639aaf9b6f4f7ba295 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/5304759 Reviewed-by: Curtis Malainey <cujomalainey@chromium.org> Reviewed-by: Sean Paul <sean@poorly.run> Commit-Queue: Curtis Malainey <cujomalainey@chromium.org>

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. Link: thesofproject/linux#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>

BugLink: https://bugs.launchpad.net/bugs/2061814 [ Upstream commit d7332c4 ] 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. Link: thesofproject/linux#4807 Fixes: 31ed8da ("ASoC: SOF: sof-audio: Modify logic for enabling/disabling topology cores") Signed-off-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> Link: https://lore.kernel.org/r/20240208133432.1688-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Portia Stephens <portia.stephens@canonical.com> Signed-off-by: Roxana Nicolescu <roxana.nicolescu@canonical.com>

135e2 mentioned this issue Feb 6, 2024

linux-6.7rc5 breaks sof-rt5682 WeirdTreeThing/chromebook-linux-audio#70

Closed

ranj063 mentioned this issue Feb 7, 2024

ASoC: SOF: ipc3-topology: Fix pipeline tear down logic #4808

Merged

ujfalusi closed this as completed in #4808 Feb 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] refactoring breaks sof-rt5682 audio on chromebook; successfully bisected #4807

[BUG] refactoring breaks sof-rt5682 audio on chromebook; successfully bisected #4807

135e2 commented Feb 6, 2024

ujfalusi commented Feb 6, 2024 •

edited

Loading

ujfalusi commented Feb 6, 2024

ujfalusi commented Feb 6, 2024

plbossart commented Feb 6, 2024

ranj063 commented Feb 6, 2024

ranj063 commented Feb 6, 2024 •

edited

Loading

plbossart commented Feb 6, 2024

ranj063 commented Feb 6, 2024

135e2 commented Feb 7, 2024

ranj063 commented Feb 7, 2024

ranj063 commented Feb 7, 2024

plbossart commented Feb 7, 2024

ujfalusi commented Feb 8, 2024 •

edited

Loading

135e2 commented Feb 8, 2024

ujfalusi commented Feb 8, 2024

ujfalusi commented Feb 8, 2024

ujfalusi commented Feb 8, 2024

[BUG] refactoring breaks sof-rt5682 audio on chromebook; successfully bisected #4807

[BUG] refactoring breaks sof-rt5682 audio on chromebook; successfully bisected #4807

Comments

135e2 commented Feb 6, 2024

revert 31ed8da, working

vanilla linux-git kernel, no sound

changes suggested in the email, still no sound

ujfalusi commented Feb 6, 2024 • edited Loading

ujfalusi commented Feb 6, 2024

ujfalusi commented Feb 6, 2024

plbossart commented Feb 6, 2024

ranj063 commented Feb 6, 2024

ranj063 commented Feb 6, 2024 • edited Loading

plbossart commented Feb 6, 2024

ranj063 commented Feb 6, 2024

135e2 commented Feb 7, 2024

ranj063 commented Feb 7, 2024

ranj063 commented Feb 7, 2024

plbossart commented Feb 7, 2024

ujfalusi commented Feb 8, 2024 • edited Loading

135e2 commented Feb 8, 2024

ujfalusi commented Feb 8, 2024

ujfalusi commented Feb 8, 2024

ujfalusi commented Feb 8, 2024

ujfalusi commented Feb 6, 2024 •

edited

Loading

ranj063 commented Feb 6, 2024 •

edited

Loading

ujfalusi commented Feb 8, 2024 •

edited

Loading