Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Waldek Andrukiewicz]

On 28.07.23 15:55, Stefan Binding wrote:
>
> On 28/07/2023 12:10, Takashi Iwai wrote:
>> On Mon, 24 Jul 2023 11:00:38 +0200,
>> Takashi Iwai wrote:
>>> On Fri, 21 Jul 2023 17:18:05 +0200,
>>> Stefan Binding wrote:
>>>> There have been a couple of customer reports of intermittant issues
>>>> after
>>>> system resume, where sometimes the DSP firmware stops responding.
>>>> Investigations into this issue show that there is a race between
>>>> receiving
>>>> a prepare from the HDA core, and the firmware reload which is
>>>> started by
>>>> the system resume. This can causes the Global Enable on the CS35L41
>>>> to be
>>>> enabled during the firmware load, which can sometimes cause issues
>>>> in the
>>>> DSP.
>>>>
>>>> The existing system resume behaviour also did not resume the audio, if
>>>> audio was previously playing when it was suspended.
>>>> In addition, during investigation, it was found there were additional
>>>> problems in the System Resume sequence, as well as the Playback
>>>> sequence
>>>> with External Boost, where the driver does not correctly follow its
>>>> enable sequence for this mode. This can cause additional issues
>>>> such as
>>>> pops and clicks.
>>>>
>>>> This chain intends to correct the sequences for playback and system
>>>> suspend/resume so that the driver: obeys the external boost enable
>>>> sequence;
>>>> resumes audio on system resume; and avoids the race condition on
>>>> firmware
>>>> load and playback during system resume.
>>>>
>>>> Changes since v1:
>>>> - Split patch 1 into 2 separate patches
>>>> - Combine Patches 6 and 9
>>>>
>>>> Stefan Binding (11):
>>>>    ALSA: cs35l41: Use mbox command to enable speaker output for
>>>> external
>>>>      boost
>>>>    ALSA: cs35l41: Poll for Power Up/Down rather than waiting a fixed
>>>>      delay
>>>>    ALSA: hda: cs35l41: Check mailbox status of pause command after
>>>>      firmware load
>>>>    ALSA: hda: cs35l41: Ensure we correctly re-sync regmap before
>>>> system
>>>>      suspending.
>>>>    ALSA: hda: cs35l41: Ensure we pass up any errors during system
>>>>      suspend.
>>>>    ALSA: hda: cs35l41: Move Play and Pause into separate functions
>>>>    ALSA: hda: hda_component: Add pre and post playback hooks to
>>>>      hda_component
>>>>    ALSA: hda: cs35l41: Use pre and post playback hooks
>>>>    ALSA: hda: cs35l41: Rework System Suspend to ensure correct call
>>>>      separation
>>>>    ALSA: hda: cs35l41: Add device_link between HDA and cs35l41_hda
>>>>    ALSA: hda: cs35l41: Ensure amp is only unmuted during playback
>>> Applied all patches now to for-next branch.
>> It seems that this patch set causes occasional freeze at suspend:
>>    https://bugzilla.suse.com/show_bug.cgi?id=1213745
>>
>> Could you take a look?
>>
>>
>> thanks,
>>
>> Takashi
>
> Hi Takashi,
>
> The initial bug report shows one of the original issues that this patch
> chain was trying to fix.
> From what I can tell from the second issue, something has caused the
> CS35L41 to stop responding,
> which in turn caused the system suspend call to fail, and the error is
> passed up. Since system suspend
> failed, there was no corresponding system resume, which means the
> CS35L41 was stuck broken.
>
> I'm not sure what was meant by "freeze" and "overheating" in the bug
> report, since the log seems to
> indicate the laptop is still responsive, even if audio is broken.
>
> There is some oddity in the log, since one of the errors that was
> printed should only be printed when
> the CS35L41 is using External Boost, but I think this laptop is supposed
> to use Internal Boost.
> We'll investigate further.
>
> Thanks,
>
> Stefan Binding


Hello,

I would like to inform you that (one of) those patches is probably
breaking sound on Lenovo Legion 7 16ACHg6. They were applied in Manjaro
Linux kernel here:

https://gitlab.manjaro.org/packages/core/linux64/-/commit/742e66f525170fe02dec42e47aedf53d3dc85195

and when I install this kernel, there is no sound anymore. There is
nothing more than those patches in this commit. A kernel compiled from
the previous commit works fine.

Sound used to work fine from 5.17 if I remember correctly.

I hope I replied correctly to this thread, if not, apologies.

Best Regards,

Waldek Andrukiewicz

Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

On Fri, Sep 01, 2023 at 04:48:21PM +0000, Waldek Andrukiewicz wrote:

[An encrypted message which probably nobody can read, it's not even
encrypted to me even though I was a recipient]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Stefan Binding]

On 01/09/2023 17:48, Waldek Andrukiewicz wrote:
> On 28.07.23 15:55, Stefan Binding wrote:
>> On 28/07/2023 12:10, Takashi Iwai wrote:
>>> On Mon, 24 Jul 2023 11:00:38 +0200,
>>> Takashi Iwai wrote:
>>>> On Fri, 21 Jul 2023 17:18:05 +0200,
>>>> Stefan Binding wrote:
>>>>> There have been a couple of customer reports of intermittant issues
>>>>> after
>>>>> system resume, where sometimes the DSP firmware stops responding.
>>>>> Investigations into this issue show that there is a race between
>>>>> receiving
>>>>> a prepare from the HDA core, and the firmware reload which is
>>>>> started by
>>>>> the system resume. This can causes the Global Enable on the CS35L41
>>>>> to be
>>>>> enabled during the firmware load, which can sometimes cause issues
>>>>> in the
>>>>> DSP.
>>>>>
>>>>> The existing system resume behaviour also did not resume the audio, if
>>>>> audio was previously playing when it was suspended.
>>>>> In addition, during investigation, it was found there were additional
>>>>> problems in the System Resume sequence, as well as the Playback
>>>>> sequence
>>>>> with External Boost, where the driver does not correctly follow its
>>>>> enable sequence for this mode. This can cause additional issues
>>>>> such as
>>>>> pops and clicks.
>>>>>
>>>>> This chain intends to correct the sequences for playback and system
>>>>> suspend/resume so that the driver: obeys the external boost enable
>>>>> sequence;
>>>>> resumes audio on system resume; and avoids the race condition on
>>>>> firmware
>>>>> load and playback during system resume.
>>>>>
>>>>> Changes since v1:
>>>>> - Split patch 1 into 2 separate patches
>>>>> - Combine Patches 6 and 9
>>>>>
>>>>> Stefan Binding (11):
>>>>>     ALSA: cs35l41: Use mbox command to enable speaker output for
>>>>> external
>>>>>       boost
>>>>>     ALSA: cs35l41: Poll for Power Up/Down rather than waiting a fixed
>>>>>       delay
>>>>>     ALSA: hda: cs35l41: Check mailbox status of pause command after
>>>>>       firmware load
>>>>>     ALSA: hda: cs35l41: Ensure we correctly re-sync regmap before
>>>>> system
>>>>>       suspending.
>>>>>     ALSA: hda: cs35l41: Ensure we pass up any errors during system
>>>>>       suspend.
>>>>>     ALSA: hda: cs35l41: Move Play and Pause into separate functions
>>>>>     ALSA: hda: hda_component: Add pre and post playback hooks to
>>>>>       hda_component
>>>>>     ALSA: hda: cs35l41: Use pre and post playback hooks
>>>>>     ALSA: hda: cs35l41: Rework System Suspend to ensure correct call
>>>>>       separation
>>>>>     ALSA: hda: cs35l41: Add device_link between HDA and cs35l41_hda
>>>>>     ALSA: hda: cs35l41: Ensure amp is only unmuted during playback
>>>> Applied all patches now to for-next branch.
>>> It seems that this patch set causes occasional freeze at suspend:
>>>     https://bugzilla.suse.com/show_bug.cgi?id=1213745
>>>
>>> Could you take a look?
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>> Hi Takashi,
>>
>> The initial bug report shows one of the original issues that this patch
>> chain was trying to fix.
>>  From what I can tell from the second issue, something has caused the
>> CS35L41 to stop responding,
>> which in turn caused the system suspend call to fail, and the error is
>> passed up. Since system suspend
>> failed, there was no corresponding system resume, which means the
>> CS35L41 was stuck broken.
>>
>> I'm not sure what was meant by "freeze" and "overheating" in the bug
>> report, since the log seems to
>> indicate the laptop is still responsive, even if audio is broken.
>>
>> There is some oddity in the log, since one of the errors that was
>> printed should only be printed when
>> the CS35L41 is using External Boost, but I think this laptop is supposed
>> to use Internal Boost.
>> We'll investigate further.
>>
>> Thanks,
>>
>> Stefan Binding
>
> Hello,
>
> I would like to inform you that (one of) those patches is probably
> breaking sound on Lenovo Legion 7 16ACHg6. They were applied in Manjaro
> Linux kernel here:
>
> https://gitlab.manjaro.org/packages/core/linux64/-/commit/742e66f525170fe02dec42e47aedf53d3dc85195
>
> and when I install this kernel, there is no sound anymore. There is
> nothing more than those patches in this commit. A kernel compiled from
> the previous commit works fine.
>
> Sound used to work fine from 5.17 if I remember correctly.
>
> I hope I replied correctly to this thread, if not, apologies.
>
> Best Regards,
>
> Waldek Andrukiewicz
>
>
Hi,

