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

Re: [PATCH v3 1/2] ASoC: SOF: amd: Move signed_fw_image to struct acp_quirk_entry
by AngeloGioacchino Del Regno 21 Feb '24

21 Feb '24

Il 21/02/24 11:29, Cristian Ciocaltea ha scritto: > On 2/21/24 11:35, AngeloGioacchino Del Regno wrote: >> Il 20/02/24 21:16, Cristian Ciocaltea ha scritto: >>> The signed_fw_image member of struct sof_amd_acp_desc is used to enable >>> signed firmware support in the driver via the acp_sof_quirk_table. >>> >>> In preparation to support additional use cases of the quirk table (i.e. >>> adding new flags), move signed_fw_image to a new struct acp_quirk_entry >>> and update all references to it accordingly. >>> >>> No functional changes intended. >>> >>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea(a)collabora.com> >>> --- >>> sound/soc/sof/amd/acp-loader.c | 2 +- >>> sound/soc/sof/amd/acp.c | 47 ++++++++++++++++++---------------- >>> sound/soc/sof/amd/acp.h | 6 ++++- >>> sound/soc/sof/amd/vangogh.c | 9 +++++-- >>> 4 files changed, 38 insertions(+), 26 deletions(-) >>> >>> diff --git a/sound/soc/sof/amd/acp-loader.c >>> b/sound/soc/sof/amd/acp-loader.c >>> index d2d21478399e..aad904839b81 100644 >>> --- a/sound/soc/sof/amd/acp-loader.c >>> +++ b/sound/soc/sof/amd/acp-loader.c >>> @@ -173,7 +173,7 @@ int acp_dsp_pre_fw_run(struct snd_sof_dev *sdev) >>> adata = sdev->pdata->hw_pdata; >>> - if (adata->signed_fw_image) >>> + if (adata->quirks && adata->quirks->signed_fw_image) >>> size_fw = adata->fw_bin_size - ACP_FIRMWARE_SIGNATURE; >>> else >>> size_fw = adata->fw_bin_size; >>> diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c >>> index 9b3c26210db3..9d9197fa83ed 100644 >>> --- a/sound/soc/sof/amd/acp.c >>> +++ b/sound/soc/sof/amd/acp.c >>> @@ -20,12 +20,14 @@ >>> #include "acp.h" >>> #include "acp-dsp-offset.h" >>> -#define SECURED_FIRMWARE 1 >>> - >>> static bool enable_fw_debug; >>> module_param(enable_fw_debug, bool, 0444); >>> MODULE_PARM_DESC(enable_fw_debug, "Enable Firmware debug"); >>> +static struct acp_quirk_entry quirk_valve_galileo = { >>> + .signed_fw_image = true, >> >> Hello Cristian, >> >> are you sure that a structure holding "quirks" is the right choice here? >> >> That probably comes as a personal preference, but I would simply pass a >> `u32 flags` >> and structure the quirks as bits. >> >> #define ACP_SIGNED_FW_IMAGE BIT(0) >> #define ACP_SOMETHING_ELSE BIT(1) >> >> flags = BIT(SIGNED_FW_IMAGE) | BIT(SOMETHING_ELSE); That should've been flags = SIGNED_FW_IMAGE | SOMETHING_ELSE; >> >> if (flags & BIT(SIGNED_FW_IMAGE)) and this if (flags & SIGNED_FW_IMAGE) ... I have no idea why I added a nested BIT(BIT()) in there, something in my brain started ticking sideways, lol. >> do_something() >> >> What do you think? > > Hi Angelo, > > The flags approach was actually my first thought and I think that would > have been the best choice if the quirks usage was limited to a single > file (acp.c). > > Since they need to be exposed externally as well (acp-loader.c, > vangogh.c) and already using a dedicated member in struct > sof_amd_acp_desc related to the existing quirk, I found the "quirks" > struct solution a bit more natural/convenient to follow (I've done a bit > of research before and noticed other drivers having similar handling). > > However, as you already pointed out, it may also come down to individual > preferences, so I'm open to using the flags if there is not enough > reasoning to stick with the current implementation. Of course the definitions should be put in a "common header" for that to actually work in your described situation, but it's not a big deal. Mine wasn't a strong opinion: that does actually matter in case you expect more quirks (or something that is not a quirk, but a functional flag instead) to eventually get there... but otherwise, it's actually the same. It's your choice in the end, I'm fine with both anyway :-) Cheers, Angelo

1 0

Re: [PATCH v3 1/2] ASoC: SOF: amd: Move signed_fw_image to struct acp_quirk_entry
by AngeloGioacchino Del Regno 21 Feb '24

21 Feb '24

