Message-ID: <2165924.irdbgypaU6@nobara-desktop-pc>
Date:   Mon, 23 Oct 2023 00:38:42 -0700
From:   Jonathan LoBue <jlobue10@...il.com>
To:     Stefan Binding <sbinding@...nsource.cirrus.com>,
        Huayu Zhang <leviz.kernel.dev@...il.com>
Cc:     Luke Jones <luke@...nes.dev>, Takashi Iwai <tiwai@...e.de>,
        tiwai@...e.com, james.schulman@...rus.com, david.rhodes@...rus.com,
        rf@...nsource.cirrus.com, linux-kernel@...r.kernel.org,
        patches@...nsource.cirrus.com
Subject: Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

On Sunday, October 8, 2023 10:19:18 AM PDT Huayu Zhang wrote:
> Hi Stefan and all,
> 
> Thanks for examine my email. I'm just interesting in Linux kernel
> development and met sound issue with my `21J8 Lenovo ThinkBook 16p Gen
> 4`.
> Sorry for not familiar with the email process if any.
> 
> I wrote following changes based on some discovery and the downside
> speakers (bass) seems begin to work. But the volumn keys actually
> adjusting the frequence (I suppose). (Louder for high freq, and lower
> volumn for low freq)
> 
> Wondering if any suggestions on the patch or any plan for officially
> supporting of Lenovo ThinkBook 16p Gen 4. ^_^
> I'll also provide the alsa-info and dmesg output below. Thanks a lot~
> 
> Patch:
> 
> From 124161547483109cbb491a8e39d1b5ef0973cd80 Mon Sep 17 00:00:00 2001
> From: Huayu Zhang <zhanghuayu.dev@...il.com>
> Date: Mon, 9 Oct 2023 00:59:56 +0800
> Subject: [PATCH] thinkbook 16p gen4 sound fix
> 
> ---
> sound/pci/hda/cs35l41_hda_property.c | 41 ++++++++++++++++++++++++++++
> sound/pci/hda/patch_realtek.c | 1 +
> 2 files changed, 42 insertions(+)
> 
> diff --git a/sound/pci/hda/cs35l41_hda_property.c
> b/sound/pci/hda/cs35l41_hda_property.c
> index b62a4e6968e2..af359fbeb671 100644
> --- a/sound/pci/hda/cs35l41_hda_property.c
> +++ b/sound/pci/hda/cs35l41_hda_property.c
> @@ -74,6 +74,46 @@ static int hp_vision_acpi_fix(struct cs35l41_hda
> *cs35l41, struct device *physde
> return 0;
> }
> +static int lenovo_thinkbook16pgen4_no_acpi(struct cs35l41_hda
> *cs35l41, struct device *physdev, int id,
> + const char *hid)
> +{
> + struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
> +
> + dev_info(cs35l41->dev, "Adding DSD properties for %s\n",
> cs35l41->acpi_subsystem_id);
> +
> + printk("CSC3551: id == 0x%x\n", id);
> +
> + // cirrus,dev-index
> + cs35l41->index = id == 0x40 ? 0 : 1;
> + cs35l41->channel_index = 0;
> +
> + // cs35l41->reset_gpio = gpiod_get_index(physdev, NULL,
> cs35l41->index, GPIOD_OUT_LOW);
> + cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH);
> + printk("CS3551: reset_gpio == 0x%x\n", cs35l41->reset_gpio);
> +
> + // cs35l41->speaker_id = cs35l41_get_speaker_id(physdev,
> cs35l41->index, nval, -1);
> + cs35l41->speaker_id = cs35l41_get_speaker_id(physdev, 0, 0, 2);
> +
> + // cirrus,speaker-position
> + hw_cfg->spk_pos = cs35l41->index;
> +
> + // cirrus,gpio1-func
> + hw_cfg->gpio1.func = CS35l41_VSPK_SWITCH;
> + hw_cfg->gpio1.valid = true;
> +
> + // cirrus,gpio2-func
> + hw_cfg->gpio2.func = CS35L41_INTERRUPT;
> + hw_cfg->gpio2.valid = true;
> +
> + hw_cfg->bst_type = CS35L41_EXT_BOOST;
> + hw_cfg->valid = true;
> +
> + put_device(physdev);
> + printk("CSC3551: Done.\n");
> +
> + return 0;
> +}
> +
> struct cs35l41_prop_model {
> const char *hid;
> const char *ssid;
> @@ -85,6 +125,7 @@ static const struct cs35l41_prop_model
> cs35l41_prop_model_table[] = {
> { "CLSA0100", NULL, lenovo_legion_no_acpi },
> { "CLSA0101", NULL, lenovo_legion_no_acpi },
> { "CSC3551", "103C89C6", hp_vision_acpi_fix },
> + { "CSC3551", "17AA38A9", lenovo_thinkbook16pgen4_no_acpi },
> {}
> };
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 751783f3a15c..fc884fdcec5f 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -10031,6 +10031,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[]
> = { SND_PCI_QUIRK(0x17aa, 0x3886, "Y780 VECO DUAL",
> ALC287_FIXUP_TAS2781_I2C), SND_PCI_QUIRK(0x17aa, 0x38a7, "Y780P AMD YG
> dual", ALC287_FIXUP_TAS2781_I2C), SND_PCI_QUIRK(0x17aa, 0x38a8, "Y780P AMD
> VECO dual", ALC287_FIXUP_TAS2781_I2C), + SND_PCI_QUIRK(0x17aa, 0x38a9,
> "Lenovo ThinkBook 16p Gen 4",
> ALC287_FIXUP_CS35L41_I2C_2),
> SND_PCI_QUIRK(0x17aa, 0x38ba, "Yoga S780-14.5 Air AMD quad YC",
> ALC287_FIXUP_TAS2781_I2C),
> SND_PCI_QUIRK(0x17aa, 0x38bb, "Yoga S780-14.5 Air AMD quad AAC",
> ALC287_FIXUP_TAS2781_I2C),
> SND_PCI_QUIRK(0x17aa, 0x38be, "Yoga S980-14.5 proX YC Dual",
> ALC287_FIXUP_TAS2781_I2C),
> 
> > On 03/10/2023 15:45, Luke Jones wrote:
> > > On Thu, Aug 24 2023 at 08:31:06 AM +12:00:00, Luke Jones
> > > 
> > > <luke@...nes.dev> wrote:
> > >>> The second member variable in cs35l41_prop_model_table is the SSID to
> > >>> match against.
> > >>> The Lenovo laptops in the initial patch didn't have different SSIDs so
> > >>> the entry was set to NULL for those.
> > >>> Future entries using CSC3551 MUST always have an accompanying SSID
> > >>> with this entry.
> > >>> Takashi was correct, the implementation is intended to also be used to
> > >>> patch incorrect DSD.
> > >>> 
> > >>> We have a potential solution to workaround the SPI cs-gpios issue
> > >>> inside here,
> > >>> though the drawback for that is that it only works for laptops with 2
> > >>> SPI amps.
> > >> 
> > >> Can you provide me this so I can test? I have laptops with SPI 2 and
> > >> 4 speaker setups.
> > > 
> > > Hi Stefan,
> > > 
> > > Do you have any further information about the status of this in
> > > regards to the 2023 laptops?
> > 
> > Hi,
> > 
> > We are currently working on adding support for 2023 ASUS laptops without
> > _DSD.
> > 
> > >>> I also took a look at the function for applying DSD properties for the
> > >>> 2023 ROG laptops.
> > >>> Unfortunately the one-size-fits-all approach will not work, some of
> > >>> these laptops are i2c
> > >>> and some are SPI, meaning the GPIO indexes are different for different
> > >>> laptops.
> > >> 
> > >> Do you mean "spk-id-gpios"? For all the laptops I know of this seems
> > >> to be
> > >> Package () { "spk-id-gpios", Package () {
> > >> 
> > >>    SPK1, 0x02, Zero, Zero,
> > >>    SPK1, 0x02, Zero, Zero
> > >> 
> > >> } },
> > >> 
> > >> There is one laptop where it is One not 0x02 (the GA402N)
> > >> 
> > >>> Some of the laptops do no have Speaker IDs.
> > >>> Also, no laptop other than the 2 I added already should ever use
> > >>> CS35L41_EXT_BOOST_NO_VSPK_SWITCH (in fact I believe all these laptops
> > >>> are internal
> > >>> boost anyway).
> > >> 
> > >> Grazie.
> > >> 
> > >>> We are currently working internally on adding support for the 2023 ROG
> > >>> laptops, so we
> > >>> ask for you guys to hold off on trying to upstream support for these
> > >>> laptops.
> > >> 
> > >> Ah great. Thank you. I apologise for trying to rush things, but I do
> > >> have a discord server of over 4000 people, many of whom have laptops
> > >> with cirrus amps.
> > >> 
> > >> For now I'm including a patch in my kernel builds with this mapping:
> > >> 
> > >> const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
> > >> 
> > >>     { "CLSA0100", NULL, lenovo_legion_no_acpi },
> > >>     { "CLSA0101", NULL, lenovo_legion_no_acpi },
> > >>     { "CSC3551", "10431433", asus_rog_2023_no_acpi }, // ASUS GS650P
> > >> 
> > >> - i2c
> > >> 
> > >>     { "CSC3551", "10431463", asus_rog_2023_no_acpi }, // ASUS GA402X
> > >> 
> > >> - i2c
> > >> 
> > >>     { "CSC3551", "10431473", asus_rog_2023_no_acpi }, // ASUS GU604V
> > >> 
> > >> - spi
> > >> 
> > >>     { "CSC3551", "10431483", asus_rog_2023_no_acpi }, // ASUS GU603V
> > >> 
> > >> - spi
> > >> 
> > >>     { "CSC3551", "10431493", asus_rog_2023_no_acpi }, // ASUS GV601V
> > >> 
> > >> - spi
> > >> 
> > >>     { "CSC3551", "10431573", asus_rog_2023_no_acpi }, // ASUS GZ301V
> > >> 
> > >> - spi
> > >> 
> > >>     { "CSC3551", "104317F3", asus_rog_2023_no_acpi }, // ASUS ROG
> > >> 
> > >> ALLY - i2c
> > >> 
> > >>     { "CSC3551", "10431B93", asus_rog_2023_no_acpi }, // ASUS G614J -
> > >> 
> > >> spi
> > >> 
> > >>     { "CSC3551", "10431CAF", asus_rog_2023_no_acpi }, // ASUS G634J -
> > >> 
> > >> spi
> > >> 
> > >>     { "CSC3551", "10431C9F", asus_rog_2023_no_acpi }, // ASUS G614JI
> > >> 
> > >> -spi
> > >> 
> > >>     { "CSC3551", "10431D1F", asus_rog_2023_no_acpi }, // ASUS G713P -
> > >> 
> > >> i2c
> > >> 
> > >>     { "CSC3551", "10431F1F", asus_rog_2023_no_acpi }, // ASUS H7604JV
> > >> 
> > >> - spi
> > >> 
> > >>     {}
> > >> 
> > >> };
> > >> 
> > >> These are the machines I have verified the gpios and such for.
> > > 
> > > I have a new version of this patch with all listed models confirmed as
> > > working, and with slightly different settings for some. The only thing
> > > missing in a solution to the gpio-cs issue.
> > > 
> > > Can you please provide an update on where you are with ASUS support in
> > > particular so that I may consider if it is worth my time submitting
> > > the updated patch.
> > 
> > We would prefer for you to wait, as we are looking to push up this
> > support in the coming weeks.
> > 
> > >> Cheers,
> > >> Luke.

Stefan and all,

Thanks for the hard work getting the DSD properties sorted and installed on 
various BIOSes. The one that I'm most concerned with and what got me 
interested in this patching mechanism in the first place is the ASUS ROG ALLY. 
>From a DSD dump, it's clear that the latest ROG ALLY BIOS (330) has the proper 
audio DSD properties. Unfortunately, certain things on many people's setups 
across numerous ROG ALLYs (using various Linux distros) have broken with the 
330 BIOS. This led many of us to revert back to the previous BIOS, 323. 
Knowing the proper internal boost corresponding values now (cap, inductor, and 
peak current), it's ideal to push these properties out for those people who 
prefer to stay on an older BIOS for the ROG ALLY. I've developed a small patch 
that I've used on a custom 6.5.8 Nobara kernel. Line numbers and whatnot may 
be different according to latest on official Linux git, but I'd still like to 
share what I have. I think it is a good addition. There's basically a small 
logic check versus BIOS number. If the BIOS is 330 or greater, then the 
function to override DSD properties is exited. If the BIOS has a smaller 
number than 330, then the DSD overrides are applied. Here is my patch.

diff --git a/sound/pci/hda/cs35l41_hda_property.c b/sound/pci/hda/
cs35l41_hda_property.c
index b39f944..b67c636 100644
--- a/sound/pci/hda/cs35l41_hda_property.c
+++ b/sound/pci/hda/cs35l41_hda_property.c
@@ -6,7 +6,9 @@
 //
 // Author: Stefan Binding <sbinding@...nsource.cirrus.com>
 
+#include <linux/dmi.h>
 #include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
 #include <linux/string.h>
 #include "cs35l41_hda_property.h"
 
@@ -78,6 +80,40 @@ static int asus_rog_2023_spkr_id2(struct cs35l41_hda 
*cs35l41, struct device *ph
 	return 0;
 }
 
+static int asus_rog_2023_ally_fix(struct cs35l41_hda *cs35l41, struct device 
*physdev, int id,
+				const char *hid)
+{
+	const char *rog_ally_bios_ver = 
dmi_get_system_info(DMI_BIOS_VERSION);
+	const char *rog_ally_bios_num = rog_ally_bios_ver + 6; // Dropping 
the RC71L. part before the number
+	int rog_ally_bios_int;
+	kstrtoint(rog_ally_bios_num, 10, &rog_ally_bios_int);
+	if(rog_ally_bios_int >= 330){
+		printk(KERN_INFO "DSD properties exist in the %d 
BIOS\n", rog_ally_bios_int);
+		return -ENOENT;
+	}
+
+	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
+
+	dev_info(cs35l41->dev, "Adding DSD properties for %s\n", cs35l41-
>acpi_subsystem_id);
+
+	cs35l41->index = id == 0x40 ? 0 : 1;
+	cs35l41->channel_index = 0;
+	cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, 
GPIOD_OUT_HIGH);
+	cs35l41->speaker_id = cs35l41_get_speaker_id(physdev, 0, 0, 2);
+	hw_cfg->spk_pos = cs35l41->index;
+	hw_cfg->gpio1.func = CS35L41_NOT_USED;
+	hw_cfg->gpio1.valid = true;
+	hw_cfg->gpio2.func = CS35L41_INTERRUPT;
+	hw_cfg->gpio2.valid = true;
+	hw_cfg->bst_type = CS35L41_INT_BOOST;
+	hw_cfg->bst_ind = 1000; /* 1,000nH Inductance value */
+	hw_cfg->bst_ipk = 4500; /* 4,500mA peak current */
+	hw_cfg->bst_cap = 24; /* 24 microFarad cap value */
+	hw_cfg->valid = true;
+
+	return 0;
+}
+
 struct cs35l41_prop_model {
 	const char *hid;
 	const char *ssid;
@@ -94,7 +130,7 @@ const struct cs35l41_prop_model cs35l41_prop_model_table[] 
= {
 	{ "CSC3551", "10431483", asus_rog_2023_spkr_id2 }, // ASUS GU603V - 
spi, reset gpio 1
 	{ "CSC3551", "10431493", asus_rog_2023_spkr_id2 }, // ASUS GV601V - 
spi, reset gpio 1
 	{ "CSC3551", "10431573", asus_rog_2023_spkr_id2 }, // ASUS GZ301V - 
spi, reset gpio 0
-	{ "CSC3551", "104317F3", asus_rog_2023_spkr_id2 }, // ASUS ROG ALLY 
- i2c
+	{ "CSC3551", "104317F3", asus_rog_2023_ally_fix }, // ASUS ROG ALLY 
- i2c
 	{ "CSC3551", "10431B93", asus_rog_2023_spkr_id2 }, // ASUS G614J - 
spi, reset gpio 0
 	{ "CSC3551", "10431CAF", asus_rog_2023_spkr_id2 }, // ASUS G634J - 
spi, reset gpio 0
 	{ "CSC3551", "10431C9F", asus_rog_2023_spkr_id2 }, // ASUS G614JI -
spi, reset gpio 0

This patch is what my changes are after unpacking the kernel source RPM for a 
Nobara kernel and applying a version of Luke's patches.

I think a similar BIOS version checking logic may come in handy again in the 
future. Thanks.

Best Regards,
Jon LoBue

Powered by blists - more mailing lists