[PATCH] ASoC: tas2783-sdw: drop stale regcache on uninitialized re-attach

[PATCH] ASoC: tas2783-sdw: drop stale regcache on uninitialized re-attach

[Andrey Golovko]

When the peripheral re-attaches after the SoundWire controller was
power-gated during system suspend (s2idle reaching S0i3 on AMD ACP), the
amplifier has lost all of its register and DSP state. tas_update_status()
handles that by re-running tas_io_init(), which soft-resets the device
and re-downloads the firmware, but before doing so it syncs back a
register cache that still holds the pre-suspend values.

That sync is useless, since the soft reset immediately wipes whatever it
wrote, and it leaves the cache claiming that the amplifier is already
powered up and unmuted. Subsequent read-modify-write updates - DAPM
amplifier power-up, SDCA PDE transitions at stream start - then see "no
change" and skip the hardware write. Playback runs without a single
error while the speakers stay silent. Unbinding and rebinding the driver
restores audio, since probe starts from a fresh cache.

Drop the cache instead of syncing it when an uninitialized device
attaches, so that later accesses see the real hardware state.
regcache_mark_dirty() + regcache_sync() is not an option here: the cache
can also hold registers outside the SDCA MBQ map, written during the
init sequence, which the MBQ backend refuses to write back. The sync
then fails with -EINVAL and takes initialization down with it.

Cached user settings fall back to hardware defaults across such a power
loss, which seems clearly preferable to a silent amplifier - the device
is being reset and its firmware reloaded at this point anyway.

Tested on an ASUS ProArt PX13 HN7306EAC (AMD Strix Halo, ACP7.0, two
TAS2783 amplifiers plus RT721 on SoundWire link 1): the speakers work
after an s2idle resume with ~51 s of S0i3 residency, where previously
they stayed silent despite a complete firmware re-download.