Il 20/02/24 21:16, Cristian Ciocaltea ha scritto: > The signed_fw_image member of struct sof_amd_acp_desc is used to enable > signed firmware support in the driver via the acp_sof_quirk_table. > > In preparation to support additional use cases of the quirk table (i.e. > adding new flags), move signed_fw_image to a new struct acp_quirk_entry > and update all references to it accordingly. > > No functional changes intended. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea(a)collabora.com> > --- > sound/soc/sof/amd/acp-loader.c | 2 +- > sound/soc/sof/amd/acp.c | 47 ++++++++++++++++++---------------- > sound/soc/sof/amd/acp.h | 6 ++++- > sound/soc/sof/amd/vangogh.c | 9 +++++-- > 4 files changed, 38 insertions(+), 26 deletions(-) > > diff --git a/sound/soc/sof/amd/acp-loader.c b/sound/soc/sof/amd/acp-loader.c > index d2d21478399e..aad904839b81 100644 > --- a/sound/soc/sof/amd/acp-loader.c > +++ b/sound/soc/sof/amd/acp-loader.c > @@ -173,7 +173,7 @@ int acp_dsp_pre_fw_run(struct snd_sof_dev *sdev) > > adata = sdev->pdata->hw_pdata; > > - if (adata->signed_fw_image) > + if (adata->quirks && adata->quirks->signed_fw_image) > size_fw = adata->fw_bin_size - ACP_FIRMWARE_SIGNATURE; > else > size_fw = adata->fw_bin_size; > diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c > index 9b3c26210db3..9d9197fa83ed 100644 > --- a/sound/soc/sof/amd/acp.c > +++ b/sound/soc/sof/amd/acp.c > @@ -20,12 +20,14 @@ > #include "acp.h" > #include "acp-dsp-offset.h" > > -#define SECURED_FIRMWARE 1 > - > static bool enable_fw_debug; > module_param(enable_fw_debug, bool, 0444); > MODULE_PARM_DESC(enable_fw_debug, "Enable Firmware debug"); > > +static struct acp_quirk_entry quirk_valve_galileo = { > + .signed_fw_image = true, Hello Cristian, are you sure that a structure holding "quirks" is the right choice here? That probably comes as a personal preference, but I would simply pass a `u32 flags` and structure the quirks as bits. #define ACP_SIGNED_FW_IMAGE BIT(0) #define ACP_SOMETHING_ELSE BIT(1) flags = BIT(SIGNED_FW_IMAGE) | BIT(SOMETHING_ELSE); if (flags & BIT(SIGNED_FW_IMAGE)) do_something() What do you think? Cheers, Angelo > +}; > + > const struct dmi_system_id acp_sof_quirk_table[] = { > { > /* Steam Deck OLED device */ > @@ -33,7 +35,7 @@ const struct dmi_system_id acp_sof_quirk_table[] = { > DMI_MATCH(DMI_SYS_VENDOR, "Valve"), > DMI_MATCH(DMI_PRODUCT_NAME, "Galileo"), > }, > - .driver_data = (void *)SECURED_FIRMWARE, > + .driver_data = &quirk_valve_galileo, > }, > {} > }; > @@ -254,7 +256,7 @@ int configure_and_run_sha_dma(struct acp_dev_data *adata, void *image_addr, > } > } > > - if (adata->signed_fw_image) > + if (adata->quirks && adata->quirks->signed_fw_image) > snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SHA_DMA_INCLUDE_HDR, ACP_SHA_HEADER); > > snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SHA_DMA_STRT_ADDR, start_addr); > @@ -738,26 +740,27 @@ int amd_sof_acp_probe(struct snd_sof_dev *sdev) > sdev->debug_box.offset = sdev->host_box.offset + sdev->host_box.size; > sdev->debug_box.size = BOX_SIZE_1024; > > - adata->signed_fw_image = false; > dmi_id = dmi_first_match(acp_sof_quirk_table); > - if (dmi_id && dmi_id->driver_data) { > - adata->fw_code_bin = devm_kasprintf(sdev->dev, GFP_KERNEL, > - "sof-%s-code.bin", > - chip->name); > - if (!adata->fw_code_bin) { > - ret = -ENOMEM; > - goto free_ipc_irq; > + if (dmi_id) { > + adata->quirks = dmi_id->driver_data; > + > + if (adata->quirks->signed_fw_image) { > + adata->fw_code_bin = devm_kasprintf(sdev->dev, GFP_KERNEL, > + "sof-%s-code.bin", > + chip->name); > + if (!adata->fw_code_bin) { > + ret = -ENOMEM; > + goto free_ipc_irq; > + } > + > + adata->fw_data_bin = devm_kasprintf(sdev->dev, GFP_KERNEL, > + "sof-%s-data.bin", > + chip->name); > + if (!adata->fw_data_bin) { > + ret = -ENOMEM; > + goto free_ipc_irq; > + } > } > - > - adata->fw_data_bin = devm_kasprintf(sdev->dev, GFP_KERNEL, > - "sof-%s-data.bin", > - chip->name); > - if (!adata->fw_data_bin) { > - ret = -ENOMEM; > - goto free_ipc_irq; > - } > - > - adata->signed_fw_image = dmi_id->driver_data; > } > > adata->enable_fw_debug = enable_fw_debug; > diff --git a/sound/soc/sof/amd/acp.h b/sound/soc/sof/amd/acp.h > index 947068da39b5..b648ed194b9f 100644 > --- a/sound/soc/sof/amd/acp.h > +++ b/sound/soc/sof/amd/acp.h > @@ -207,6 +207,10 @@ struct sof_amd_acp_desc { > u64 sdw_acpi_dev_addr; > }; > > +struct acp_quirk_entry { > + bool signed_fw_image; > +}; > + > /* Common device data struct for ACP devices */ > struct acp_dev_data { > struct snd_sof_dev *dev; > @@ -236,7 +240,7 @@ struct acp_dev_data { > u8 *data_buf; > dma_addr_t sram_dma_addr; > u8 *sram_data_buf; > - bool signed_fw_image; > + struct acp_quirk_entry *quirks; > struct dma_descriptor dscr_info[ACP_MAX_DESC]; > struct acp_dsp_stream stream_buf[ACP_MAX_STREAM]; > struct acp_dsp_stream *dtrace_stream; > diff --git a/sound/soc/sof/amd/vangogh.c b/sound/soc/sof/amd/vangogh.c > index de15d21aa6d9..bc6ffdb5471a 100644 > --- a/sound/soc/sof/amd/vangogh.c > +++ b/sound/soc/sof/amd/vangogh.c > @@ -143,6 +143,7 @@ EXPORT_SYMBOL_NS(sof_vangogh_ops, SND_SOC_SOF_AMD_COMMON); > int sof_vangogh_ops_init(struct snd_sof_dev *sdev) > { > const struct dmi_system_id *dmi_id; > + struct acp_quirk_entry *quirks; > > /* common defaults */ > memcpy(&sof_vangogh_ops, &sof_acp_common_ops, sizeof(struct snd_sof_dsp_ops)); > @@ -151,8 +152,12 @@ int sof_vangogh_ops_init(struct snd_sof_dev *sdev) > sof_vangogh_ops.num_drv = ARRAY_SIZE(vangogh_sof_dai); > > dmi_id = dmi_first_match(acp_sof_quirk_table); > - if (dmi_id && dmi_id->driver_data) > - sof_vangogh_ops.load_firmware = acp_sof_load_signed_firmware; > + if (dmi_id) { > + quirks = dmi_id->driver_data; > + > + if (quirks->signed_fw_image) > + sof_vangogh_ops.load_firmware = acp_sof_load_signed_firmware; > + } > > return 0; > }

