[PATCH] ASoC: AMD Renoir - add DMI table to avoid the ACP mic probe (broken BIOS)

[Jaroslav Kysela]

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 at kernel.org>
>> Cc: Vijendar Mukunda <Vijendar.Mukunda at amd.com>
>> Cc: Mark Brown <broonie at kernel.org>
>> Signed-off-by: Jaroslav Kysela <perex at 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

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.