diff mbox series

ALSA: hda: intel-dsp-config: Fix Huawei Matebook D14 NBLB-WAX9N quirk detection

Message ID 5e6ba980c0738199589749b68b83f2d730512107.1713430105.git.mchehab@kernel.org (mailing list archive)
State New, archived
Headers show
Series ALSA: hda: intel-dsp-config: Fix Huawei Matebook D14 NBLB-WAX9N quirk detection | expand

Commit Message

Mauro Carvalho Chehab April 18, 2024, 8:48 a.m. UTC
Newer Matebook D14 model comes with essx8336 and supports SOF,
but the initial models use the legacy driver, with a Realtek ALC 256
AC97 chip on it.

The BIOS seems to be prepared to be used by both models, so
it contains an entry for ESSX8336 on its DSDT table.

Add a quirk, as otherwise dspconfig driver will try to load
SOF, causing audio probe to fail.

Cc: stable@vger.kernel.org
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 sound/hda/intel-dsp-config.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Mauro Carvalho Chehab April 18, 2024, 10:04 a.m. UTC | #1
Em Thu, 18 Apr 2024 09:48:27 +0100
Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:

> Newer Matebook D14 model comes with essx8336 and supports SOF,
> but the initial models use the legacy driver, with a Realtek ALC 256
> AC97 chip on it.
> 
> The BIOS seems to be prepared to be used by both models, so
> it contains an entry for ESSX8336 on its DSDT table.
> 
> Add a quirk, as otherwise dspconfig driver will try to load
> SOF, causing audio probe to fail.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Worth to mention that I opened an issue on Github about that:

https://github.com/thesofproject/linux/issues/4934

I added there the ACPI DSDT table as a reference.


