| Message ID | 20211117093734.17407-8-daniel.baluta@oss.nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f1bdd8d385a803565024c8feeedc17bf86aac4f5 |
| Headers | show |
| Series | ASoC: SOF: Platform updates for AMD and Mediatek | expand |
Hi Daniel, On Wed, Nov 17, 2021 at 12:39 PM Daniel Baluta <daniel.baluta@oss.nxp.com> wrote: > From: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> > > ACP hw block configuration differs across various distributions > and hence it's required to register different drivers module for > distributions. For now we support three ACP drivers: > > * ACP without SOF use case > * ACP with SOF use case > * ACP with SOF use case for DMIC and non SOF for I2S endpoints > > As all above driver registers with common PCI ID for ACP hw block > we need code to determine ACP configuration and auto select driver > module. This patch expose function that return configuration flag > based on dmi checks for a system. ACP driver module probe register > platform device based on such configuration flag to avoid conflict > with other ACP drivers probed for same PCI ID. > > Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> > Reviewed-by: Bard Liao <bard.liao@intel.com> > Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> Thanks for your patch, which is now commit f1bdd8d385a80356 ("ASoC: amd: Add module to determine ACP configuration") in sound-asoc/for-next. > --- a/sound/soc/amd/Kconfig > +++ b/sound/soc/amd/Kconfig > @@ -96,4 +96,10 @@ config SND_SOC_AMD_YC_MACH > Say m if you have such a device. > If unsure select "N". > > +config SND_AMD_ACP_CONFIG > + tristate "AMD ACP configuration selection" This definitely needs proper dependencies, to prevent asking the user about this when configuring a kernel without AMD Audio ACP support. I would have sent a patch, but... > + help > + This option adds an auto detection to determine which ACP > + driver modules to use > + > source "sound/soc/amd/acp/Kconfig" > --- /dev/null > +++ b/sound/soc/amd/acp-config.c > @@ -0,0 +1,81 @@ > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) > +// > +// This file is provided under a dual BSD/GPLv2 license. When using or > +// redistributing this file, you may do so under either license. > +// > +// Copyright(c) 2021 Advanced Micro Devices, Inc. > +// > +// Authors: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> > +// > + > +/* ACP machine configuration module */ > + > +#include <linux/acpi.h> > +#include <linux/bits.h> > +#include <linux/dmi.h> > +#include <linux/module.h> > +#include <linux/pci.h> > + > +#include "../sof/amd/acp.h" This doesn't seem to use anything from this header file? > +#include "mach-config.h" > + > +static int acp_quirk_data; > + > +static const struct config_entry config_table[] = { > + { > + .flags = FLAG_AMD_SOF, > + .device = ACP_PCI_DEV_ID, > + .dmi_table = (const struct dmi_system_id []) { > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "AMD"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Majolica-CZN"), > + }, > + }, > + {} > + }, > + }, > +}; > + > +int snd_amd_acp_find_config(struct pci_dev *pci) > +{ > + const struct config_entry *table = config_table; > + u16 device = pci->device; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(config_table); i++, table++) { > + if (table->device != device) > + continue; > + if (table->dmi_table && !dmi_check_system(table->dmi_table)) > + continue; > + acp_quirk_data = table->flags; > + return table->flags; > + } > + > + return 0; > +} > +EXPORT_SYMBOL(snd_amd_acp_find_config); > +struct snd_soc_acpi_mach snd_soc_acpi_amd_acp_machines[] = { > + { > + .id = "AMDI1019", > + .drv_name = "renoir-acp", > + .pdata = (void *)&acp_quirk_data, > + }, > + {}, > +}; > +EXPORT_SYMBOL(snd_soc_acpi_amd_acp_machines); These symbols are only used from sound/soc/sof/amd/pci-rn.c. Why is this code living under sound/soc/amd/, while the ACP code is under sound/soc/amd/acp/? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
>> --- a/sound/soc/amd/Kconfig >> +++ b/sound/soc/amd/Kconfig >> @@ -96,4 +96,10 @@ config SND_SOC_AMD_YC_MACH >> Say m if you have such a device. >> If unsure select "N". >> >> +config SND_AMD_ACP_CONFIG >> + tristate "AMD ACP configuration selection" > > This definitely needs proper dependencies, to prevent asking the user > about this when configuring a kernel without AMD Audio ACP support. > > I would have sent a patch, but... There's indeed a missing dependency that was fixed in https://github.com/thesofproject/linux/pull/3284 Daniel, you may want to squash it in an update? To Geert's point, there may be an additional need to add a depends on SND_SOC_AMD_ACP There are also a set of SND_SOC_AMD_ACPyx options, not sure if any applies as a dependency here?
On Tue, Nov 30, 2021 at 10:49:30AM -0600, Pierre-Louis Bossart wrote: > To Geert's point, there may be an additional need to add a > depends on SND_SOC_AMD_ACP > There are also a set of > SND_SOC_AMD_ACPyx options, not sure if any applies as a dependency here? Or put them in an if block (IIRC I thought they were which was why the dependency wasn't needed but I don't know what I was looking at if I did check that).
On Tue, Nov 30, 2021 at 7:05 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Nov 30, 2021 at 10:49:30AM -0600, Pierre-Louis Bossart wrote: > > > To Geert's point, there may be an additional need to add a > > > depends on SND_SOC_AMD_ACP > > > There are also a set of > > > SND_SOC_AMD_ACPyx options, not sure if any applies as a dependency here? > > Or put them in an if block (IIRC I thought they were which was why the > dependency wasn't needed but I don't know what I was looking at if I did > check that). There will be some delays in handling this as I'm in vacation until next Monday.
Hi Mark, On Tue, Nov 30, 2021 at 6:05 PM Mark Brown <broonie@kernel.org> wrote: > On Tue, Nov 30, 2021 at 10:49:30AM -0600, Pierre-Louis Bossart wrote: > > To Geert's point, there may be an additional need to add a > > > depends on SND_SOC_AMD_ACP > > > There are also a set of > > > SND_SOC_AMD_ACPyx options, not sure if any applies as a dependency here? > > Or put them in an if block (IIRC I thought they were which was why the > dependency wasn't needed but I don't know what I was looking at if I did > check that). Probably you were looking at sound/soc/amd/acp/Kconfig, where all the other ACP options live, and where you had applied my earlier patch to wrap everything in a big "if". Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig index 2c6af3f8f296..092966ff5ea7 100644 --- a/sound/soc/amd/Kconfig +++ b/sound/soc/amd/Kconfig @@ -96,4 +96,10 @@ config SND_SOC_AMD_YC_MACH Say m if you have such a device. If unsure select "N". +config SND_AMD_ACP_CONFIG + tristate "AMD ACP configuration selection" + help + This option adds an auto detection to determine which ACP + driver modules to use + source "sound/soc/amd/acp/Kconfig" diff --git a/sound/soc/amd/Makefile b/sound/soc/amd/Makefile index f1d42bbda709..4b1f77930a4a 100644 --- a/sound/soc/amd/Makefile +++ b/sound/soc/amd/Makefile @@ -3,6 +3,7 @@ acp_audio_dma-objs := acp-pcm-dma.o snd-soc-acp-da7219mx98357-mach-objs := acp-da7219-max98357a.o snd-soc-acp-rt5645-mach-objs := acp-rt5645.o snd-soc-acp-rt5682-mach-objs := acp3x-rt5682-max9836.o +snd-acp-config-objs := acp-config.o obj-$(CONFIG_SND_SOC_AMD_ACP) += acp_audio_dma.o obj-$(CONFIG_SND_SOC_AMD_CZ_DA7219MX98357_MACH) += snd-soc-acp-da7219mx98357-mach.o @@ -13,3 +14,4 @@ obj-$(CONFIG_SND_SOC_AMD_RENOIR) += renoir/ obj-$(CONFIG_SND_SOC_AMD_ACP5x) += vangogh/ obj-$(CONFIG_SND_SOC_AMD_ACP6x) += yc/ obj-$(CONFIG_SND_SOC_AMD_ACP_COMMON) += acp/ +obj-$(CONFIG_SND_AMD_ACP_CONFIG) += snd-acp-config.o diff --git a/sound/soc/amd/acp-config.c b/sound/soc/amd/acp-config.c new file mode 100644 index 000000000000..c9abbb46b6f5 --- /dev/null +++ b/sound/soc/amd/acp-config.c @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) +// +// This file is provided under a dual BSD/GPLv2 license. When using or +// redistributing this file, you may do so under either license. +// +// Copyright(c) 2021 Advanced Micro Devices, Inc. +// +// Authors: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> +// + +/* ACP machine configuration module */ + +#include <linux/acpi.h> +#include <linux/bits.h> +#include <linux/dmi.h> +#include <linux/module.h> +#include <linux/pci.h> + +#include "../sof/amd/acp.h" +#include "mach-config.h" + +static int acp_quirk_data; + +static const struct config_entry config_table[] = { + { + .flags = FLAG_AMD_SOF, + .device = ACP_PCI_DEV_ID, + .dmi_table = (const struct dmi_system_id []) { + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "AMD"), + DMI_MATCH(DMI_PRODUCT_NAME, "Majolica-CZN"), + }, + }, + {} + }, + }, +}; + +int snd_amd_acp_find_config(struct pci_dev *pci) +{ + const struct config_entry *table = config_table; + u16 device = pci->device; + int i; + + for (i = 0; i < ARRAY_SIZE(config_table); i++, table++) { + if (table->device != device) + continue; + if (table->dmi_table && !dmi_check_system(table->dmi_table)) + continue; + acp_quirk_data = table->flags; + return table->flags; + } + + return 0; +} +EXPORT_SYMBOL(snd_amd_acp_find_config); + +struct snd_soc_acpi_mach snd_soc_acpi_amd_sof_machines[] = { + { + .id = "AMDI1019", + .drv_name = "renoir-dsp", + .pdata = (void *)&acp_quirk_data, + .fw_filename = "sof-rn.ri", + .sof_tplg_filename = "sof-acp.tplg", + }, + {}, +}; +EXPORT_SYMBOL(snd_soc_acpi_amd_sof_machines); + +struct snd_soc_acpi_mach snd_soc_acpi_amd_acp_machines[] = { + { + .id = "AMDI1019", + .drv_name = "renoir-acp", + .pdata = (void *)&acp_quirk_data, + }, + {}, +}; +EXPORT_SYMBOL(snd_soc_acpi_amd_acp_machines); + +MODULE_LICENSE("Dual BSD/GPL"); diff --git a/sound/soc/amd/mach-config.h b/sound/soc/amd/mach-config.h new file mode 100644 index 000000000000..608f1e199775 --- /dev/null +++ b/sound/soc/amd/mach-config.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */ +/* + * This file is provided under a dual BSD/GPLv2 license. When using or + * redistributing this file, you may do so under either license. + * + * Copyright(c) 2021 Advanced Micro Devices, Inc. All rights reserved. + * + * Author: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> + */ +#ifndef __AMD_MACH_CONFIG_H +#define __AMD_MACH_CONFIG_H + +#include <sound/soc-acpi.h> + +#define FLAG_AMD_SOF BIT(1) +#define FLAG_AMD_SOF_ONLY_DMIC BIT(2) + +#define ACP_PCI_DEV_ID 0x15E2 + +extern struct snd_soc_acpi_mach snd_soc_acpi_amd_sof_machines[]; +extern struct snd_soc_acpi_mach snd_soc_acpi_amd_acp_machines[]; + +struct config_entry { + u32 flags; + u16 device; + const struct dmi_system_id *dmi_table; +}; + +#endif