1 0

[PATCH] ASoC: SOF: amd: fix soundwire dependencies
by Arnd Bergmann 21 Feb '24

21 Feb '24

From: Arnd Bergmann <arnd(a)arndb.de> The soundwire-amd driver has a bit of a layering violation requiring the SOF driver to directly call into its exported symbols rather than through an abstraction. The SND_SOC_SOF_AMD_SOUNDWIRE Kconfig symbol tries to deal with the dependency by selecting SOUNDWIRE_AMD in a complicated set of conditions, but gets it wrong for a configuration involving SND_SOC_SOF_AMD_COMMON=y, SND_SOC_SOF_AMD_ACP63=m, and SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE=m SOUNDWIRE_AMD=m, which results in a link failure: ld.lld: error: undefined symbol: sdw_amd_get_slave_info >>> referenced by acp-common.c ld.lld: error: undefined symbol: amd_sdw_scan_controller ld.lld: error: undefined symbol: sdw_amd_probe ld.lld: error: undefined symbol: sdw_amd_exit >>> referenced by acp.c >>> sound/soc/sof/amd/acp.o:(amd_sof_acp_remove) in archive vmlinux.a In essence, the SND_SOC_SOF_AMD_COMMON option cannot be built-in when trying to link against a modular SOUNDWIRE_AMD driver. Since CONFIG_SOUNDWIRE_AMD is a user-visible option, it really should never be selected by another driver in the first place, so replace the extra complexity with a normal Kconfig dependency in SND_SOC_SOF_AMD_SOUNDWIRE, plus a top-level check that forbids any of the AMD SOF drivers from being built-in with CONFIG_SOUNDWIRE_AMD=m. In normal configs, they should all either be built-in or all loadable modules anyway, so this simplification does not limit any real usecases. Fixes: d948218424bf ("ASoC: SOF: amd: add code for invoking soundwire manager helper functions") Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> --- sound/soc/sof/amd/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/sof/amd/Kconfig b/sound/soc/sof/amd/Kconfig index c3bbe6c70fb2..2729c6eb3feb 100644 --- a/sound/soc/sof/amd/Kconfig +++ b/sound/soc/sof/amd/Kconfig @@ -6,6 +6,7 @@ config SND_SOC_SOF_AMD_TOPLEVEL tristate "SOF support for AMD audio DSPs" + depends on SOUNDWIRE_AMD || !SOUNDWIRE_AMD depends on X86 || COMPILE_TEST help This adds support for Sound Open Firmware for AMD platforms. @@ -62,15 +63,14 @@ config SND_SOC_SOF_ACP_PROBES config SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE tristate - select SOUNDWIRE_AMD if SND_SOC_SOF_AMD_SOUNDWIRE != n select SND_AMD_SOUNDWIRE_ACPI if ACPI config SND_SOC_SOF_AMD_SOUNDWIRE tristate "SOF support for SoundWire based AMD platforms" default SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE depends on SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE - depends on ACPI && SOUNDWIRE - depends on !(SOUNDWIRE=m && SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE=y) + depends on ACPI + depends on SOUNDWIRE_AMD help This adds support for SoundWire with Sound Open Firmware for AMD platforms. -- 2.39.2

