[PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Luke D. Jones]

Support adding the missing DSD properties required  for ASUS ROG 2023
laptops and other ASUS laptops to properly utilise the cs35l41.

This support includes both I2C and SPI connected amps.

The SPI connected amps may be required to use an external DSD patch
to fix or add the "cs-gpios" property.

Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
Co-developed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 sound/pci/hda/cs35l41_hda_property.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/sound/pci/hda/cs35l41_hda_property.c b/sound/pci/hda/cs35l41_hda_property.c
index 673f23257a09..69879ab57918 100644
--- a/sound/pci/hda/cs35l41_hda_property.c
+++ b/sound/pci/hda/cs35l41_hda_property.c
@@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct cs35l41_hda *cs35l41, struct device *phy
 	return 0;
 }
 
+/*
+ * The CSC3551 is used in almost the entire ASUS ROG laptop range in 2023, this is likely to
+ * also include many non ROG labelled laptops. It is also used with either I2C connection or
+ * SPI connection. The SPI connected versions may be missing a chip select GPIO and require
+ * an DSD table patch.
+ */
+static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41, struct device *physdev, int id,
+				const char *hid)
+{
+	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
+
+	/* check SPI or I2C address to assign the index */
+	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
+	hw_cfg->gpio2.valid = true;
+	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
+	hw_cfg->valid = true;
+
+	return 0;
+}
+
 struct cs35l41_prop_model {
 	const char *hid;
 	const char *ssid;
@@ -53,6 +78,7 @@ struct cs35l41_prop_model {
 const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
 	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
 	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
+	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
 	{}
 };
 
-- 
2.41.0

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Luke Jones]

On Wed, Aug 23 2023 at 13:10:08 +12:00:00, Luke D. Jones 
<luke@ljones.dev> wrote:
> Support adding the missing DSD properties required  for ASUS ROG 2023
> laptops and other ASUS laptops to properly utilise the cs35l41.
> 
> This support includes both I2C and SPI connected amps.
> 
> The SPI connected amps may be required to use an external DSD patch
> to fix or add the "cs-gpios" property.
> 
> Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
> Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> Co-developed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  sound/pci/hda/cs35l41_hda_property.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/sound/pci/hda/cs35l41_hda_property.c 
> b/sound/pci/hda/cs35l41_hda_property.c
> index 673f23257a09..69879ab57918 100644
> --- a/sound/pci/hda/cs35l41_hda_property.c
> +++ b/sound/pci/hda/cs35l41_hda_property.c
> @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct 
> cs35l41_hda *cs35l41, struct device *phy
>  	return 0;
>  }
> 
> +/*
> + * The CSC3551 is used in almost the entire ASUS ROG laptop range in 
> 2023, this is likely to
> + * also include many non ROG labelled laptops. It is also used with 
> either I2C connection or
> + * SPI connection. The SPI connected versions may be missing a chip 
> select GPIO and require
> + * an DSD table patch.
> + */
> +static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41, struct 
> device *physdev, int id,
> +				const char *hid)
> +{
> +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
> +
> +	/* check SPI or I2C address to assign the index */
> +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
> +	hw_cfg->gpio2.valid = true;
> +	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
> +	hw_cfg->valid = true;
> +
> +	return 0;
> +}
> +
>  struct cs35l41_prop_model {
>  	const char *hid;
>  	const char *ssid;
> @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
>  const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
>  	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
>  	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
> +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
>  	{}
>  };
> 
> --
> 2.41.0

Although this does work for SPI connected amps, it still requires the 
chipselect DSD patch:

DefinitionBlock ("", "SSDT", 1, "CUSTOM", "CSC3551", 0x00000001)
{
    External (_SB_.PC00.SPI3, DeviceObj)
    External (_SB_.PC00.SPI3.SPK1, DeviceObj)

    Scope (_SB.PC00.SPI3)
    {
        Name (_DSD, Package ()
        {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package ()
            {
                Package () { "cs-gpios", Package () { Zero, SPK1, Zero, 
Zero, Zero } }
            }
        })
    }
}

I am unsure what to do about this and any advice would be appreciated.

Cheers,
Luke.

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Takashi Iwai]

On Wed, 23 Aug 2023 03:10:08 +0200,
Luke D. Jones wrote:
> 
> Support adding the missing DSD properties required  for ASUS ROG 2023
> laptops and other ASUS laptops to properly utilise the cs35l41.
> 
> This support includes both I2C and SPI connected amps.
> 
> The SPI connected amps may be required to use an external DSD patch
> to fix or add the "cs-gpios" property.
> 
> Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
> Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> Co-developed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  sound/pci/hda/cs35l41_hda_property.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/sound/pci/hda/cs35l41_hda_property.c b/sound/pci/hda/cs35l41_hda_property.c
> index 673f23257a09..69879ab57918 100644
> --- a/sound/pci/hda/cs35l41_hda_property.c
> +++ b/sound/pci/hda/cs35l41_hda_property.c
> @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct cs35l41_hda *cs35l41, struct device *phy
>  	return 0;
>  }
>  
> +/*
> + * The CSC3551 is used in almost the entire ASUS ROG laptop range in 2023, this is likely to
> + * also include many non ROG labelled laptops. It is also used with either I2C connection or
> + * SPI connection. The SPI connected versions may be missing a chip select GPIO and require
> + * an DSD table patch.
> + */
> +static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41, struct device *physdev, int id,
> +				const char *hid)
> +{
> +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
> +
> +	/* check SPI or I2C address to assign the index */
> +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
> +	hw_cfg->gpio2.valid = true;
> +	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
> +	hw_cfg->valid = true;
> +
> +	return 0;
> +}
> +
>  struct cs35l41_prop_model {
>  	const char *hid;
>  	const char *ssid;
> @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
>  const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
>  	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
>  	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
> +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },

I believe this breaks things badly.  cs35l41_add_dsd_properties() is
always called no matter whether _DSD is found or not.  So this will
override the setup of all machines with CSC3551 even if it has a
proper _DSD.

The existing entries of CLSA0100 and CLSA0101 are OK since
(supposedly) those never had _DSD.  The current code is a bit
misleading as if it's applicable easily, though.

