[PATCH v2 0/2] Fix Lenovo ThinkPad X1 Carbon 7th gen bass volume

[PATCH v2 0/2] Fix Lenovo ThinkPad X1 Carbon 7th gen bass volume

[Benjamin Poirier]

This is an updated version of the patchset to fix the low output volume on
the Lenovo ThinkPad X1 Carbon 7th gen. With these patches, the volume
output with the Linux driver matches the one with the Windows driver.
Volume control is effective and headphone output is unaffected.

Multiple users of this machine have reported a low output volume [1] when
using a kernel with commit d2cd795c4ece ("ALSA: hda - fixup for the bass
speaker on Lenovo Carbon X1 7th gen"). Many have tried these patches or a
similar workaround using `hda-verb` and prefer this configuration[2].

As noted in patch 2, the changes also depend on an associated alsa-ucm-conf
change.

[1]
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3227516
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3240090
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3296637
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3304699
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3343871
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3349803
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3361380
...

[2]
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3343252
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3344118
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3345062
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3345107
...

Changes:

v1
==
https://lore.kernel.org/alsa-devel/20200211055651.4405-1-benjamin.poirier@gmail.com/

v2
==
* patch 1
Shortened log

* patch 2
Change log to replace experimental observations by information from diagram
posted on kernel.org bugzilla.
Select exactly the desired connection for node 0x17 (bass speakers) instead of
reusing alc295_fixup_disable_dac3().
Control node 0x21 (headphones) connection to avoid the volume "wobble"
effect described in the log.
Include SET_CONNECT_SEL command in fixup since it is not done by the hda
core.
Rename one control to reflect its new function and to use as a condition in
the ucm config.


Benjamin Poirier (2):
  ALSA: hda/realtek - Fix Lenovo Thinkpad X1 Carbon 7th quirk subdevice
    id
  ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

 sound/pci/hda/patch_realtek.c | 57 ++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 5 deletions(-)

-- 
2.27.0

[PATCH v2 1/2] ALSA: hda/realtek - Fix Lenovo Thinkpad X1 Carbon 7th quirk subdevice id

[Benjamin Poirier]

1)
In snd_hda_pick_fixup(), quirks are first matched by PCI SSID and then, if
there is no match, by codec SSID. The Lenovo "ThinkPad X1 Carbon 7th" has
an audio chip with PCI SSID 0x2292 and codec SSID 0x2293[1]. Therefore, fix
the quirk meant for that device to match on .subdevice == 0x2292.

2)
The "Thinkpad X1 Yoga 7th" does not exist. The companion product to the
Carbon 7th is the Yoga 4th. That device has an audio chip with PCI SSID
0x2292 and codec SSID 0x2292[2]. Given the behavior of
snd_hda_pick_fixup(), it is not possible to have a separate quirk for the
Yoga based on SSID. Therefore, merge the quirks meant for the Carbon and
Yoga. This preserves the current behavior for the Yoga.

[1] This is the case on my own machine and can also be checked here
https://github.com/linuxhw/LsPCI/tree/master/Notebook/Lenovo/ThinkPad
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3225701
[2]
https://github.com/linuxhw/LsPCI/tree/master/Convertible/Lenovo/ThinkPad
https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3176355

Fixes: d2cd795c4ece ("ALSA: hda - fixup for the bass speaker on Lenovo Carbon X1 7th gen")
Fixes: 54a6a7dc107d ("ALSA: hda/realtek - Add quirk for the bass speaker on Lenovo Yoga X1 7th gen")
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Kailang Yang <kailang@realtek.com>
Tested-by: Vincent Bernat <vincent@bernat.ch>
Tested-by: Even Brenden <evenbrenden@gmail.com>
Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
---
 sound/pci/hda/patch_realtek.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 5a11ac2d446d..16696694da91 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -7559,8 +7559,7 @@ 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 Yoga 7th", ALC285_FIXUP_THINKPAD_HEADSET_JACK),