4 9

Re: [PATCH] ASoC: SOF: Add some bounds checking to firmware data
by Mark Brown 14 Feb '24

14 Feb '24

On Fri, 09 Feb 2024 16:02:16 +0300, Dan Carpenter wrote: > Smatch complains about "head->full_size - head->header_size" can > underflow. To some extent, we're always going to have to trust the > firmware a bit. However, it's easy enough to add a check for negatives, > and let's add a upper bounds check as well. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: SOF: Add some bounds checking to firmware data commit: 98f681b0f84cfc3a1d83287b77697679e0398306 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] ASoC: SOF: amd: Fix locking in ACP IRQ handler
by Mark Brown 12 Feb '24

12 Feb '24

On Fri, 09 Feb 2024 01:43:14 +0200, Cristian Ciocaltea wrote: > A recent change in acp_irq_thread() was meant to address a potential race > condition while trying to acquire the hardware semaphore responsible for > the synchronization between firmware and host IPC interrupts. > > This resulted in an improper use of the IPC spinlock, causing normal > kernel memory allocations (which may sleep) inside atomic contexts: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: SOF: amd: Fix locking in ACP IRQ handler commit: c4b603c6e2df3a17831731d8bbec5c16fa7bbdf8 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

[PATCH V4 11/13] ASoC: SOF: amd: update descriptor fields for acp6.3 based platform
by Vijendar Mukunda 29 Jan '24

29 Jan '24