This is a different issue that we have been able to reproduce.
It only affects that particular laptop, and is unrelated to the other issue.
We are currently working on a fix for this.

Thanks,

Stefan

[PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Stefan Binding]

There have been a couple of customer reports of intermittant issues after
system resume, where sometimes the DSP firmware stops responding.
Investigations into this issue show that there is a race between receiving
a prepare from the HDA core, and the firmware reload which is started by
the system resume. This can causes the Global Enable on the CS35L41 to be
enabled during the firmware load, which can sometimes cause issues in the
DSP.

The existing system resume behaviour also did not resume the audio, if
audio was previously playing when it was suspended.
In addition, during investigation, it was found there were additional
problems in the System Resume sequence, as well as the Playback sequence
with External Boost, where the driver does not correctly follow its
enable sequence for this mode. This can cause additional issues such as
pops and clicks.

This chain intends to correct the sequences for playback and system
suspend/resume so that the driver: obeys the external boost enable sequence;
resumes audio on system resume; and avoids the race condition on firmware
load and playback during system resume.

Changes since v1:
- Split patch 1 into 2 separate patches
- Combine Patches 6 and 9

Stefan Binding (11):
  ALSA: cs35l41: Use mbox command to enable speaker output for external
    boost
  ALSA: cs35l41: Poll for Power Up/Down rather than waiting a fixed
    delay
  ALSA: hda: cs35l41: Check mailbox status of pause command after
    firmware load
  ALSA: hda: cs35l41: Ensure we correctly re-sync regmap before system
    suspending.
  ALSA: hda: cs35l41: Ensure we pass up any errors during system
    suspend.
  ALSA: hda: cs35l41: Move Play and Pause into separate functions
  ALSA: hda: hda_component: Add pre and post playback hooks to
    hda_component
  ALSA: hda: cs35l41: Use pre and post playback hooks
  ALSA: hda: cs35l41: Rework System Suspend to ensure correct call
    separation
  ALSA: hda: cs35l41: Add device_link between HDA and cs35l41_hda
  ALSA: hda: cs35l41: Ensure amp is only unmuted during playback

 include/sound/cs35l41.h        |   5 +-
 sound/pci/hda/cs35l41_hda.c    | 288 +++++++++++++++++++++++++--------
 sound/pci/hda/hda_component.h  |   2 +
 sound/pci/hda/patch_realtek.c  |  10 +-
 sound/soc/codecs/cs35l41-lib.c | 118 ++++++++++++--
 sound/soc/codecs/cs35l41.c     |  18 +--
 6 files changed, 343 insertions(+), 98 deletions(-)

-- 
2.34.1

Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Takashi Iwai]

