September 2023 - Sound-open-firmware - mailman.alsa-project.org

Re: [PATCH] ASoC: SOF: amd: fix for firmware reload failure after playback
by Mark Brown 28 Sep '23

28 Sep '23

On Wed, 27 Sep 2023 12:44:10 +0530, Vijendar Mukunda wrote: > Setting ACP ACLK as clock source when ACP enters D0 state causing > firmware load failure as mentioned in below scenario. > > - Load snd_sof_amd_rembrandt > - Play or Record audio > - Stop audio > - Unload snd_sof_amd_rembrandt > - Reload snd_sof_amd_rembrandt > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: SOF: amd: fix for firmware reload failure after playback commit: 7e1fe5d9e7eae67e218f878195d1d348d01f9af7 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

1 0

Re: [PATCH v4 01/11] ASoC: SOF: core: add 'no_wq' probe and remove callbacks
by Kai Vehmanen 12 Sep '23

12 Sep '23

Hi, On Wed, 30 Aug 2023, Maarten Lankhorst wrote: > With the upcoming changes for i915/Xe driver relying on the > -EPROBE_DEFER mechanism, we need to have a first pass of the probe > which cannot be pushed to a workqueue. Introduce 2 new optional > callbacks. [...] > diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c > index 30db685cc5f4b..54c384a5d6140 100644 > --- a/sound/soc/sof/core.c > +++ b/sound/soc/sof/core.c > @@ -327,8 +327,6 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > dsp_err: > snd_sof_remove(sdev); > probe_err: > - sof_ops_free(sdev); > - this seems a bit out-of-place in this patch. It seems a valid change, but not really related to this patch, right? We seem to have a related fix waiting to be sent to alsa-devel, by Peter: "ASoC: SOF: core: Only call sof_ops_free() on remove if the probe wa" https://github.com/thesofproject/linux/pull/4515 ... not yet in Mark's tree. Otherwise patch looks good to me. Br, Kai

3 7

[PATCH AUTOSEL 6.1 11/22] ASoC: SOF: topology: simplify code to prevent static analysis warnings
by Sasha Levin 08 Sep '23

08 Sep '23

From: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> [ Upstream commit 55cb3dc271d81f1982c949a2ac483a6daf613b92 ] make KCFLAGS='-fanalyzer' sound/soc/sof/intel/ reports a possible NULL pointer dereference. sound/soc/sof/topology.c:1136:21: error: dereference of NULL ‘w’ [CWE-476] [-Werror=analyzer-null-dereference] 1136 | strcmp(w->sname, rtd->dai_link->stream_name)) The code is rather confusing and can be simplified to make static analysis happy. No functionality change. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Rander Wang <rander.wang(a)intel.com> Reviewed-by: Daniel Baluta <daniel.baluta(a)nxp.com> Reviewed-by: Yaochun Hung <yc.hung(a)mediatek.com> Link: https://lore.kernel.org/r/20230731213748.440285-4-pierre-louis.bossart@linu… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/topology.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 872e44408298f..e7305ce57ea1f 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1086,16 +1086,17 @@ static void sof_disconnect_dai_widget(struct snd_soc_component *scomp, { struct snd_soc_card *card = scomp->card; struct snd_soc_pcm_runtime *rtd; + const char *sname = w->sname; struct snd_soc_dai *cpu_dai; int i; - if (!w->sname) + if (!sname) return; list_for_each_entry(rtd, &card->rtd_list, list) { /* does stream match DAI link ? */ if (!rtd->dai_link->stream_name || - strcmp(w->sname, rtd->dai_link->stream_name)) + strcmp(sname, rtd->dai_link->stream_name)) continue; switch (w->id) { -- 2.40.1

1 0

[PATCH AUTOSEL 6.4 30/31] ASoC: SOF: amd: clear panic mask status when panic occurs
by Sasha Levin 08 Sep '23

08 Sep '23

From: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> [ Upstream commit 3d02e1c439b4140215b624d423aa3c7554b17a5a ] Due to scratch memory persistence, Once the DSP panic is reported, need to clear the panic mask after handling DSP panic. Otherwise, It results in DSP panic on next reboot. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> Link: https://lore.kernel.org/r/20230823073340.2829821-6-Vijendar.Mukunda@amd.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/amd/acp-ipc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/soc/sof/amd/acp-ipc.c b/sound/soc/sof/amd/acp-ipc.c index 749e856dc6011..6951c67bc599f 100644 --- a/sound/soc/sof/amd/acp-ipc.c +++ b/sound/soc/sof/amd/acp-ipc.c @@ -161,6 +161,8 @@ irqreturn_t acp_sof_ipc_irq_thread(int irq, void *context) if ((status & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { snd_sof_dsp_panic(sdev, sdev->dsp_box.offset + sizeof(status), true); + status = 0; + acp_mailbox_write(sdev, sdev->dsp_box.offset, &status, sizeof(status)); return IRQ_HANDLED; } snd_sof_ipc_msgs_rx(sdev); @@ -190,6 +192,8 @@ irqreturn_t acp_sof_ipc_irq_thread(int irq, void *context) acp_mailbox_read(sdev, sdev->debug_box.offset, &status, sizeof(u32)); if ((status & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { snd_sof_dsp_panic(sdev, sdev->dsp_oops_offset, true); + status = 0; + acp_mailbox_write(sdev, sdev->debug_box.offset, &status, sizeof(status)); return IRQ_HANDLED; } -- 2.40.1

