How to solve the speaker problem - xuwd1/lenovo-legion-slim7i-gen7-knowledges GitHub Wiki

How to solve the speaker problem

The problem that the speakers being unable to make any sound on Slim 7i IAH7 has been solved.

We do some quick review on this problem first:

1. The laptop has two cs35l41 amplifiers from Cirrus Logics (this dual cs35l41 config is called CSC3551) that drive the speaker:

  • Each cs35l41 is connected to PCH I2C_3 controller via I2C bus. On alderlake the I2C controllers are integrated into the PCH and is handled by the intel_lpss_pci driver. The two amplifiers have I2C address 0x40 and 0x41 respectively.
  • Each cs35l41 has three pins: (at least I guess so)
    • The first is the reset pin. Both of the two cs35l41s have their reset pin connected to PCH GPIO.
    • The second is the interrupt pin. Both of the two cs35l41s have it connected to APIC.
    • The third is a GPIO whose function is usually assigned VSPK_SWITCH in cs35l41_hda driver, and is sometimes also called spkid. I guess it is related to indexing/chip selecting.

2. For the two amps to work below has to be done:

  • Each cs35l41 device has to be instantiated as i2c device first. (or some spi device. For example the SteamDeck has three cs35l41s that are connected via spi) This instantiation is handled by the serial-multi-instantiate driver in the kernel.
  • After the i2c/spi device instantiation, the cs35l41_hda driver has to properly initialize the amp. More specifically, it has to read a bunch of configs for the amp from the CSC3551 ACPI _DSD method, apply the configs and reset the amp.
  • Before playback starts, correct firmware has to be loaded for the cs35l41 (and yes, this thing has firmware!). The loading of the firmware is done by cs35l41_hda driver registering a playback hook on the hda driver first and later the hda driver calling the hook function at its initialization time.

3. Why the amps fail to work on Slim 7i gen7

  • It is because none of the above would work properly. LOL.
  • In the current kernel, the serial-multi-instantiate driver always assume the cs35l41 has its interrupt pin connected to PCH GPIO as this really is a common case. However Lenovo engineers thought that it would be fantastic if the interrupt pin were connected to the APIC, so the i2c device instantiation would just fail, being unable to acquire IRQ resource.
  • So quite naturally the cs35l41_hda driver would never do probing without the serial-multi-instantiate driver asking it to do so. However even if the serial-multi-instantiate driver were patched to work the cs35l41_hda driver would still fail since Lenovo did not provide the CSC3551 _DSD method in the ACPI table.
  • At last even after patching for the above two problems, the hda driver would never run the playback hook for loading firmware since there is no corresponding quirk entry in the patch_realtek.c file.

How to solve the problem

Sadly, the only way to address the problem is by patching the kernel. The kernel patch has two parts:

Part 1: patching the serial-multi-instantiate driver

Please note that this patch has been accepted by the maintainer and would show up in mainline kernel code soon.

 .../platform/x86/serial-multi-instantiate.c   | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index f3dcbdd72fec..2c2abf69f049 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -21,6 +21,7 @@
 #define IRQ_RESOURCE_NONE	0
 #define IRQ_RESOURCE_GPIO	1
 #define IRQ_RESOURCE_APIC	2