On Fri, 21 Jul 2023 17:18:05 +0200,
Stefan Binding wrote:
> 
> There have been a couple of customer reports of intermittant issues after
> system resume, where sometimes the DSP firmware stops responding.
> Investigations into this issue show that there is a race between receiving
> a prepare from the HDA core, and the firmware reload which is started by
> the system resume. This can causes the Global Enable on the CS35L41 to be
> enabled during the firmware load, which can sometimes cause issues in the
> DSP.
> 
> The existing system resume behaviour also did not resume the audio, if
> audio was previously playing when it was suspended.
> In addition, during investigation, it was found there were additional
> problems in the System Resume sequence, as well as the Playback sequence
> with External Boost, where the driver does not correctly follow its
> enable sequence for this mode. This can cause additional issues such as
> pops and clicks.
> 
> This chain intends to correct the sequences for playback and system
> suspend/resume so that the driver: obeys the external boost enable sequence;
> resumes audio on system resume; and avoids the race condition on firmware
> load and playback during system resume.
> 
> Changes since v1:
> - Split patch 1 into 2 separate patches
> - Combine Patches 6 and 9
> 
> Stefan Binding (11):
>   ALSA: cs35l41: Use mbox command to enable speaker output for external
>     boost
>   ALSA: cs35l41: Poll for Power Up/Down rather than waiting a fixed
>     delay
>   ALSA: hda: cs35l41: Check mailbox status of pause command after
>     firmware load
>   ALSA: hda: cs35l41: Ensure we correctly re-sync regmap before system
>     suspending.
>   ALSA: hda: cs35l41: Ensure we pass up any errors during system
>     suspend.
>   ALSA: hda: cs35l41: Move Play and Pause into separate functions
>   ALSA: hda: hda_component: Add pre and post playback hooks to
>     hda_component
>   ALSA: hda: cs35l41: Use pre and post playback hooks
>   ALSA: hda: cs35l41: Rework System Suspend to ensure correct call
>     separation
>   ALSA: hda: cs35l41: Add device_link between HDA and cs35l41_hda
>   ALSA: hda: cs35l41: Ensure amp is only unmuted during playback

Applied all patches now to for-next branch.


thanks,

Takashi

Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Takashi Iwai]

On Mon, 24 Jul 2023 11:00:38 +0200,
Takashi Iwai wrote:
> 
> On Fri, 21 Jul 2023 17:18:05 +0200,
> Stefan Binding wrote:
> > 
> > There have been a couple of customer reports of intermittant issues after
> > system resume, where sometimes the DSP firmware stops responding.
> > Investigations into this issue show that there is a race between receiving
> > a prepare from the HDA core, and the firmware reload which is started by
> > the system resume. This can causes the Global Enable on the CS35L41 to be
> > enabled during the firmware load, which can sometimes cause issues in the
> > DSP.
> > 
> > The existing system resume behaviour also did not resume the audio, if
> > audio was previously playing when it was suspended.
> > In addition, during investigation, it was found there were additional
> > problems in the System Resume sequence, as well as the Playback sequence
> > with External Boost, where the driver does not correctly follow its
> > enable sequence for this mode. This can cause additional issues such as
> > pops and clicks.
> > 
> > This chain intends to correct the sequences for playback and system
> > suspend/resume so that the driver: obeys the external boost enable sequence;
> > resumes audio on system resume; and avoids the race condition on firmware
> > load and playback during system resume.
> > 
> > Changes since v1:
> > - Split patch 1 into 2 separate patches
> > - Combine Patches 6 and 9
> > 
> > Stefan Binding (11):
> >   ALSA: cs35l41: Use mbox command to enable speaker output for external
> >     boost
> >   ALSA: cs35l41: Poll for Power Up/Down rather than waiting a fixed
> >     delay
> >   ALSA: hda: cs35l41: Check mailbox status of pause command after
> >     firmware load
> >   ALSA: hda: cs35l41: Ensure we correctly re-sync regmap before system
> >     suspending.
> >   ALSA: hda: cs35l41: Ensure we pass up any errors during system
> >     suspend.
> >   ALSA: hda: cs35l41: Move Play and Pause into separate functions
> >   ALSA: hda: hda_component: Add pre and post playback hooks to
> >     hda_component
> >   ALSA: hda: cs35l41: Use pre and post playback hooks
> >   ALSA: hda: cs35l41: Rework System Suspend to ensure correct call
> >     separation
> >   ALSA: hda: cs35l41: Add device_link between HDA and cs35l41_hda
> >   ALSA: hda: cs35l41: Ensure amp is only unmuted during playback
> 
> Applied all patches now to for-next branch.