-	SND_PCI_QUIRK(0x17aa, 0x2293, "Thinkpad X1 Carbon 7th", ALC285_FIXUP_THINKPAD_HEADSET_JACK),
+	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, 0x30bb, "ThinkCentre AIO", ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY),
 	SND_PCI_QUIRK(0x17aa, 0x30e2, "ThinkCentre AIO", ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY),
-- 
2.27.0

Re: [PATCH v2 1/2] ALSA: hda/realtek - Fix Lenovo Thinkpad X1 Carbon 7th quirk subdevice id

[Takashi Iwai]

On Fri, 03 Jul 2020 10:00:04 +0200,
Benjamin Poirier wrote:
> 
> 1)
> In snd_hda_pick_fixup(), quirks are first matched by PCI SSID and then, if
> there is no match, by codec SSID. The Lenovo "ThinkPad X1 Carbon 7th" has
> an audio chip with PCI SSID 0x2292 and codec SSID 0x2293[1]. Therefore, fix
> the quirk meant for that device to match on .subdevice == 0x2292.
> 
> 2)
> The "Thinkpad X1 Yoga 7th" does not exist. The companion product to the
> Carbon 7th is the Yoga 4th. That device has an audio chip with PCI SSID
> 0x2292 and codec SSID 0x2292[2]. Given the behavior of
> snd_hda_pick_fixup(), it is not possible to have a separate quirk for the
> Yoga based on SSID. Therefore, merge the quirks meant for the Carbon and
> Yoga. This preserves the current behavior for the Yoga.
> 
> [1] This is the case on my own machine and can also be checked here
> https://github.com/linuxhw/LsPCI/tree/master/Notebook/Lenovo/ThinkPad
> https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3225701
> [2]
> https://github.com/linuxhw/LsPCI/tree/master/Convertible/Lenovo/ThinkPad
> https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3176355
> 
> Fixes: d2cd795c4ece ("ALSA: hda - fixup for the bass speaker on Lenovo Carbon X1 7th gen")
> Fixes: 54a6a7dc107d ("ALSA: hda/realtek - Add quirk for the bass speaker on Lenovo Yoga X1 7th gen")
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Kailang Yang <kailang@realtek.com>
> Tested-by: Vincent Bernat <vincent@bernat.ch>
> Tested-by: Even Brenden <evenbrenden@gmail.com>
> Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>

Applied this one alone for now.  Let me check the second patch later.


thanks,

Takashi

[PATCH v2 2/2] ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

[Benjamin Poirier]

As a result of commit d2cd795c4ece ("ALSA: hda - fixup for the bass speaker
on Lenovo Carbon X1 7th gen"), the sound output level on my machine, an X1
Carbon 7th gen, was reduced to ~65% of its previous level when playing
certain sounds. [1]

Internally, this laptop model has three outputs (PCM-OUT1, connection 0x02;
PCM-OUT2, connection 0x03; SP-OUT PCM, connection 0x06) which can be routed
to two sets of stereo speakers: Front (tweeters, node 0x14) and Bass
(woofers, node 0x17, aka Rear in some contexts) and one headphone output
(node 0x21). The tweeters are noticeably less powerful than the woofers.
[2]

Before commit d2cd795c4ece, the bass speakers were connected to SP-OUT PCM.
SP-OUT PCM is meant for s/pdif output and does not have volume control.
This connection made volume control commonly ineffective (using the Master
slider in alsa or pulseaudio apparently had little effect or alternated
between mute or max with nothing in between).

commit d2cd795c4ece added quirk ALC285_FIXUP_SPEAKER2_TO_DAC1 which
resulted in assigning both sets of speakers to PCM-OUT1, bringing
the two sets of speakers under one effective volume control but also
lowering the output volume noticeably.

Fix this by connecting PCM-OUT1 to Front speakers and PCM-OUT2 to Rear
speakers. Each set of speakers gets its own volume control and the max
output volume is restored to what it was before commit d2cd795c4ece. This
is done by setting the connection of node 0x17 to 0x03.

However, when we do this, the HDA auto config automatically changes the
connection of node 0x21 to 0x02. This output, meant for the front speakers,
has some "secret" equalizer which changes the output volume according to
the level of what's being played, after some delay[3]. This is undesirable
with headphones. Therefore, this patch manually limits the connection of
node 0x21 to 0x03.

The volume control for PCM-OUT2 is renamed to reflect its dual effect.
This name is also used in a modified alsa UCM profile. [4]

It is possible that the X1 Carbon 8th gen would benefit from the same
changes but I don't have a device to test that. Fixups are reordered so
that the devices for 7th & 8th gen can share the same chain after the first
fixup. The resulting chain is:
ALC295_FIXUP_TPX17_DUAL_SPEAKERS/ALC285_FIXUP_SPEAKER2_TO_DAC1
ALC285_FIXUP_THINKPAD_HEADSET_JACK
ALC269_FIXUP_THINKPAD_ACPI
ALC269_FIXUP_SKU_IGNORE

[1] https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3214171
[2] https://bugzilla.kernel.org/show_bug.cgi?id=207407#c10
[3] https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3276276
[4] https://lore.kernel.org/alsa-devel/20200703072302.16876-1-benjamin.poirier@gmail.com/

Fixes: d2cd795c4ece ("ALSA: hda - fixup for the bass speaker on Lenovo Carbon X1 7th gen")
Link: https://lore.kernel.org/alsa-devel/20200210025249.GA2700@f3/
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Kailang Yang <kailang@realtek.com>
Tested-by: Vincent Bernat <vincent@bernat.ch>
Tested-by: Even Brenden <evenbrenden@gmail.com>
Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
---
 sound/pci/hda/patch_realtek.c | 56 ++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 16696694da91..ef3dbf83e42b 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5825,6 +5825,46 @@ static void alc285_fixup_speaker2_to_dac1(struct hda_codec *codec,
 	}
 }
 