Update acp descriptor fields for acp6.3 version based platform. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> --- sound/soc/sof/amd/acp-dsp-offset.h | 3 +++ sound/soc/sof/amd/acp.h | 2 ++ sound/soc/sof/amd/pci-acp63.c | 7 +++++++ 3 files changed, 12 insertions(+) diff --git a/sound/soc/sof/amd/acp-dsp-offset.h b/sound/soc/sof/amd/acp-dsp-offset.h index 7ba6492b8e99..c1bdc028a61a 100644 --- a/sound/soc/sof/amd/acp-dsp-offset.h +++ b/sound/soc/sof/amd/acp-dsp-offset.h @@ -65,7 +65,10 @@ /* Registers from ACP_INTR block */ #define ACP3X_EXT_INTR_STAT 0x1808 #define ACP5X_EXT_INTR_STAT 0x1808 +#define ACP6X_EXTERNAL_INTR_ENB 0x1A00 +#define ACP6X_EXTERNAL_INTR_CNTL 0x1A04 #define ACP6X_EXT_INTR_STAT 0x1A0C +#define ACP6X_EXT_INTR_STAT1 0x1A10 #define ACP3X_DSP_SW_INTR_BASE 0x1814 #define ACP5X_DSP_SW_INTR_BASE 0x1814 diff --git a/sound/soc/sof/amd/acp.h b/sound/soc/sof/amd/acp.h index 2058dae32659..e94713d7ff1d 100644 --- a/sound/soc/sof/amd/acp.h +++ b/sound/soc/sof/amd/acp.h @@ -96,8 +96,10 @@ #define ACP_ERROR_IRQ_MASK BIT(29) #define ACP_SDW0_IRQ_MASK BIT(21) #define ACP_SDW1_IRQ_MASK BIT(2) +#define SDW_ACPI_ADDR_ACP63 5 #define ACP_DEFAULT_SRAM_LENGTH 0x00080000 #define ACP_SRAM_PAGE_COUNT 128 +#define ACP6X_SDW_MAX_MANAGER_COUNT 2 enum clock_source { ACP_CLOCK_96M = 0, diff --git a/sound/soc/sof/amd/pci-acp63.c b/sound/soc/sof/amd/pci-acp63.c index bceb94ac80a9..eeaa12cceb23 100644 --- a/sound/soc/sof/amd/pci-acp63.c +++ b/sound/soc/sof/amd/pci-acp63.c @@ -31,12 +31,19 @@ static const struct sof_amd_acp_desc acp63_chip_info = { .rev = 6, .host_bridge_id = HOST_BRIDGE_ACP63, .pgfsm_base = ACP6X_PGFSM_BASE, + .ext_intr_enb = ACP6X_EXTERNAL_INTR_ENB, + .ext_intr_cntl = ACP6X_EXTERNAL_INTR_CNTL, .ext_intr_stat = ACP6X_EXT_INTR_STAT, + .ext_intr_stat1 = ACP6X_EXT_INTR_STAT1, .dsp_intr_base = ACP6X_DSP_SW_INTR_BASE, .sram_pte_offset = ACP6X_SRAM_PTE_OFFSET, .hw_semaphore_offset = ACP6X_AXI2DAGB_SEM_0, .fusion_dsp_offset = ACP6X_DSP_FUSION_RUNSTALL, .probe_reg_offset = ACP6X_FUTURE_REG_ACLK_0, + .sdw_max_link_count = ACP6X_SDW_MAX_MANAGER_COUNT, + .sdw_acpi_dev_addr = SDW_ACPI_ADDR_ACP63, + .reg_start_addr = ACP6x_REG_START, + .reg_end_addr = ACP6x_REG_END, }; static const struct sof_dev_desc acp63_desc = { -- 2.34.1

1 0

[PATCH V4 10/13] ASoC: SOF: amd: add machine select logic for soundwire based platforms
by Vijendar Mukunda 29 Jan '24

29 Jan '24

Add machine select logic for soundwire endpoints for AMD platforms. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> --- sound/soc/sof/amd/acp-common.c | 65 +++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/amd/acp-common.c b/sound/soc/sof/amd/acp-common.c index 2d72c6d55dc8..0fc4e20ec673 100644 --- a/sound/soc/sof/amd/acp-common.c +++ b/sound/soc/sof/amd/acp-common.c @@ -118,16 +118,72 @@ void amd_sof_dump(struct snd_sof_dev *sdev, u32 flags) &panic_info, stack, AMD_STACK_DUMP_SIZE); } +#if IS_ENABLED(CONFIG_SND_SOC_SOF_AMD_SOUNDWIRE) +static int amd_sof_sdw_get_slave_info(struct snd_sof_dev *sdev) +{ + struct acp_dev_data *acp_data = sdev->pdata->hw_pdata; + + return sdw_amd_get_slave_info(acp_data->sdw); +} + +static struct snd_soc_acpi_mach *amd_sof_sdw_machine_select(struct snd_sof_dev *sdev) +{ + struct snd_soc_acpi_mach *mach; + const struct snd_soc_acpi_link_adr *link; + struct acp_dev_data *acp_data = sdev->pdata->hw_pdata; + int ret, i; + + if (acp_data->info.count) { + ret = amd_sof_sdw_get_slave_info(sdev); + if (ret) { + dev_info(sdev->dev, "failed to read slave information\n"); + return NULL; + } + for (mach = sdev->pdata->desc->alt_machines; mach; mach++) { + if (!mach->links) + break; + link = mach->links; + for (i = 0; i < acp_data->info.count && link->num_adr; link++, i++) { + if (!snd_soc_acpi_sdw_link_slaves_found(sdev->dev, link, + acp_data->sdw->ids, + acp_data->sdw->num_slaves)) + break; + } + if (i == acp_data->info.count || !link->num_adr) + break; + } + if (mach && mach->link_mask) { + mach->mach_params.links = mach->links; + mach->mach_params.link_mask = mach->link_mask; + mach->mach_params.platform = dev_name(sdev->dev); + return mach; + } + } + dev_info(sdev->dev, "No SoundWire machine driver found\n"); + return NULL; +} + +#else +static struct snd_soc_acpi_mach *amd_sof_sdw_machine_select(struct snd_sof_dev *sdev) +{ + return NULL; +} +#endif + struct snd_soc_acpi_mach *amd_sof_machine_select(struct snd_sof_dev *sdev) { struct snd_sof_pdata *sof_pdata = sdev->pdata; const struct sof_dev_desc *desc = sof_pdata->desc; - struct snd_soc_acpi_mach *mach; + struct snd_soc_acpi_mach *mach = NULL; - mach = snd_soc_acpi_find_machine(desc->machines); + if (desc->machines) + mach = snd_soc_acpi_find_machine(desc->machines); if (!mach) { - dev_warn(sdev->dev, "No matching ASoC machine driver found\n"); - return NULL; + mach = amd_sof_sdw_machine_select(sdev); + if (!mach) { + dev_warn(sdev->dev, "No matching ASoC machine driver found\n"); + return NULL; + } } sof_pdata->tplg_filename = mach->sof_tplg_filename; @@ -204,5 +260,6 @@ EXPORT_SYMBOL_NS(sof_acp_common_ops, SND_SOC_SOF_AMD_COMMON); MODULE_IMPORT_NS(SND_SOC_SOF_AMD_COMMON); MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA); +MODULE_IMPORT_NS(SOUNDWIRE_AMD_INIT); MODULE_DESCRIPTION("ACP SOF COMMON Driver"); MODULE_LICENSE("Dual BSD/GPL"); -- 2.34.1