> ---
>  sound/hda/intel-dsp-config.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
> index 6a384b922e4f..8e728f0585dd 100644
> --- a/sound/hda/intel-dsp-config.c
> +++ b/sound/hda/intel-dsp-config.c
> @@ -46,6 +46,22 @@ static const struct snd_soc_acpi_codecs __maybe_unused essx_83x6 = {
>   * - the first successful match will win
>   */
>  static const struct config_entry config_table[] = {
> +	/* Quirks */
> +	{
> +		.flags = 0,	/* Model uses AC97 with Realtek ALC 256 */
> +		.device = PCI_DEVICE_ID_INTEL_HDA_CML_LP,
> +		.dmi_table = (const struct dmi_system_id []) {
> +			{
> +				.ident = "Huawei NBLB-WAX9N",
> +				.matches = {
> +					DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"),
> +					DMI_MATCH(DMI_PRODUCT_NAME, "NBLB-WAX9N"),
> +				}
> +			},
> +			{}
> +		}
> +	},
> +
>  /* Merrifield */
>  #if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
>  	{
Pierre-Louis Bossart April 18, 2024, 1:24 p.m. UTC | #2
On 4/18/24 05:04, Mauro Carvalho Chehab wrote:
> Em Thu, 18 Apr 2024 09:48:27 +0100
> Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:
> 
>> Newer Matebook D14 model comes with essx8336 and supports SOF,
>> but the initial models use the legacy driver, with a Realtek ALC 256
>> AC97 chip on it.
>>
>> The BIOS seems to be prepared to be used by both models, so
>> it contains an entry for ESSX8336 on its DSDT table.
>>
>> Add a quirk, as otherwise dspconfig driver will try to load
>> SOF, causing audio probe to fail.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Worth to mention that I opened an issue on Github about that:
> 
> https://github.com/thesofproject/linux/issues/4934
> 
> I added there the ACPI DSDT table as a reference.

This sounds like an 'easy enough' fix, but I don't have a burning desire
to start adding quirks of this nature. To be clear, the entire support
of the ES8336 is an absolute nightmare that I've stopped looking at
completely given the lack of support of vendor/OEMs.

In this case, the ACPI table is completely wrong, we should try to
'mark' the ES8336 device as 'not present' or detect the presence of HDaudio.

Andy, what do you think and what would be your recommendation?

>> ---
>>  sound/hda/intel-dsp-config.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
>> index 6a384b922e4f..8e728f0585dd 100644
>> --- a/sound/hda/intel-dsp-config.c
>> +++ b/sound/hda/intel-dsp-config.c
>> @@ -46,6 +46,22 @@ static const struct snd_soc_acpi_codecs __maybe_unused essx_83x6 = {
>>   * - the first successful match will win
>>   */
>>  static const struct config_entry config_table[] = {
>> +	/* Quirks */
>> +	{
>> +		.flags = 0,	/* Model uses AC97 with Realtek ALC 256 */
>> +		.device = PCI_DEVICE_ID_INTEL_HDA_CML_LP,
>> +		.dmi_table = (const struct dmi_system_id []) {
>> +			{
>> +				.ident = "Huawei NBLB-WAX9N",
>> +				.matches = {
>> +					DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"),
>> +					DMI_MATCH(DMI_PRODUCT_NAME, "NBLB-WAX9N"),
>> +				}
>> +			},
>> +			{}
>> +		}
>> +	},
>> +
>>  /* Merrifield */
>>  #if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
>>  	{
Andy Shevchenko April 18, 2024, 2:06 p.m. UTC | #3
On Thu, Apr 18, 2024 at 11:04:53AM +0100, Mauro Carvalho Chehab wrote:
> Em Thu, 18 Apr 2024 09:48:27 +0100
> Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:
> 
> > Newer Matebook D14 model comes with essx8336 and supports SOF,
> > but the initial models use the legacy driver, with a Realtek ALC 256
> > AC97 chip on it.
> > 
> > The BIOS seems to be prepared to be used by both models, so
> > it contains an entry for ESSX8336 on its DSDT table.
> > 
> > Add a quirk, as otherwise dspconfig driver will try to load
> > SOF, causing audio probe to fail.

...

> Worth to mention that I opened an issue on Github about that:
> 
> https://github.com/thesofproject/linux/issues/4934
> 
> I added there the ACPI DSDT table as a reference.

Yeah, please add a BugLink tag.
Andy Shevchenko April 18, 2024, 2:07 p.m. UTC | #4
On Thu, Apr 18, 2024 at 09:48:27AM +0100, Mauro Carvalho Chehab wrote:
> Newer Matebook D14 model comes with essx8336 and supports SOF,
> but the initial models use the legacy driver, with a Realtek ALC 256
> AC97 chip on it.
> 
> The BIOS seems to be prepared to be used by both models, so
> it contains an entry for ESSX8336 on its DSDT table.
> 
> Add a quirk, as otherwise dspconfig driver will try to load
> SOF, causing audio probe to fail.

...

> +				.ident = "Huawei NBLB-WAX9N",
> +				.matches = {
> +					DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"),
> +					DMI_MATCH(DMI_PRODUCT_NAME, "NBLB-WAX9N"),
> +				}

I would leave a trailing comma to avoid unneeded churn in the future in case
this gets extended.

> +			},
> +			{}
> +		}

Ditto.

> +	},
Takashi Iwai April 18, 2024, 2:09 p.m. UTC | #5
On Thu, 18 Apr 2024 16:06:00 +0200,
Andy Shevchenko wrote:
> 
> On Thu, Apr 18, 2024 at 11:04:53AM +0100, Mauro Carvalho Chehab wrote:
> > Em Thu, 18 Apr 2024 09:48:27 +0100
> > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:
> > 
> > > Newer Matebook D14 model comes with essx8336 and supports SOF,
> > > but the initial models use the legacy driver, with a Realtek ALC 256
> > > AC97 chip on it.
> > > 
> > > The BIOS seems to be prepared to be used by both models, so
> > > it contains an entry for ESSX8336 on its DSDT table.
> > > 
> > > Add a quirk, as otherwise dspconfig driver will try to load
> > > SOF, causing audio probe to fail.
> 
> ...
> 
> > Worth to mention that I opened an issue on Github about that:
> > 
> > https://github.com/thesofproject/linux/issues/4934
> > 
> > I added there the ACPI DSDT table as a reference.
> 
> Yeah, please add a BugLink tag.

I thought a Link tag is preferred over a BugLink tag nowadays.

I can add it in my side, but let's wait for the review by Intel people
at first.


thanks,

Takashi
Mauro Carvalho Chehab April 18, 2024, 8:22 p.m. UTC | #6
Em Thu, 18 Apr 2024 08:24:10 -0500
Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> escreveu:

> On 4/18/24 05:04, Mauro Carvalho Chehab wrote:
> > Em Thu, 18 Apr 2024 09:48:27 +0100
> > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:
> >   
> >> Newer Matebook D14 model comes with essx8336 and supports SOF,
> >> but the initial models use the legacy driver, with a Realtek ALC 256
> >> AC97 chip on it.
> >>
> >> The BIOS seems to be prepared to be used by both models, so
> >> it contains an entry for ESSX8336 on its DSDT table.
> >>
> >> Add a quirk, as otherwise dspconfig driver will try to load
> >> SOF, causing audio probe to fail.
> >>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>  
> > 
> > Worth to mention that I opened an issue on Github about that:
> > 
> > https://github.com/thesofproject/linux/issues/4934
> > 
> > I added there the ACPI DSDT table as a reference.  
> 
> This sounds like an 'easy enough' fix, but I don't have a burning desire
> to start adding quirks of this nature. To be clear, the entire support
> of the ES8336 is an absolute nightmare that I've stopped looking at
> completely given the lack of support of vendor/OEMs.

Heh, I know the pain, having working myself to have some support for audio
on two different notebooks with my siblings, both with es8336. On both 
cases, the BIOS info was not really useful, requiring quirks to make device
to work properly.

This is btw a common issue I'm aware for a long time: BIOS data is
not reliable, as vendors tend to re-use BIOS from one device on others,
without actually reflecting what it is on each device.

The EDAC subsystem never relies on DMI data for memory banks - as even
server BIOS from top tear manufacturers usually report wrong data for
motherboard's DIMM labels. Instead, an userspace application reads
DMI data and propose changes, but patches for rasdaemon are required
to add such labels to a database.

-

In any case, this specific device doesn't have es8336 ;-)

Also, I don't think this problem will remain confined to es8336: any
other SOF driver may have the same problem: a BIOS (or a BIOS update)
may add non-existent _HID devices to DSDT, causing the driver to
disable AC97 support, enabling SOF instead and causing regressions.

As I wrote at the Github issue, one solution would be to do an I2C
scan to detect if the SOF device(s) reported by BIOS are really
present. This would require that, before calling 
snd_intel_acpi_dsp_driver_probe():

	- the I2C bus to be created;
	- Runtime PM for the audio device needs to put resume the
	  device and I2C bus controller, if suspended;
	- the I2C address of the audio device needs to be known
	  by sound/hda/intel-dsp-config.c

With that, a zero-byte (or one-byte if zero-byte not support)
read or write could detect if the device is there, before
initializing it - or calling the device-specific driver.

Another solution would be to probe the SOF driver, falling back
to AC97 if SOF init fails.

> 
> In this case, the ACPI table is completely wrong, we should try to
> 'mark' the ES8336 device as 'not present' or detect the presence of HDaudio.
> 
> Andy, what do you think and what would be your recommendation?
> 
> >> ---
> >>  sound/hda/intel-dsp-config.c | 16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
> >> index 6a384b922e4f..8e728f0585dd 100644
> >> --- a/sound/hda/intel-dsp-config.c
> >> +++ b/sound/hda/intel-dsp-config.c
> >> @@ -46,6 +46,22 @@ static const struct snd_soc_acpi_codecs __maybe_unused essx_83x6 = {
> >>   * - the first successful match will win
> >>   */
> >>  static const struct config_entry config_table[] = {
> >> +	/* Quirks */
> >> +	{
> >> +		.flags = 0,	/* Model uses AC97 with Realtek ALC 256 */
> >> +		.device = PCI_DEVICE_ID_INTEL_HDA_CML_LP,
> >> +		.dmi_table = (const struct dmi_system_id []) {
> >> +			{
> >> +				.ident = "Huawei NBLB-WAX9N",
> >> +				.matches = {
> >> +					DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"),
> >> +					DMI_MATCH(DMI_PRODUCT_NAME, "NBLB-WAX9N"),
> >> +				}
> >> +			},
> >> +			{}
> >> +		}
> >> +	},
> >> +
> >>  /* Merrifield */
> >>  #if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
> >>  	{
Takashi Iwai April 29, 2024, 12:17 p.m. UTC | #7
On Thu, 18 Apr 2024 15:24:10 +0200,
Pierre-Louis Bossart wrote:
> 
> On 4/18/24 05:04, Mauro Carvalho Chehab wrote:
> > Em Thu, 18 Apr 2024 09:48:27 +0100
> > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:
> > 
> >> Newer Matebook D14 model comes with essx8336 and supports SOF,
> >> but the initial models use the legacy driver, with a Realtek ALC 256
> >> AC97 chip on it.
> >>
> >> The BIOS seems to be prepared to be used by both models, so
> >> it contains an entry for ESSX8336 on its DSDT table.
> >>
> >> Add a quirk, as otherwise dspconfig driver will try to load
> >> SOF, causing audio probe to fail.
> >>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > 
> > Worth to mention that I opened an issue on Github about that:
> > 
> > https://github.com/thesofproject/linux/issues/4934
> > 
> > I added there the ACPI DSDT table as a reference.
> 
> This sounds like an 'easy enough' fix, but I don't have a burning desire
> to start adding quirks of this nature. To be clear, the entire support
> of the ES8336 is an absolute nightmare that I've stopped looking at
> completely given the lack of support of vendor/OEMs.
> 
> In this case, the ACPI table is completely wrong, we should try to
> 'mark' the ES8336 device as 'not present' or detect the presence of HDaudio.
> 
> Andy, what do you think and what would be your recommendation?

This one is still pending, and I'd like to hear the decision from
Intel people.  Let me know if this hack is acceptable.


thanks,

Takashi

> 
> >> ---
> >>  sound/hda/intel-dsp-config.c | 16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
> >> index 6a384b922e4f..8e728f0585dd 100644
> >> --- a/sound/hda/intel-dsp-config.c
> >> +++ b/sound/hda/intel-dsp-config.c
> >> @@ -46,6 +46,22 @@ static const struct snd_soc_acpi_codecs __maybe_unused essx_83x6 = {
> >>   * - the first successful match will win
> >>   */
> >>  static const struct config_entry config_table[] = {
> >> +	/* Quirks */
> >> +	{
> >> +		.flags = 0,	/* Model uses AC97 with Realtek ALC 256 */
> >> +		.device = PCI_DEVICE_ID_INTEL_HDA_CML_LP,
> >> +		.dmi_table = (const struct dmi_system_id []) {
> >> +			{
> >> +				.ident = "Huawei NBLB-WAX9N",
> >> +				.matches = {
> >> +					DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"),
> >> +					DMI_MATCH(DMI_PRODUCT_NAME, "NBLB-WAX9N"),
> >> +				}
> >> +			},
> >> +			{}
> >> +		}
> >> +	},
> >> +
> >>  /* Merrifield */
> >>  #if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
> >>  	{
Andy Shevchenko April 29, 2024, 12:25 p.m. UTC | #8
On Mon, Apr 29, 2024 at 02:17:34PM +0200, Takashi Iwai wrote:
> On Thu, 18 Apr 2024 15:24:10 +0200,
> Pierre-Louis Bossart wrote:
> > On 4/18/24 05:04, Mauro Carvalho Chehab wrote:

...

> > Andy, what do you think and what would be your recommendation?
> 
> This one is still pending, and I'd like to hear the decision from
> Intel people.  Let me know if this hack is acceptable.

IIRC it was resolved on GitHub by Pierre, anything else I need to contribute?
Pierre-Louis Bossart April 29, 2024, 1:51 p.m. UTC | #9
On 4/29/24 07:25, Andy Shevchenko wrote:
> On Mon, Apr 29, 2024 at 02:17:34PM +0200, Takashi Iwai wrote:
>> On Thu, 18 Apr 2024 15:24:10 +0200,
>> Pierre-Louis Bossart wrote:
>>> On 4/18/24 05:04, Mauro Carvalho Chehab wrote:
> 
> ...
> 
>>> Andy, what do you think and what would be your recommendation?
>>
>> This one is still pending, and I'd like to hear the decision from
>> Intel people.  Let me know if this hack is acceptable.
> 
> IIRC it was resolved on GitHub by Pierre, anything else I need to contribute?

This initial patch was replaced by the one I sent on Friday
"[PATCH] ALSA: hda: intel-dsp-config: harden I2C/I2S codec detection"

I should have made it clearer in the commit message, sorry.
Takashi Iwai April 29, 2024, 3 p.m. UTC | #10
On Mon, 29 Apr 2024 15:51:03 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 4/29/24 07:25, Andy Shevchenko wrote:
> > On Mon, Apr 29, 2024 at 02:17:34PM +0200, Takashi Iwai wrote:
> >> On Thu, 18 Apr 2024 15:24:10 +0200,
> >> Pierre-Louis Bossart wrote:
> >>> On 4/18/24 05:04, Mauro Carvalho Chehab wrote:
> > 
> > ...
> > 
> >>> Andy, what do you think and what would be your recommendation?
> >>
> >> This one is still pending, and I'd like to hear the decision from
> >> Intel people.  Let me know if this hack is acceptable.
> > 
> > IIRC it was resolved on GitHub by Pierre, anything else I need to contribute?
> 
> This initial patch was replaced by the one I sent on Friday
> "[PATCH] ALSA: hda: intel-dsp-config: harden I2C/I2S codec detection"
> 
> I should have made it clearer in the commit message, sorry.

OK, thanks for the updates!


Takashi
diff mbox series

Patch

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 6a384b922e4f..8e728f0585dd 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -46,6 +46,22 @@  static const struct snd_soc_acpi_codecs __maybe_unused essx_83x6 = {
  * - the first successful match will win
  */
 static const struct config_entry config_table[] = {
+	/* Quirks */
+	{
+		.flags = 0,	/* Model uses AC97 with Realtek ALC 256 */
+		.device = PCI_DEVICE_ID_INTEL_HDA_CML_LP,
+		.dmi_table = (const struct dmi_system_id []) {
+			{
+				.ident = "Huawei NBLB-WAX9N",
+				.matches = {
+					DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"),
+					DMI_MATCH(DMI_PRODUCT_NAME, "NBLB-WAX9N"),
+				}
+			},
+			{}
+		}
+	},
+
 /* Merrifield */
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
 	{