August 2022 - Sound-open-firmware - mailman.alsa-project.org

[Sound-open-firmware] [PATCH AUTOSEL 5.19 14/48] ASoC: SOF: sof-client-probes: Only load the driver if IPC3 is used
by Sasha Levin 14 Aug '22

14 Aug '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit 9b93eda355089b36482f7a2f134bdd24be70f907 ] The current implementation of probes only supports IPC3 and should not be loaded for other IPC implementation. Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://lore.kernel.org/r/20220712131022.1124-1-peter.ujfalusi@linux.intel.… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/sof-client-probes.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/soc/sof/sof-client-probes.c b/sound/soc/sof/sof-client-probes.c index 34e6bd356e71..60e4250fac87 100644 --- a/sound/soc/sof/sof-client-probes.c +++ b/sound/soc/sof/sof-client-probes.c @@ -693,6 +693,10 @@ static int sof_probes_client_probe(struct auxiliary_device *auxdev, if (!sof_probes_enabled) return -ENXIO; + /* only ipc3 is supported */ + if (sof_client_get_ipc_type(cdev) != SOF_IPC) + return -ENXIO; + if (!dev->platform_data) { dev_err(dev, "missing platform data\n"); return -ENODEV; -- 2.35.1

1 0

[Sound-open-firmware] [PATCH AUTOSEL 5.19 13/48] ASoC: SOF: Intel: hda-ipc: Do not process IPC reply before firmware boot
by Sasha Levin 14 Aug '22

14 Aug '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit 499cc881b09c8283ab5e75b0d6d21cb427722161 ] It is not yet clear, but it is possible to create a firmware so broken that it will send a reply message before a FW_READY message (it is not yet clear if FW_READY will arrive later). Since the reply_data is allocated only after the FW_READY message, this will lead to a NULL pointer dereference if not filtered out. The issue was reported with IPC4 firmware but the same condition is present for IPC3. Reported-by: Kai Vehmanen <kai.vehmanen(a)linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Link: https://lore.kernel.org/r/20220712122357.31282-3-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/hda-ipc.c | 39 ++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index f08011249955..65e688f749ea 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -148,17 +148,23 @@ irqreturn_t hda_dsp_ipc4_irq_thread(int irq, void *context) if (primary & SOF_IPC4_MSG_DIR_MASK) { /* Reply received */ - struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data; + if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) { + struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data; - data->primary = primary; - data->extension = extension; + data->primary = primary; + data->extension = extension; - spin_lock_irq(&sdev->ipc_lock); + spin_lock_irq(&sdev->ipc_lock); - snd_sof_ipc_get_reply(sdev); - snd_sof_ipc_reply(sdev, data->primary); + snd_sof_ipc_get_reply(sdev); + snd_sof_ipc_reply(sdev, data->primary); - spin_unlock_irq(&sdev->ipc_lock); + spin_unlock_irq(&sdev->ipc_lock); + } else { + dev_dbg_ratelimited(sdev->dev, + "IPC reply before FW_READY: %#x|%#x\n", + primary, extension); + } } else { /* Notification received */ @@ -225,16 +231,21 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context) * place, the message might not yet be marked as expecting a * reply. */ - spin_lock_irq(&sdev->ipc_lock); + if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) { + spin_lock_irq(&sdev->ipc_lock); - /* handle immediate reply from DSP core */ - hda_dsp_ipc_get_reply(sdev); - snd_sof_ipc_reply(sdev, msg); + /* handle immediate reply from DSP core */ + hda_dsp_ipc_get_reply(sdev); + snd_sof_ipc_reply(sdev, msg); - /* set the done bit */ - hda_dsp_ipc_dsp_done(sdev); + /* set the done bit */ + hda_dsp_ipc_dsp_done(sdev); - spin_unlock_irq(&sdev->ipc_lock); + spin_unlock_irq(&sdev->ipc_lock); + } else { + dev_dbg_ratelimited(sdev->dev, "IPC reply before FW_READY: %#x\n", + msg); + } ipc_irq = true; } -- 2.35.1

1 0

[Sound-open-firmware] [PATCH AUTOSEL 5.19 12/48] ASoC: SOF: Intel: cnl: Do not process IPC reply before firmware boot
by Sasha Levin 14 Aug '22