1 0

[PATCH V4 09/13] ASoC: SOF: amd: Add Soundwire DAI configuration support for AMD platforms
by Vijendar Mukunda 29 Jan '24

29 Jan '24

Add support for configuring AMD Soundwire DAI from topology. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> --- include/sound/sof/dai-amd.h | 7 ++++++ include/sound/sof/dai.h | 2 ++ include/uapi/sound/sof/tokens.h | 4 ++++ sound/soc/sof/ipc3-pcm.c | 25 +++++++++++++++++++++ sound/soc/sof/ipc3-topology.c | 40 +++++++++++++++++++++++++++++++++ sound/soc/sof/sof-audio.h | 1 + sound/soc/sof/topology.c | 5 +++++ 7 files changed, 84 insertions(+) diff --git a/include/sound/sof/dai-amd.h b/include/sound/sof/dai-amd.h index 9df7ac824efe..59cd014392c1 100644 --- a/include/sound/sof/dai-amd.h +++ b/include/sound/sof/dai-amd.h @@ -26,4 +26,11 @@ struct sof_ipc_dai_acpdmic_params { uint32_t pdm_ch; } __packed; +/* ACP_SDW Configuration Request - SOF_IPC_DAI_AMD_SDW_CONFIG */ +struct sof_ipc_dai_acp_sdw_params { + struct sof_ipc_hdr hdr; + u32 rate; + u32 channels; +} __packed; + #endif diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 4773a5f616a4..0764a80c17a9 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -89,6 +89,7 @@ enum sof_ipc_dai_type { SOF_DAI_AMD_SP_VIRTUAL, /**< AMD ACP SP VIRTUAL */ SOF_DAI_AMD_HS_VIRTUAL, /**< AMD ACP HS VIRTUAL */ SOF_DAI_IMX_MICFIL, /** < i.MX MICFIL PDM */ + SOF_DAI_AMD_SDW, /**< AMD ACP SDW */ }; /* general purpose DAI configuration */ @@ -119,6 +120,7 @@ struct sof_ipc_dai_config { struct sof_ipc_dai_acp_params acphs; struct sof_ipc_dai_mtk_afe_params afe; struct sof_ipc_dai_micfil_params micfil; + struct sof_ipc_dai_acp_sdw_params acp_sdw; }; } __packed; diff --git a/include/uapi/sound/sof/tokens.h b/include/uapi/sound/sof/tokens.h index ee5708934614..6bf00c09d30d 100644 --- a/include/uapi/sound/sof/tokens.h +++ b/include/uapi/sound/sof/tokens.h @@ -218,4 +218,8 @@ #define SOF_TKN_IMX_MICFIL_RATE 2000 #define SOF_TKN_IMX_MICFIL_CH 2001 +/* ACP SDW */ +#define SOF_TKN_AMD_ACP_SDW_RATE 2100 +#define SOF_TKN_AMD_ACP_SDW_CH 2101 + #endif diff --git a/sound/soc/sof/ipc3-pcm.c b/sound/soc/sof/ipc3-pcm.c index 330f04bcd75d..35769dd7905e 100644 --- a/sound/soc/sof/ipc3-pcm.c +++ b/sound/soc/sof/ipc3-pcm.c @@ -395,6 +395,31 @@ static int sof_ipc3_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, dev_dbg(component->dev, "MICFIL PDM channels_min: %d channels_max: %d\n", channels->min, channels->max); break; + case SOF_DAI_AMD_SDW: + /* change the default trigger sequence as per HW implementation */ + for_each_dpcm_fe(rtd, SNDRV_PCM_STREAM_PLAYBACK, dpcm) { + struct snd_soc_pcm_runtime *fe = dpcm->fe; + + fe->dai_link->trigger[SNDRV_PCM_STREAM_PLAYBACK] = + SND_SOC_DPCM_TRIGGER_POST; + } + + for_each_dpcm_fe(rtd, SNDRV_PCM_STREAM_CAPTURE, dpcm) { + struct snd_soc_pcm_runtime *fe = dpcm->fe; + + fe->dai_link->trigger[SNDRV_PCM_STREAM_CAPTURE] = + SND_SOC_DPCM_TRIGGER_POST; + } + rate->min = private->dai_config->acp_sdw.rate; + rate->max = private->dai_config->acp_sdw.rate; + channels->min = private->dai_config->acp_sdw.channels; + channels->max = private->dai_config->acp_sdw.channels; + + dev_dbg(component->dev, + "AMD_SDW rate_min: %d rate_max: %d\n", rate->min, rate->max); + dev_dbg(component->dev, "AMD_SDW channels_min: %d channels_max: %d\n", + channels->min, channels->max); + break; default: dev_err(component->dev, "Invalid DAI type %d\n", private->dai_config->type); break; diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index a8832a1c1a24..0970dbdfa78a 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -298,6 +298,14 @@ static const struct sof_topology_token micfil_pdm_tokens[] = { offsetof(struct sof_ipc_dai_micfil_params, pdm_ch)}, }; +/* ACP_SDW */ +static const struct sof_topology_token acp_sdw_tokens[] = { + {SOF_TKN_AMD_ACP_SDW_RATE, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, + offsetof(struct sof_ipc_dai_acp_sdw_params, rate)}, + {SOF_TKN_AMD_ACP_SDW_CH, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, + offsetof(struct sof_ipc_dai_acp_sdw_params, channels)}, +}; + /* Core tokens */ static const struct sof_topology_token core_tokens[] = { {SOF_TKN_COMP_CORE_ID, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, @@ -336,6 +344,7 @@ static const struct sof_token_info ipc3_token_list[SOF_TOKEN_COUNT] = { [SOF_ACPI2S_TOKENS] = {"ACPI2S tokens", acpi2s_tokens, ARRAY_SIZE(acpi2s_tokens)}, [SOF_MICFIL_TOKENS] = {"MICFIL PDM tokens", micfil_pdm_tokens, ARRAY_SIZE(micfil_pdm_tokens)}, + [SOF_ACP_SDW_TOKENS] = {"ACP_SDW tokens", acp_sdw_tokens, ARRAY_SIZE(acp_sdw_tokens)}, }; /** @@ -1315,6 +1324,34 @@ static int sof_link_acp_hs_load(struct snd_soc_component *scomp, struct snd_sof_ return 0; } +static int sof_link_acp_sdw_load(struct snd_soc_component *scomp, struct snd_sof_dai_link *slink, + struct sof_ipc_dai_config *config, struct snd_sof_dai *dai) +{ + struct sof_dai_private_data *private = dai->private; + u32 size = sizeof(*config); + int ret; + + /* parse the required set of ACP_SDW tokens based on num_hw_cfgs */ + ret = sof_update_ipc_object(scomp, &config->acp_sdw, SOF_ACP_SDW_TOKENS, slink->tuples, + slink->num_tuples, size, slink->num_hw_configs); + if (ret < 0) + return ret; + + /* init IPC */ + config->hdr.size = size; + dev_dbg(scomp->dev, "ACP SDW config rate %d channels %d\n", + config->acp_sdw.rate, config->acp_sdw.channels); + + /* set config for all DAI's with name matching the link name */ + dai->number_configs = 1; + dai->current_config = 0; + private->dai_config = kmemdup(config, size, GFP_KERNEL); + if (!private->dai_config) + return -ENOMEM; + + return 0; +} + static int sof_link_afe_load(struct snd_soc_component *scomp, struct snd_sof_dai_link *slink, struct sof_ipc_dai_config *config, struct snd_sof_dai *dai) { @@ -1629,6 +1666,9 @@ static int sof_ipc3_widget_setup_comp_dai(struct snd_sof_widget *swidget) case SOF_DAI_MEDIATEK_AFE: ret = sof_link_afe_load(scomp, slink, config, dai); break; + case SOF_DAI_AMD_SDW: + ret = sof_link_acp_sdw_load(scomp, slink, config, dai); + break; default: break; } diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 8874ee5f557f..f98242a404db 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -276,6 +276,7 @@ enum sof_tokens { SOF_ACPDMIC_TOKENS, SOF_ACPI2S_TOKENS, SOF_MICFIL_TOKENS, + SOF_ACP_SDW_TOKENS, /* this should be the last */ SOF_TOKEN_COUNT, diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 617a225fff24..25fb0d1443b6 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -297,6 +297,7 @@ static const struct sof_dai_types sof_dais[] = { {"ACPSP_VIRTUAL", SOF_DAI_AMD_SP_VIRTUAL}, {"ACPHS_VIRTUAL", SOF_DAI_AMD_HS_VIRTUAL}, {"MICFIL", SOF_DAI_IMX_MICFIL}, + {"ACP_SDW", SOF_DAI_AMD_SDW}, }; @@ -1968,6 +1969,10 @@ static int sof_link_load(struct snd_soc_component *scomp, int index, struct snd_ token_id = SOF_MICFIL_TOKENS; num_tuples += token_list[SOF_MICFIL_TOKENS].count; break; + case SOF_DAI_AMD_SDW: + token_id = SOF_ACP_SDW_TOKENS; + num_tuples += token_list[SOF_ACP_SDW_TOKENS].count; + break; default: break; } -- 2.34.1

