| Message ID | 20201208153654.2733354-1-perex@perex.cz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: AMD Renoir - add DMI table to avoid the ACP mic probe (broken BIOS) | expand |
On Tue, 08 Dec 2020 16:36:54 +0100, Jaroslav Kysela wrote: > > Users reported that some Lenovo AMD platforms do not have ACP microphone, > but the BIOS advertises it via ACPI. > > This patch create a simple DMI table, where those machines with the broken > BIOS can be added. The DMI description for Lenovo IdeaPad 5 and > IdeaPad Flex 5 devices are added there. > > Also describe the dmic_acpi_check kernel module parameter in a more > understandable way. > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1892115 > Cc: <stable@kernel.org> > Cc: Vijendar Mukunda <Vijendar.Mukunda@amd.com> > Cc: Mark Brown <broonie@kernel.org> > Signed-off-by: Jaroslav Kysela <perex@perex.cz> > --- > sound/soc/amd/renoir/rn-pci-acp3x.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/sound/soc/amd/renoir/rn-pci-acp3x.c b/sound/soc/amd/renoir/rn-pci-acp3x.c > index b943e59fc302..3289ab3eae6f 100644 > --- a/sound/soc/amd/renoir/rn-pci-acp3x.c > +++ b/sound/soc/amd/renoir/rn-pci-acp3x.c > @@ -6,6 +6,7 @@ > > #include <linux/pci.h> > #include <linux/acpi.h> > +#include <linux/dmi.h> > #include <linux/module.h> > #include <linux/io.h> > #include <linux/delay.h> > @@ -20,14 +21,13 @@ module_param(acp_power_gating, int, 0644); > MODULE_PARM_DESC(acp_power_gating, "Enable acp power gating"); > > /** > - * dmic_acpi_check = -1 - Checks ACPI method to know DMIC hardware status runtime > - * = 0 - Skips the DMIC device creation and returns probe failure > - * = 1 - Assumes that platform has DMIC support and skips ACPI > - * method check > + * dmic_acpi_check = -1 - Use ACPI/DMI method to detect the DMIC hardware presence at runtime > + * = 0 - Skip the DMIC device creation and return probe failure > + * = 1 - Force DMIC support > */ > static int dmic_acpi_check = ACP_DMIC_AUTO; > module_param(dmic_acpi_check, bint, 0644); > -MODULE_PARM_DESC(dmic_acpi_check, "checks Dmic hardware runtime"); > +MODULE_PARM_DESC(dmic_acpi_check, "Digital microphone presence (-1=auto, 0=none, 1=force)"); > > struct acp_dev_data { > void __iomem *acp_base; > @@ -163,6 +163,17 @@ static int rn_acp_deinit(void __iomem *acp_base) > return 0; > } > > +static const struct dmi_system_id rn_acp_quirk_table[] = { > + { > + /* Lenovo IdeaPad Flex 5 14ARE05, IdeaPad 5 15ARE05 */ > + .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "LENOVO"), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "LNVNB161216"), > + } > + }, > + {} > +}; > + > static int snd_rn_acp_probe(struct pci_dev *pci, > const struct pci_device_id *pci_id) > { > @@ -172,6 +183,7 @@ static int snd_rn_acp_probe(struct pci_dev *pci, > acpi_handle handle; > acpi_integer dmic_status; > #endif > + const struct dmi_system_id *dmi_id; > unsigned int irqflags; > int ret, index; > u32 addr; > @@ -232,6 +244,12 @@ static int snd_rn_acp_probe(struct pci_dev *pci, > goto de_init; > } > #endif > + dmi_id = dmi_first_match(rn_acp_quirk_table); > + if (dmi_id && !dmi_id->driver_data) { > + dev_warn(&pci->dev, "ACPI settings override using DMI (ACP mic is not present)"); IMO, better to be dev_info() here. It's the correct set up, hence it should be neither error nor warning that appears in the boot screen over the boot splash. BTW, both Raven and Reonir drivers point to the very same PCI ID, and both drivers will be probed for this machine (and both to be skipped). Also, I noticed that Renoir driver tries to detect the dmic at the late stage; this could be done at the very beginning, so the whole allocation and initialization could be simply skipped. But this can be done in a separate cleanup patch. thanks, Takashi
Dne 08. 12. 20 v 17:24 Takashi Iwai napsal(a): > On Tue, 08 Dec 2020 16:36:54 +0100, > Jaroslav Kysela wrote: >> >> Users reported that some Lenovo AMD platforms do not have ACP microphone, >> but the BIOS advertises it via ACPI. >> >> This patch create a simple DMI table, where those machines with the broken >> BIOS can be added. The DMI description for Lenovo IdeaPad 5 and >> IdeaPad Flex 5 devices are added there. >> >> Also describe the dmic_acpi_check kernel module parameter in a more >> understandable way. >> >> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1892115 >> Cc: <stable@kernel.org> >> Cc: Vijendar Mukunda <Vijendar.Mukunda@amd.com> >> Cc: Mark Brown <broonie@kernel.org> >> Signed-off-by: Jaroslav Kysela <perex@perex.cz> >> --- >> sound/soc/amd/renoir/rn-pci-acp3x.c | 28 +++++++++++++++++++++++----- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/sound/soc/amd/renoir/rn-pci-acp3x.c b/sound/soc/amd/renoir/rn-pci-acp3x.c >> index b943e59fc302..3289ab3eae6f 100644 >> --- a/sound/soc/amd/renoir/rn-pci-acp3x.c >> +++ b/sound/soc/amd/renoir/rn-pci-acp3x.c >> @@ -6,6 +6,7 @@ >> >> #include <linux/pci.h> >> #include <linux/acpi.h> >> +#include <linux/dmi.h> >> #include <linux/module.h> >> #include <linux/io.h> >> #include <linux/delay.h> >> @@ -20,14 +21,13 @@ module_param(acp_power_gating, int, 0644); >> MODULE_PARM_DESC(acp_power_gating, "Enable acp power gating"); >> >> /** >> - * dmic_acpi_check = -1 - Checks ACPI method to know DMIC hardware status runtime >> - * = 0 - Skips the DMIC device creation and returns probe failure >> - * = 1 - Assumes that platform has DMIC support and skips ACPI >> - * method check >> + * dmic_acpi_check = -1 - Use ACPI/DMI method to detect the DMIC hardware presence at runtime >> + * = 0 - Skip the DMIC device creation and return probe failure >> + * = 1 - Force DMIC support >> */ >> static int dmic_acpi_check = ACP_DMIC_AUTO; >> module_param(dmic_acpi_check, bint, 0644); >> -MODULE_PARM_DESC(dmic_acpi_check, "checks Dmic hardware runtime"); >> +MODULE_PARM_DESC(dmic_acpi_check, "Digital microphone presence (-1=auto, 0=none, 1=force)"); >> >> struct acp_dev_data { >> void __iomem *acp_base; >> @@ -163,6 +163,17 @@ static int rn_acp_deinit(void __iomem *acp_base) >> return 0; >> } >> >> +static const struct dmi_system_id rn_acp_quirk_table[] = { >> + { >> + /* Lenovo IdeaPad Flex 5 14ARE05, IdeaPad 5 15ARE05 */ >> + .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "LENOVO"), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "LNVNB161216"), >> + } >> + }, >> + {} >> +}; >> + >> static int snd_rn_acp_probe(struct pci_dev *pci, >> const struct pci_device_id *pci_id) >> { >> @@ -172,6 +183,7 @@ static int snd_rn_acp_probe(struct pci_dev *pci, >> acpi_handle handle; >> acpi_integer dmic_status; >> #endif >> + const struct dmi_system_id *dmi_id; >> unsigned int irqflags; >> int ret, index; >> u32 addr; >> @@ -232,6 +244,12 @@ static int snd_rn_acp_probe(struct pci_dev *pci, >> goto de_init; >> } >> #endif >> + dmi_id = dmi_first_match(rn_acp_quirk_table); >> + if (dmi_id && !dmi_id->driver_data) { >> + dev_warn(&pci->dev, "ACPI settings override using DMI (ACP mic is not present)"); > > IMO, better to be dev_info() here. It's the correct set up, hence > it should be neither error nor warning that appears in the boot screen > over the boot splash. I sent v2 with this change. Thanks. > BTW, both Raven and Reonir drivers point to the very same PCI ID, > and both drivers will be probed for this machine (and both to be > skipped). It looks like a bad design. I believe that only one PCI module (driver) should handle this PCI device and do the I2S / PDM redirection in the PCI probe callback. > Also, I noticed that Renoir driver tries to detect the dmic at the > late stage; this could be done at the very beginning, so the whole > allocation and initialization could be simply skipped. But this can > be done in a separate cleanup patch. Vijendar, could you give a look? Thanks, Jaroslav
On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: > BTW, both Raven and Reonir drivers point to the very same PCI ID, > and both drivers will be probed for this machine (and both to be > skipped). Ugh, that's not good. It's not even super obvious from the code that this is happening. Seems like it should be one core driver which instantiates the components for Raven and Reonir as appropriate, the PCI driver is pretty thin at present anyway.
Dne 08. 12. 20 v 19:06 Mukunda,Vijendar napsal(a): > > > On 08/12/20 11:10 pm, Mark Brown wrote: >> On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: >> >>> BTW, both Raven and Reonir drivers point to the very same PCI ID, >>> and both drivers will be probed for this machine (and both to be >>> skipped). >> >> Ugh, that's not good. It's not even super obvious from the code that >> this is happening. Seems like it should be one core driver which >> instantiates the components for Raven and Reonir as appropriate, the PCI >> driver is pretty thin at present anyway. >> > > Raven and Renoir has same PCI ID but both platforms have different > revision ID. Raven platform revision id is 0x00 where as for Renoir it > is 0x01. I don't see pci->revision early check in the probe callbacks. Jaroslav
On Tue, 08 Dec 2020 19:06:21 +0100, Mukunda,Vijendar wrote: > > > > On 08/12/20 11:10 pm, Mark Brown wrote: > > On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: > > > >> BTW, both Raven and Reonir drivers point to the very same PCI ID, > >> and both drivers will be probed for this machine (and both to be > >> skipped). > > > > Ugh, that's not good. It's not even super obvious from the code that > > this is happening. Seems like it should be one core driver which > > instantiates the components for Raven and Reonir as appropriate, the PCI > > driver is pretty thin at present anyway. > > > > Raven and Renoir has same PCI ID but both platforms have different > revision ID. Raven platform revision id is 0x00 where as for Renoir it > is 0x01. But your drivers don't check the revision ID, as far as I see? The linux PCI driver doesn't distinguish the revision id at the matching time, unfortunately. Takashi
On 08/12/20 11:10 pm, Mark Brown wrote: > On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: > >> BTW, both Raven and Reonir drivers point to the very same PCI ID, >> and both drivers will be probed for this machine (and both to be >> skipped). > > Ugh, that's not good. It's not even super obvious from the code that > this is happening. Seems like it should be one core driver which > instantiates the components for Raven and Reonir as appropriate, the PCI > driver is pretty thin at present anyway. > Raven and Renoir has same PCI ID but both platforms have different revision ID. Raven platform revision id is 0x00 where as for Renoir it is 0x01.
Dne 08. 12. 20 v 19:23 Mukunda,Vijendar napsal(a): > > > On 08/12/20 11:51 pm, Mukunda,Vijendar wrote: >> >> >> On 08/12/20 11:27 pm, Takashi Iwai wrote: >>> On Tue, 08 Dec 2020 19:06:21 +0100, >>> Mukunda,Vijendar wrote: >>>> >>>> >>>> >>>> On 08/12/20 11:10 pm, Mark Brown wrote: >>>>> On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: >>>>> >>>>>> BTW, both Raven and Reonir drivers point to the very same PCI ID, >>>>>> and both drivers will be probed for this machine (and both to be >>>>>> skipped). >>>>> >>>>> Ugh, that's not good. It's not even super obvious from the code that >>>>> this is happening. Seems like it should be one core driver which >>>>> instantiates the components for Raven and Reonir as appropriate, the >>>>> PCI >>>>> driver is pretty thin at present anyway. >>>>> >>>> >>>> Raven and Renoir has same PCI ID but both platforms have different >>>> revision ID. Raven platform revision id is 0x00 where as for Renoir it >>>> is 0x01. >>> >>> But your drivers don't check the revision ID, as far as I see? >>> >>> The linux PCI driver doesn't distinguish the revision id at the >>> matching time, unfortunately. >>> >>> >>> Takashi >>> >> Apart from Revision ID difference, There are few hardware differences >> specific to ACP IP. >> ACP IP hardware versions are different for Raven and Renoir. >> Unfortunately we don't have specific logic to distinguish ACP hardware >> versions for Raven and Renoir. >> > But build wise both Raven and Renoir uses different Kconfig options. We need to build all drivers for the universal distros to one kernel. The Kconfig does not help here. Jaroslav
On 08/12/20 11:27 pm, Takashi Iwai wrote: > On Tue, 08 Dec 2020 19:06:21 +0100, > Mukunda,Vijendar wrote: >> >> >> >> On 08/12/20 11:10 pm, Mark Brown wrote: >>> On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: >>> >>>> BTW, both Raven and Reonir drivers point to the very same PCI ID, >>>> and both drivers will be probed for this machine (and both to be >>>> skipped). >>> >>> Ugh, that's not good. It's not even super obvious from the code that >>> this is happening. Seems like it should be one core driver which >>> instantiates the components for Raven and Reonir as appropriate, the PCI >>> driver is pretty thin at present anyway. >>> >> >> Raven and Renoir has same PCI ID but both platforms have different >> revision ID. Raven platform revision id is 0x00 where as for Renoir it >> is 0x01. > > But your drivers don't check the revision ID, as far as I see? > > The linux PCI driver doesn't distinguish the revision id at the > matching time, unfortunately. > > > Takashi > Apart from Revision ID difference, There are few hardware differences specific to ACP IP. ACP IP hardware versions are different for Raven and Renoir. Unfortunately we don't have specific logic to distinguish ACP hardware versions for Raven and Renoir.
On 08/12/20 11:51 pm, Mukunda,Vijendar wrote: > > > On 08/12/20 11:27 pm, Takashi Iwai wrote: >> On Tue, 08 Dec 2020 19:06:21 +0100, >> Mukunda,Vijendar wrote: >>> >>> >>> >>> On 08/12/20 11:10 pm, Mark Brown wrote: >>>> On Tue, Dec 08, 2020 at 05:24:32PM +0100, Takashi Iwai wrote: >>>> >>>>> BTW, both Raven and Reonir drivers point to the very same PCI ID, >>>>> and both drivers will be probed for this machine (and both to be >>>>> skipped). >>>> >>>> Ugh, that's not good. It's not even super obvious from the code that >>>> this is happening. Seems like it should be one core driver which >>>> instantiates the components for Raven and Reonir as appropriate, the >>>> PCI >>>> driver is pretty thin at present anyway. >>>> >>> >>> Raven and Renoir has same PCI ID but both platforms have different >>> revision ID. Raven platform revision id is 0x00 where as for Renoir it >>> is 0x01. >> >> But your drivers don't check the revision ID, as far as I see? >> >> The linux PCI driver doesn't distinguish the revision id at the >> matching time, unfortunately. >> >> >> Takashi >> > Apart from Revision ID difference, There are few hardware differences > specific to ACP IP. > ACP IP hardware versions are different for Raven and Renoir. > Unfortunately we don't have specific logic to distinguish ACP hardware > versions for Raven and Renoir. > But build wise both Raven and Renoir uses different Kconfig options.
On Tue, Dec 08, 2020 at 07:14:06PM +0100, Jaroslav Kysela wrote: > Dne 08. 12. 20 v 19:23 Mukunda,Vijendar napsal(a): > > But build wise both Raven and Renoir uses different Kconfig options. > We need to build all drivers for the universal distros to one kernel. The > Kconfig does not help here. Yeah, even for embedded targets like phones this is getting less and less useful over time - for systems with PCI like this people really want a single kernel image for every user, not per-board images.
diff --git a/sound/soc/amd/renoir/rn-pci-acp3x.c b/sound/soc/amd/renoir/rn-pci-acp3x.c index b943e59fc302..3289ab3eae6f 100644 --- a/sound/soc/amd/renoir/rn-pci-acp3x.c +++ b/sound/soc/amd/renoir/rn-pci-acp3x.c @@ -6,6 +6,7 @@ #include <linux/pci.h> #include <linux/acpi.h> +#include <linux/dmi.h> #include <linux/module.h> #include <linux/io.h> #include <linux/delay.h> @@ -20,14 +21,13 @@ module_param(acp_power_gating, int, 0644); MODULE_PARM_DESC(acp_power_gating, "Enable acp power gating"); /** - * dmic_acpi_check = -1 - Checks ACPI method to know DMIC hardware status runtime - * = 0 - Skips the DMIC device creation and returns probe failure - * = 1 - Assumes that platform has DMIC support and skips ACPI - * method check + * dmic_acpi_check = -1 - Use ACPI/DMI method to detect the DMIC hardware presence at runtime + * = 0 - Skip the DMIC device creation and return probe failure + * = 1 - Force DMIC support */ static int dmic_acpi_check = ACP_DMIC_AUTO; module_param(dmic_acpi_check, bint, 0644); -MODULE_PARM_DESC(dmic_acpi_check, "checks Dmic hardware runtime"); +MODULE_PARM_DESC(dmic_acpi_check, "Digital microphone presence (-1=auto, 0=none, 1=force)"); struct acp_dev_data { void __iomem *acp_base; @@ -163,6 +163,17 @@ static int rn_acp_deinit(void __iomem *acp_base) return 0; } +static const struct dmi_system_id rn_acp_quirk_table[] = { + { + /* Lenovo IdeaPad Flex 5 14ARE05, IdeaPad 5 15ARE05 */ + .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "LNVNB161216"), + } + }, + {} +}; + static int snd_rn_acp_probe(struct pci_dev *pci, const struct pci_device_id *pci_id) { @@ -172,6 +183,7 @@ static int snd_rn_acp_probe(struct pci_dev *pci, acpi_handle handle; acpi_integer dmic_status; #endif + const struct dmi_system_id *dmi_id; unsigned int irqflags; int ret, index; u32 addr; @@ -232,6 +244,12 @@ static int snd_rn_acp_probe(struct pci_dev *pci, goto de_init; } #endif + dmi_id = dmi_first_match(rn_acp_quirk_table); + if (dmi_id && !dmi_id->driver_data) { + dev_warn(&pci->dev, "ACPI settings override using DMI (ACP mic is not present)"); + ret = -ENODEV; + goto de_init; + } } adata->res = devm_kzalloc(&pci->dev,
Users reported that some Lenovo AMD platforms do not have ACP microphone, but the BIOS advertises it via ACPI. This patch create a simple DMI table, where those machines with the broken BIOS can be added. The DMI description for Lenovo IdeaPad 5 and IdeaPad Flex 5 devices are added there. Also describe the dmic_acpi_check kernel module parameter in a more understandable way. BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1892115 Cc: <stable@kernel.org> Cc: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Cc: Mark Brown <broonie@kernel.org> Signed-off-by: Jaroslav Kysela <perex@perex.cz> --- sound/soc/amd/renoir/rn-pci-acp3x.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)