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

[Takashi Iwai <tiwai@xxxxxxx>]

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