ALSA: hda/realtek - Improved routing for Thinkpad X1 7th/8th Gen

There've been quite a few regression reports about the lowered volume
(reduced to ca 65% from the previous level) on Lenovo Thinkpad X1
after the commit ed545e254c65 ("ALSA: hda - fixup for the bass speaker
on Lenovo Carbon X1 7th gen").  Although the commit itself does the
right thing from HD-audio POV in order to have a volume control for
bass speakers, it seems that the machine has some secret recipe under
the hood.

Through experiments, Benjamin Poirier found out that the following
routing gives the best result:
* DAC1 (NID 0x02) -> Speaker pin (NID 0x14)
* DAC2 (NID 0x03) -> Shared by both Bass Speaker pin (NID 0x17) &
                     Headphone pin (0x21)
* DAC3 (NID 0x06) -> Unused

DAC1 seems to have some equalizer internally applied, and you'd get
again the output in a bad quality if you connect this to the
headphone pin.  Hence the headphone is connected to DAC2, which is now
shared with the bass speaker pin.  DAC3 has no volume amp, hence it's
not connected at all.

For achieving the routing above, this patch introduced a couple of
workarounds:

* The connection list of bass speaker pin (NID 0x17) is reduced not to
  include DAC3 (NID 0x06)
* Pass preferred_pairs array to specify the fixed connection

Here, both workarounds are needed because the generic parser prefers
the individual DAC assignment over others.

When the routing above is applied, the generic parser creates the two
volume controls "Front" and "Bass Speaker".  Since we have only two
DACs for three output pins, those are not fully controlling each
output individually, and it would confuse PulseAudio.  For avoiding
the pitfall, in this patch, we rename those volume controls to some
unique ones ("DAC1" and "DAC2").  Then PulseAudio ignore them and
concentrate only on the still good-working "Master" volume control.
If a user still wants to control each DAC volume, they can still
change manually via "DAC1" and "DAC2" volume controls.

Fixes: ed545e254c65 ("ALSA: hda - fixup for the bass speaker on Lenovo Carbon X1 7th gen")
Reported-by: Benjamin Poirier <benjamin.poirier@gmail.com>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Tested-by: Benjamin Poirier <benjamin.poirier@gmail.com>
Cc: <stable@vger.kernel.org>
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=207407#c10
BugLink: https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3214171
BugLink: https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3276276
Link: https://lore/kernel.org/r/20200829112746.3118-1-benjamin.poirier@gmail.com
Link: https://lore.kernel.org/r/20200903083300.6333-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 2ef8b080d84b1aa26e5875ae8b572739a3d381ce..c521a1f170969aabea99b5c627486d0985c97de8 100644 (file)
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5868,6 +5868,39 @@ static void alc275_fixup_gpio4_off(struct hda_codec *codec,
        }
 }
 
+/* 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;
+       }
+}
+
 static void alc233_alc662_fixup_lenovo_dual_codecs(struct hda_codec *codec,
                                         const struct hda_fixup *fix,
                                         int action)
@@ -6136,6 +6169,7 @@ enum {
        ALC289_FIXUP_DUAL_SPK,
        ALC294_FIXUP_SPK2_TO_DAC1,
        ALC294_FIXUP_ASUS_DUAL_SPK,
+       ALC285_FIXUP_THINKPAD_X1_GEN7,
        ALC285_FIXUP_THINKPAD_HEADSET_JACK,
        ALC294_FIXUP_ASUS_HPE,
        ALC294_FIXUP_ASUS_COEF_1B,
@@ -7281,11 +7315,17 @@ static const struct hda_fixup alc269_fixups[] = {
                .chained = true,
                .chain_id = ALC294_FIXUP_SPK2_TO_DAC1
        },
+       [ALC285_FIXUP_THINKPAD_X1_GEN7] = {
+               .type = HDA_FIXUP_FUNC,
+               .v.func = alc285_fixup_thinkpad_x1_gen7,
+               .chained = true,
+               .chain_id = ALC269_FIXUP_THINKPAD_ACPI
+       },
        [ALC285_FIXUP_THINKPAD_HEADSET_JACK] = {
                .type = HDA_FIXUP_FUNC,
                .v.func = alc_fixup_headset_jack,
                .chained = true,
-               .chain_id = ALC285_FIXUP_SPEAKER2_TO_DAC1
+               .chain_id = ALC285_FIXUP_THINKPAD_X1_GEN7
        },
        [ALC294_FIXUP_ASUS_HPE] = {
                .type = HDA_FIXUP_VERBS,