Messages in this thread | | | From | Jonathan LoBue <> | Subject | Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD | Date | Mon, 23 Oct 2023 09:35:55 -0700 |
| |
On Monday, October 23, 2023 12:38:42 AM PDT Jonathan LoBue wrote: > 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@gmail.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@ljones.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@opensource.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
One more minor change to my proposed patch is to replace the "return -ENOENT;" line in the patch routine not applying and exiting to "return 0; //Patch not applicable. Exiting successfully" since it's actually a successful exit if the BIOS version is detected as 330 or greater and no patch is applied. Thanks.
Best Regards, Jon LoBue
| |