That said, we have to apply the setup only conditionally for each
specific device.  One easy thing would be to move the function call
after _DSD check.  But, I *guess* that Stefan applied the function at
the top so that it may cover the all cases including incorrect _DSD
properties.

And, the question is how to be specific to each device.  This can be
messy, as the sub-codec driver is probed independently from Realtek
codec driver, hence you can't pass any flag from Realtek to CS driver
at the probe time.  In the end, we might need to keep another table of
IDs (either the same SSID or DMI) to distinguish which machine needs
which properties.


thanks,

Takashi

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Luke Jones]

On Wed, Aug 23 2023 at 08:24:51 +02:00:00, Takashi Iwai <tiwai@suse.de> 
wrote:
> On Wed, 23 Aug 2023 03:10:08 +0200,
> Luke D. Jones wrote:
>> 
>>  Support adding the missing DSD properties required  for ASUS ROG 
>> 2023
>>  laptops and other ASUS laptops to properly utilise the cs35l41.
>> 
>>  This support includes both I2C and SPI connected amps.
>> 
>>  The SPI connected amps may be required to use an external DSD patch
>>  to fix or add the "cs-gpios" property.
>> 
>>  Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
>>  Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
>>  Co-developed-by: Luke D. Jones <luke@ljones.dev>
>>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
>>  ---
>>   sound/pci/hda/cs35l41_hda_property.c | 26 
>> ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>> 
>>  diff --git a/sound/pci/hda/cs35l41_hda_property.c 
>> b/sound/pci/hda/cs35l41_hda_property.c
>>  index 673f23257a09..69879ab57918 100644
>>  --- a/sound/pci/hda/cs35l41_hda_property.c
>>  +++ b/sound/pci/hda/cs35l41_hda_property.c
>>  @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct 
>> cs35l41_hda *cs35l41, struct device *phy
>>   	return 0;
>>   }
>> 
>>  +/*
>>  + * The CSC3551 is used in almost the entire ASUS ROG laptop range 
>> in 2023, this is likely to
>>  + * also include many non ROG labelled laptops. It is also used 
>> with either I2C connection or
>>  + * SPI connection. The SPI connected versions may be missing a 
>> chip select GPIO and require
>>  + * an DSD table patch.
>>  + */
>>  +static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41, 
>> struct device *physdev, int id,
>>  +				const char *hid)
>>  +{
>>  +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
>>  +
>>  +	/* check SPI or I2C address to assign the index */
>>  +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
>>  +	hw_cfg->gpio2.valid = true;
>>  +	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
>>  +	hw_cfg->valid = true;
>>  +
>>  +	return 0;
>>  +}
>>  +
>>   struct cs35l41_prop_model {
>>   	const char *hid;
>>   	const char *ssid;
>>  @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
>>   const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
>>   	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
>>   	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
>>  +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
> 
> I believe this breaks things badly.  cs35l41_add_dsd_properties() is
> always called no matter whether _DSD is found or not.  So this will
> override the setup of all machines with CSC3551 even if it has a
> proper _DSD.

These are the entries I know of so far since they definitely had to be 
added and have a DSD patch:

- SPI_2
    - 0x1043, 0x1473, "ASUS GU604V"
    - 0x1043, 0x1483, "ASUS GU603V"
    - 0x1043, 0x1493, "ASUS GV601V"
    - 0x1043, 0x1573, "ASUS GZ301V"
    - 0x1043, 0x1c9f, "ASUS G614JI"
- SPI_4
    - 0x1043, 0x1caf, "ASUS G634JYR/JZR"
- I2C_2
    - 0x1043, 0x1d1f, "ASUS ROG Strix G17
    - 0x1043, 0x1463, "Asus GA402X"
    - 0x1043, 0x1433, "ASUS GX650P"
    - ROG ALLY

You can see the variants are V, J, X, and P. A grep through the DSL 
dumps I have collected reveals that these machines are all indeed 
missing DSD entries. These are a mix of I2C and SPI.

The patch I submitted was based on Stefan's work only, and tested on 3 
SPI machines plus 2 I2C machines with no issues except the SPI machines 
needing a chipselect DSD patch.

It's worth stating that the DSD patches people were using all followed 
the exact same template except for the SPI number, or speaker ID.

I'd wager it being a safe bet to assume that every one of the ASUS 
laptops this year using the CSC3551 will be missing the required DSD 
entries given the trend so far.

> The existing entries of CLSA0100 and CLSA0101 are OK since
> (supposedly) those never had _DSD.  The current code is a bit
> misleading as if it's applicable easily, though.
> 
> That said, we have to apply the setup only conditionally for each
> specific device.  One easy thing would be to move the function call
> after _DSD check.  But, I *guess* that Stefan applied the function at
> the top so that it may cover the all cases including incorrect _DSD
> properties.

Given the trend of what I've seen this seems like a reasonable 
assumption and desired.

> And, the question is how to be specific to each device.  This can be
> messy, as the sub-codec driver is probed independently from Realtek
> codec driver, hence you can't pass any flag from Realtek to CS driver
> at the probe time.  In the end, we might need to keep another table of
> IDs (either the same SSID or DMI) to distinguish which machine needs
> which properties.

Is there some other ID we can use? I see:
[   13.569242] cs35l41-hda spi0-CSC3551:00-cs35l41-hda.1: Cirrus Logic 
CS35L41 (35a40), Revision: B2
and I'd assume that 35a40 is unique? A bit of searching on my discord 
reveals that all the machines I listed that require the same patch also 
have this identifier (including a ProArt laptop).

I can get a fairly large test base if required.
> 
Cheers,
Luke.

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Takashi Iwai]