14 Aug '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit acacd9eefd0def5a83244d88e5483b5f38ee7287 ] It is not yet clear, but it is possible to create a firmware so broken that it will send a reply message before a FW_READY message (it is not yet clear if FW_READY will arrive later). Since the reply_data is allocated only after the FW_READY message, this will lead to a NULL pointer dereference if not filtered out. The issue was reported with IPC4 firmware but the same condition is present for IPC3. Reported-by: Kai Vehmanen <kai.vehmanen(a)linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Link: https://lore.kernel.org/r/20220712122357.31282-2-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/cnl.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index cd6e5f8a5eb4..6c98f65635fc 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -60,17 +60,23 @@ irqreturn_t cnl_ipc4_irq_thread(int irq, void *context) if (primary & SOF_IPC4_MSG_DIR_MASK) { /* Reply received */ - struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data; + if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) { + struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data; - data->primary = primary; - data->extension = extension; + data->primary = primary; + data->extension = extension; - spin_lock_irq(&sdev->ipc_lock); + spin_lock_irq(&sdev->ipc_lock); - snd_sof_ipc_get_reply(sdev); - snd_sof_ipc_reply(sdev, data->primary); + snd_sof_ipc_get_reply(sdev); + snd_sof_ipc_reply(sdev, data->primary); - spin_unlock_irq(&sdev->ipc_lock); + spin_unlock_irq(&sdev->ipc_lock); + } else { + dev_dbg_ratelimited(sdev->dev, + "IPC reply before FW_READY: %#x|%#x\n", + primary, extension); + } } else { /* Notification received */ notification_data.primary = primary; @@ -124,15 +130,20 @@ irqreturn_t cnl_ipc_irq_thread(int irq, void *context) CNL_DSP_REG_HIPCCTL, CNL_DSP_REG_HIPCCTL_DONE, 0); - spin_lock_irq(&sdev->ipc_lock); + if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) { + spin_lock_irq(&sdev->ipc_lock); - /* handle immediate reply from DSP core */ - hda_dsp_ipc_get_reply(sdev); - snd_sof_ipc_reply(sdev, msg); + /* handle immediate reply from DSP core */ + hda_dsp_ipc_get_reply(sdev); + snd_sof_ipc_reply(sdev, msg); - cnl_ipc_dsp_done(sdev); + cnl_ipc_dsp_done(sdev); - spin_unlock_irq(&sdev->ipc_lock); + spin_unlock_irq(&sdev->ipc_lock); + } else { + dev_dbg_ratelimited(sdev->dev, "IPC reply before FW_READY: %#x\n", + msg); + } ipc_irq = true; } -- 2.35.1

1 0

[Sound-open-firmware] FYI: setup config was removed from codec adapter topologies
by Daniel Baluta 11 Aug '22

11 Aug '22

Hi Joe, I hope you are right person to direct this email to. Looking at commit: commit bfd5e61768713bf25fe85030268a2f860eb26cf4 Author: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Date: Fri Jan 14 13:18:48 2022 -0800 topology1: remove setup config info from all codec_adapter topologies Remove the setup config info from all initial config data in the codec_adapter kcontrol's. This is no longer needed. This removed setup config from config data. The only place which didn't get this seems to be the DTS codec file: topology/topology1/m4/dts_codec_adapter.m4 Did you notice any bugs in your tests?

1 0

[Sound-open-firmware] [PATCH resend] ASoC: SOF: ipc3-topology: Fix clang -Wformat warning
by Justin Stitt 05 Aug '22

05 Aug '22

When building with Clang we encounter these warnings: | sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type | 'unsigned char' but the argument has type 'int' [-Werror,-Wformat] | SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH); | ^~~~~~~~~~~~~~~^~~~~~~~~~~~~~~^~~~~~~~~~~~~ Use correct format specifier `%d` since args are of type int. Link: https://github.com/ClangBuiltLinux/linux/issues/378 Reported-by: Nathan Chancellor <nathan(a)kernel.org> Suggested-by: Nathan Chancellor <nathan(a)kernel.org> Signed-off-by: Justin Stitt <justinstitt(a)google.com> --- Note: resending with the subject corrected, this patch fixes a build warning that will break the Clang build when CONFIG_WERROR=y. Original patch: https://lore.kernel.org/all/20220721211218.4039288-1-justinstitt@google.com/ Reported by Nathan here: https://lore.kernel.org/all/YtmrCJjQrSbv8Aj1@dev-arch.thelio-3990X/ sound/soc/sof/ipc3-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index b2cc046b9f60..65923e7a5976 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index, } dev_info(scomp->dev, - "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n", + "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n", man->priv.data[0], man->priv.data[1], man->priv.data[2], SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH); -- 2.37.1.359.gd136c6c3e2-goog

4 3

Re: [Sound-open-firmware] [PATCH 2/2] ASoC: SOF: mediatek: Add .of_machine_select field for mt8186
by chunxu.li 04 Aug '22

04 Aug '22

On Thu, 2022-08-04 at 14:33 +0100, Mark Brown wrote: > On Thu, Aug 04, 2022 at 09:21:37PM +0800, chunxu.li wrote: > > On Thu, 2022-08-04 at 13:41 +0100, Mark Brown wrote: > > > On Thu, Aug 04, 2022 at 05:13:59PM +0800, Chunxu Li wrote: > > > > + .board = "mediatek,mt8186", > > > > + .sof_tplg_filename = "sof-mt8186.tplg", > > > The mediatek,mt8186 compatible looks like a SoC compatible not a > > > board > > > compatible... > > Our dts config as below: > > kingler board: > > compatible = "google,corsola", "google,kingler", "mediatek,mt8186"; > > krabby board: > > compatible = "google,corsola", "google,krabby", "mediatek,mt8186"; > > So the SoC is listed as a fallback for the board and things work out > fine. > > > Which of the two approaches do you prefer? > > I think it would be clearer to keep what's being matched the same but > rename the .board field to be .compatible, or possibly > .system_compatible > or something if it's unclear what's being matched. That'd be more > normal for specifying a compatible string anyway. Appreciated for you advice, I'll rename the .board to .compatible as following: + .compatible = "mediatek,mt8186", + .sof_tplg_filename = "sof-mt8186.tplg",

