2023-10-26 15:08:08

by Stefan Binding

[permalink] [raw]
Subject: [PATCH v1 0/8] System Suspend fixes and improvements for CS35L41 HDA

There is a report of a single laptop which uses CS35L41 HDA having an
issue with System Suspend. This particular laptop uses S3 (Deep) Sleep.
The reported issue states that when the laptop resumes from a system
suspend, audio no longer works.

The root cause of this issue is due to the CS35L41 being returned to us
in an unexpected state after a suspend/resume cycle.
When the driver resumes, it expects the parts to have been reset, which
leads to issues with audio and firmware loading.

To prevent this issue, and the possibility of similar issues, patches
2-5 force the driver to reset during probe, system suspend, and system
resume, which ensures that the part is always in the correct state.
Patches 6-8 are improvements in the suspend and firmware loading code,
which makes it easier to detect issues in the future, as well as
simplifiying the suspend code.

Patch 1 is a fix for an incorrect configuration for the HP Zbook Fury
17, which is the laptop which had the original issue.

Stefan Binding (8):
ALSA: hda: cs35l41: Use reset label to get GPIO for HP Zbook Fury 17
G9
ALSA: hda: cs35l41: Assert reset before system suspend
ALSA: hda: cs35l41: Assert Reset prior to de-asserting in probe and
system resume
ALSA: hda: cs35l41: Run boot process during resume callbacks
ALSA: hda: cs35l41: Force a software reset after hardware reset
ALSA: hda: cs35l41: Do not unload firmware before reset in system
suspend
ALSA: hda: cs35l41: Check CSPL state after loading firmware
ASoC: cs35l41: Detect CSPL errors when sending CSPL commands

include/sound/cs35l41.h | 3 +
sound/pci/hda/cs35l41_hda.c | 170 +++++++++++++++++----------
sound/pci/hda/cs35l41_hda_property.c | 11 +-
sound/soc/codecs/cs35l41-lib.c | 6 +
4 files changed, 124 insertions(+), 66 deletions(-)

--
2.34.1


2023-10-26 15:08:21

by Stefan Binding

[permalink] [raw]
Subject: [PATCH v1 7/8] ALSA: hda: cs35l41: Check CSPL state after loading firmware

CSPL firmware should be in RUNNING or PAUSED state after loading.
If not, the firmware has not been loaded correctly, and we can unload
it and pass the error up.

Signed-off-by: Stefan Binding <[email protected]>
---
sound/pci/hda/cs35l41_hda.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index 69303888be1a..496ff6a9d300 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -994,6 +994,7 @@ static int cs35l41_runtime_resume(struct device *dev)

static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41)
{
+ unsigned int fw_status;
__be32 halo_sts;
int ret;

@@ -1027,6 +1028,23 @@ static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41)
goto clean_dsp;
}

+ ret = regmap_read(cs35l41->regmap, CS35L41_DSP_MBOX_2, &fw_status);
+ if (ret < 0) {
+ dev_err(cs35l41->dev,
+ "Failed to read firmware status: %d\n", ret);
+ goto clean_dsp;
+ }
+
+ switch (fw_status) {
+ case CSPL_MBOX_STS_RUNNING:
+ case CSPL_MBOX_STS_PAUSED:
+ break;
+ default:
+ dev_err(cs35l41->dev, "Firmware status is invalid: %u\n",
+ fw_status);
+ goto clean_dsp;
+ }
+
ret = cs35l41_set_cspl_mbox_cmd(cs35l41->dev, cs35l41->regmap, CSPL_MBOX_CMD_PAUSE);
if (ret) {
dev_err(cs35l41->dev, "Error waiting for DSP to pause: %u\n", ret);
--
2.34.1

2023-10-26 16:03:35

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH v1 0/8] System Suspend fixes and improvements for CS35L41 HDA

On Thu, 26 Oct 2023 17:05:50 +0200,
Stefan Binding wrote:
>
> There is a report of a single laptop which uses CS35L41 HDA having an
> issue with System Suspend. This particular laptop uses S3 (Deep) Sleep.
> The reported issue states that when the laptop resumes from a system
> suspend, audio no longer works.
>
> The root cause of this issue is due to the CS35L41 being returned to us
> in an unexpected state after a suspend/resume cycle.
> When the driver resumes, it expects the parts to have been reset, which
> leads to issues with audio and firmware loading.
>
> To prevent this issue, and the possibility of similar issues, patches
> 2-5 force the driver to reset during probe, system suspend, and system
> resume, which ensures that the part is always in the correct state.
> Patches 6-8 are improvements in the suspend and firmware loading code,
> which makes it easier to detect issues in the future, as well as
> simplifiying the suspend code.
>
> Patch 1 is a fix for an incorrect configuration for the HP Zbook Fury
> 17, which is the laptop which had the original issue.
>
> Stefan Binding (8):
> ALSA: hda: cs35l41: Use reset label to get GPIO for HP Zbook Fury 17
> G9
> ALSA: hda: cs35l41: Assert reset before system suspend
> ALSA: hda: cs35l41: Assert Reset prior to de-asserting in probe and
> system resume
> ALSA: hda: cs35l41: Run boot process during resume callbacks
> ALSA: hda: cs35l41: Force a software reset after hardware reset
> ALSA: hda: cs35l41: Do not unload firmware before reset in system
> suspend
> ALSA: hda: cs35l41: Check CSPL state after loading firmware
> ASoC: cs35l41: Detect CSPL errors when sending CSPL commands

Applied to for-next branch now. Thanks.


Takashi