On Wed, 23 Aug 2023 09:28:39 +0200,
Luke Jones wrote:
> 
> 
> 
> On Wed, Aug 23 2023 at 08:24:51 +02:00:00, Takashi Iwai
> <tiwai@suse.de> wrote:
> > On Wed, 23 Aug 2023 03:10:08 +0200,
> > Luke D. Jones wrote:
> >> 
> >>  Support adding the missing DSD properties required  for ASUS ROG
> >> 2023
> >>  laptops and other ASUS laptops to properly utilise the cs35l41.
> >> 
> >>  This support includes both I2C and SPI connected amps.
> >> 
> >>  The SPI connected amps may be required to use an external DSD patch
> >>  to fix or add the "cs-gpios" property.
> >> 
> >>  Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
> >>  Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> >>  Co-developed-by: Luke D. Jones <luke@ljones.dev>
> >>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
> >>  ---
> >>   sound/pci/hda/cs35l41_hda_property.c | 26
> >> ++++++++++++++++++++++++++
> >>   1 file changed, 26 insertions(+)
> >> 
> >>  diff --git a/sound/pci/hda/cs35l41_hda_property.c
> >> b/sound/pci/hda/cs35l41_hda_property.c
> >>  index 673f23257a09..69879ab57918 100644
> >>  --- a/sound/pci/hda/cs35l41_hda_property.c
> >>  +++ b/sound/pci/hda/cs35l41_hda_property.c
> >>  @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct
> >> cs35l41_hda *cs35l41, struct device *phy
> >>   	return 0;
> >>   }
> >> 
> >>  +/*
> >>  + * The CSC3551 is used in almost the entire ASUS ROG laptop range
> >> in 2023, this is likely to
> >>  + * also include many non ROG labelled laptops. It is also used
> >> with either I2C connection or
> >>  + * SPI connection. The SPI connected versions may be missing a
> >> chip select GPIO and require
> >>  + * an DSD table patch.
> >>  + */
> >>  +static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41,
> >> struct device *physdev, int id,
> >>  +				const char *hid)
> >>  +{
> >>  +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
> >>  +
> >>  +	/* check SPI or I2C address to assign the index */
> >>  +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
> >>  +	hw_cfg->gpio2.valid = true;
> >>  +	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
> >>  +	hw_cfg->valid = true;
> >>  +
> >>  +	return 0;
> >>  +}
> >>  +
> >>   struct cs35l41_prop_model {
> >>   	const char *hid;
> >>   	const char *ssid;
> >>  @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
> >>   const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
> >>   	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
> >>   	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
> >>  +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
> > 
> > I believe this breaks things badly.  cs35l41_add_dsd_properties() is
> > always called no matter whether _DSD is found or not.  So this will
> > override the setup of all machines with CSC3551 even if it has a
> > proper _DSD.
> 
> These are the entries I know of so far since they definitely had to be
> added and have a DSD patch:
> 
> - SPI_2
>    - 0x1043, 0x1473, "ASUS GU604V"
>    - 0x1043, 0x1483, "ASUS GU603V"
>    - 0x1043, 0x1493, "ASUS GV601V"
>    - 0x1043, 0x1573, "ASUS GZ301V"
>    - 0x1043, 0x1c9f, "ASUS G614JI"
> - SPI_4
>    - 0x1043, 0x1caf, "ASUS G634JYR/JZR"
> - I2C_2
>    - 0x1043, 0x1d1f, "ASUS ROG Strix G17
>    - 0x1043, 0x1463, "Asus GA402X"
>    - 0x1043, 0x1433, "ASUS GX650P"
>    - ROG ALLY
> 
> You can see the variants are V, J, X, and P. A grep through the DSL
> dumps I have collected reveals that these machines are all indeed
> missing DSD entries. These are a mix of I2C and SPI.
> 
> The patch I submitted was based on Stefan's work only, and tested on 3
> SPI machines plus 2 I2C machines with no issues except the SPI
> machines needing a chipselect DSD patch.
> 
> It's worth stating that the DSD patches people were using all followed
> the exact same template except for the SPI number, or speaker ID.
> 
> I'd wager it being a safe bet to assume that every one of the ASUS
> laptops this year using the CSC3551 will be missing the required DSD
> entries given the trend so far.

Yes, I know that there are tons of such devices that need the
workaround for missing _DSD.  My point is, however, that your patch
breaks the devices that *do have* _DSD, because the function always
overrides.

> > The existing entries of CLSA0100 and CLSA0101 are OK since
> > (supposedly) those never had _DSD.  The current code is a bit
> > misleading as if it's applicable easily, though.
> > 
> > That said, we have to apply the setup only conditionally for each
> > specific device.  One easy thing would be to move the function call
> > after _DSD check.  But, I *guess* that Stefan applied the function at
> > the top so that it may cover the all cases including incorrect _DSD
> > properties.
> 
> Given the trend of what I've seen this seems like a reasonable
> assumption and desired.

... and it's not enough.  That's another point.  The parameters aren't
always same for all devices but may vary.  That is, it must have some
check of devices and models to identify which parameters may be used.

> > And, the question is how to be specific to each device.  This can be
> > messy, as the sub-codec driver is probed independently from Realtek
> > codec driver, hence you can't pass any flag from Realtek to CS driver
> > at the probe time.  In the end, we might need to keep another table of
> > IDs (either the same SSID or DMI) to distinguish which machine needs
> > which properties.
> 
> Is there some other ID we can use? I see:
> [   13.569242] cs35l41-hda spi0-CSC3551:00-cs35l41-hda.1: Cirrus Logic
> CS35L41 (35a40), Revision: B2
> and I'd assume that 35a40 is unique? A bit of searching on my discord
> reveals that all the machines I listed that require the same patch
> also have this identifier (including a ProArt laptop).

That's not unique to each *device* (not the chip model).  Say, we want
to distinguish ASUS GU604V and ASUS G64JYR.  They may have different
setups.

I guess only DMI is suitable at the time of probing this driver.


Takashi

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Luke Jones]