1 0

Re: [PATCH v2 0/8] Improve SOF support for Steam Deck OLED
by Mark Brown 22 Jan '24

22 Jan '24

On Tue, 19 Dec 2023 05:07:18 +0200, Cristian Ciocaltea wrote: > This patch series is a continuation of [1] to provide several fixes and > improvements to the SOF drivers targeting the Vangogh platform, as found on > Valve's Steam Deck OLED. The previous series only handled the legacy ACP > drivers. > > Also note that, since v2, this patch series requires an updated SOF > topology to provide a correct DAI link ID for the BT codec. The binary > file should be located under: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/8] ASoC: amd: acp: Drop redundant initialization of machine driver data commit: f0f1021fc9cb88ebdc241b6121107399ee4f2eb7 [2/8] ASoC: amd: acp: Make use of existing *_CODEC_DAI macros commit: 68ab29426d88294d16170919a6a6e764f375113f [3/8] ASoC: amd: acp: Add missing error handling in sof-mach commit: d0ada20279db2649a7549a2b8a4a3379c59f238d [4/8] ASoC: amd: acp: Update MODULE_DESCRIPTION for sof-mach commit: a4832a94688000662d4ebb8a1c05f086a9c98826 [5/8] ASoC: SOF: amd: Fix memory leak in amd_sof_acp_probe() commit: 222be59e5eed1554119294edc743ee548c2371d0 [6/8] ASoC: SOF: amd: Optimize quirk for Valve Galileo commit: a13f0c3c0e8fb3e61fbfd99c6b350cf9be0c4660 [7/8] ASoC: SOF: core: Skip firmware test for custom loaders commit: 369b997a1371aeecd0a1fb0f9f4ef3747a1d07a4 [8/8] ASoC: SOF: amd: Compute file paths on firmware load commit: d9cacc1a2af2e1cd781b5cd2a3e57fbde64f5a2d 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

