Re: ASoC: tas2783-sdw: calibration firmware not re-downloaded after s2idle resume (AMD ACP SoundWire, ASUS ProArt PX13)

[Andrey Golovko]

Hi Antoine,

Same machine here (ASUS ProArt PX13 HN7306EAC, ACP rev 0x70, RT721 +
2x TAS2783 on link 1).

First, the reason you never saw a re-download at all: on <= v7.2-rc3 the
peripherals do not re-attach after s2idle, so tas_update_status() is
never called with ATTACHED and nothing downstream of it can run. That is
a separate ACP bug, fixed in v7.2-rc4 by

  5893013efabb ("ASoC: amd: ps: disable MSI on resume in ACP PCI driver")

Details in my reply on your other thread [1].

With that fix in place, the firmware IS re-downloaded on every resume and
the one-shot ->hw_init gate is not the blocker: tas_update_status() sets
hw_init = false on UNATTACHED, so the ATTACHED transition re-runs
tas_io_init() and request_firmware_nowait() as intended. I confirmed the
download really happens over the wire with ftrace: ~81k calls to
amd_sdw_send_cmd_get_resp() during a single resume, i.e. an honest full
re-download of both amps, not a cached no-op.

One trap worth naming, since it fooled me for a while: in the resume path
printk timestamps make the download look impossible (32 KB in ~150 us,
5 ns/byte), because sched_clock is not running yet that early. Do not
trust the log deltas there - use ftrace.

So the symptom you filed is real, but the mechanism is elsewhere. On this
board, after a resume with genuine deep S0i3 residency (51 s of a 57 s
sleep, per /sys/kernel/debug/amd_pmc/s0ix_stats), all three peripherals
are Attached, the firmware is reloaded, every log line is clean - and the
speakers are silent.

Root cause #1: stale regmap cache
---------------------------------
In tas_update_status(), on the UNATTACHED -> ATTACHED transition, the
driver does:

	regcache_cache_only(tas_dev->regmap, false);
	regcache_sync(tas_dev->regmap);
	/* then */
	return tas_io_init(&slave->dev, slave);

Two problems: the sync runs *before* tas_io_init(), which performs a
software reset and thus wipes whatever was just written; and there is no
regcache_mark_dirty(), so the sync is close to a no-op to begin with.
The cache therefore survives the power cycle claiming that the DAPM
power/unmute bits and the SDCA PDE entity are already at their target
values. Every subsequent regmap_update_bits() - amp power-up from DAPM,
PDE programming at stream start - sees "no change" and skips the write.
The amplifier stays powered down and nothing in the log complains.

Consistent with that: unbind/rebind of slave-tas2783 (fresh cache)
restores sound immediately, while toggling mixer controls does not.

What does not work as a fix: adding regcache_mark_dirty() + a real
regcache_sync() after tas_io_init(). This regmap is SDW-MBQ
(devm_regmap_init_sdw_mbq_cfg) with per-register mbq_size, and the cache
also holds non-MBQ registers written during init, so regcache_sync()
fails with -EINVAL on the first such register, and the failure then also
breaks probe ("Update Slave status failed: -22"). A full sync is simply
not usable for this regmap.

What does work here: drop the stale cache instead of syncing it, i.e.
regcache_drop_region(0, UINT_MAX) before tas_io_init() when the device
re-attaches uninitialized, so the following update_bits() calls read real
hardware. Side effect: user-set controls fall back to hardware defaults
after a resume, which seems the lesser evil versus a silent amp.

I will send this as a proper patch. Note for Mark/TI: it will be based on
top of 0d6b2d6f93a6 ("ASoC: codecs: tas2783-sdw: Propagate
regcache_sync() errors") currently in for-next, which touches the same
two call sites.

Root cause #2: ACP SoundWire DMA config not reprogrammed on recovery
--------------------------------------------------------------------
Even with the cache fixed, sound only returns after the PCM is fully
recreated (pactl profile off/on), not after a plain userspace resume.
sound/soc/amd/ps/ps-sdw-dma.c advertises SNDRV_PCM_INFO_RESUME, so
userspace issues TRIGGER_RESUME (or prepare without hw_params), while the
ring-buffer registers (RINGBUFADDR/RINGBUFSIZE, watermark, IRQ masks) are
only programmed in hw_params. The DMA is then started with unprogrammed
registers: silence, followed by an XRUN. Intel does not set INFO_RESUME
on SoundWire for exactly this reason.

Dropping INFO_RESUME plus reprogramming the DMA config from .prepare
still does not fully restore audio here, so something else is lost across
the power gate on the ACP/manager side. I will post that part separately,
with traces, addressed to the AMD folks - it does not belong in this
codec thread.

Happy to test patches on this hardware.

[1] https://lore.kernel.org/all/466a905d-8203-46d2-bfe4-a3b3f9b5d68b@xxxxxxxxxxxx/

Thanks,
Andrey