On Wed, Aug 23 2023 at 09:37:08 +02:00:00, Takashi Iwai <tiwai@suse.de> 
wrote:
> On Wed, 23 Aug 2023 09:28:39 +0200,
> Luke Jones wrote:
>> 
>> 
>> 
>>  On Wed, Aug 23 2023 at 08:24:51 +02:00:00, Takashi Iwai
>>  <tiwai@suse.de> wrote:
>>  > On Wed, 23 Aug 2023 03:10:08 +0200,
>>  > Luke D. Jones wrote:
>>  >>
>>  >>  Support adding the missing DSD properties required  for ASUS ROG
>>  >> 2023
>>  >>  laptops and other ASUS laptops to properly utilise the cs35l41.
>>  >>
>>  >>  This support includes both I2C and SPI connected amps.
>>  >>
>>  >>  The SPI connected amps may be required to use an external DSD 
>> patch
>>  >>  to fix or add the "cs-gpios" property.
>>  >>
>>  >>  Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
>>  >>  Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
>>  >>  Co-developed-by: Luke D. Jones <luke@ljones.dev>
>>  >>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
>>  >>  ---
>>  >>   sound/pci/hda/cs35l41_hda_property.c | 26
>>  >> ++++++++++++++++++++++++++
>>  >>   1 file changed, 26 insertions(+)
>>  >>
>>  >>  diff --git a/sound/pci/hda/cs35l41_hda_property.c
>>  >> b/sound/pci/hda/cs35l41_hda_property.c
>>  >>  index 673f23257a09..69879ab57918 100644
>>  >>  --- a/sound/pci/hda/cs35l41_hda_property.c
>>  >>  +++ b/sound/pci/hda/cs35l41_hda_property.c
>>  >>  @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct
>>  >> cs35l41_hda *cs35l41, struct device *phy
>>  >>   	return 0;
>>  >>   }
>>  >>
>>  >>  +/*
>>  >>  + * The CSC3551 is used in almost the entire ASUS ROG laptop 
>> range
>>  >> in 2023, this is likely to
>>  >>  + * also include many non ROG labelled laptops. It is also used
>>  >> with either I2C connection or
>>  >>  + * SPI connection. The SPI connected versions may be missing a
>>  >> chip select GPIO and require
>>  >>  + * an DSD table patch.
>>  >>  + */
>>  >>  +static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41,
>>  >> struct device *physdev, int id,
>>  >>  +				const char *hid)
>>  >>  +{
>>  >>  +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
>>  >>  +
>>  >>  +	/* check SPI or I2C address to assign the index */
>>  >>  +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
>>  >>  +	hw_cfg->gpio2.valid = true;
>>  >>  +	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
>>  >>  +	hw_cfg->valid = true;
>>  >>  +
>>  >>  +	return 0;
>>  >>  +}
>>  >>  +
>>  >>   struct cs35l41_prop_model {
>>  >>   	const char *hid;
>>  >>   	const char *ssid;
>>  >>  @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
>>  >>   const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
>>  >>   	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
>>  >>   	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
>>  >>  +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
>>  >
>>  > I believe this breaks things badly.  cs35l41_add_dsd_properties() 
>> is
>>  > always called no matter whether _DSD is found or not.  So this 
>> will
>>  > override the setup of all machines with CSC3551 even if it has a
>>  > proper _DSD.
>> 
>>  These are the entries I know of so far since they definitely had to 
>> be
>>  added and have a DSD patch:
>> 
>>  - SPI_2
>>     - 0x1043, 0x1473, "ASUS GU604V"
>>     - 0x1043, 0x1483, "ASUS GU603V"
>>     - 0x1043, 0x1493, "ASUS GV601V"
>>     - 0x1043, 0x1573, "ASUS GZ301V"
>>     - 0x1043, 0x1c9f, "ASUS G614JI"
>>  - SPI_4
>>     - 0x1043, 0x1caf, "ASUS G634JYR/JZR"
>>  - I2C_2
>>     - 0x1043, 0x1d1f, "ASUS ROG Strix G17
>>     - 0x1043, 0x1463, "Asus GA402X"
>>     - 0x1043, 0x1433, "ASUS GX650P"
>>     - ROG ALLY
>> 
>>  You can see the variants are V, J, X, and P. A grep through the DSL
>>  dumps I have collected reveals that these machines are all indeed
>>  missing DSD entries. These are a mix of I2C and SPI.
>> 
>>  The patch I submitted was based on Stefan's work only, and tested 
>> on 3
>>  SPI machines plus 2 I2C machines with no issues except the SPI
>>  machines needing a chipselect DSD patch.
>> 
>>  It's worth stating that the DSD patches people were using all 
>> followed
>>  the exact same template except for the SPI number, or speaker ID.
>> 
>>  I'd wager it being a safe bet to assume that every one of the ASUS
>>  laptops this year using the CSC3551 will be missing the required DSD
>>  entries given the trend so far.
> 
> Yes, I know that there are tons of such devices that need the
> workaround for missing _DSD.  My point is, however, that your patch
> breaks the devices that *do have* _DSD, because the function always
> overrides.

Ah, because CSC3551 is not unique at all. I see.

> 
>>  > The existing entries of CLSA0100 and CLSA0101 are OK since
>>  > (supposedly) those never had _DSD.  The current code is a bit
>>  > misleading as if it's applicable easily, though.
>>  >
>>  > That said, we have to apply the setup only conditionally for each
>>  > specific device.  One easy thing would be to move the function 
>> call
>>  > after _DSD check.  But, I *guess* that Stefan applied the 
>> function at
>>  > the top so that it may cover the all cases including incorrect 
>> _DSD
>>  > properties.
>> 
>>  Given the trend of what I've seen this seems like a reasonable
>>  assumption and desired.
> 
> ... and it's not enough.  That's another point.  The parameters aren't
> always same for all devices but may vary.  That is, it must have some
> check of devices and models to identify which parameters may be used.
> 
>>  > And, the question is how to be specific to each device.  This can 
>> be
>>  > messy, as the sub-codec driver is probed independently from 
>> Realtek
>>  > codec driver, hence you can't pass any flag from Realtek to CS 
>> driver
>>  > at the probe time.  In the end, we might need to keep another 
>> table of
>>  > IDs (either the same SSID or DMI) to distinguish which machine 
>> needs
>>  > which properties.
>> 
>>  Is there some other ID we can use? I see:
>>  [   13.569242] cs35l41-hda spi0-CSC3551:00-cs35l41-hda.1: Cirrus 
>> Logic
>>  CS35L41 (35a40), Revision: B2
>>  and I'd assume that 35a40 is unique? A bit of searching on my 
>> discord
>>  reveals that all the machines I listed that require the same patch
>>  also have this identifier (including a ProArt laptop).
> 
> That's not unique to each *device* (not the chip model).  Say, we want
> to distinguish ASUS GU604V and ASUS G64JYR.  They may have different
> setups.
> 
> I guess only DMI is suitable at the time of probing this driver.

Oh, I'm sorry I should have paid more attention the first time. I can 
match against acpi_subsystem_id which would be for example 10431caf yes?

I'll begin working on a patch series for each.

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Takashi Iwai]