[PATCH AUTOSEL 6.1 07/53] ASoC: SOF: icp3-dtrace: Fix wrong kfree() usage
by Sasha Levin 22 Jan '24

22 Jan '24

From: Kamil Duljas <kamil.duljas(a)gmail.com> [ Upstream commit 8c91ca76f44804868d12aed20ebdbc2f89aa7d60 ] trace_filter_parse() allocs memory for *out and when -ENOMEM is returned, caller function, dfsentry_trace_filter_write() trying to freed this memory. After this patch, the memory is freed in trace_filter_parse() before -EINVAL returned. In caller function removed kfree(elms) from error label Signed-off-by: Kamil Duljas <kamil.duljas(a)gmail.com> Link: https://lore.kernel.org/r/20231116220102.2097-2-kamil.duljas@gmail.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/ipc3-dtrace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/ipc3-dtrace.c b/sound/soc/sof/ipc3-dtrace.c index 8cf421577378..c626cefa0ec5 100644 --- a/sound/soc/sof/ipc3-dtrace.c +++ b/sound/soc/sof/ipc3-dtrace.c @@ -137,6 +137,7 @@ static int trace_filter_parse(struct snd_sof_dev *sdev, char *string, dev_err(sdev->dev, "Parsing filter entry '%s' failed with %d\n", entry, entry_len); + kfree(*out); return -EINVAL; } } @@ -215,13 +216,13 @@ static ssize_t dfsentry_trace_filter_write(struct file *file, const char __user ret = ipc3_trace_update_filter(sdev, num_elems, elems); if (ret < 0) { dev_err(sdev->dev, "Filter update failed: %d\n", ret); + kfree(elems); goto error; } } ret = count; error: kfree(string); - kfree(elems); return ret; } -- 2.43.0

1 0