PCI SSID and Codec SSID collision?

PCI SSID and Codec SSID collision?

[Gergo Koteles]

Hi,

The Lenovo Yoga Slim Pro 9 14IRP8 has tas2781 amplifiers, but it can
only pickup its fixup with the hda_model=17aa:38be module parameter.

The 17aa:38be is the SSID of its Codec. The PCI SSID is 17aa:3802.

The patch_realtek.c has these fixup:
SND_PCI_QUIRK(0x17aa, 0x3802, "Lenovo Yoga DuetITL 2021",
ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
SND_PCI_QUIRK(0x17aa, 0x38be, "Yoga S980-14.5 proX YC Dual",
ALC287_FIXUP_TAS2781_I2C),

And the snd_hda_pick_fixup() picks the former fixup because of the same
PCI SSID, but that is a Codec SSID?

At least it worked without that line.
https://bugzilla.kernel.org/show_bug.cgi?id=208555#c839

It would be nice if it can be work without the module parameter.

What is the best way to add this strangeness to patch_realtek.c?

Thanks,
Gergo

Re: PCI SSID and Codec SSID collision?

[Takashi Iwai]

On Fri, 23 Feb 2024 13:05:12 +0100,
Gergo Koteles wrote:
> 
> Hi,
> 
> The Lenovo Yoga Slim Pro 9 14IRP8 has tas2781 amplifiers, but it can
> only pickup its fixup with the hda_model=17aa:38be module parameter.
> 
> The 17aa:38be is the SSID of its Codec. The PCI SSID is 17aa:3802.
> 
> The patch_realtek.c has these fixup:
> SND_PCI_QUIRK(0x17aa, 0x3802, "Lenovo Yoga DuetITL 2021",
> ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
> SND_PCI_QUIRK(0x17aa, 0x38be, "Yoga S980-14.5 proX YC Dual",
> ALC287_FIXUP_TAS2781_I2C),
> 
> And the snd_hda_pick_fixup() picks the former fixup because of the same
> PCI SSID, but that is a Codec SSID?
> 
> At least it worked without that line.
> https://bugzilla.kernel.org/show_bug.cgi?id=208555#c839
> 
> It would be nice if it can be work without the module parameter.
> 
> What is the best way to add this strangeness to patch_realtek.c?

That's a bad collision that happens rarely but more than once...

In this case, we'd need to fix 17aa:3802 matching to be a special
function that checks the model more closely.  e.g. create a quirk
function that checks the codec SSID additionally and apply the
different quirks from there.


thanks,

Takashi

Re: PCI SSID and Codec SSID collision?

[Willian Wang]

Hi, I'm curious about the reasons for the fixup table to be the same
for PCI and Codec SSIDs and if the PCI SSID is usually more important
for it to be checked before the Codec. Anyway, would a simple
conditional statement to match these exact PCI and Codec SSID to set
the fixup manually be enough? There seems to already exist a manual
fix for TX300 and ROG Strix G17 there (not the same kind of fix and
with a FIXME comment, though). I have this exact laptop model and
would like to try to contribute to the code if it's not too
complicated.

Re: PCI SSID and Codec SSID collision?

[Takashi Iwai]

On Fri, 23 Feb 2024 15:41:26 +0100,
Willian Wang wrote:
> 
> Hi, I'm curious about the reasons for the fixup table to be the same
> for PCI and Codec SSIDs and if the PCI SSID is usually more important
> for it to be checked before the Codec.

Sometimes the manufacturers are lazy and didn't check the conflict in
their own products :-<

> Anyway, would a simple
> conditional statement to match these exact PCI and Codec SSID to set
> the fixup manually be enough? There seems to already exist a manual
> fix for TX300 and ROG Strix G17 there (not the same kind of fix and
> with a FIXME comment, though). I have this exact laptop model and
> would like to try to contribute to the code if it's not too
> complicated.

Yes, that's my suggestion.  I don't have the data of the other
17aa:3802, but I guess it should have a different codec SSID.


thanks,

Takashi

Re: PCI SSID and Codec SSID collision?

[Gergo Koteles]

Hi Willian,

On Fri, 2024-02-23 at 11:41 -0300, Willian Wang wrote:
> Hi, I'm curious about the reasons for the fixup table to be the same
> for PCI and Codec SSIDs and if the PCI SSID is usually more important
> for it to be checked before the Codec. Anyway, would a simple
> conditional statement to match these exact PCI and Codec SSID to set
> the fixup manually be enough? There seems to already exist a manual
> fix for TX300 and ROG Strix G17 there (not the same kind of fix and
> with a FIXME comment, though). I have this exact laptop model and
> would like to try to contribute to the code if it's not too
> complicated.
> 

I think it could be simple, like this function:
alc298_fixup_lenovo_c940_duet7

Feel free to contribute. Easier for you to test.

This can help:
The output of DuetITL 2021 amixer:
https://linux-hardware.org/?probe=19173612af&log=amixer


Regards,
Gergo

Re: PCI SSID and Codec SSID collision?

[Willian Wang]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

Here is my attempt. I hope it doesn't break with the DuetITL mode, I
chose to create a special case for it instead of otherwise because
there is an european version of the 14IRP8 with different codec SSID
(https://bugzilla.kernel.org/show_bug.cgi?id=208555#c778, 0x17aa38bf
vs 0x17aa38be).

This is my first time contributing with a commit and I'm not sure if I
did everything correctly so please let me know. I also don't know if I
should send it to a specific email with you as CC or something like
that.

On Fri, Feb 23, 2024 at 12:10 PM Gergo Koteles
<soyer@irl.hu> wrote:
>
> Hi Willian,
>
> On Fri, 2024-02-23 at 11:41 -0300, Willian Wang wrote:
> > Hi, I'm curious about the reasons for the fixup table to be the same
> > for PCI and Codec SSIDs and if the PCI SSID is usually more important
> > for it to be checked before the Codec. Anyway, would a simple
> > conditional statement to match these exact PCI and Codec SSID to set
> > the fixup manually be enough? There seems to already exist a manual
> > fix for TX300 and ROG Strix G17 there (not the same kind of fix and
> > with a FIXME comment, though). I have this exact laptop model and
> > would like to try to contribute to the code if it's not too
> > complicated.
> >
>
> I think it could be simple, like this function:
> alc298_fixup_lenovo_c940_duet7
>
> Feel free to contribute. Easier for you to test.
>
> This can help:
> The output of DuetITL 2021 amixer:
> https://linux-hardware.org/?probe=19173612af&log=amixer
>
>
> Regards,
> Gergo
>

[-- Attachment #2: 0001-ALSA-hda-realtek-Add-special-fixup-for-Lenovo-14IRP8.patch --]
[-- Type: text/x-patch, Size: 3294 bytes --]

From 9b88c274e6c7933b4d05848928296de61905f493 Mon Sep 17 00:00:00 2001
From: Willian Wang <git@willian.wang>
Date: Fri, 23 Feb 2024 18:38:48 -0300
Subject: [PATCH] ALSA: hda/realtek: Add special fixup for Lenovo 14IRP8

Lenovo Slim/Yoga Pro 9 14IRP8 requires a special fixup because there is
a collision of its PCI SSID (17aa:3802) with Lenovo Yoga DuetITL 2021
codec SSID.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=208555
Link: https://lore.kernel.org/all/d5b42e483566a3815d229270abd668131a0d9f3a.camel@irl.hu
Signed-off-by: Willian Wang <git@willian.wang>
---
 sound/pci/hda/patch_realtek.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 0ec1312bffd5..aa16944d5c11 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -7444,6 +7444,7 @@ enum {
 	ALC287_FIXUP_LEGION_15IMHG05_AUTOMUTE,
 	ALC287_FIXUP_YOGA7_14ITL_SPEAKERS,
 	ALC298_FIXUP_LENOVO_C940_DUET7,
+	ALC287_FIXUP_LENOVO_14IRP8_DUETITL,
 	ALC287_FIXUP_13S_GEN2_SPEAKERS,
 	ALC256_FIXUP_SET_COEF_DEFAULTS,
 	ALC256_FIXUP_SYSTEM76_MIC_NO_PRESENCE,
@@ -7495,6 +7496,24 @@ static void alc298_fixup_lenovo_c940_duet7(struct hda_codec *codec,
 	__snd_hda_apply_fixup(codec, id, action, 0);
 }
 
+/* A special fixup for Lenovo Slim/Yoga Pro 9 14IRP8 and Yoga DuetITL 2021;
+ * 14IRP8 PCI SSID will mistakenly be matched with the DuetITL codec SSID,
+ * so we need to apply different fixup on this case. If it weren't for the
+ * PCI SSID, 14IRP8 would be matched with the correct codec SSID
+ */
+static void alc287_fixup_lenovo_14Irp8_duetitl(struct hda_codec *codec,
+					      const struct hda_fixup *fix,
+					      int action)
+{
+	int id;
+
+	if (codec->core.subsystem_id == 0x17aa3802)
+		id = ALC287_FIXUP_YOGA7_14ITL_SPEAKERS; /* DuetITL */
+	else
+		id = ALC287_FIXUP_TAS2781_I2C; /* 14IRP8 */
+	__snd_hda_apply_fixup(codec, id, action, 0);
+}
+
 static const struct hda_fixup alc269_fixups[] = {
 	[ALC269_FIXUP_GPIO2] = {
 		.type = HDA_FIXUP_FUNC,
@@ -9379,6 +9398,10 @@ static const struct hda_fixup alc269_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc298_fixup_lenovo_c940_duet7,
 	},
+	[ALC287_FIXUP_LENOVO_14IRP8_DUETITL] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc287_fixup_lenovo_14Irp8_duetitl,
+	},
 	[ALC287_FIXUP_13S_GEN2_SPEAKERS] = {
 		.type = HDA_FIXUP_VERBS,
 		.v.verbs = (const struct hda_verb[]) {
@@ -10247,7 +10270,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x31af, "ThinkCentre Station", ALC623_FIXUP_LENOVO_THINKSTATION_P340),
 	SND_PCI_QUIRK(0x17aa, 0x334b, "Lenovo ThinkCentre M70 Gen5", ALC283_FIXUP_HEADSET_MIC),
 	SND_PCI_QUIRK(0x17aa, 0x3801, "Lenovo Yoga9 14IAP7", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
-	SND_PCI_QUIRK(0x17aa, 0x3802, "Lenovo Yoga DuetITL 2021", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
+	SND_PCI_QUIRK(0x17aa, 0x3802, "Lenovo Yoga DuetITL 2021", ALC287_FIXUP_LENOVO_14IRP8_DUETITL),
 	SND_PCI_QUIRK(0x17aa, 0x3813, "Legion 7i 15IMHG05", ALC287_FIXUP_LEGION_15IMHG05_SPEAKERS),
 	SND_PCI_QUIRK(0x17aa, 0x3818, "Lenovo C940 / Yoga Duet 7", ALC298_FIXUP_LENOVO_C940_DUET7),
 	SND_PCI_QUIRK(0x17aa, 0x3819, "Lenovo 13s Gen2 ITL", ALC287_FIXUP_13S_GEN2_SPEAKERS),
-- 
2.43.2

Re: PCI SSID and Codec SSID collision?

[Takashi Iwai]

On Fri, 23 Feb 2024 22:55:40 +0100,
Willian Wang wrote:
> 
> Here is my attempt. I hope it doesn't break with the DuetITL mode, I
> chose to create a special case for it instead of otherwise because
> there is an european version of the 14IRP8 with different codec SSID
> (https://bugzilla.kernel.org/show_bug.cgi?id=208555#c778, 0x17aa38bf
> vs 0x17aa38be).
> 
> This is my first time contributing with a commit and I'm not sure if I
> did everything correctly so please let me know. I also don't know if I
> should send it to a specific email with you as CC or something like
> that.

Looks good.  And yes, could you resubmit only the patch without
hanging on the thread?


thanks,

Takashi