+#define IRQ_RESOURCE_AUTO   3
 
 enum smi_bus_type {
 	SMI_I2C,
@@ -52,6 +53,18 @@ static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
 	int ret;
 
 	switch (inst->flags & IRQ_RESOURCE_TYPE) {
+	case IRQ_RESOURCE_AUTO:
+		ret = acpi_dev_gpio_irq_get(adev, inst->irq_idx);
+		if (ret > 0) {
+			dev_dbg(&pdev->dev, "Using gpio irq\n");
+			break;
+		}
+		ret = platform_get_irq(pdev, inst->irq_idx);
+		if (ret > 0) {
+			dev_dbg(&pdev->dev, "Using platform irq\n");
+			break;
+		}
+		break;
 	case IRQ_RESOURCE_GPIO:
 		ret = acpi_dev_gpio_irq_get(adev, inst->irq_idx);
 		break;
@@ -307,10 +320,10 @@ static const struct smi_node int3515_data = {
 
 static const struct smi_node cs35l41_hda = {
 	.instances = {
-		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
-		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
-		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
-		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
+		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
 		{}
 	},
 	.bus_type = SMI_AUTO_DETECT,

Part 2: patching the cs35l41_hda and the hda driver

Please note that the maintainers from Cirrus Logics rejected this patch and said that a new patch for the problem which has taken some cleaner approach would come in few weeks. I think we could expect their work to completely solve the problem and then the only thing we have to do is updating our kernel.

 sound/pci/hda/cs35l41_hda.c | 160 ++++++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)

diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index ce5faa620517..d957458dd4e6 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -1211,6 +1211,159 @@ static int cs35l41_get_speaker_id(struct device *dev, int amp_index,
 	return speaker_id;
 }
 
+#define CS35L41_FIXUP_CFG_MAX_DEVICES 4
+
+struct cs35l41_fixup_cfg {
+	unsigned short vender;
+	unsigned short device;
+	unsigned int num_device;  /* The num of cs35l41 instances */
+	/* cs35l41 instance ids, can be i2c index or spi index */
+	int ids[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	unsigned int reset_gpio_idx[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	enum gpiod_flags reset_gpio_flags[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	int spkid_gpio_idx[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	unsigned int spk_pos[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	enum cs35l41_hda_gpio_function gpio1_func[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	enum cs35l41_hda_gpio_function gpio2_func[CS35L41_FIXUP_CFG_MAX_DEVICES];
+	enum cs35l41_boost_type bst_type[CS35L41_FIXUP_CFG_MAX_DEVICES];
+};
+
+static const struct cs35l41_fixup_cfg cs35l41_fixup_cfgtbl[] = {
+	{ // Lenovo Legion Slim 7i 16IAH7
+	.vender = 0x17aa,
+	.device = 0x386e,
+	.num_device = 2,
+	.ids = {0x40, 0x41},
+	.reset_gpio_idx = {0, 0},
+	.reset_gpio_flags = {GPIOD_OUT_LOW, GPIOD_OUT_LOW},
+	.spkid_gpio_idx = {1, 1},
+	.spk_pos = {0, 1},
+	.gpio1_func = {CS35l41_VSPK_SWITCH, CS35l41_VSPK_SWITCH},
+	.gpio2_func = {CS35L41_INTERRUPT, CS35L41_NOT_USED},
+	.bst_type = {CS35L41_EXT_BOOST, CS35L41_EXT_BOOST}
+	},
+	{ // Lenovo Legion Slim 7i 16IAH7 type2
+	.vender = 0x17aa,
+	.device = 0x3803,
+	.num_device = 2,
+	.ids = {0x40, 0x41},
+	.reset_gpio_idx = {0, 0},
+	.reset_gpio_flags = {GPIOD_OUT_LOW, GPIOD_OUT_LOW},
+	.spkid_gpio_idx = {1, 1},
+	.spk_pos = {0, 1},
+	.gpio1_func = {CS35l41_VSPK_SWITCH, CS35l41_VSPK_SWITCH},
+	.gpio2_func = {CS35L41_INTERRUPT, CS35L41_NOT_USED},
+	.bst_type = {CS35L41_EXT_BOOST, CS35L41_EXT_BOOST}
+	},
+	{ // Lenovo Legion 7i 16IAX7
+	.vender = 0x17aa,
+	.device = 0x3874,
+	.num_device = 2,
+	.ids = {0x40, 0x41},
+	.reset_gpio_idx = {0, 0},
+	.reset_gpio_flags = {GPIOD_OUT_LOW, GPIOD_OUT_LOW},
+	.spkid_gpio_idx = {1, 1},
+	.spk_pos = {0, 1},
+	.gpio1_func = {CS35l41_VSPK_SWITCH, CS35l41_VSPK_SWITCH},
+	.gpio2_func = {CS35L41_INTERRUPT, CS35L41_INTERRUPT},
+	.bst_type = {CS35L41_EXT_BOOST, CS35L41_EXT_BOOST}
+	},
+	{ // Lenovo Legion 7i 16IAX7 type 2
+	.vender = 0x17aa,
+	.device = 0x386f,
+	.num_device = 2,
+	.ids = {0x40, 0x41},
+	.reset_gpio_idx = {0, 0},
+	.reset_gpio_flags = {GPIOD_OUT_LOW, GPIOD_OUT_LOW},
+	.spkid_gpio_idx = {1, 1},
+	.spk_pos = {0, 1},
+	.gpio1_func = {CS35l41_VSPK_SWITCH, CS35l41_VSPK_SWITCH},
+	.gpio2_func = {CS35L41_INTERRUPT, CS35L41_INTERRUPT},
+	.bst_type = {CS35L41_EXT_BOOST, CS35L41_EXT_BOOST}
+	},
+	{ // Lenovo Legion Slim 7 16ARHA7
+	.vender = 0x17aa,
+	.device = 0x3877,
+	.num_device = 2,
+	.ids = {0x40, 0x41},
+	.reset_gpio_idx = {0, 0},
+	.reset_gpio_flags = {GPIOD_OUT_LOW, GPIOD_OUT_LOW},
+	.spkid_gpio_idx = {1, 1},
+	.spk_pos = {0, 1},
+	.gpio1_func = {CS35l41_VSPK_SWITCH, CS35l41_VSPK_SWITCH},
+	.gpio2_func = {CS35L41_INTERRUPT, CS35L41_INTERRUPT},
+	.bst_type = {CS35L41_EXT_BOOST, CS35L41_EXT_BOOST}
+	},
+	{} // terminator
+};
+
+static inline int cs35l41_fixup_get_index(const struct cs35l41_fixup_cfg *fixup, int cs35l41_addr)
+{
+	int i;
+
+	for (i = 0; i < fixup->num_device; i++) {
+		if (fixup->ids[i] == cs35l41_addr)
+			return i;
+	}
+	return -ENODEV;
+}
+
+static int apply_cs35l41_fixup_cfg(struct cs35l41_hda *cs35l41,
+				struct device *physdev,
+				int cs35l41_addr,
+				const struct cs35l41_fixup_cfg *fixup_tbl)
+{
+	const char *ssid;
+	unsigned int vendid;
+	unsigned int devid;
+	const struct cs35l41_fixup_cfg *cur_fixup;
+	struct cs35l41_hw_cfg *hw_cfg;
+	int cs35l41_index;
+	int ret;
+	int i;
+
+	ssid = cs35l41->acpi_subsystem_id;
+	ret = sscanf(ssid, "%04x%04x", &vendid, &devid);
+	if (ret != 2)
+		return -EINVAL;
+
+	hw_cfg = &cs35l41->hw_cfg;
+	for (cur_fixup = fixup_tbl; cur_fixup->vender; cur_fixup++) {
+		if (cur_fixup->vender == vendid && cur_fixup->device == devid) {
+			cs35l41_index = cs35l41_fixup_get_index(cur_fixup, cs35l41_addr);
+			if (cs35l41_index == -ENODEV)
+				return -ENODEV;
+			cs35l41->index = cs35l41_index;
+			cs35l41->reset_gpio = gpiod_get_index(
+				physdev,
+				NULL,
+				cur_fixup->reset_gpio_idx[cs35l41_index],
+				cur_fixup->reset_gpio_flags[cs35l41_index]
+				);
+			cs35l41->speaker_id = cs35l41_get_speaker_id(physdev,
+				cs35l41_index,
+				cur_fixup->num_device,
+				cur_fixup->spkid_gpio_idx[cs35l41_index]
+				);
+			hw_cfg->spk_pos = cur_fixup->spk_pos[cs35l41_index];
+			cs35l41->channel_index = 0;
+			for (i = 0; i < cs35l41->index; i++)
+				if (cur_fixup->spk_pos[i] == hw_cfg->spk_pos)
+					cs35l41->channel_index++;
+
+			hw_cfg->gpio1.func = cur_fixup->gpio1_func[cs35l41_index];
+			hw_cfg->gpio1.valid = true;
+			hw_cfg->gpio2.func = cur_fixup->gpio2_func[cs35l41_index];
+			hw_cfg->gpio2.valid = true;
+			hw_cfg->bst_type = cur_fixup->bst_type[cs35l41_index];
+			dev_dbg(physdev, "Fixup applied.\n");
+			break;
+		}
+	}
+	return 0;
+
+}
+
 /*
  * Device CLSA010(0/1) doesn't have _DSD so a gpiod_get by the label reset won't work.
  * And devices created by serial-multi-instantiate don't have their device struct
@@ -1221,6 +1374,7 @@ static int cs35l41_get_speaker_id(struct device *dev, int amp_index,
 static int cs35l41_no_acpi_dsd(struct cs35l41_hda *cs35l41, struct device *physdev, int id,
 			       const char *hid)
 {
+	int ret;
 	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
 
 	/* check I2C address to assign the index */
@@ -1243,7 +1397,13 @@ static int cs35l41_no_acpi_dsd(struct cs35l41_hda *cs35l41, struct device *physd
 		/*
 		 * Note: CLSA010(0/1) are special cases which use a slightly different design.
 		 * All other HIDs e.g. CSC3551 require valid ACPI _DSD properties to be supported.
+		 * However many OEMs hardcoded the configurations into their proprietary software
+		 * thus leaving our Linux installation with no speaker sound at all while we see
+		 * no hope those OEMs would fix it. So we apply a ssid specific fixup to fix it.
 		 */
+		if (apply_cs35l41_fixup_cfg(cs35l41, physdev, id, cs35l41_fixup_cfgtbl) == 0)
+			return 0;
+
 		dev_err(cs35l41->dev, "Error: ACPI _DSD Properties are missing for HID %s.\n", hid);
 		hw_cfg->valid = false;
 		hw_cfg->gpio1.valid = false;
 sound/pci/hda/patch_realtek.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index e2f8b608de82..cc10bb8b75d1 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -9831,6 +9831,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x31af, "ThinkCentre Station", ALC623_FIXUP_LENOVO_THINKSTATION_P340),
 	SND_PCI_QUIRK(0x17aa, 0x3801, "Lenovo Yoga9 14IAP7", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
 	SND_PCI_QUIRK(0x17aa, 0x3802, "Lenovo Yoga DuetITL 2021", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
+	SND_PCI_QUIRK(0x17aa, 0x3803, "Legion Slim 7i 16IAH7", ALC287_FIXUP_CS35L41_I2C_2),
 	SND_PCI_QUIRK(0x17aa, 0x3813, "Legion 7i 15IMHG05", ALC287_FIXUP_LEGION_15IMHG05_SPEAKERS),
 	SND_PCI_QUIRK(0x17aa, 0x3818, "Lenovo C940 / Yoga Duet 7", ALC298_FIXUP_LENOVO_C940_DUET7),
 	SND_PCI_QUIRK(0x17aa, 0x3819, "Lenovo 13s Gen2 ITL", ALC287_FIXUP_13S_GEN2_SPEAKERS),
@@ -9846,6 +9847,10 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x3853, "Lenovo Yoga 7 15ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
 	SND_PCI_QUIRK(0x17aa, 0x3855, "Legion 7 16ITHG6", ALC287_FIXUP_LEGION_16ITHG6),
 	SND_PCI_QUIRK(0x17aa, 0x3869, "Lenovo Yoga7 14IAL7", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
+	SND_PCI_QUIRK(0x17aa, 0x386e, "Legion Slim 7i 16IAH7", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x386f, "Legion 7i 16IAX7", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x3874, "Legion 7i 16IAX7", ALC287_FIXUP_CS35L41_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x3877, "Legion 7 16ARHA7", ALC287_FIXUP_CS35L41_I2C_2),
 	SND_PCI_QUIRK(0x17aa, 0x3902, "Lenovo E50-80", ALC269_FIXUP_DMIC_THINKPAD_ACPI),
 	SND_PCI_QUIRK(0x17aa, 0x3977, "IdeaPad S210", ALC283_FIXUP_INT_MIC),
 	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo B50-70", ALC269_FIXUP_DMIC_THINKPAD_ACPI),

Some other things to notice

After applying the patches you should have your laptop speakers working. Few things to notice:

  1. As the patches shows, these patches could also be used for Legion Slim 7 16ARHA7 and Legion 7i 16IAX7.
  2. You might find that the speakers are somehow quiet (that is, the volume is quite low). This is because that the cs35l41 amp actually has two boosting modes, namely internal mode and external mode. The patches above only uses external mode, which in fact means that the audio signal is not boosted, thus giving rise to the problem. The internal boosting mode is tricky that it also requires a series of configs to work, while Cirrus people stated that an incorrect internal boost config has greater than 0 probability to permanently damage the speaker, and getting a correct internal boosting config is out of my ability.
⚠️ **GitHub.com Fallback** ⚠️