+static void alc295_fixup_tpx17_dual_speakers(struct hda_codec *codec,
+					     const struct hda_fixup *fix,
+					     int action)
+{
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		static const hda_nid_t conn[] = { 0x03 };
+
+		/* For NID 0x17 (bass speakers), the connection list is {0x02,
+		 * 0x03, 0x06}. Disable SP-OUT PCM (0x06) selection since it
+		 * has no volume control, disable PCM1 (0x02) selection since
+		 * it is for front speakers. This leaves PCM2 (0x03).
+		 */
+		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn),
+					   conn);
+		/* For NID 0x21 (headphone out), the connection list is {0x02,
+		 * 0x03}. Disable LOUT1 (0x02) selection since its volume
+		 * fluctuates according to input level. This leaves LOUT2
+		 * (0x03).
+		 */
+		snd_hda_override_conn_list(codec, 0x21, ARRAY_SIZE(conn),
+					   conn);
+	} else if (action == HDA_FIXUP_ACT_INIT) {
+		/* Because the overridden connection lists contain only a
+		 * single node, __parse_nid_path() does not label the output
+		 * as "multi". This leads snd_hda_activate_path() to skip the
+		 * AC_VERB_SET_CONNECT_SEL even though it might be needed. Do
+		 * it here instead.
+		 * Note that when doing AC_VERB_SET_CONNECT_SEL, the
+		 * connection is specified by index instead of nid.
+		 */
+		snd_hda_codec_write(codec, 0x17, 0, AC_VERB_SET_CONNECT_SEL,
+				    0x1);
+		snd_hda_codec_write(codec, 0x21, 0, AC_VERB_SET_CONNECT_SEL,
+				    0x1);
+	} else if (action == HDA_FIXUP_ACT_BUILD) {
+		rename_ctl(codec, "Headphone Playback Volume",
+			   "Headphone/Bass Speaker Playback Volume");
+	}
+}
+
 /* Hook to update amp GPIO4 for automute */
 static void alc280_hp_gpio4_automute_hook(struct hda_codec *codec,
 					  struct hda_jack_callback *jack)
@@ -6077,6 +6117,7 @@ enum {
 	ALC225_FIXUP_DISABLE_MIC_VREF,
 	ALC225_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC295_FIXUP_DISABLE_DAC3,
+	ALC295_FIXUP_TPX17_DUAL_SPEAKERS,
 	ALC285_FIXUP_SPEAKER2_TO_DAC1,
 	ALC280_FIXUP_HP_HEADSET_MIC,
 	ALC221_FIXUP_HP_FRONT_MIC,
@@ -6886,11 +6927,17 @@ static const struct hda_fixup alc269_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc295_fixup_disable_dac3,
 	},
