[PATCH] ASoC: tas2783-sdw: restore the volume controls after re-initialisation

[PATCH] ASoC: tas2783-sdw: restore the volume controls after re-initialisation

[Lee Mann]

When the SoundWire controller is power-gated during system suspend, the
amplifier loses its register state and tas_update_status() re-runs
tas_io_init() on re-attach, which resets the device and reloads the
firmware. Since commit b627da430357 ("ASoC: tas2783-sdw: drop stale
regcache on uninitialized re-attach") the cache is dropped first, and
that commit notes that cached user settings then fall back to hardware
defaults. Before it, the cache was synced back before the reset, which
wiped the values again, so they were lost either way.

For the two volume controls this is audible: the digital volume comes
back at its default, full scale (0 dB), while userspace still holds the
level it set before suspend and does not write it again. The first
sound after resume plays at maximum volume until the user changes the
volume. On an ASUS ProArt PX13 with PipeWire at 48% the "Speaker Volume"
control reads 200 after resume instead of 162.

Remember the register values written through the "Amp Volume" and
"Speaker Volume" controls and write them back once the init sequence
has completed. Only values userspace has set are restored, so the
first initialisation is unchanged.

Fixes: 4cc9bd8d7b32 ("ASoc: tas2783A: Add soundwire based codec driver")
Assisted-by: LLM
Signed-off-by: Lee Mann <leepaulmann@gmail.com>
---
Notes (not for the changelog):

AI assistance: I worked on this with Claude Code, as I don't normally
get this low level.

It drafted the code and the changelog from my measurements of the problem;
I reviewed the change, built it and tested it on my machine as described
below.

Testing: ASUS ProArt PX13 HN7306EA (two TAS2783 on SoundWire link 1, AMD
ACP), v7.3-rc6 with this patch applied to the in-tree driver, PipeWire at
48%. After s2idle, "tas2783-N Speaker Volume" read 200 without the patch
and 162 with it (the value set before suspend), with nothing done in
userspace; stereo playback is unaffected. checkpatch --strict is clean
apart from the Fixes/commit references checkpatch cannot resolve in my
tree.

This posting is rebased onto broonie/sound for-next (686b3eff60). The only
conflict was the new "Re-active AMP after resume" write in tas_io_init(),
which the restore now follows. The rebased driver (for-next's
tas2783-sdw.c with this patch, W=1 clean) was also run on the same
machine: after ~60 s of s2idle both amps read 162 again, matching
PipeWire. for-next does not have 173cb3147ba2 ("ASoC: tas2783-sdw: split a
stereo stream across the two mono amps") yet, so only the left speaker
played on that build; the volume check covered both amps.

 sound/soc/codecs/tas2783-sdw.c | 60 +++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index a57230e01f..5235a7ddc4 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -84,6 +84,14 @@ struct tas2783_prv {
 	struct calibration_data cali_data;
 	struct sdw_slave *sdw_peripheral;
 	struct sdca_function_data *sa_func_data;
+	/*
+	 * Last values userspace wrote to the two volume controls. Re-applied
+	 * after tas_io_init(), which resets the device and drops the cache.
+	 */
+	unsigned int user_amp_lvl;
+	unsigned int user_dvc_lvl;
+	bool user_amp_set;
+	bool user_dvc_set;
 	enum sdw_slave_status status;
 	/* calibration */
 	struct mutex calib_lock;
@@ -536,10 +544,25 @@ static s32 tas2783_digital_getvol(struct snd_kcontrol *kcontrol,
 	return snd_soc_get_volsw(kcontrol, ucontrol);
 }
 
+static void tas2783_save_vol(struct tas2783_prv *tas_dev, unsigned int reg,
+			     unsigned int *val, bool *set)
+{
+	if (!regmap_read(tas_dev->regmap, reg, val))
+		*set = true;
+}
+
 static s32 tas2783_digital_putvol(struct snd_kcontrol *kcontrol,
 				  struct snd_ctl_elem_value *ucontrol)
 {
-	return snd_soc_put_volsw(kcontrol, ucontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct tas2783_prv *tas_dev = snd_soc_component_get_drvdata(component);
+	s32 ret = snd_soc_put_volsw(kcontrol, ucontrol);
+
+	if (ret >= 0)
+		tas2783_save_vol(tas_dev, TAS2783_DVC_LVL,
+				 &tas_dev->user_dvc_lvl, &tas_dev->user_dvc_set);
+
+	return ret;
 }
 
 static s32 tas2783_amp_getvol(struct snd_kcontrol *kcontrol,
@@ -551,7 +574,34 @@ static s32 tas2783_amp_getvol(struct snd_kcontrol *kcontrol,
 static s32 tas2783_amp_putvol(struct snd_kcontrol *kcontrol,
 			      struct snd_ctl_elem_value *ucontrol)
 {
-	return snd_soc_put_volsw(kcontrol, ucontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct tas2783_prv *tas_dev = snd_soc_component_get_drvdata(component);
+	s32 ret = snd_soc_put_volsw(kcontrol, ucontrol);
+
+	if (ret >= 0)
+		tas2783_save_vol(tas_dev, TAS2783_AMP_LEVEL,
+				 &tas_dev->user_amp_lvl, &tas_dev->user_amp_set);
+
+	return ret;
+}
+
+static void tas2783_restore_vol(struct tas2783_prv *tas_dev)
+{
+	int ret;
+
+	if (tas_dev->user_amp_set) {
+		ret = regmap_write(tas_dev->regmap, TAS2783_AMP_LEVEL,
+				   tas_dev->user_amp_lvl);
+		if (ret)
+			dev_warn(tas_dev->dev, "amp level restore failed: %d\n", ret);
+	}
+
+	if (tas_dev->user_dvc_set) {
+		ret = regmap_write(tas_dev->regmap, TAS2783_DVC_LVL,
+				   tas_dev->user_dvc_lvl);
+		if (ret)
+			dev_warn(tas_dev->dev, "volume restore failed: %d\n", ret);
+	}
 }
 
 static const struct snd_kcontrol_new tas2783_snd_controls[] = {
@@ -1245,11 +1295,13 @@ static s32 tas_io_init(struct device *dev, struct sdw_slave *slave)
 
 		/* Re-active AMP after resume. */
 		regmap_write(tas_dev->regmap, TASDEV_REG_SDW(0, 0, 2), 0);
-		if (ret)
+		if (ret) {
 			dev_err(tas_dev->dev,
 				"init writes failed, err=%d", ret);
-		else
+		} else {
 			tas_dev->hw_init = true;
+			tas2783_restore_vol(tas_dev);
+		}
 	}
 
 	return ret;

base-commit: 686b3eff603871bd500434e00ff15394a0c782bc
-- 
2.55.0