[PATCH 0/2] Fix CSC3551 speaker sound problem for machines without a valid ACPI _DSD

[PATCH 0/2] Fix CSC3551 speaker sound problem for machines without a valid ACPI _DSD

[David Xu]

As the comments added in commit 4d4c4bff4f8ed79d95e05 ("ALSA: hda:
cs35l41: Clarify support for CSC3551 without _DSD Properties"), CSC3551
requires a valid _DSD to work and the current implementation just
fails when no _DSD can be found for CSC3551. However it is a fact
that many OEMs hardcoded the configurations needed by CSC3551 into their
proprietary software for various 2022 and later laptop models,
and this makes the Linux installations on these models cannot make
any speaker sound. Meanwhile, at this point of time, we see no hope
that these OEMs would ever fix this problem via a BIOS update. 

To address the problem, this patch series contains two patches: 

Patch 1 for cs35l41 hda driver: a fixup mechanism is introduced that 
when the driver found there is no valid _DSD that contains the 
configurations, a fixup function would try to find a fixup entry that
contains a proper cs35l41 configuration from a pre-defined fixup table
by matching the CSC3551 ACPI _SUB id. If found, the fixup function
would apply the cs35l41 configurations retrived from the entry. 
In this patch the fixup table only contains some entries for three 
Lenovo laptop models: namely 16IAH7, 16IAX7 and 16ARHA7. However 
as is known, several other laptop models from ASUS and HP also suffer
from this no valid _DSD problem and could have it addressed with this 
fixup mechanism when proper fixup entries are inserted.


Patch 2 for realtek hda driver: add quirks for Lenovo 16IAH7, 16IAX7 
and 16ARHA7 so that cs35l41 playback hook could be registered. Please 
note that for these quirks to work patch 1 has to be applied.


David Xu (2):
  ALSA: hda: cs35l41: Add fixups for machines without a valid ACPI _DSD
  ALSA: hda/realtek: Add quirks for Lenovo 16IAH7, 16IAX7 and 16ARHA7

 sound/pci/hda/cs35l41_hda.c   | 160 ++++++++++++++++++++++++++++++++++
 sound/pci/hda/patch_realtek.c |   5 ++
 2 files changed, 165 insertions(+)

-- 
2.41.0

Re: [PATCH 0/2] Fix CSC3551 speaker sound problem for machines without a valid ACPI _DSD

[Takashi Iwai]

On Thu, 13 Jul 2023 18:29:53 +0200,
David Xu wrote:
> 
> 
> As the comments added in commit 4d4c4bff4f8ed79d95e05 ("ALSA: hda:
> cs35l41: Clarify support for CSC3551 without _DSD Properties"), CSC3551
> requires a valid _DSD to work and the current implementation just
> fails when no _DSD can be found for CSC3551. However it is a fact
> that many OEMs hardcoded the configurations needed by CSC3551 into their
> proprietary software for various 2022 and later laptop models,
> and this makes the Linux installations on these models cannot make
> any speaker sound. Meanwhile, at this point of time, we see no hope
> that these OEMs would ever fix this problem via a BIOS update. 
> 
> To address the problem, this patch series contains two patches: 
> 
> Patch 1 for cs35l41 hda driver: a fixup mechanism is introduced that 
> when the driver found there is no valid _DSD that contains the 
> configurations, a fixup function would try to find a fixup entry that
> contains a proper cs35l41 configuration from a pre-defined fixup table
> by matching the CSC3551 ACPI _SUB id. If found, the fixup function
> would apply the cs35l41 configurations retrived from the entry. 
> In this patch the fixup table only contains some entries for three 
> Lenovo laptop models: namely 16IAH7, 16IAX7 and 16ARHA7. However 
> as is known, several other laptop models from ASUS and HP also suffer
> from this no valid _DSD problem and could have it addressed with this 
> fixup mechanism when proper fixup entries are inserted.
> 
> 
> Patch 2 for realtek hda driver: add quirks for Lenovo 16IAH7, 16IAX7 
> and 16ARHA7 so that cs35l41 playback hook could be registered. Please 
> note that for these quirks to work patch 1 has to be applied.

Thanks for the patches.

I've seen the lots of pains with CS35L41 codec stuff on the recent
machines.  But, first of all, it still needs to be agreed by Cirrus
people whether this approach is acceptable.  Judging from the current 
situation, such workaround appears inevitable, but we need a
consensus.

So, Cirrus people, please check this.


Also, some ideas about the current patch set:

- Do we need yet another listing and check of each ID in another
  place?  The existing entry in the SSID quirk table is already unique
  enough to identify which configuration is taken, I suppose.

- The quirk entries can be gathered in patch_realtek.c, and the hw_cfg
  and other items are overwritten in cs35l41_no_acpi_dsd() when no
  _DSD is found.  In that way, we can avoid fixing two places for each
  update.

- The workaround is a workaround, and it's fundamentally dangerous.
  We should warn it in a kernel message.


Takashi

Re: [PATCH 0/2] Fix CSC3551 speaker sound problem for machines without a valid ACPI _DSD

[Stuart Henderson]

On 16/07/2023 13:49, Takashi Iwai wrote:
> On Thu, 13 Jul 2023 18:29:53 +0200,
> David Xu wrote:
>>
>> As the comments added in commit 4d4c4bff4f8ed79d95e05 ("ALSA: hda:
>> cs35l41: Clarify support for CSC3551 without _DSD Properties"), CSC3551
>> requires a valid _DSD to work and the current implementation just
>> fails when no _DSD can be found for CSC3551. However it is a fact
>> that many OEMs hardcoded the configurations needed by CSC3551 into their
>> proprietary software for various 2022 and later laptop models,
>> and this makes the Linux installations on these models cannot make
>> any speaker sound. Meanwhile, at this point of time, we see no hope
>> that these OEMs would ever fix this problem via a BIOS update.
>>
>> To address the problem, this patch series contains two patches:
>>
>> Patch 1 for cs35l41 hda driver: a fixup mechanism is introduced that
>> when the driver found there is no valid _DSD that contains the
>> configurations, a fixup function would try to find a fixup entry that
>> contains a proper cs35l41 configuration from a pre-defined fixup table
>> by matching the CSC3551 ACPI _SUB id. If found, the fixup function
>> would apply the cs35l41 configurations retrived from the entry.
>> In this patch the fixup table only contains some entries for three
>> Lenovo laptop models: namely 16IAH7, 16IAX7 and 16ARHA7. However
>> as is known, several other laptop models from ASUS and HP also suffer
>> from this no valid _DSD problem and could have it addressed with this
>> fixup mechanism when proper fixup entries are inserted.
>>
>>
>> Patch 2 for realtek hda driver: add quirks for Lenovo 16IAH7, 16IAX7
>> and 16ARHA7 so that cs35l41 playback hook could be registered. Please
>> note that for these quirks to work patch 1 has to be applied.
> Thanks for the patches.
>
> I've seen the lots of pains with CS35L41 codec stuff on the recent
> machines.  But, first of all, it still needs to be agreed by Cirrus
> people whether this approach is acceptable.  Judging from the current
> situation, such workaround appears inevitable, but we need a
> consensus.
>
> So, Cirrus people, please check this.
>
>
> Also, some ideas about the current patch set:
>
> - Do we need yet another listing and check of each ID in another
>    place?  The existing entry in the SSID quirk table is already unique
>    enough to identify which configuration is taken, I suppose.
>
> - The quirk entries can be gathered in patch_realtek.c, and the hw_cfg
>    and other items are overwritten in cs35l41_no_acpi_dsd() when no
>    _DSD is found.  In that way, we can avoid fixing two places for each
>    update.
>
> - The workaround is a workaround, and it's fundamentally dangerous.
>    We should warn it in a kernel message.
>
>
> Takashi

Hi David, Takashi,
We're looking into supporting some of these older devices at the moment, 
and have a patch chain in development at the moment.
The approach we've taken is a lot closer to the one taken by the github 
Luke links through to elsewhere in this chain, which we think might be a 
cleaner approach.  We do have concerns about the current approach of 
identifying the SPI device though, as we've seen "spi1" become "spi2" as 
additional devices become supported in the kernel and more SPI 
controllers come into use.  We'll look into this more and hopefully get 
a patch chain up in the coming weeks.
This patch chain looks like it might also be missing support for 
different boost configurations.
I wouldn't recommend this patch be merged.
Thanks,
Stu

Re: [PATCH 0/2] Fix CSC3551 speaker sound problem for machines without a valid ACPI _DSD

[David Xu]

On Mon, 17 Jul 2023 13:48:28 +0100, Stuart Henderson wrote:
> On 16/07/2023 13:49, Takashi Iwai wrote:
> > On Thu, 13 Jul 2023 18:29:53 +0200,
> > David Xu wrote:
> >>
> >> As the comments added in commit 4d4c4bff4f8ed79d95e05 ("ALSA: hda:
> >> cs35l41: Clarify support for CSC3551 without _DSD Properties"), CSC3551
> >> requires a valid _DSD to work and the current implementation just
> >> fails when no _DSD can be found for CSC3551. However it is a fact
> >> that many OEMs hardcoded the configurations needed by CSC3551 into their
> >> proprietary software for various 2022 and later laptop models,
> >> and this makes the Linux installations on these models cannot make
> >> any speaker sound. Meanwhile, at this point of time, we see no hope
> >> that these OEMs would ever fix this problem via a BIOS update.
> >>
> >> To address the problem, this patch series contains two patches:
> >>
> >> Patch 1 for cs35l41 hda driver: a fixup mechanism is introduced that
> >> when the driver found there is no valid _DSD that contains the
> >> configurations, a fixup function would try to find a fixup entry that
> >> contains a proper cs35l41 configuration from a pre-defined fixup table
> >> by matching the CSC3551 ACPI _SUB id. If found, the fixup function
> >> would apply the cs35l41 configurations retrived from the entry.
> >> In this patch the fixup table only contains some entries for three
> >> Lenovo laptop models: namely 16IAH7, 16IAX7 and 16ARHA7. However
> >> as is known, several other laptop models from ASUS and HP also suffer
> >> from this no valid _DSD problem and could have it addressed with this
> >> fixup mechanism when proper fixup entries are inserted.
> >>
> >>
> >> Patch 2 for realtek hda driver: add quirks for Lenovo 16IAH7, 16IAX7
> >> and 16ARHA7 so that cs35l41 playback hook could be registered. Please
> >> note that for these quirks to work patch 1 has to be applied.
> > Thanks for the patches.
> >
> > I've seen the lots of pains with CS35L41 codec stuff on the recent
> > machines.  But, first of all, it still needs to be agreed by Cirrus
> > people whether this approach is acceptable.  Judging from the current
> > situation, such workaround appears inevitable, but we need a
> > consensus.
> >
> > So, Cirrus people, please check this.
> >
> >
> > Also, some ideas about the current patch set:
> >
> > - Do we need yet another listing and check of each ID in another
> >    place?  The existing entry in the SSID quirk table is already unique
> >    enough to identify which configuration is taken, I suppose.
> >
> > - The quirk entries can be gathered in patch_realtek.c, and the hw_cfg
> >    and other items are overwritten in cs35l41_no_acpi_dsd() when no
> >    _DSD is found.  In that way, we can avoid fixing two places for each
> >    update.
> >
> > - The workaround is a workaround, and it's fundamentally dangerous.
> >    We should warn it in a kernel message.
> >
> >
> > Takashi
> 
> Hi David, Takashi,
> We're looking into supporting some of these older devices at the moment, 
> and have a patch chain in development at the moment.
> The approach we've taken is a lot closer to the one taken by the github 
> Luke links through to elsewhere in this chain, which we think might be a 
> cleaner approach.  We do have concerns about the current approach of 
> identifying the SPI device though, as we've seen "spi1" become "spi2" as 
> additional devices become supported in the kernel and more SPI 
> controllers come into use.  We'll look into this more and hopefully get 
> a patch chain up in the coming weeks.
> This patch chain looks like it might also be missing support for 
> different boost configurations.
> I wouldn't recommend this patch be merged.
> Thanks,
> Stu

Hi Stuart,

Thank you for replying. I am looking forward to your work for fixing
this problem and hopefully I could see it in the following weeks. In 
addtion I'd like to justify this lack of support for boost configurations
as without a valid _DSD, it is nearly impossible for us users to know the 
correct and safe configuartion for internal boosting. The current situation
is that without this boosting config, we indeed get a quite low speaker
sound. It would be much appreciated if you could also solve this problem. 

Regards,

David

Re: [PATCH 0/2] Fix CSC3551 speaker sound problem for machines without a valid ACPI _DSD

[David Xu]

Hi Takashi,

On Sun, 16 Jul 2023 14:49:18 +0200 you wrote:

> Thanks for the patches.
> 
> I've seen the lots of pains with CS35L41 codec stuff on the recent
> machines.  But, first of all, it still needs to be agreed by Cirrus
> people whether this approach is acceptable.  Judging from the current 
> situation, such workaround appears inevitable, but we need a
> consensus.
> 
> So, Cirrus people, please check this.

Agreed.
 
> Also, some ideas about the current patch set:
> 
> - Do we need yet another listing and check of each ID in another
>   place?  The existing entry in the SSID quirk table is already unique
>   enough to identify which configuration is taken, I suppose.
>
> - The quirk entries can be gathered in patch_realtek.c, and the hw_cfg
>   and other items are overwritten in cs35l41_no_acpi_dsd() when no
>   _DSD is found.  In that way, we can avoid fixing two places for each
>   update.

I do have noticed that the existing entries in patch_realtek.c are enough
for locating the right quirk. However I'm unsure if these entries can 
be moved to that file since the cs35l41 i2c/spi devices are instantiated
by the serial-multi-instantiate driver, and if no proper _DSD is found 
the cs35l41 driver would just fail with no corresponding i2c/spi devices.
I don't know if it is possible for the hda driver to intervene this 
probing process.


> - The workaround is a workaround, and it's fundamentally dangerous.
>   We should warn it in a kernel message.

Agreed. I will update my patches to emit a warning message, however as
Stuart has replied, a patch series which is cleaner is on its way in few 
weeks and I would not submit a new version for now. I am looking forward 
to Cirrus's work. 

Regards,

David.