+	[ALC295_FIXUP_TPX17_DUAL_SPEAKERS] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc295_fixup_tpx17_dual_speakers,
+		.chained = true,
+		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK
+	},
 	[ALC285_FIXUP_SPEAKER2_TO_DAC1] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc285_fixup_speaker2_to_dac1,
 		.chained = true,
-		.chain_id = ALC269_FIXUP_THINKPAD_ACPI
+		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK
 	},
 	[ALC256_FIXUP_DELL_INSPIRON_7559_SUBWOOFER] = {
 		.type = HDA_FIXUP_PINS,
@@ -7263,7 +7310,7 @@ static const struct hda_fixup alc269_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_headset_jack,
 		.chained = true,
-		.chain_id = ALC285_FIXUP_SPEAKER2_TO_DAC1
+		.chain_id = ALC269_FIXUP_THINKPAD_ACPI
 	},
 	[ALC294_FIXUP_ASUS_HPE] = {
 		.type = HDA_FIXUP_VERBS,
@@ -7559,8 +7606,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", ALC295_FIXUP_TPX17_DUAL_SPEAKERS),
+	SND_PCI_QUIRK(0x17aa, 0x22be, "Thinkpad X1 Carbon 8th", ALC285_FIXUP_SPEAKER2_TO_DAC1),
 	SND_PCI_QUIRK(0x17aa, 0x30bb, "ThinkCentre AIO", ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY),
 	SND_PCI_QUIRK(0x17aa, 0x30e2, "ThinkCentre AIO", ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY),
 	SND_PCI_QUIRK(0x17aa, 0x310c, "ThinkCentre Station", ALC294_FIXUP_LENOVO_MIC_LOCATION),
@@ -7746,6 +7793,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC255_FIXUP_DELL_SPK_NOISE, .name = "dell-spk-noise"},
 	{.id = ALC225_FIXUP_DELL1_MIC_NO_PRESENCE, .name = "alc225-dell1"},
 	{.id = ALC295_FIXUP_DISABLE_DAC3, .name = "alc295-disable-dac3"},
+	{.id = ALC295_FIXUP_TPX17_DUAL_SPEAKERS, .name = "alc295-thinkpad-x1-gen7"},
 	{.id = ALC285_FIXUP_SPEAKER2_TO_DAC1, .name = "alc285-speaker2-to-dac1"},
 	{.id = ALC280_FIXUP_HP_HEADSET_MIC, .name = "alc280-hp-headset"},
 	{.id = ALC221_FIXUP_HP_FRONT_MIC, .name = "alc221-hp-mic"},
-- 
2.27.0

Re: [PATCH v2 2/2] ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

[Jaroslav Kysela]

