[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

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

[Matthew Schwartz]

On 10/16/26 3:28 AM, Lee Mann wrote:
> -	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);


Hi Lee,

I ran into the same bug bringing up the ASUS ROG Xbox Ally X20 (two
TAS2783 on one AMD link), and there's one more case this doesn't cover.

On for-next idle_bias_on is 0, so the amps runtime suspend whenever
nothing is playing. The cache is dropped on attach and after the
firmware download, so until something writes the volume registers
again they aren't cached, and snd_soc_get_volsw()/put_volsw() fail with
-EBUSY from regmap in cache-only mode while the amp is suspended. Your
restore fixes that once a value has been set during playback, but at
boot nothing has set it yet, so the first mixer accesses fail and every
boot logs "ASoC error (-16)" for both registers. On the X20 the link
is also powered off when idle, so the amps are re-initialized on every
wake, not just after system suspend.

I have a fix I was working on that addresses the same issue but keeps
both values in the driver instead of the register cache, so get/put
work while the amp is suspended and the values are programmed again
after every init and resume. It's part of a small series with two
other tas2783 fixes. I'll post it shortly with you on Cc with a
Reported-by on the relevant patch if that's okay with you.

Thanks,
Matt

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

[Lee Mann]

Hi Matt,

Thanks, that sounds like the better fix. A Reported-by is fine with me,
and please consider my patch superseded by your series.

One data point from the ProArt PX13 (two TAS2783 on one AMD ACP link):
with for-next plus my patch I didn't see the -16 errors at boot over
several boots, so this machine may not hit the idle runtime-suspend
path the way the X20 does. I'm happy to test your series here and send
a Tested-by.

Thanks,
Lee


On Sun, 18 Oct 2026 at 06:37, Matthew Schwartz
<matthew.schwartz@linux.dev> wrote:
>
> On 10/16/26 3:28 AM, Lee Mann wrote:
> > -     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);
>
>
> Hi Lee,
>
> I ran into the same bug bringing up the ASUS ROG Xbox Ally X20 (two
> TAS2783 on one AMD link), and there's one more case this doesn't cover.
>
> On for-next idle_bias_on is 0, so the amps runtime suspend whenever
> nothing is playing. The cache is dropped on attach and after the
> firmware download, so until something writes the volume registers
> again they aren't cached, and snd_soc_get_volsw()/put_volsw() fail with
> -EBUSY from regmap in cache-only mode while the amp is suspended. Your
> restore fixes that once a value has been set during playback, but at
> boot nothing has set it yet, so the first mixer accesses fail and every
> boot logs "ASoC error (-16)" for both registers. On the X20 the link
> is also powered off when idle, so the amps are re-initialized on every
> wake, not just after system suspend.
>
> I have a fix I was working on that addresses the same issue but keeps
> both values in the driver instead of the register cache, so get/put
> work while the amp is suspended and the values are programmed again
> after every init and resume. It's part of a small series with two
> other tas2783 fixes. I'll post it shortly with you on Cc with a
> Reported-by on the relevant patch if that's okay with you.
>
> Thanks,
> Matt