1 0

[PATCH AUTOSEL 6.4 15/31] ASoC: SOF: topology: simplify code to prevent static analysis warnings
by Sasha Levin 08 Sep '23

08 Sep '23

From: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> [ Upstream commit 55cb3dc271d81f1982c949a2ac483a6daf613b92 ] make KCFLAGS='-fanalyzer' sound/soc/sof/intel/ reports a possible NULL pointer dereference. sound/soc/sof/topology.c:1136:21: error: dereference of NULL ‘w’ [CWE-476] [-Werror=analyzer-null-dereference] 1136 | strcmp(w->sname, rtd->dai_link->stream_name)) The code is rather confusing and can be simplified to make static analysis happy. No functionality change. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Rander Wang <rander.wang(a)intel.com> Reviewed-by: Daniel Baluta <daniel.baluta(a)nxp.com> Reviewed-by: Yaochun Hung <yc.hung(a)mediatek.com> Link: https://lore.kernel.org/r/20230731213748.440285-4-pierre-louis.bossart@linu… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/topology.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index f160dc454b448..8e6f6148f0a07 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1117,10 +1117,11 @@ static void sof_disconnect_dai_widget(struct snd_soc_component *scomp, { struct snd_soc_card *card = scomp->card; struct snd_soc_pcm_runtime *rtd; + const char *sname = w->sname; struct snd_soc_dai *cpu_dai; int i, stream; - if (!w->sname) + if (!sname) return; if (w->id == snd_soc_dapm_dai_out) @@ -1133,7 +1134,7 @@ static void sof_disconnect_dai_widget(struct snd_soc_component *scomp, list_for_each_entry(rtd, &card->rtd_list, list) { /* does stream match DAI link ? */ if (!rtd->dai_link->stream_name || - strcmp(w->sname, rtd->dai_link->stream_name)) + strcmp(sname, rtd->dai_link->stream_name)) continue; for_each_rtd_cpu_dais(rtd, i, cpu_dai) -- 2.40.1

1 0

[PATCH AUTOSEL 6.5 35/36] ASoC: SOF: amd: clear panic mask status when panic occurs
by Sasha Levin 08 Sep '23

08 Sep '23

From: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> [ Upstream commit 3d02e1c439b4140215b624d423aa3c7554b17a5a ] Due to scratch memory persistence, Once the DSP panic is reported, need to clear the panic mask after handling DSP panic. Otherwise, It results in DSP panic on next reboot. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> Link: https://lore.kernel.org/r/20230823073340.2829821-6-Vijendar.Mukunda@amd.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/amd/acp-ipc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/soc/sof/amd/acp-ipc.c b/sound/soc/sof/amd/acp-ipc.c index 8a0fc635a997c..d07dc78074cc3 100644 --- a/sound/soc/sof/amd/acp-ipc.c +++ b/sound/soc/sof/amd/acp-ipc.c @@ -168,6 +168,8 @@ irqreturn_t acp_sof_ipc_irq_thread(int irq, void *context) if ((status & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { snd_sof_dsp_panic(sdev, sdev->dsp_box.offset + sizeof(status), true); + status = 0; + acp_mailbox_write(sdev, sdev->dsp_box.offset, &status, sizeof(status)); return IRQ_HANDLED; } snd_sof_ipc_msgs_rx(sdev); @@ -197,6 +199,8 @@ irqreturn_t acp_sof_ipc_irq_thread(int irq, void *context) acp_mailbox_read(sdev, sdev->debug_box.offset, &status, sizeof(u32)); if ((status & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { snd_sof_dsp_panic(sdev, sdev->dsp_oops_offset, true); + status = 0; + acp_mailbox_write(sdev, sdev->debug_box.offset, &status, sizeof(status)); return IRQ_HANDLED; } -- 2.40.1

1 0

[PATCH AUTOSEL 6.5 17/36] ASoC: SOF: topology: simplify code to prevent static analysis warnings
by Sasha Levin 08 Sep '23

08 Sep '23

