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

[José Relvas <josemonsantorelvas@xxxxxxxxx>]

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