On Wed, 23 Aug 2023 10:02:11 +0200,
Luke Jones wrote:
> 
> 
> 
> On Wed, Aug 23 2023 at 09:37:08 +02:00:00, Takashi Iwai
> <tiwai@suse.de> wrote:
> > On Wed, 23 Aug 2023 09:28:39 +0200,
> > Luke Jones wrote:
> >> 
> >> 
> >> 
> >>  On Wed, Aug 23 2023 at 08:24:51 +02:00:00, Takashi Iwai
> >>  <tiwai@suse.de> wrote:
> >>  > On Wed, 23 Aug 2023 03:10:08 +0200,
> >>  > Luke D. Jones wrote:
> >>  >>
> >>  >>  Support adding the missing DSD properties required  for ASUS ROG
> >>  >> 2023
> >>  >>  laptops and other ASUS laptops to properly utilise the cs35l41.
> >>  >>
> >>  >>  This support includes both I2C and SPI connected amps.
> >>  >>
> >>  >>  The SPI connected amps may be required to use an external DSD
> >> patch
> >>  >>  to fix or add the "cs-gpios" property.
> >>  >>
> >>  >>  Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
> >>  >>  Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> >>  >>  Co-developed-by: Luke D. Jones <luke@ljones.dev>
> >>  >>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
> >>  >>  ---
> >>  >>   sound/pci/hda/cs35l41_hda_property.c | 26
> >>  >> ++++++++++++++++++++++++++
> >>  >>   1 file changed, 26 insertions(+)
> >>  >>
> >>  >>  diff --git a/sound/pci/hda/cs35l41_hda_property.c
> >>  >> b/sound/pci/hda/cs35l41_hda_property.c
> >>  >>  index 673f23257a09..69879ab57918 100644
> >>  >>  --- a/sound/pci/hda/cs35l41_hda_property.c
> >>  >>  +++ b/sound/pci/hda/cs35l41_hda_property.c
> >>  >>  @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct
> >>  >> cs35l41_hda *cs35l41, struct device *phy
> >>  >>   	return 0;
> >>  >>   }
> >>  >>
> >>  >>  +/*
> >>  >>  + * The CSC3551 is used in almost the entire ASUS ROG laptop
> >> range
> >>  >> in 2023, this is likely to
> >>  >>  + * also include many non ROG labelled laptops. It is also used
> >>  >> with either I2C connection or
> >>  >>  + * SPI connection. The SPI connected versions may be missing a
> >>  >> chip select GPIO and require
> >>  >>  + * an DSD table patch.
> >>  >>  + */
> >>  >>  +static int asus_rog_2023_no_acpi(struct cs35l41_hda *cs35l41,
> >>  >> struct device *physdev, int id,
> >>  >>  +				const char *hid)
> >>  >>  +{
> >>  >>  +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
> >>  >>  +
> >>  >>  +	/* check SPI or I2C address to assign the index */
> >>  >>  +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
> >>  >>  +	hw_cfg->gpio2.valid = true;
> >>  >>  +	hw_cfg->bst_type = CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
> >>  >>  +	hw_cfg->valid = true;
> >>  >>  +
> >>  >>  +	return 0;
> >>  >>  +}
> >>  >>  +
> >>  >>   struct cs35l41_prop_model {
> >>  >>   	const char *hid;
> >>  >>   	const char *ssid;
> >>  >>  @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
> >>  >>   const struct cs35l41_prop_model cs35l41_prop_model_table[] = {
> >>  >>   	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
> >>  >>   	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
> >>  >>  +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
> >>  >
> >>  > I believe this breaks things badly.
> >> cs35l41_add_dsd_properties() is
> >>  > always called no matter whether _DSD is found or not.  So this
> >> will
> >>  > override the setup of all machines with CSC3551 even if it has a
> >>  > proper _DSD.
> >> 
> >>  These are the entries I know of so far since they definitely had
> >> to be
> >>  added and have a DSD patch:
> >> 
> >>  - SPI_2
> >>     - 0x1043, 0x1473, "ASUS GU604V"
> >>     - 0x1043, 0x1483, "ASUS GU603V"
> >>     - 0x1043, 0x1493, "ASUS GV601V"
> >>     - 0x1043, 0x1573, "ASUS GZ301V"
> >>     - 0x1043, 0x1c9f, "ASUS G614JI"
> >>  - SPI_4
> >>     - 0x1043, 0x1caf, "ASUS G634JYR/JZR"
> >>  - I2C_2
> >>     - 0x1043, 0x1d1f, "ASUS ROG Strix G17
> >>     - 0x1043, 0x1463, "Asus GA402X"
> >>     - 0x1043, 0x1433, "ASUS GX650P"
> >>     - ROG ALLY
> >> 
> >>  You can see the variants are V, J, X, and P. A grep through the DSL
> >>  dumps I have collected reveals that these machines are all indeed
> >>  missing DSD entries. These are a mix of I2C and SPI.
> >> 
> >>  The patch I submitted was based on Stefan's work only, and tested
> >> on 3
> >>  SPI machines plus 2 I2C machines with no issues except the SPI
> >>  machines needing a chipselect DSD patch.
> >> 
> >>  It's worth stating that the DSD patches people were using all
> >> followed
> >>  the exact same template except for the SPI number, or speaker ID.
> >> 
> >>  I'd wager it being a safe bet to assume that every one of the ASUS
> >>  laptops this year using the CSC3551 will be missing the required DSD
> >>  entries given the trend so far.
> > 
> > Yes, I know that there are tons of such devices that need the
> > workaround for missing _DSD.  My point is, however, that your patch
> > breaks the devices that *do have* _DSD, because the function always
> > overrides.
> 
> Ah, because CSC3551 is not unique at all. I see.
> 
> > 
> >>  > The existing entries of CLSA0100 and CLSA0101 are OK since
> >>  > (supposedly) those never had _DSD.  The current code is a bit
> >>  > misleading as if it's applicable easily, though.
> >>  >
> >>  > That said, we have to apply the setup only conditionally for each
> >>  > specific device.  One easy thing would be to move the function
> >> call
> >>  > after _DSD check.  But, I *guess* that Stefan applied the
> >> function at
> >>  > the top so that it may cover the all cases including incorrect
> >> _DSD
> >>  > properties.
> >> 
> >>  Given the trend of what I've seen this seems like a reasonable
> >>  assumption and desired.
> > 
> > ... and it's not enough.  That's another point.  The parameters aren't
> > always same for all devices but may vary.  That is, it must have some
> > check of devices and models to identify which parameters may be used.
> > 
> >>  > And, the question is how to be specific to each device.  This
> >> can be
> >>  > messy, as the sub-codec driver is probed independently from
> >> Realtek
> >>  > codec driver, hence you can't pass any flag from Realtek to CS
> >> driver
> >>  > at the probe time.  In the end, we might need to keep another
> >> table of
> >>  > IDs (either the same SSID or DMI) to distinguish which machine
> >> needs
> >>  > which properties.
> >> 
> >>  Is there some other ID we can use? I see:
> >>  [   13.569242] cs35l41-hda spi0-CSC3551:00-cs35l41-hda.1: Cirrus
> >> Logic
> >>  CS35L41 (35a40), Revision: B2
> >>  and I'd assume that 35a40 is unique? A bit of searching on my
> >> discord
> >>  reveals that all the machines I listed that require the same patch
> >>  also have this identifier (including a ProArt laptop).
> > 
> > That's not unique to each *device* (not the chip model).  Say, we want
> > to distinguish ASUS GU604V and ASUS G64JYR.  They may have different
> > setups.
> > 
> > I guess only DMI is suitable at the time of probing this driver.
> 
> Oh, I'm sorry I should have paid more attention the first time. I can
> match against acpi_subsystem_id which would be for example 10431caf
> yes?

I believe many other drivers match with vendor name, product name, or
whatever strings.

> I'll begin working on a patch series for each.

Thanks!


Takashi

RE: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Stefan Binding]

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Wednesday, August 23, 2023 9:44 AM
> To: Luke Jones <luke@ljones.dev>
> Cc: Takashi Iwai <tiwai@suse.de>; tiwai@suse.com;
> james.schulman@cirrus.com; david.rhodes@cirrus.com;
> rf@opensource.cirrus.com; linux-kernel@vger.kernel.org;
> sbinding@opensource.cirrus.com; Jonathan LoBue <jlobue10@gmail.com>
> Subject: Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops
with
> missing DSD
> 
> On Wed, 23 Aug 2023 10:02:11 +0200,
> Luke Jones wrote:
> >
> >
> >
> > On Wed, Aug 23 2023 at 09:37:08 +02:00:00, Takashi Iwai
> > <tiwai@suse.de> wrote:
> > > On Wed, 23 Aug 2023 09:28:39 +0200,
> > > Luke Jones wrote:
> > >>
> > >>
> > >>
> > >>  On Wed, Aug 23 2023 at 08:24:51 +02:00:00, Takashi Iwai
> > >>  <tiwai@suse.de> wrote:
> > >>  > On Wed, 23 Aug 2023 03:10:08 +0200,
> > >>  > Luke D. Jones wrote:
> > >>  >>
> > >>  >>  Support adding the missing DSD properties required  for
ASUS
> ROG
> > >>  >> 2023
> > >>  >>  laptops and other ASUS laptops to properly utilise the
cs35l41.
> > >>  >>
> > >>  >>  This support includes both I2C and SPI connected amps.
> > >>  >>
> > >>  >>  The SPI connected amps may be required to use an external
DSD
> > >> patch
> > >>  >>  to fix or add the "cs-gpios" property.
> > >>  >>
> > >>  >>  Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
> > >>  >>  Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> > >>  >>  Co-developed-by: Luke D. Jones <luke@ljones.dev>
> > >>  >>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > >>  >>  ---
> > >>  >>   sound/pci/hda/cs35l41_hda_property.c | 26
> > >>  >> ++++++++++++++++++++++++++
> > >>  >>   1 file changed, 26 insertions(+)
> > >>  >>
> > >>  >>  diff --git a/sound/pci/hda/cs35l41_hda_property.c
> > >>  >> b/sound/pci/hda/cs35l41_hda_property.c
> > >>  >>  index 673f23257a09..69879ab57918 100644
> > >>  >>  --- a/sound/pci/hda/cs35l41_hda_property.c
> > >>  >>  +++ b/sound/pci/hda/cs35l41_hda_property.c
> > >>  >>  @@ -43,6 +43,31 @@ static int lenovo_legion_no_acpi(struct
> > >>  >> cs35l41_hda *cs35l41, struct device *phy
> > >>  >>   	return 0;
> > >>  >>   }
> > >>  >>
> > >>  >>  +/*
> > >>  >>  + * The CSC3551 is used in almost the entire ASUS ROG
laptop
> > >> range
> > >>  >> in 2023, this is likely to
> > >>  >>  + * also include many non ROG labelled laptops. It is also
used
> > >>  >> with either I2C connection or
> > >>  >>  + * SPI connection. The SPI connected versions may be
missing a
> > >>  >> chip select GPIO and require
> > >>  >>  + * an DSD table patch.
> > >>  >>  + */
> > >>  >>  +static int asus_rog_2023_no_acpi(struct cs35l41_hda
*cs35l41,
> > >>  >> struct device *physdev, int id,
> > >>  >>  +				const char *hid)
> > >>  >>  +{
> > >>  >>  +	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
> > >>  >>  +
> > >>  >>  +	/* check SPI or I2C address to assign the index */
> > >>  >>  +	cs35l41->index = (id == 0 || 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->gpio2.func = CS35L41_INTERRUPT;
> > >>  >>  +	hw_cfg->gpio2.valid = true;
> > >>  >>  +	hw_cfg->bst_type =
> CS35L41_EXT_BOOST_NO_VSPK_SWITCH;
> > >>  >>  +	hw_cfg->valid = true;
> > >>  >>  +
> > >>  >>  +	return 0;
> > >>  >>  +}
> > >>  >>  +
> > >>  >>   struct cs35l41_prop_model {
> > >>  >>   	const char *hid;
> > >>  >>   	const char *ssid;
> > >>  >>  @@ -53,6 +78,7 @@ struct cs35l41_prop_model {
> > >>  >>   const struct cs35l41_prop_model
cs35l41_prop_model_table[] =
> {
> > >>  >>   	{ "CLSA0100", NULL, lenovo_legion_no_acpi },
> > >>  >>   	{ "CLSA0101", NULL, lenovo_legion_no_acpi },
> > >>  >>  +	{ "CSC3551", NULL, asus_rog_2023_no_acpi },
> > >>  >
> > >>  > I believe this breaks things badly.
> > >> cs35l41_add_dsd_properties() is
> > >>  > always called no matter whether _DSD is found or not.  So
this
> > >> will
> > >>  > override the setup of all machines with CSC3551 even if it
has a
> > >>  > proper _DSD.
> > >>
> > >>  These are the entries I know of so far since they definitely
had
> > >> to be
> > >>  added and have a DSD patch:
> > >>
> > >>  - SPI_2
> > >>     - 0x1043, 0x1473, "ASUS GU604V"
> > >>     - 0x1043, 0x1483, "ASUS GU603V"
> > >>     - 0x1043, 0x1493, "ASUS GV601V"
> > >>     - 0x1043, 0x1573, "ASUS GZ301V"
> > >>     - 0x1043, 0x1c9f, "ASUS G614JI"
> > >>  - SPI_4
> > >>     - 0x1043, 0x1caf, "ASUS G634JYR/JZR"
> > >>  - I2C_2
> > >>     - 0x1043, 0x1d1f, "ASUS ROG Strix G17
> > >>     - 0x1043, 0x1463, "Asus GA402X"
> > >>     - 0x1043, 0x1433, "ASUS GX650P"
> > >>     - ROG ALLY
> > >>
> > >>  You can see the variants are V, J, X, and P. A grep through
the DSL
> > >>  dumps I have collected reveals that these machines are all
indeed
> > >>  missing DSD entries. These are a mix of I2C and SPI.
> > >>
> > >>  The patch I submitted was based on Stefan's work only, and
tested
> > >> on 3
> > >>  SPI machines plus 2 I2C machines with no issues except the SPI
> > >>  machines needing a chipselect DSD patch.
> > >>
> > >>  It's worth stating that the DSD patches people were using all
> > >> followed
> > >>  the exact same template except for the SPI number, or speaker
ID.
> > >>
> > >>  I'd wager it being a safe bet to assume that every one of the
ASUS
> > >>  laptops this year using the CSC3551 will be missing the
required DSD
> > >>  entries given the trend so far.
> > >
> > > Yes, I know that there are tons of such devices that need the
> > > workaround for missing _DSD.  My point is, however, that your
patch
> > > breaks the devices that *do have* _DSD, because the function
always
> > > overrides.
> >
> > Ah, because CSC3551 is not unique at all. I see.
> >
> > >
> > >>  > The existing entries of CLSA0100 and CLSA0101 are OK since
> > >>  > (supposedly) those never had _DSD.  The current code is a
bit
> > >>  > misleading as if it's applicable easily, though.
> > >>  >
> > >>  > That said, we have to apply the setup only conditionally for
each
> > >>  > specific device.  One easy thing would be to move the
function
> > >> call
> > >>  > after _DSD check.  But, I *guess* that Stefan applied the
> > >> function at
> > >>  > the top so that it may cover the all cases including
incorrect
> > >> _DSD
> > >>  > properties.
> > >>
> > >>  Given the trend of what I've seen this seems like a reasonable
> > >>  assumption and desired.
> > >
> > > ... and it's not enough.  That's another point.  The parameters
aren't
> > > always same for all devices but may vary.  That is, it must have
some
> > > check of devices and models to identify which parameters may be
> used.
> > >
> > >>  > And, the question is how to be specific to each device.
This
> > >> can be
> > >>  > messy, as the sub-codec driver is probed independently from
> > >> Realtek
> > >>  > codec driver, hence you can't pass any flag from Realtek to
CS
> > >> driver
> > >>  > at the probe time.  In the end, we might need to keep
another
> > >> table of
> > >>  > IDs (either the same SSID or DMI) to distinguish which
machine
> > >> needs
> > >>  > which properties.
> > >>
> > >>  Is there some other ID we can use? I see:
> > >>  [   13.569242] cs35l41-hda spi0-CSC3551:00-cs35l41-hda.1:
Cirrus
> > >> Logic
> > >>  CS35L41 (35a40), Revision: B2
> > >>  and I'd assume that 35a40 is unique? A bit of searching on my
> > >> discord
> > >>  reveals that all the machines I listed that require the same
patch
> > >>  also have this identifier (including a ProArt laptop).
> > >
> > > That's not unique to each *device* (not the chip model).  Say,
we want
> > > to distinguish ASUS GU604V and ASUS G64JYR.  They may have
> different
> > > setups.
> > >
> > > I guess only DMI is suitable at the time of probing this driver.
> >
> > Oh, I'm sorry I should have paid more attention the first time. I
can
> > match against acpi_subsystem_id which would be for example
10431caf
> > yes?

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.

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.
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).

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.

Thanks,
Stefan Binding

> 
> I believe many other drivers match with vendor name, product name,
or
> whatever strings.
> 
> > I'll begin working on a patch series for each.
> 
> Thanks!
> 
> 
> Takashi

RE: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Luke Jones]

> 
> 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.

> 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.

Cheers,
Luke.

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Jonathan LoBue]

On Wednesday, August 23, 2023 1:31:06 PM PDT Luke Jones 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.
> 
> > 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.
> 
> Cheers,
> Luke.

Stefan,

Based on your comment, "in fact I believe all these laptops are internal boost 
anyway," that would imply that we want to update our temporary patches that we 
are using for kernel creation (and sharing via RPM or other means). Right now 
we had updated that line to hw_cfg->bst_type = CS35L41_EXT_BOOST; but it seems 
that it should actually be hw_cfg->bst_type = CS35L41_INT_BOOST. The external 
boost setting seems to work for the ASUS ROG ALLY in my own testing, but 
perhaps it is driving the amp harder than intended. Based on available 
documentation, the internal boost setting must be accompanied by the peak 
current, inductor and capacitor values of the boost circuit. It's possible 
that ASUS used some different values here for board components across different 
laptop models (and the ALLY). If you have the table with accurate values for 
the capacitance and inductance values (and for which models they apply), that 
would be very valuable information to share. As Luke pointed out, many people 
are wanting to use these patches now to avoid the need for DSD ACPI overrides 
to get audio working on their devices.

I'm most interested in the capacitance and inductance values for the ROG ALLY, 
so that I can fill in the values for hw_cfg->bst_ind and hw_cfg->bst_cap. Peak 
current seems to be okay to leave at 4,500mA as I can't imagine that Cirrus 
would allow an unsafe current value there, and in documentation it's listed as 
default anyways. We will wait for your upstream patch, but we would also very 
much appreciate if you could share this information if you have it and are 
able to share it. Thanks.

Best Regards,
Jon LoBue

RE: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Luke Jones]

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?

> 
>> 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.

> 
> Cheers,
> Luke.
> 
> 

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Stefan Binding]

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.
>>
>>
>
>

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Huayu Zhang]

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),
-- 
2.42.0