From: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> [ Upstream commit 55cb3dc271d81f1982c949a2ac483a6daf613b92 ] make KCFLAGS='-fanalyzer' sound/soc/sof/intel/ reports a possible NULL pointer dereference. sound/soc/sof/topology.c:1136:21: error: dereference of NULL ‘w’ [CWE-476] [-Werror=analyzer-null-dereference] 1136 | strcmp(w->sname, rtd->dai_link->stream_name)) The code is rather confusing and can be simplified to make static analysis happy. No functionality change. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Rander Wang <rander.wang(a)intel.com> Reviewed-by: Daniel Baluta <daniel.baluta(a)nxp.com> Reviewed-by: Yaochun Hung <yc.hung(a)mediatek.com> Link: https://lore.kernel.org/r/20230731213748.440285-4-pierre-louis.bossart@linu… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/topology.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 698129dccc7df..3866dd3cba695 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1117,10 +1117,11 @@ static void sof_disconnect_dai_widget(struct snd_soc_component *scomp, { struct snd_soc_card *card = scomp->card; struct snd_soc_pcm_runtime *rtd; + const char *sname = w->sname; struct snd_soc_dai *cpu_dai; int i, stream; - if (!w->sname) + if (!sname) return; if (w->id == snd_soc_dapm_dai_out) @@ -1133,7 +1134,7 @@ static void sof_disconnect_dai_widget(struct snd_soc_component *scomp, list_for_each_entry(rtd, &card->rtd_list, list) { /* does stream match DAI link ? */ if (!rtd->dai_link->stream_name || - strcmp(w->sname, rtd->dai_link->stream_name)) + strcmp(sname, rtd->dai_link->stream_name)) continue; for_each_rtd_cpu_dais(rtd, i, cpu_dai) -- 2.40.1

1 0

Re: [PATCH v4 10/11] ASoC: SOF: Intel: Move binding to display driver outside of deferred probe
by Kai Vehmanen 01 Sep '23

01 Sep '23

Hey, On Wed, 30 Aug 2023, Maarten Lankhorst wrote: > Now that we can use -EPROBE_DEFER, it's no longer required to spin off > the snd_hdac_i915_init into a workqueue. > > Use the -EPROBE_DEFER mechanism instead, which must be returned in the > probe function. > > The previously added probe_no_wq can be used for this, > and we also use the newly added remove_no_wq for unbinding afterwards. [...] > @@ -1317,7 +1313,6 @@ int hda_dsp_remove(struct snd_sof_dev *sdev) > { > struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; > const struct sof_intel_dsp_desc *chip = hda->desc; > - struct hdac_bus *bus = sof_to_bus(sdev); > struct pci_dev *pci = to_pci_dev(sdev->dev); > struct nhlt_acpi_table *nhlt = hda->nhlt; > > @@ -1368,10 +1363,12 @@ int hda_dsp_remove(struct snd_sof_dev *sdev) > if (!sdev->dspless_mode_selected) > iounmap(sdev->bar[HDA_DSP_BAR]); > > - iounmap(bus->remap_addr); > - > - sof_hda_bus_exit(sdev); > + return 0; > +} > > +int hda_dsp_remove_no_wq(struct snd_sof_dev *sdev) > +{ > + iounmap(sof_to_bus(sdev)->remap_addr); > hda_codec_i915_exit(sdev); I think here we drop the call to sof_hda_bus_exit() which should be done in hda_dsp_remove_no_wq() to counter hda_init(). Rest looks good to me, the "no_wq" variants do symmetric ops, so we can handle both wq an non-wq cases. Br, Kai

1 0

Re: [PATCH v4 01/11] ASoC: SOF: core: add 'no_wq' probe and remove callbacks
by Pierre-Louis Bossart 01 Sep '23

01 Sep '23

> +static inline int snd_sof_remove_no_wq(struct snd_sof_dev *sdev) > +{ > + if (sof_ops(sdev)->remove_no_wq) > + return sof_ops(sdev)->remove_no_wq(sdev); > + > + return 0; > +} > /* probe/remove/shutdown */ > + int (*probe_no_wq)(struct snd_sof_dev *sof_dev); /* optional */ > + int (*remove_no_wq)(struct snd_sof_dev *sof_dev); /* optional */ My initial PR didn't have this remove_no_wq() callback. For symmetry it could be useful if it is meant to undo what the probe_no_wq() did, but conceptually the first thing we do in the remove is make sure that workqueue is either not scheduled or we wait until it completes. int snd_sof_device_remove(struct device *dev) { struct snd_sof_dev *sdev = dev_get_drvdata(dev); struct snd_sof_pdata *pdata = sdev->pdata; int ret; if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) cancel_work_sync(&sdev->probe_work);

2 1