Dne 03. 07. 20 v 10:00 Benjamin Poirier napsal(a):
> As a result of commit d2cd795c4ece ("ALSA: hda - fixup for the bass speaker
> on Lenovo Carbon X1 7th gen"), the sound output level on my machine, an X1
> Carbon 7th gen, was reduced to ~65% of its previous level when playing
> certain sounds. [1]
> 
> Internally, this laptop model has three outputs (PCM-OUT1, connection 0x02;
> PCM-OUT2, connection 0x03; SP-OUT PCM, connection 0x06) which can be routed
> to two sets of stereo speakers: Front (tweeters, node 0x14) and Bass
> (woofers, node 0x17, aka Rear in some contexts) and one headphone output
> (node 0x21). The tweeters are noticeably less powerful than the woofers.
> [2]
> 
> Before commit d2cd795c4ece, the bass speakers were connected to SP-OUT PCM.
> SP-OUT PCM is meant for s/pdif output and does not have volume control.
> This connection made volume control commonly ineffective (using the Master
> slider in alsa or pulseaudio apparently had little effect or alternated
> between mute or max with nothing in between).
> 
> commit d2cd795c4ece added quirk ALC285_FIXUP_SPEAKER2_TO_DAC1 which
> resulted in assigning both sets of speakers to PCM-OUT1, bringing
> the two sets of speakers under one effective volume control but also
> lowering the output volume noticeably.
> 
> Fix this by connecting PCM-OUT1 to Front speakers and PCM-OUT2 to Rear
> speakers. Each set of speakers gets its own volume control and the max
> output volume is restored to what it was before commit d2cd795c4ece. This
> is done by setting the connection of node 0x17 to 0x03.
> 
> However, when we do this, the HDA auto config automatically changes the
> connection of node 0x21 to 0x02. This output, meant for the front speakers,
> has some "secret" equalizer which changes the output volume according to
> the level of what's being played, after some delay[3]. This is undesirable
> with headphones. Therefore, this patch manually limits the connection of
> node 0x21 to 0x03.

Thank you for this work. Perhaps, Takashi will have some comments to improve 
this quirk.

> The volume control for PCM-OUT2 is renamed to reflect its dual effect.
> This name is also used in a modified alsa UCM profile. [4]

The new name is not ideal. Perhaps, a mirror (joined) control might be better 
in this case. I mean to create both "Bass Speaker Playback Volume" and 
"Headphone Playback Volume" with the similar NID control.

Or create "Speaker Playback" controls with 4 channels (and joined "Headphone 
Playback" controls).

> It is possible that the X1 Carbon 8th gen would benefit from the same
> changes but I don't have a device to test that. Fixups are reordered so
> that the devices for 7th & 8th gen can share the same chain after the first
> fixup. The resulting chain is:

8th gen hardware should be similar, so the new fixup should be applied to this 
hw, too.

> ALC295_FIXUP_TPX17_DUAL_SPEAKERS/ALC285_FIXUP_SPEAKER2_TO_DAC1
> ALC285_FIXUP_THINKPAD_HEADSET_JACK
> ALC269_FIXUP_THINKPAD_ACPI
> ALC269_FIXUP_SKU_IGNORE
> 
> [1] https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3214171
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=207407#c10
> [3] https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3276276
> [4] https://lore.kernel.org/alsa-devel/20200703072302.16876-1-benjamin.poirier@gmail.com/
> 
> Fixes: d2cd795c4ece ("ALSA: hda - fixup for the bass speaker on Lenovo Carbon X1 7th gen")
> Link: https://lore.kernel.org/alsa-devel/20200210025249.GA2700@f3/
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Kailang Yang <kailang@realtek.com>
> Tested-by: Vincent Bernat <vincent@bernat.ch>
> Tested-by: Even Brenden <evenbrenden@gmail.com>
> Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
> ---
>   sound/pci/hda/patch_realtek.c | 56 ++++++++++++++++++++++++++++++++---
>   1 file changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 16696694da91..ef3dbf83e42b 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -5825,6 +5825,46 @@ static void alc285_fixup_speaker2_to_dac1(struct hda_codec *codec,
>   	}
>   }
>   
> +static void alc295_fixup_tpx17_dual_speakers(struct hda_codec *codec,
> +					     const struct hda_fixup *fix,
> +					     int action)
> +{
> +	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> +		static const hda_nid_t conn[] = { 0x03 };
> +
> +		/* For NID 0x17 (bass speakers), the connection list is {0x02,
> +		 * 0x03, 0x06}. Disable SP-OUT PCM (0x06) selection since it
> +		 * has no volume control, disable PCM1 (0x02) selection since
> +		 * it is for front speakers. This leaves PCM2 (0x03).
> +		 */
> +		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn),
> +					   conn);
> +		/* For NID 0x21 (headphone out), the connection list is {0x02,
> +		 * 0x03}. Disable LOUT1 (0x02) selection since its volume
> +		 * fluctuates according to input level. This leaves LOUT2
> +		 * (0x03).
> +		 */
> +		snd_hda_override_conn_list(codec, 0x21, ARRAY_SIZE(conn),
> +					   conn);
> +	} else if (action == HDA_FIXUP_ACT_INIT) {
> +		/* Because the overridden connection lists contain only a
> +		 * single node, __parse_nid_path() does not label the output
> +		 * as "multi". This leads snd_hda_activate_path() to skip the
> +		 * AC_VERB_SET_CONNECT_SEL even though it might be needed. Do
> +		 * it here instead.
> +		 * Note that when doing AC_VERB_SET_CONNECT_SEL, the
> +		 * connection is specified by index instead of nid.
> +		 */
> +		snd_hda_codec_write(codec, 0x17, 0, AC_VERB_SET_CONNECT_SEL,
> +				    0x1);
> +		snd_hda_codec_write(codec, 0x21, 0, AC_VERB_SET_CONNECT_SEL,
> +				    0x1);
> +	} else if (action == HDA_FIXUP_ACT_BUILD) {
> +		rename_ctl(codec, "Headphone Playback Volume",
> +			   "Headphone/Bass Speaker Playback Volume");
> +	}
> +}
> +
>   /* Hook to update amp GPIO4 for automute */
>   static void alc280_hp_gpio4_automute_hook(struct hda_codec *codec,
>   					  struct hda_jack_callback *jack)
> @@ -6077,6 +6117,7 @@ enum {
>   	ALC225_FIXUP_DISABLE_MIC_VREF,
>   	ALC225_FIXUP_DELL1_MIC_NO_PRESENCE,
>   	ALC295_FIXUP_DISABLE_DAC3,
> +	ALC295_FIXUP_TPX17_DUAL_SPEAKERS,
>   	ALC285_FIXUP_SPEAKER2_TO_DAC1,
>   	ALC280_FIXUP_HP_HEADSET_MIC,
>   	ALC221_FIXUP_HP_FRONT_MIC,
> @@ -6886,11 +6927,17 @@ static const struct hda_fixup alc269_fixups[] = {
>   		.type = HDA_FIXUP_FUNC,
>   		.v.func = alc295_fixup_disable_dac3,
>   	},
> +	[ALC295_FIXUP_TPX17_DUAL_SPEAKERS] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = alc295_fixup_tpx17_dual_speakers,
> +		.chained = true,
> +		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK
> +	},
>   	[ALC285_FIXUP_SPEAKER2_TO_DAC1] = {
>   		.type = HDA_FIXUP_FUNC,
>   		.v.func = alc285_fixup_speaker2_to_dac1,
>   		.chained = true,
> -		.chain_id = ALC269_FIXUP_THINKPAD_ACPI
> +		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK
>   	},
>   	[ALC256_FIXUP_DELL_INSPIRON_7559_SUBWOOFER] = {
>   		.type = HDA_FIXUP_PINS,
> @@ -7263,7 +7310,7 @@ static const struct hda_fixup alc269_fixups[] = {
>   		.type = HDA_FIXUP_FUNC,
>   		.v.func = alc_fixup_headset_jack,
>   		.chained = true,
> -		.chain_id = ALC285_FIXUP_SPEAKER2_TO_DAC1
> +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI
>   	},
>   	[ALC294_FIXUP_ASUS_HPE] = {
>   		.type = HDA_FIXUP_VERBS,
> @@ -7559,8 +7606,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", ALC295_FIXUP_TPX17_DUAL_SPEAKERS),
> +	SND_PCI_QUIRK(0x17aa, 0x22be, "Thinkpad X1 Carbon 8th", ALC285_FIXUP_SPEAKER2_TO_DAC1),

It will cause regression with the old UCM configs. I would preper the manual 
model selection and switch this when things settle a bit in the user space. We 
definitely need a better way to control this volume separately for both 
tweeters and the bass speakers in the user space. The "Master" volume hack is 
far from the ideal solution.

For this time, keep ALC285_FIXUP_SPEAKER2_TO_DAC1 as the default fixup.

						Jaroslav


>   	SND_PCI_QUIRK(0x17aa, 0x30bb, "ThinkCentre AIO", ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY),
>   	SND_PCI_QUIRK(0x17aa, 0x30e2, "ThinkCentre AIO", ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY),
>   	SND_PCI_QUIRK(0x17aa, 0x310c, "ThinkCentre Station", ALC294_FIXUP_LENOVO_MIC_LOCATION),
> @@ -7746,6 +7793,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
>   	{.id = ALC255_FIXUP_DELL_SPK_NOISE, .name = "dell-spk-noise"},
>   	{.id = ALC225_FIXUP_DELL1_MIC_NO_PRESENCE, .name = "alc225-dell1"},
>   	{.id = ALC295_FIXUP_DISABLE_DAC3, .name = "alc295-disable-dac3"},
> +	{.id = ALC295_FIXUP_TPX17_DUAL_SPEAKERS, .name = "alc295-thinkpad-x1-gen7"},
>   	{.id = ALC285_FIXUP_SPEAKER2_TO_DAC1, .name = "alc285-speaker2-to-dac1"},
>   	{.id = ALC280_FIXUP_HP_HEADSET_MIC, .name = "alc280-hp-headset"},
>   	{.id = ALC221_FIXUP_HP_FRONT_MIC, .name = "alc221-hp-mic"},
> 


-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

RE: [External] Re: [PATCH v2 2/2] ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

[Mark Pearson]

> -----Original Message-----
> From: Alsa-devel <alsa-devel-bounces@alsa-project.org> On Behalf Of
> Jaroslav Kysela
> Sent: Friday, July 3, 2020 6:34 AM
> 
> Dne 03. 07. 20 v 10:00 Benjamin Poirier napsal(a):
> > As a result of commit d2cd795c4ece ("ALSA: hda - fixup for the bass
> > speaker on Lenovo Carbon X1 7th gen"), the sound output level on my machine, an
> > X1 Carbon 7th gen, was reduced to ~65% of its previous level when playing
> > certain sounds. [1]
> >
<snip>
> 
> Thank you for this work. Perhaps, Takashi will have some comments to
> improve this quirk.
Seconded - thank you!

<snip>
> 
> > It is possible that the X1 Carbon 8th gen would benefit from the same
> > changes but I don't have a device to test that. Fixups are reordered so
> > that the devices for 7th & 8th gen can share the same chain after the first
> > fixup. The resulting chain is:
> 
> 8th gen hardware should be similar, so the new fixup should be applied to this
> hw, too.
> 
We'll do some testing here and confirm the fixes on the X1C7 and X1C8 (and Yoga)

Do let me know if there is any details Lenovo can provide that would help

Mark

Re: [External] Re: [PATCH v2 2/2] ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

[Jaroslav Kysela]

Dne 03. 07. 20 v 12:59 Mark Pearson napsal(a):
> 
>> -----Original Message-----
>> From: Alsa-devel <alsa-devel-bounces@alsa-project.org> On Behalf Of
>> Jaroslav Kysela
>> Sent: Friday, July 3, 2020 6:34 AM
>>
>> Dne 03. 07. 20 v 10:00 Benjamin Poirier napsal(a):
>>> As a result of commit d2cd795c4ece ("ALSA: hda - fixup for the bass
>>> speaker on Lenovo Carbon X1 7th gen"), the sound output level on my machine, an
>>> X1 Carbon 7th gen, was reduced to ~65% of its previous level when playing
>>> certain sounds. [1]
>>>
> <snip>
>>
>> Thank you for this work. Perhaps, Takashi will have some comments to
>> improve this quirk.
> Seconded - thank you!
> 
> <snip>
>>
>>> It is possible that the X1 Carbon 8th gen would benefit from the same
>>> changes but I don't have a device to test that. Fixups are reordered so
>>> that the devices for 7th & 8th gen can share the same chain after the first
>>> fixup. The resulting chain is:
>>
>> 8th gen hardware should be similar, so the new fixup should be applied to this
>> hw, too.
>>
> We'll do some testing here and confirm the fixes on the X1C7 and X1C8 (and Yoga)
> 
> Do let me know if there is any details Lenovo can provide that would help

The functionality of this patch is same like the hda-verb command is executed 
with the current kernel (run as root):

hda-verb /dev/snd/hwC0D0 0x17 SET_CONNECT_SEL 1

You can control tweeters with 'Speaker' volume control.
And headphones and bass speakers with the 'Headphone' volume control.

						Jaroslav


> 
> Mark
> 


-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [External] Re: [PATCH v2 2/2] ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

[Benjamin Poirier]

On 2020-07-03 14:43 +0200, Jaroslav Kysela wrote:
> Dne 03. 07. 20 v 12:59 Mark Pearson napsal(a):
> > 
> > > -----Original Message-----
> > > From: Alsa-devel <alsa-devel-bounces@alsa-project.org> On Behalf Of
> > > Jaroslav Kysela
> > > Sent: Friday, July 3, 2020 6:34 AM
> > > 
> > > Dne 03. 07. 20 v 10:00 Benjamin Poirier napsal(a):
> > > > As a result of commit d2cd795c4ece ("ALSA: hda - fixup for the bass
> > > > speaker on Lenovo Carbon X1 7th gen"), the sound output level on my machine, an
> > > > X1 Carbon 7th gen, was reduced to ~65% of its previous level when playing
> > > > certain sounds. [1]
> > > > 
> > <snip>
> > > 
> > > Thank you for this work. Perhaps, Takashi will have some comments to
> > > improve this quirk.
> > Seconded - thank you!
> > 
> > <snip>
> > > 
> > > > It is possible that the X1 Carbon 8th gen would benefit from the same
> > > > changes but I don't have a device to test that. Fixups are reordered so
> > > > that the devices for 7th & 8th gen can share the same chain after the first
> > > > fixup. The resulting chain is:
> > > 
> > > 8th gen hardware should be similar, so the new fixup should be applied to this
> > > hw, too.
> > > 
> > We'll do some testing here and confirm the fixes on the X1C7 and X1C8 (and Yoga)
> > 
> > Do let me know if there is any details Lenovo can provide that would help
> 
> The functionality of this patch is same like the hda-verb command is
> executed with the current kernel (run as root):
> 
> hda-verb /dev/snd/hwC0D0 0x17 SET_CONNECT_SEL 1
> 
> You can control tweeters with 'Speaker' volume control.
> And headphones and bass speakers with the 'Headphone' volume control.

For other X1 users who might stumble upon this discussion; practical
usage of the hda-verb trick requires a way to apply it automatically[1]
and changes to the ucm profile[2].

[1] https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3345062
[2] https://gist.github.com/hamidzr/dd81e429dc86f4327ded7a2030e7d7d9#gistcomment-3335517

I've added more information here:
https://github.com/gobenji/thinkpad-x1-gen7-sound/tree/c5ebc9166e955934dab03d6c4fb5928cf28496b0#alternative-2---runtime-reconfiguration

Re: [PATCH v2 2/2] ALSA: hda/realtek - Replace Lenovo Thinkpad X1 Carbon 7th quirk

[Benjamin Poirier]

On 2020-07-03 12:33 +0200, Jaroslav Kysela wrote:
[...]

> > @@ -7559,8 +7606,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", ALC295_FIXUP_TPX17_DUAL_SPEAKERS),
> > +	SND_PCI_QUIRK(0x17aa, 0x22be, "Thinkpad X1 Carbon 8th", ALC285_FIXUP_SPEAKER2_TO_DAC1),
> 
> It will cause regression with the old UCM configs.

Yes, good point. I'll try to find a way that doesn't need ucm changes.

> I would preper the manual
> model selection and switch this when things settle a bit in the user space.
> We definitely need a better way to control this volume separately for both
> tweeters and the bass speakers in the user space. The "Master" volume hack
> is far from the ideal solution.

[...]

> 
> > The volume control for PCM-OUT2 is renamed to reflect its dual effect.
> > This name is also used in a modified alsa UCM profile. [4]
> 
> The new name is not ideal. Perhaps, a mirror (joined) control might be
> better in this case. I mean to create both "Bass Speaker Playback Volume"
> and "Headphone Playback Volume" with the similar NID control.
> 
> Or create "Speaker Playback" controls with 4 channels (and joined "Headphone
> Playback" controls).

I'll look into that. As I'm working on this in my spare time, no
guarantee on how long it'll take.

Thanks for the review.