1 0

Re: [Sound-open-firmware] [PATCH 1/2] ASoC: SOF: Introduce optional callback of_machine_select
by chunxu.li 04 Aug '22

04 Aug '22

On Thu, 2022-08-04 at 14:17 +0100, Mark Brown wrote: > On Thu, Aug 04, 2022 at 05:13:58PM +0800, Chunxu Li wrote: > > > @@ -284,6 +284,7 @@ struct snd_sof_dsp_ops { > > void (*machine_unregister)(struct snd_sof_dev *sdev, > > void *pdata); /* optional */ > > struct snd_soc_acpi_mach * (*machine_select)(struct snd_sof_dev > > *sdev); /* > > optional */ > > + struct snd_sof_of_mach * (*of_machine_select)(struct > > snd_sof_dev *sdev); > > I don't understand why we pass this in as a function when as far as I > can see it should always be the standard operation provided by the > core > - why not just always call the function? We can tell at runtime if > the > system is using DT so there's no issue there and there shouldn't be > any > concerns with ACPI or other firmware interfaces. Thanks for you advice, I'll remove the callback function, and directly call sof_of_machine_select() in sof_machine_check() as following. int sof_machine_check(struct snd_sof_dev *sdev) { snip... const struct snd_sof_of_mach *of_mach; /* find machine */ mach = snd_sof_machine_select(sdev); if (mach) { sof_pdata->machine = mach; snd_sof_set_mach_params(mach, sdev); return 0; } + of_mach = sof_of_machine_select(sdev); + if (of_mach) { + sof_pdata->of_machine = of_mach; + return 0; + } snip ... }

1 0

Re: [Sound-open-firmware] [PATCH 2/2] ASoC: SOF: mediatek: Add .of_machine_select field for mt8186
by chunxu.li 04 Aug '22

04 Aug '22

On Thu, 2022-08-04 at 13:41 +0100, Mark Brown wrote: > On Thu, Aug 04, 2022 at 05:13:59PM +0800, Chunxu Li wrote: > > > +static struct snd_sof_of_mach sof_mt8186_machs[] = { > > + { > > + .board = "mediatek,mt8186", > > + .sof_tplg_filename = "sof-mt8186.tplg", > > The mediatek,mt8186 compatible looks like a SoC compatible not a > board > compatible... Our dts config as below: kingler board: compatible = "google,corsola", "google,kingler", "mediatek,mt8186"; krabby board: compatible = "google,corsola", "google,krabby", "mediatek,mt8186"; they all use the same sof_tplg_filename, so i use "mediatek,mt8186" to match these. The original code looks like the following: static struct snd_soc_acpi_mach sof_mt8186_machs[] = { { .board = "google,kingler", .sof_tplg_filename = "sof-mt8186.tplg", }, { .board = "google,krabby", .sof_tplg_filename = "sof-mt8186.tplg", }, {} }; Which of the two approaches do you prefer?

1 0

[Sound-open-firmware] [PATCH 0/2] ASoC: SOF: Introduce of_machine_select
by Chunxu Li 04 Aug '22

04 Aug '22

From: "chunxu.li" <chunxu.li(a)mediatek.com> In these patches, we introduce of_machine_select for SOF Chunxu Li (2): ASoC: SOF: Introduce optional callback of_machine_select ASoC: SOF: mediatek: Add .of_machine_select field for mt8186 include/sound/sof.h | 2 ++ sound/soc/sof/mediatek/mt8186/mt8186.c | 11 +++++++++++ sound/soc/sof/ops.h | 9 +++++++++ sound/soc/sof/pcm.c | 8 +++++++- sound/soc/sof/sof-audio.c | 7 +++++++ sound/soc/sof/sof-of-dev.c | 23 +++++++++++++++++++++++ sound/soc/sof/sof-of-dev.h | 8 ++++++++ sound/soc/sof/sof-priv.h | 1 + 8 files changed, 68 insertions(+), 1 deletion(-) -- 2.25.1

1 2

[Sound-open-firmware] change in west usage for SOF Zephyr builds
by Kai Vehmanen 04 Aug '22

04 Aug '22

Hi, just a heads up that the recent change to sof/scripts/xtensa-build-zephyr.py in commit "xtensa-build-zephyr.py: use west manifest", merged to SOF main yesterday, has been causing trouble to some users. The change adds a west manifest to SOF repository. This is now used to check out zephyr in a specific place in the work area, and most importantly, to have a version controlled way to specify which version of Zephyr (and other subprojects) to build against, when building SOF with Zephyr. Easiest way to recover is to check out a new work area to build SOF with Zephyr. Br, Kai

1 0