Here is also my alsa info output:
http://alsa-project.org/db/?f=1ad9f2709886f0ec5d2d87d5d8e59a0ac05384be

And the output of `dmesg | grep CSC` using the kernel with above patch:

[    4.509280] Serial bus multi instantiate pseudo device driver
CSC3551:00: Instantiated 2 I2C devices.
[    4.716009] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: Adding DSD
properties for 17AA38A9
[    4.716011] CSC3551: id == 0x40
[    4.716728] CSC3551: Done.
[    4.716729] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: Using extra
_DSD properties, bypassing _DSD in ACPI
[    4.760514] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: Cirrus Logic
CS35L41 (35a40), Revision: B2
[    4.760776] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: Adding DSD
properties for 17AA38A9
[    4.760777] CSC3551: id == 0x41
[    4.761107] CSC3551: Done.
[    4.761108] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: Using extra
_DSD properties, bypassing _DSD in ACPI
[    4.761108] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: Reset line
busy, assuming shared reset
[    4.802203] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: Cirrus Logic
CS35L41 (35a40), Revision: B2
[    5.041334] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: Falling back
to default firmware.
[    5.041896] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: DSP1:
Firmware version: 3
[    5.041897] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: DSP1:
cirrus/cs35l41-dsp1-spk-prot.wmfw: Fri 24 Jun 2022 14:55:56 GMT
Daylight Time
[    5.501809] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: DSP1:
Firmware: 400a4 vendor: 0x2 v0.58.0, 2 algorithms
[    5.503073] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: DSP1:
cirrus/cs35l41-dsp1-spk-prot.bin: v0.58.0
[    5.503079] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: DSP1:
spk-prot: e:\workspace\workspace\tibranch_release_playback_6.76_2\ormis\staging\default_tunings\internal\CS35L53\Fixed_Attenuation_Mono_48000_29.78.0\full\Fixed_Attenuation_Mono_48000_29.78.0_full.bin
[    5.538264] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: CS35L41 Bound
- SSID: 17AA38A9, BST: 1, VSPK: 1, CH: L, FW EN: 1, SPKID: 0
[    5.538277] snd_hda_codec_realtek ehdaudio0D0: bound
i2c-CSC3551:00-cs35l41-hda.0 (ops cs35l41_hda_comp_ops
[snd_hda_scodec_cs35l41])
[    5.538545] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: Falling back
to default firmware.
[    5.539404] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: DSP1:
Firmware version: 3
[    5.539409] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: DSP1:
cirrus/cs35l41-dsp1-spk-prot.wmfw: Fri 24 Jun 2022 14:55:56 GMT
Daylight Time
[    5.999448] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: DSP1:
Firmware: 400a4 vendor: 0x2 v0.58.0, 2 algorithms
[    6.000638] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: DSP1:
cirrus/cs35l41-dsp1-spk-prot.bin: v0.58.0
[    6.000640] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: DSP1:
spk-prot: e:\workspace\workspace\tibranch_release_playback_6.76_2\ormis\staging\default_tunings\internal\CS35L53\Fixed_Attenuation_Mono_48000_29.78.0\full\Fixed_Attenuation_Mono_48000_29.78.0_full.bin
[    6.031058] cs35l41-hda i2c-CSC3551:00-cs35l41-hda.1: CS35L41 Bound
- SSID: 17AA38A9, BST: 1, VSPK: 1, CH: R, FW EN: 1, SPKID: 0
[    6.031062] snd_hda_codec_realtek ehdaudio0D0: bound
i2c-CSC3551:00-cs35l41-hda.1 (ops cs35l41_hda_comp_ops
[snd_hda_scodec_cs35l41])


Thanks again. Nice day~

Cheers,
Huayu

On Tue, Oct 3, 2023 at 11:07 PM Stefan Binding
<sbinding@opensource.cirrus.com> wrote:
>
>
> 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.
> >>
> >>
> >
> >

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Jonathan LoBue]

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

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Jonathan LoBue]

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

Re: [PATCH] ALSA: hda: cs35l41: Support ASUS 2023 laptops with missing DSD

[Jonathan LoBue]

On Monday, October 23, 2023 9:35:55 AM PDT Jonathan LoBue wrote:
> 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

Replying again here to clarify that the exit code for the DSD override patch 
not being necessary and therefore exiting the patching process for BIOSes 
equal to or greater than 330 (on ROG ALLY) should in fact be "return -
ENOENT;". This was confirmed in testing by me today for ChimeraOS. Changing 
that line to "return 0;" actually causes the code logic to take an incorrect 
branch and subsequently audio ends up not working with that compiled module. 
This reply is mainly for educational and informational purposes. I don't 
expect this patch to be merged. Fortunately, the new 331 BIOS that came out 
today for ASUS ROG ALLY fixes all my known issues that I had with the 330 BIOS 
on Nobara, and is actually usable for me. Thanks.

Best Regards,
Jon LoBue