[PATCH] Apply headset jack fixup for alc287 thinkpads

[PATCH] Apply headset jack fixup for alc287 thinkpads

[José Relvas]

Thinkpads have a fixup for alc285 which enables headset media buttons, but not for alc287.
This patch removes the chain to the X1 gen 7 fixup in the alc285 headset fixup, making it applicable for models without bass speakers.
It then chains said headset fixup with every alc287 thinkpad fixup, making headset media buttons work for these laptops.

Signed-off-by: José Relvas <josemonsantorelvas@gmail.com>
---
 sound/pci/hda/patch_realtek.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index c80e2f3acfcc..21d10c62964c 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -8796,13 +8796,13 @@ static const struct hda_fixup alc269_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc285_fixup_thinkpad_x1_gen7,
 		.chained = true,
-		.chain_id = ALC269_FIXUP_THINKPAD_ACPI
+		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK
 	},
 	[ALC285_FIXUP_THINKPAD_HEADSET_JACK] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_headset_jack,
 		.chained = true,
-		.chain_id = ALC285_FIXUP_THINKPAD_X1_GEN7
+		.chain_id = ALC269_FIXUP_THINKPAD_ACPI
 	},
 	[ALC294_FIXUP_ASUS_HPE] = {
 		.type = HDA_FIXUP_VERBS,
@@ -9577,7 +9577,7 @@ static const struct hda_fixup alc269_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = cs35l41_fixup_i2c_two,
 		.chained = true,
-		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
+		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK,
 	},
 	[ALC287_FIXUP_TAS2781_I2C] = {
 		.type = HDA_FIXUP_FUNC,
@@ -9604,6 +9604,8 @@ static const struct hda_fixup alc269_fixups[] = {
 	[ALC287_FIXUP_THINKPAD_I2S_SPK] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc287_fixup_bind_dacs,
+                .chained = true,
+                .chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK
 	},
 	[ALC287_FIXUP_MG_RTKC_CSAMP_CS35L41_I2C_THINKPAD] = {
 		.type = HDA_FIXUP_FUNC,
@@ -10206,8 +10208,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x224c, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
 	SND_PCI_QUIRK(0x17aa, 0x224d, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
 	SND_PCI_QUIRK(0x17aa, 0x225d, "Thinkpad T480", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
-	SND_PCI_QUIRK(0x17aa, 0x2292, "Thinkpad X1 Carbon 7th", ALC285_FIXUP_THINKPAD_HEADSET_JACK),
-	SND_PCI_QUIRK(0x17aa, 0x22be, "Thinkpad X1 Carbon 8th", ALC285_FIXUP_THINKPAD_HEADSET_JACK),
+	SND_PCI_QUIRK(0x17aa, 0x2292, "Thinkpad X1 Carbon 7th", ALC285_FIXUP_THINKPAD_X1_GEN7),
+	SND_PCI_QUIRK(0x17aa, 0x22be, "Thinkpad X1 Carbon 8th", ALC285_FIXUP_THINKPAD_X1_GEN7),
 	SND_PCI_QUIRK(0x17aa, 0x22c1, "Thinkpad P1 Gen 3", ALC285_FIXUP_THINKPAD_NO_BASS_SPK_HEADSET_JACK),
 	SND_PCI_QUIRK(0x17aa, 0x22c2, "Thinkpad X1 Extreme Gen 3", ALC285_FIXUP_THINKPAD_NO_BASS_SPK_HEADSET_JACK),
 	SND_PCI_QUIRK(0x17aa, 0x22f1, "Thinkpad", ALC287_FIXUP_MG_RTKC_CSAMP_CS35L41_I2C_THINKPAD),
-- 
2.43.0

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[Takashi Iwai]

On Tue, 30 Jan 2024 20:25:36 +0100,
José Relvas wrote:
> 
> Thinkpads have a fixup for alc285 which enables headset media buttons, but not for alc287.
> This patch removes the chain to the X1 gen 7 fixup in the alc285 headset fixup, making it applicable for models without bass speakers.
> It then chains said headset fixup with every alc287 thinkpad fixup, making headset media buttons work for these laptops.
> 
> Signed-off-by: José Relvas <josemonsantorelvas@gmail.com>

Thanks for the patch.  But the changes look too intrusive.  We don't
want to change the behavior of the existing quirk entries
unnecessarily.  Your change will break if anyone explicitly uses a
specific model option, for example.
So, please rather create a new entry for handling the headset jack and
use it (maybe better with some comments).

Also the patch isn't cleanly applicable.  Your MUA broke tabs and
spaces.  Please fix the setup, or use git-send-email.


thanks,

Takashi

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[José Relvas]

Thanks for the reply!

Sorry about the misformatted patch. I did send in a second one through git-send-email. Is it not correct either? 

Anyways, the reason I'm editing the existing quirks is mostly because ALC285_FIXUP_THINKPAD_HEADSET_JACK was actually wrong.
It was actually always linking to the X1 gen 7 quirk, which has some kcontrol changes only meant for a few models with bass speakers.

It looks like this: 

/* Quirk for Thinkpad X1 7th and 8th Gen
 * The following fixed routing needed
 * DAC1 (NID 0x02) -> Speaker (NID 0x14); some eq applied secretly
 * DAC2 (NID 0x03) -> Bass (NID 0x17) & Headphone (NID 0x21); sharing a DAC
 * DAC3 (NID 0x06) -> Unused, due to the lack of volume amp
*/
static void alc285_fixup_thinkpad_x1_gen7(struct hda_codec *codec,
					  const struct hda_fixup *fix, int action)
{
	static const hda_nid_t conn[] = { 0x02, 0x03 }; /* exclude 0x06 */
	static const hda_nid_t preferred_pairs[] = {
		0x14, 0x02, 0x17, 0x03, 0x21, 0x03, 0
	};
	struct alc_spec *spec = codec->spec;

	switch (action) {
	case HDA_FIXUP_ACT_PRE_PROBE:
		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
		spec->gen.preferred_dacs = preferred_pairs;
		break;
	case HDA_FIXUP_ACT_BUILD:
		/* The generic parser creates somewhat unintuitive volume ctls
		 * with the fixed routing above, and the shared DAC2 may be
		 * confusing for PA.
		 * Rename those to unique names so that PA doesn't touch them
		 * and use only Master volume.
		 */
		rename_ctl(codec, "Front Playback Volume", "DAC1 Playback Volume");
		rename_ctl(codec, "Bass Speaker Playback Volume", "DAC2 Playback Volume");
		break;
	}
}

On second review, this does break some routing with older alc285, as the speaker and headphone NIDs are still 0x02 and 0x03.
Maybe cloning the x1 gen 7 fixup, but without the renamed ctls, is the more appropriate solution here? Let me know what your thoughts are.

Sorry for the confusion.
José Relvas

On Wed, Jan 31, 2024 at 08:20:29AM +0100, Takashi Iwai wrote:
> On Tue, 30 Jan 2024 20:25:36 +0100,
> José Relvas wrote:
> > 
> > Thinkpads have a fixup for alc285 which enables headset media buttons, but not for alc287.
> > This patch removes the chain to the X1 gen 7 fixup in the alc285 headset fixup, making it applicable for models without bass speakers.
> > It then chains said headset fixup with every alc287 thinkpad fixup, making headset media buttons work for these laptops.
> > 
> > Signed-off-by: José Relvas <josemonsantorelvas@gmail.com>
> 
> Thanks for the patch.  But the changes look too intrusive.  We don't
> want to change the behavior of the existing quirk entries
> unnecessarily.  Your change will break if anyone explicitly uses a
> specific model option, for example.
> So, please rather create a new entry for handling the headset jack and
> use it (maybe better with some comments).
> 
> Also the patch isn't cleanly applicable.  Your MUA broke tabs and
> spaces.  Please fix the setup, or use git-send-email.
> 
> 
> thanks,
> 
> Takashi

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[Takashi Iwai]

On Wed, 31 Jan 2024 11:28:15 +0100,
José Relvas wrote:
> 
> Thanks for the reply!
> 
> Sorry about the misformatted patch. I did send in a second one through git-send-email. Is it not correct either? 
> 
> Anyways, the reason I'm editing the existing quirks is mostly because ALC285_FIXUP_THINKPAD_HEADSET_JACK was actually wrong.
> It was actually always linking to the X1 gen 7 quirk, which has some kcontrol changes only meant for a few models with bass speakers.

Yes, and that's the intentional behavior.
There is the match with the pin table, not only the explicit PCI
SSID, and those are with bass speakers.

> It looks like this: 
> 
> /* Quirk for Thinkpad X1 7th and 8th Gen
>  * The following fixed routing needed
>  * DAC1 (NID 0x02) -> Speaker (NID 0x14); some eq applied secretly
>  * DAC2 (NID 0x03) -> Bass (NID 0x17) & Headphone (NID 0x21); sharing a DAC
>  * DAC3 (NID 0x06) -> Unused, due to the lack of volume amp
> */
> static void alc285_fixup_thinkpad_x1_gen7(struct hda_codec *codec,
> 					  const struct hda_fixup *fix, int action)
> {
> 	static const hda_nid_t conn[] = { 0x02, 0x03 }; /* exclude 0x06 */
> 	static const hda_nid_t preferred_pairs[] = {
> 		0x14, 0x02, 0x17, 0x03, 0x21, 0x03, 0
> 	};
> 	struct alc_spec *spec = codec->spec;
> 
> 	switch (action) {
> 	case HDA_FIXUP_ACT_PRE_PROBE:
> 		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> 		spec->gen.preferred_dacs = preferred_pairs;
> 		break;
> 	case HDA_FIXUP_ACT_BUILD:
> 		/* The generic parser creates somewhat unintuitive volume ctls
> 		 * with the fixed routing above, and the shared DAC2 may be
> 		 * confusing for PA.
> 		 * Rename those to unique names so that PA doesn't touch them
> 		 * and use only Master volume.
> 		 */
> 		rename_ctl(codec, "Front Playback Volume", "DAC1 Playback Volume");
> 		rename_ctl(codec, "Bass Speaker Playback Volume", "DAC2 Playback Volume");
> 		break;
> 	}
> }
> 
> On second review, this does break some routing with older alc285, as the speaker and headphone NIDs are still 0x02 and 0x03.
> Maybe cloning the x1 gen 7 fixup, but without the renamed ctls, is the more appropriate solution here? Let me know what your thoughts are.

I believe it's safer to make a specific quirk for X7 gen7.
There are too many dependencies on the existing chains.


Takashi

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[José Relvas]

Thanks for the feedback! So if my understanding is correct, the headset jack quirk is currently *only* applied to models with bass speakers?

bass thinkpad -> headset thinkpad quirk -> bass thinkpad quirk (both quirks applied)
normal thinkpad -X> headset thinkpad quirk -X> bass thinkpad quirk (neither quirks applied) 

I'll go back to the drawing board, just making sure I'm not missing anything.

José Relvas

On Wed, Jan 31, 2024 at 11:49:28AM +0100, Takashi Iwai wrote:
> On Wed, 31 Jan 2024 11:28:15 +0100,
> José Relvas wrote:
> > 
> > Thanks for the reply!
> > 
> > Sorry about the misformatted patch. I did send in a second one through git-send-email. Is it not correct either? 
> > 
> > Anyways, the reason I'm editing the existing quirks is mostly because ALC285_FIXUP_THINKPAD_HEADSET_JACK was actually wrong.
> > It was actually always linking to the X1 gen 7 quirk, which has some kcontrol changes only meant for a few models with bass speakers.
> 
> Yes, and that's the intentional behavior.
> There is the match with the pin table, not only the explicit PCI
> SSID, and those are with bass speakers.
> 
> > It looks like this: 
> > 
> > /* Quirk for Thinkpad X1 7th and 8th Gen
> >  * The following fixed routing needed
> >  * DAC1 (NID 0x02) -> Speaker (NID 0x14); some eq applied secretly
> >  * DAC2 (NID 0x03) -> Bass (NID 0x17) & Headphone (NID 0x21); sharing a DAC
> >  * DAC3 (NID 0x06) -> Unused, due to the lack of volume amp
> > */
> > static void alc285_fixup_thinkpad_x1_gen7(struct hda_codec *codec,
> > 					  const struct hda_fixup *fix, int action)
> > {
> > 	static const hda_nid_t conn[] = { 0x02, 0x03 }; /* exclude 0x06 */
> > 	static const hda_nid_t preferred_pairs[] = {
> > 		0x14, 0x02, 0x17, 0x03, 0x21, 0x03, 0
> > 	};
> > 	struct alc_spec *spec = codec->spec;
> > 
> > 	switch (action) {
> > 	case HDA_FIXUP_ACT_PRE_PROBE:
> > 		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> > 		spec->gen.preferred_dacs = preferred_pairs;
> > 		break;
> > 	case HDA_FIXUP_ACT_BUILD:
> > 		/* The generic parser creates somewhat unintuitive volume ctls
> > 		 * with the fixed routing above, and the shared DAC2 may be
> > 		 * confusing for PA.
> > 		 * Rename those to unique names so that PA doesn't touch them
> > 		 * and use only Master volume.
> > 		 */
> > 		rename_ctl(codec, "Front Playback Volume", "DAC1 Playback Volume");
> > 		rename_ctl(codec, "Bass Speaker Playback Volume", "DAC2 Playback Volume");
> > 		break;
> > 	}
> > }
> > 
> > On second review, this does break some routing with older alc285, as the speaker and headphone NIDs are still 0x02 and 0x03.
> > Maybe cloning the x1 gen 7 fixup, but without the renamed ctls, is the more appropriate solution here? Let me know what your thoughts are.
> 
> I believe it's safer to make a specific quirk for X7 gen7.
> There are too many dependencies on the existing chains.
> 
> 
> Takashi

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[Takashi Iwai]

On Wed, 31 Jan 2024 12:09:49 +0100,
José Relvas wrote:
> 
> Thanks for the feedback! So if my understanding is correct, the headset jack quirk is currently *only* applied to models with bass speakers?

AFAIK, it looks yes; at least the majority of machines are actually
with bass speakers.

> bass thinkpad -> headset thinkpad quirk -> bass thinkpad quirk (both quirks applied)
> normal thinkpad -X> headset thinkpad quirk -X> bass thinkpad quirk (neither quirks applied) 
>
> I'll go back to the drawing board, just making sure I'm not missing anything.

Maybe it'd be helpful to rename the current one to be more explicit
with bass speakers at first :)


Takashi

> José Relvas
> 
> On Wed, Jan 31, 2024 at 11:49:28AM +0100, Takashi Iwai wrote:
> > On Wed, 31 Jan 2024 11:28:15 +0100,
> > José Relvas wrote:
> > > 
> > > Thanks for the reply!
> > > 
> > > Sorry about the misformatted patch. I did send in a second one through git-send-email. Is it not correct either? 
> > > 
> > > Anyways, the reason I'm editing the existing quirks is mostly because ALC285_FIXUP_THINKPAD_HEADSET_JACK was actually wrong.
> > > It was actually always linking to the X1 gen 7 quirk, which has some kcontrol changes only meant for a few models with bass speakers.
> > 
> > Yes, and that's the intentional behavior.
> > There is the match with the pin table, not only the explicit PCI
> > SSID, and those are with bass speakers.
> > 
> > > It looks like this: 
> > > 
> > > /* Quirk for Thinkpad X1 7th and 8th Gen
> > >  * The following fixed routing needed
> > >  * DAC1 (NID 0x02) -> Speaker (NID 0x14); some eq applied secretly
> > >  * DAC2 (NID 0x03) -> Bass (NID 0x17) & Headphone (NID 0x21); sharing a DAC
> > >  * DAC3 (NID 0x06) -> Unused, due to the lack of volume amp
> > > */
> > > static void alc285_fixup_thinkpad_x1_gen7(struct hda_codec *codec,
> > > 					  const struct hda_fixup *fix, int action)
> > > {
> > > 	static const hda_nid_t conn[] = { 0x02, 0x03 }; /* exclude 0x06 */
> > > 	static const hda_nid_t preferred_pairs[] = {
> > > 		0x14, 0x02, 0x17, 0x03, 0x21, 0x03, 0
> > > 	};
> > > 	struct alc_spec *spec = codec->spec;
> > > 
> > > 	switch (action) {
> > > 	case HDA_FIXUP_ACT_PRE_PROBE:
> > > 		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> > > 		spec->gen.preferred_dacs = preferred_pairs;
> > > 		break;
> > > 	case HDA_FIXUP_ACT_BUILD:
> > > 		/* The generic parser creates somewhat unintuitive volume ctls
> > > 		 * with the fixed routing above, and the shared DAC2 may be
> > > 		 * confusing for PA.
> > > 		 * Rename those to unique names so that PA doesn't touch them
> > > 		 * and use only Master volume.
> > > 		 */
> > > 		rename_ctl(codec, "Front Playback Volume", "DAC1 Playback Volume");
> > > 		rename_ctl(codec, "Bass Speaker Playback Volume", "DAC2 Playback Volume");
> > > 		break;
> > > 	}
> > > }
> > > 
> > > On second review, this does break some routing with older alc285, as the speaker and headphone NIDs are still 0x02 and 0x03.
> > > Maybe cloning the x1 gen 7 fixup, but without the renamed ctls, is the more appropriate solution here? Let me know what your thoughts are.
> > 
> > I believe it's safer to make a specific quirk for X7 gen7.
> > There are too many dependencies on the existing chains.
> > 
> > 
> > Takashi

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[José Relvas]

Oops, turns out there's a non-bass variant of this quirk: ALC285_FIXUP_THINKPAD_NO_BASS_SPK_HEADSET_JACK
I've made a V2 of this patch using it - it's much less invasive.

I'm in agreement. If the quirk's name says it targets bass systems, then that'd imply a non-bass version is available, which should prevent future confusion.

On Wed, Jan 31, 2024 at 12:39:00PM +0100, Takashi Iwai wrote:
> On Wed, 31 Jan 2024 12:09:49 +0100,
> José Relvas wrote:
> > 
> > Thanks for the feedback! So if my understanding is correct, the headset jack quirk is currently *only* applied to models with bass speakers?
> 
> AFAIK, it looks yes; at least the majority of machines are actually
> with bass speakers.
> 
> > bass thinkpad -> headset thinkpad quirk -> bass thinkpad quirk (both quirks applied)
> > normal thinkpad -X> headset thinkpad quirk -X> bass thinkpad quirk (neither quirks applied) 
> >
> > I'll go back to the drawing board, just making sure I'm not missing anything.
> 
> Maybe it'd be helpful to rename the current one to be more explicit
> with bass speakers at first :)
> 
> 
> Takashi
> 
> > José Relvas
> > 
> > On Wed, Jan 31, 2024 at 11:49:28AM +0100, Takashi Iwai wrote:
> > > On Wed, 31 Jan 2024 11:28:15 +0100,
> > > José Relvas wrote:
> > > > 
> > > > Thanks for the reply!
> > > > 
> > > > Sorry about the misformatted patch. I did send in a second one through git-send-email. Is it not correct either? 
> > > > 
> > > > Anyways, the reason I'm editing the existing quirks is mostly because ALC285_FIXUP_THINKPAD_HEADSET_JACK was actually wrong.
> > > > It was actually always linking to the X1 gen 7 quirk, which has some kcontrol changes only meant for a few models with bass speakers.
> > > 
> > > Yes, and that's the intentional behavior.
> > > There is the match with the pin table, not only the explicit PCI
> > > SSID, and those are with bass speakers.
> > > 
> > > > It looks like this: 
> > > > 
> > > > /* Quirk for Thinkpad X1 7th and 8th Gen
> > > >  * The following fixed routing needed
> > > >  * DAC1 (NID 0x02) -> Speaker (NID 0x14); some eq applied secretly
> > > >  * DAC2 (NID 0x03) -> Bass (NID 0x17) & Headphone (NID 0x21); sharing a DAC
> > > >  * DAC3 (NID 0x06) -> Unused, due to the lack of volume amp
> > > > */
> > > > static void alc285_fixup_thinkpad_x1_gen7(struct hda_codec *codec,
> > > > 					  const struct hda_fixup *fix, int action)
> > > > {
> > > > 	static const hda_nid_t conn[] = { 0x02, 0x03 }; /* exclude 0x06 */
> > > > 	static const hda_nid_t preferred_pairs[] = {
> > > > 		0x14, 0x02, 0x17, 0x03, 0x21, 0x03, 0
> > > > 	};
> > > > 	struct alc_spec *spec = codec->spec;
> > > > 
> > > > 	switch (action) {
> > > > 	case HDA_FIXUP_ACT_PRE_PROBE:
> > > > 		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> > > > 		spec->gen.preferred_dacs = preferred_pairs;
> > > > 		break;
> > > > 	case HDA_FIXUP_ACT_BUILD:
> > > > 		/* The generic parser creates somewhat unintuitive volume ctls
> > > > 		 * with the fixed routing above, and the shared DAC2 may be
> > > > 		 * confusing for PA.
> > > > 		 * Rename those to unique names so that PA doesn't touch them
> > > > 		 * and use only Master volume.
> > > > 		 */
> > > > 		rename_ctl(codec, "Front Playback Volume", "DAC1 Playback Volume");
> > > > 		rename_ctl(codec, "Bass Speaker Playback Volume", "DAC2 Playback Volume");
> > > > 		break;
> > > > 	}
> > > > }
> > > > 
> > > > On second review, this does break some routing with older alc285, as the speaker and headphone NIDs are still 0x02 and 0x03.
> > > > Maybe cloning the x1 gen 7 fixup, but without the renamed ctls, is the more appropriate solution here? Let me know what your thoughts are.
> > > 
> > > I believe it's safer to make a specific quirk for X7 gen7.
> > > There are too many dependencies on the existing chains.
> > > 
> > > 
> > > Takashi

Re: [PATCH] Apply headset jack fixup for alc287 thinkpads

[Takashi Iwai]

On Wed, 31 Jan 2024 12:45:47 +0100,
José Relvas wrote:
> 
> Oops, turns out there's a non-bass variant of this quirk: ALC285_FIXUP_THINKPAD_NO_BASS_SPK_HEADSET_JACK
> I've made a V2 of this patch using it - it's much less invasive.

Yeah, I saw it, and it looks better.


thanks,

Takashi

> I'm in agreement. If the quirk's name says it targets bass systems, then that'd imply a non-bass version is available, which should prevent future confusion.
> 
> On Wed, Jan 31, 2024 at 12:39:00PM +0100, Takashi Iwai wrote:
> > On Wed, 31 Jan 2024 12:09:49 +0100,
> > José Relvas wrote:
> > > 
> > > Thanks for the feedback! So if my understanding is correct, the headset jack quirk is currently *only* applied to models with bass speakers?
> > 
> > AFAIK, it looks yes; at least the majority of machines are actually
> > with bass speakers.
> > 
> > > bass thinkpad -> headset thinkpad quirk -> bass thinkpad quirk (both quirks applied)
> > > normal thinkpad -X> headset thinkpad quirk -X> bass thinkpad quirk (neither quirks applied) 
> > >
> > > I'll go back to the drawing board, just making sure I'm not missing anything.
> > 
> > Maybe it'd be helpful to rename the current one to be more explicit
> > with bass speakers at first :)
> > 
> > 
> > Takashi
> > 
> > > José Relvas
> > > 
> > > On Wed, Jan 31, 2024 at 11:49:28AM +0100, Takashi Iwai wrote:
> > > > On Wed, 31 Jan 2024 11:28:15 +0100,
> > > > José Relvas wrote:
> > > > > 
> > > > > Thanks for the reply!
> > > > > 
> > > > > Sorry about the misformatted patch. I did send in a second one through git-send-email. Is it not correct either? 
> > > > > 
> > > > > Anyways, the reason I'm editing the existing quirks is mostly because ALC285_FIXUP_THINKPAD_HEADSET_JACK was actually wrong.
> > > > > It was actually always linking to the X1 gen 7 quirk, which has some kcontrol changes only meant for a few models with bass speakers.
> > > > 
> > > > Yes, and that's the intentional behavior.
> > > > There is the match with the pin table, not only the explicit PCI
> > > > SSID, and those are with bass speakers.
> > > > 
> > > > > It looks like this: 
> > > > > 
> > > > > /* Quirk for Thinkpad X1 7th and 8th Gen
> > > > >  * The following fixed routing needed
> > > > >  * DAC1 (NID 0x02) -> Speaker (NID 0x14); some eq applied secretly
> > > > >  * DAC2 (NID 0x03) -> Bass (NID 0x17) & Headphone (NID 0x21); sharing a DAC
> > > > >  * DAC3 (NID 0x06) -> Unused, due to the lack of volume amp
> > > > > */
> > > > > static void alc285_fixup_thinkpad_x1_gen7(struct hda_codec *codec,
> > > > > 					  const struct hda_fixup *fix, int action)
> > > > > {
> > > > > 	static const hda_nid_t conn[] = { 0x02, 0x03 }; /* exclude 0x06 */
> > > > > 	static const hda_nid_t preferred_pairs[] = {
> > > > > 		0x14, 0x02, 0x17, 0x03, 0x21, 0x03, 0
> > > > > 	};
> > > > > 	struct alc_spec *spec = codec->spec;
> > > > > 
> > > > > 	switch (action) {
> > > > > 	case HDA_FIXUP_ACT_PRE_PROBE:
> > > > > 		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> > > > > 		spec->gen.preferred_dacs = preferred_pairs;
> > > > > 		break;
> > > > > 	case HDA_FIXUP_ACT_BUILD:
> > > > > 		/* The generic parser creates somewhat unintuitive volume ctls
> > > > > 		 * with the fixed routing above, and the shared DAC2 may be
> > > > > 		 * confusing for PA.
> > > > > 		 * Rename those to unique names so that PA doesn't touch them
> > > > > 		 * and use only Master volume.
> > > > > 		 */
> > > > > 		rename_ctl(codec, "Front Playback Volume", "DAC1 Playback Volume");
> > > > > 		rename_ctl(codec, "Bass Speaker Playback Volume", "DAC2 Playback Volume");
> > > > > 		break;
> > > > > 	}
> > > > > }
> > > > > 
> > > > > On second review, this does break some routing with older alc285, as the speaker and headphone NIDs are still 0x02 and 0x03.
> > > > > Maybe cloning the x1 gen 7 fixup, but without the renamed ctls, is the more appropriate solution here? Let me know what your thoughts are.
> > > > 
> > > > I believe it's safer to make a specific quirk for X7 gen7.
> > > > There are too many dependencies on the existing chains.
> > > > 
> > > > 
> > > > Takashi

[PATCH] Apply headset jack fixup for alc287 thinkpads

[José Relvas]

Thinkpads have a fixup for alc285 which enables headset media buttons, 
but not for alc287. This patch removes the chain to the X1 gen 7 fixup 
in the alc285 headset fixup, making it applicable for models without 
bass speakers. It then chains said headset fixup with every alc287 
thinkpad fixup, making headset media buttons work for these laptops. 
Signed-off-by: José Relvas --- sound/pci/hda/patch_realtek.c | 12 
+++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git 
a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 
c80e2f3acfcc..21d10c62964c 100644 --- a/sound/pci/hda/patch_realtek.c 
+++ b/sound/pci/hda/patch_realtek.c @@ -8796,13 +8796,13 @@ static const 
struct hda_fixup alc269_fixups[] = { .type = HDA_FIXUP_FUNC, .v.func = 
alc285_fixup_thinkpad_x1_gen7, .chained = true, - .chain_id = 
ALC269_FIXUP_THINKPAD_ACPI + .chain_id = 
ALC285_FIXUP_THINKPAD_HEADSET_JACK }, 
[ALC285_FIXUP_THINKPAD_HEADSET_JACK] = { .type = HDA_FIXUP_FUNC, .v.func 
= alc_fixup_headset_jack, .chained = true, - .chain_id = 
ALC285_FIXUP_THINKPAD_X1_GEN7 + .chain_id = ALC269_FIXUP_THINKPAD_ACPI 
}, [ALC294_FIXUP_ASUS_HPE] = { .type = HDA_FIXUP_VERBS, @@ -9576,7 
+9577,7 @@ static const struct hda_fixup alc269_fixups[] = { .type = 
HDA_FIXUP_FUNC, .v.func = cs36l41_fixup_i2c_two, .chained = true, - 
.chain_id = ALC270_FIXUP_THINKPAD_ACPI, + .chain_id = 
ALC286_FIXUP_THINKPAD_HEADSET_JACK, }, [ALC288_FIXUP_TAS2781_I2C] = { 
.type = HDA_FIXUP_FUNC, @@ -9603,6 +9604,8 @@ static const struct 
hda_fixup alc269_fixups[] = { [ALC288_FIXUP_THINKPAD_I2S_SPK] = { .type 
= HDA_FIXUP_FUNC, .v.func = alc288_fixup_bind_dacs, + .chained = true, + 
.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK }, 
[ALC287_FIXUP_MG_RTKC_CSAMP_CS35L41_I2C_THINKPAD] = { .type = 
HDA_FIXUP_FUNC, @@ -10206,8 +10208,8 @@ static const struct 
snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x17aa, 0x224c, 
"Thinkpad", ALC298_FIXUP_TPT470_DOCK), SND_PCI_QUIRK(0x17aa, 0x224d, 
"Thinkpad", ALC298_FIXUP_TPT470_DOCK), SND_PCI_QUIRK(0x17aa, 0x225d, 
"Thinkpad T480", ALC269_FIXUP_LIMIT_INT_MIC_BOOST), - 
SND_PCI_QUIRK(0x17aa, 0x2292, "Thinkpad X1 Carbon 7th", 
ALC285_FIXUP_THINKPAD_HEADSET_JACK), - SND_PCI_QUIRK(0x17aa, 0x22be, 
"Thinkpad X1 Carbon 8th", ALC285_FIXUP_THINKPAD_HEADSET_JACK), + 
SND_PCI_QUIRK(0x17aa, 0x2292, "Thinkpad X1 Carbon 7th", 
ALC285_FIXUP_THINKPAD_X1_GEN7), + SND_PCI_QUIRK(0x17aa, 0x22be, 
"Thinkpad X1 Carbon 8th", ALC285_FIXUP_THINKPAD_X1_GEN7), 
SND_PCI_QUIRK(0x17aa, 0x22c1, "Thinkpad P1 Gen 3", 
ALC285_FIXUP_THINKPAD_NO_BASS_SPK_HEADSET_JACK), SND_PCI_QUIRK(0x17aa, 
0x22c2, "Thinkpad X1 Extreme Gen 3", 
ALC285_FIXUP_THINKPAD_NO_BASS_SPK_HEADSET_JACK), SND_PCI_QUIRK(0x17aa, 
0x22f1, "Thinkpad", ALC287_FIXUP_MG_RTKC_CSAMP_CS35L41_I2C_THINKPAD), -- 
2.43.0