It seems that this patch set causes occasional freeze at suspend:
  https://bugzilla.suse.com/show_bug.cgi?id=1213745

Could you take a look?


thanks,

Takashi

Re: [PATCH v2 00/11] Fix support for System Suspend for CS35L41 HDA

[Stefan Binding]

On 28/07/2023 12:10, Takashi Iwai wrote:
> On Mon, 24 Jul 2023 11:00:38 +0200,
> Takashi Iwai wrote:
>> On Fri, 21 Jul 2023 17:18:05 +0200,
>> Stefan Binding wrote:
>>> There have been a couple of customer reports of intermittant issues after
>>> system resume, where sometimes the DSP firmware stops responding.
>>> Investigations into this issue show that there is a race between receiving
>>> a prepare from the HDA core, and the firmware reload which is started by
>>> the system resume. This can causes the Global Enable on the CS35L41 to be
>>> enabled during the firmware load, which can sometimes cause issues in the
>>> DSP.
>>>
>>> The existing system resume behaviour also did not resume the audio, if
>>> audio was previously playing when it was suspended.
>>> In addition, during investigation, it was found there were additional
>>> problems in the System Resume sequence, as well as the Playback sequence
>>> with External Boost, where the driver does not correctly follow its
>>> enable sequence for this mode. This can cause additional issues such as
>>> pops and clicks.
>>>
>>> This chain intends to correct the sequences for playback and system
>>> suspend/resume so that the driver: obeys the external boost enable sequence;
>>> resumes audio on system resume; and avoids the race condition on firmware
>>> load and playback during system resume.
>>>
>>> Changes since v1:
>>> - Split patch 1 into 2 separate patches
>>> - Combine Patches 6 and 9
>>>
>>> Stefan Binding (11):
>>>    ALSA: cs35l41: Use mbox command to enable speaker output for external
>>>      boost
>>>    ALSA: cs35l41: Poll for Power Up/Down rather than waiting a fixed
>>>      delay
>>>    ALSA: hda: cs35l41: Check mailbox status of pause command after
>>>      firmware load
>>>    ALSA: hda: cs35l41: Ensure we correctly re-sync regmap before system
>>>      suspending.
>>>    ALSA: hda: cs35l41: Ensure we pass up any errors during system
>>>      suspend.
>>>    ALSA: hda: cs35l41: Move Play and Pause into separate functions
>>>    ALSA: hda: hda_component: Add pre and post playback hooks to
>>>      hda_component
>>>    ALSA: hda: cs35l41: Use pre and post playback hooks
>>>    ALSA: hda: cs35l41: Rework System Suspend to ensure correct call
>>>      separation
>>>    ALSA: hda: cs35l41: Add device_link between HDA and cs35l41_hda
>>>    ALSA: hda: cs35l41: Ensure amp is only unmuted during playback
>> Applied all patches now to for-next branch.
> It seems that this patch set causes occasional freeze at suspend:
>    https://bugzilla.suse.com/show_bug.cgi?id=1213745
>
> Could you take a look?
>
>
> thanks,
>
> Takashi

Hi Takashi,

The initial bug report shows one of the original issues that this patch 
chain was trying to fix.
 From what I can tell from the second issue, something has caused the 
CS35L41 to stop responding,
which in turn caused the system suspend call to fail, and the error is 
passed up. Since system suspend
failed, there was no corresponding system resume, which means the 
CS35L41 was stuck broken.

I'm not sure what was meant by "freeze" and "overheating" in the bug 
report, since the log seems to
indicate the laptop is still responsive, even if audio is broken.

There is some oddity in the log, since one of the errors that was 
printed should only be printed when
the CS35L41 is using External Boost, but I think this laptop is supposed 
to use Internal Boost.
We'll investigate further.

Thanks,

Stefan Binding