Fixes: 4cc9bd8d7b32 ("ASoc: tas2783A: Add soundwire based codec driver")
Reported-by: Antoine Monnet <antoine@montane.tech>
Closes: https://lore.kernel.org/all/c66ae00a-e878-4af0-a05a-272e9574eaa5@montane.tech/
Signed-off-by: Andrey Golovko <andrey.golovko@gmail.com>
---
Based on broonie/sound for-next (asoc-next), i.e. on top of
0d6b2d6f93a6 ("ASoC: codecs: tas2783-sdw: Propagate regcache_sync()
errors"), which touches the same call site.

Tested on 7.2-rc4 plus the ACP MSI-on-resume fix 5893013efabb, which is
a prerequisite for the peripherals to re-attach at all on this board:
https://lore.kernel.org/all/466a905d-8203-46d2-bfe4-a3b3f9b5d68b@montane.tech/

 sound/soc/codecs/tas2783-sdw.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index db58c50e8a83..e62470671951 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -1216,7 +1216,6 @@ static s32 tas_update_status(struct sdw_slave *slave,
 {
 	struct tas2783_prv *tas_dev = dev_get_drvdata(&slave->dev);
 	struct device *dev = &slave->dev;
-	int ret;
 
 	dev_dbg(dev, "Peripheral status = %s",
 		status == SDW_SLAVE_UNATTACHED ? "unattached" :
@@ -1232,14 +1231,23 @@ static s32 tas_update_status(struct sdw_slave *slave,
 	if (tas_dev->hw_init || tas_dev->status != SDW_SLAVE_ATTACHED)
 		return 0;
 
-	/* updated the cache data to device */
 	regcache_cache_only(tas_dev->regmap, false);
-	ret = regcache_sync(tas_dev->regmap);
-	if (ret) {
-		regcache_cache_only(tas_dev->regmap, true);
-		regcache_mark_dirty(tas_dev->regmap);
-		return ret;
-	}
+
+	/*
+	 * The device is attaching uninitialized: either this is the first
+	 * attach, or it lost power (and with it all register and DSP state)
+	 * while the controller was power-gated during system suspend. The
+	 * cache still holds the pre-suspend values, and tas_io_init() below
+	 * soft-resets the device anyway, so syncing it back is both useless
+	 * and harmful: later read-modify-write updates would compare against
+	 * stale data and skip the hardware write.
+	 *
+	 * Drop the cache instead, so that subsequent accesses see the real
+	 * hardware state. regcache_mark_dirty() + regcache_sync() cannot be
+	 * used here: the cache may hold registers outside the SDCA MBQ map,
+	 * which the MBQ backend refuses to write back.
+	 */
+	regcache_drop_region(tas_dev->regmap, 0, UINT_MAX);
 
 	/* perform I/O transfers required for Slave initialization */
 	return tas_io_init(&slave->dev, slave);
-- 
2.53.0

Re: [PATCH] ASoC: tas2783-sdw: drop stale regcache on uninitialized re-attach

[Pierre-Louis Bossart]

On 7/27/26 10:35, Andrey Golovko wrote:
> When the peripheral re-attaches after the SoundWire controller was
> power-gated during system suspend (s2idle reaching S0i3 on AMD ACP), the
> amplifier has lost all of its register and DSP state. tas_update_status()
> handles that by re-running tas_io_init(), which soft-resets the device
> and re-downloads the firmware, but before doing so it syncs back a
> register cache that still holds the pre-suspend values.
> 
> That sync is useless, since the soft reset immediately wipes whatever it
> wrote, and it leaves the cache claiming that the amplifier is already
> powered up and unmuted. Subsequent read-modify-write updates - DAPM
> amplifier power-up, SDCA PDE transitions at stream start - then see "no
> change" and skip the hardware write. Playback runs without a single
> error while the speakers stay silent. Unbinding and rebinding the driver
> restores audio, since probe starts from a fresh cache.
> 
> Drop the cache instead of syncing it when an uninitialized device
> attaches, so that later accesses see the real hardware state.
> regcache_mark_dirty() + regcache_sync() is not an option here: the cache
> can also hold registers outside the SDCA MBQ map, written during the
> init sequence, which the MBQ backend refuses to write back. The sync
> then fails with -EINVAL and takes initialization down with it.
> 
> Cached user settings fall back to hardware defaults across such a power
> loss, which seems clearly preferable to a silent amplifier - the device
> is being reset and its firmware reloaded at this point anyway.
> 
> Tested on an ASUS ProArt PX13 HN7306EAC (AMD Strix Halo, ACP7.0, two
> TAS2783 amplifiers plus RT721 on SoundWire link 1): the speakers work
> after an s2idle resume with ~51 s of S0i3 residency, where previously
> they stayed silent despite a complete firmware re-download.
> 
> Fixes: 4cc9bd8d7b32 ("ASoc: tas2783A: Add soundwire based codec driver")
> Reported-by: Antoine Monnet <antoine@montane.tech>
> Closes: https://lore.kernel.org/all/c66ae00a-e878-4af0-a05a-272e9574eaa5@montane.tech/
> Signed-off-by: Andrey Golovko <andrey.golovko@gmail.com>
> ---
> Based on broonie/sound for-next (asoc-next), i.e. on top of
> 0d6b2d6f93a6 ("ASoC: codecs: tas2783-sdw: Propagate regcache_sync()
> errors"), which touches the same call site.
> 
> Tested on 7.2-rc4 plus the ACP MSI-on-resume fix 5893013efabb, which is
> a prerequisite for the peripherals to re-attach at all on this board:
> https://lore.kernel.org/all/466a905d-8203-46d2-bfe4-a3b3f9b5d68b@montane.tech/
> 
>  sound/soc/codecs/tas2783-sdw.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
> index db58c50e8a83..e62470671951 100644
> --- a/sound/soc/codecs/tas2783-sdw.c
> +++ b/sound/soc/codecs/tas2783-sdw.c
> @@ -1216,7 +1216,6 @@ static s32 tas_update_status(struct sdw_slave *slave,
>  {
>  	struct tas2783_prv *tas_dev = dev_get_drvdata(&slave->dev);
>  	struct device *dev = &slave->dev;
> -	int ret;
>  
>  	dev_dbg(dev, "Peripheral status = %s",
>  		status == SDW_SLAVE_UNATTACHED ? "unattached" :
> @@ -1232,14 +1231,23 @@ static s32 tas_update_status(struct sdw_slave *slave,
>  	if (tas_dev->hw_init || tas_dev->status != SDW_SLAVE_ATTACHED)
>  		return 0;
>  
> -	/* updated the cache data to device */
>  	regcache_cache_only(tas_dev->regmap, false);
> -	ret = regcache_sync(tas_dev->regmap);
> -	if (ret) {
> -		regcache_cache_only(tas_dev->regmap, true);
> -		regcache_mark_dirty(tas_dev->regmap);
> -		return ret;
> -	}

Agree that this sequence didn't make sense, but you have a set of
comments below that could be clearer.

> +
> +	/*
> +	 * The device is attaching uninitialized: either this is the first
> +	 * attach, or it lost power (and with it all register and DSP state)
> +	 * while the controller was power-gated during system suspend. The
> +	 * cache still holds the pre-suspend values, and tas_io_init() below
> +	 * soft-resets the device anyway, so syncing it back is both useless

you may want to clarify what 'soft-reset' means. This isn't a SoundWire
term, all forms of reset defined in the standard will require
re-enumeration. Some devices from Cirrus Logic perform a 'device reset'
and a second enumeration, if that was the case here then you could
end-up in a boot loop.

> +	 * and harmful: later read-modify-write updates would compare against
> +	 * stale data and skip the hardware write.
> +	 *
> +	 * Drop the cache instead, so that subsequent accesses see the real
> +	 * hardware state. regcache_mark_dirty() + regcache_sync() cannot be
> +	 * used here: the cache may hold registers outside the SDCA MBQ map,
> +	 * which the MBQ backend refuses to write back.

Not following this comment, there's a single cache with specific
registers tagged as requiring the MBQ-specific sequence with multiple
ordered read/writes. it doesn't matter whether the registers are in the
MBQ area and I don't know what the 'MBQ backend' refers to.

> +	 */
> +	regcache_drop_region(tas_dev->regmap, 0, UINT_MAX);
>  
>  	/* perform I/O transfers required for Slave initialization */
>  	return tas_io_init(&slave->dev, slave);

[PATCH v2] ASoC: tas2783-sdw: drop stale regcache on uninitialized re-attach

[Andrey Golovko]

When the peripheral re-attaches after the SoundWire controller was
power-gated during system suspend (s2idle reaching S0i3 on AMD ACP), the
amplifier has lost all of its register and DSP state. tas_update_status()
handles that by re-running tas_io_init(), which writes the device's
TAS2783_SW_RESET register - a vendor register write that clears the
device's register file and DSP state, not a SoundWire reset, so no
re-enumeration is involved - and re-downloads the firmware. Before doing
any of that, it syncs back a register cache that still holds the
pre-suspend values.

That sync is useless, since the reset immediately wipes whatever it
wrote, and it leaves the cache claiming that the amplifier is already
powered up and unmuted. Subsequent read-modify-write updates - DAPM
amplifier power-up, SDCA PDE transitions at stream start - then see "no
change" and skip the hardware write. Playback runs without a single
error while the speakers stay silent. Unbinding and rebinding the driver
restores audio, since probe starts from a fresh cache.

Drop the cache instead of syncing it when an uninitialized device
attaches, so that later accesses see the real hardware state.

Reordering the sync after tas_io_init() and marking the cache dirty is
not a workable alternative here: tas_regmap has no .writeable_reg, so
the cache accepts every register up to .max_register, including ones for
which tas2783_sdca_mbq_size() returns 0. regmap_sdw_mbq_size() rejects
those with -EINVAL, so the replay fails on the first such register and
takes initialization down with it.

Cached user settings fall back to hardware defaults across such a power
loss, which seems clearly preferable to a silent amplifier - the device
is being reset and its firmware reloaded at this point anyway.

Tested on an ASUS ProArt PX13 HN7306EAC (AMD Strix Halo, ACP7.0, two
TAS2783 amplifiers plus RT721 on SoundWire link 1): the speakers work
after an s2idle resume with ~51 s of S0i3 residency, where previously
they stayed silent despite a complete firmware re-download.

Fixes: 4cc9bd8d7b32 ("ASoc: tas2783A: Add soundwire based codec driver")
Reported-by: Antoine Monnet <antoine@montane.tech>
Closes: https://lore.kernel.org/all/c66ae00a-e878-4af0-a05a-272e9574eaa5@montane.tech/
Signed-off-by: Andrey Golovko <andrey.golovko@gmail.com>
---
Changes since v1 (thanks Pierre-Louis for the review):
 - describe the reset precisely: tas_io_init() writes the vendor
   TAS2783_SW_RESET register, which is not a SoundWire reset and does
   not trigger re-enumeration. 'soft reset' is gone from both the
   commit message and the code comment.
 - drop the vague 'MBQ backend' wording. The reason a sync is not
   usable is concrete: tas_regmap has no .writeable_reg, so the cache
   takes registers for which tas2783_sdca_mbq_size() returns 0, and
   regmap_sdw_mbq_size() then returns -EINVAL for them.
 - no functional change; the diff is the same modulo the comment.

Based on broonie/sound for-next, on top of 0d6b2d6f93a6 ("ASoC: codecs:
tas2783-sdw: Propagate regcache_sync() errors").

 sound/soc/codecs/tas2783-sdw.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index db58c50e8a83..f96a53a08175 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -1216,7 +1216,6 @@ static s32 tas_update_status(struct sdw_slave *slave,
 {
 	struct tas2783_prv *tas_dev = dev_get_drvdata(&slave->dev);
 	struct device *dev = &slave->dev;
-	int ret;
 
 	dev_dbg(dev, "Peripheral status = %s",
 		status == SDW_SLAVE_UNATTACHED ? "unattached" :
@@ -1232,14 +1231,23 @@ static s32 tas_update_status(struct sdw_slave *slave,
 	if (tas_dev->hw_init || tas_dev->status != SDW_SLAVE_ATTACHED)
 		return 0;
 
-	/* updated the cache data to device */
 	regcache_cache_only(tas_dev->regmap, false);
-	ret = regcache_sync(tas_dev->regmap);
-	if (ret) {
-		regcache_cache_only(tas_dev->regmap, true);
-		regcache_mark_dirty(tas_dev->regmap);
-		return ret;
-	}
+
+	/*
+	 * The device is attaching uninitialized: either this is the first
+	 * attach, or it lost power (and with it all register and DSP state)
+	 * while the controller was power-gated during system suspend. The
+	 * cache still holds the pre-suspend values, and tas_io_init() below
+	 * resets the device via TAS2783_SW_RESET anyway, so syncing it back
+	 * is both useless and harmful: later read-modify-write updates would
+	 * compare against stale data and skip the hardware write.
+	 *
+	 * Drop the cache instead, so that subsequent accesses see the real
+	 * hardware state. Syncing after the reset is not an option either:
+	 * the cache accepts registers for which tas2783_sdca_mbq_size()
+	 * returns 0, and writing those back fails with -EINVAL.
+	 */
+	regcache_drop_region(tas_dev->regmap, 0, UINT_MAX);
 
 	/* perform I/O transfers required for Slave initialization */
 	return tas_io_init(&slave->dev, slave);
-- 
2.53.0

Re: [PATCH v2] ASoC: tas2783-sdw: drop stale regcache on uninitialized re-attach

[Mark Brown]

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

On Mon, Jul 27, 2026 at 12:33:09PM +0300, Andrey Golovko wrote:
> When the peripheral re-attaches after the SoundWire controller was
> power-gated during system suspend (s2idle reaching S0i3 on AMD ACP), the
> amplifier has lost all of its register and DSP state. tas_update_status()

Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.

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