Re: [PATCH 3/3] ASoC: tas2783: drop firmware-owned registers from the regmap cache

Thank you for the time on this.

Context you may not have seen: I withdrew this series on 11 August, in
<[email protected]>, so none of what follows is
an argument for the patch. It is just the answers, in case they save
somebody a day. No credit or attribution is wanted for any of it -- take,
adapt or drop it as suits.

The numbers below come from the image this laptop loads,
ti/audio/tas2783/8EA1-0-0-0x9.bin.zst, parsed with the driver's own
header layout: 52 files, 39606 distinct destination addresses.

> Are there any controls with values that come from the firmware,

Both of them. The driver registers two, "Amp Volume" at
TAS2783_AMP_LEVEL (0x800003) and "Speaker Volume" at TAS2783_DVC_LVL
(0x80001a). File 19 of the image writes 8 bytes at 0x800003 and file 24
writes 4 bytes at 0x80001a, so both control registers are firmware
owned, and one of them disagrees with the cache: the image puts 0x12
there, an Amp Volume of 9 out of 20, where tas2783_reg_default[] holds
0x28, which is 20.

It is not confined to the controls. 69 of the 70 reg_defaults entries
fall inside a range this image writes -- only 0x800021 does not -- and
23 of those get a value different from the table. So the cache and the
device disagree from the moment the download finishes, suspend or no
suspend.

> and shouldn't we do this before rather than after the firmware
> download?

Yes, before is better, and I have no defence for after. The loop breaks
on a failed write, so doing it afterwards leaves the cache asserting
defaults over a range that may already be partly overwritten, and
tas2783_fw_ready() is an async callback, so a control read can race the
download and pick up a stale cached value. Nothing about it requires
the drop to come second.

> I think a more robust fix is probably a combination of writing the
> firmware values into the cache and marking registers that get
> programmed by firmware download but should never be cached for
> whatever reason as volatile.

The volatile half fits this part well. The caching half is where the
image is awkward: nearly all of those 39606 addresses are DSP memory.
File 0 is 32632 bytes at 0x00c60088, which is PRAM_ADDR_START, and file
1 is 6904 bytes at 0x800108, which is YRAM_ADDR_START. Neither is a
readable register by the driver's own definition, since
tas2783_sdca_mbq_size() returns 0 outside 0x800000-0x803fff and
0x807e80-0x807eff and readable_reg is mbq_size > 0; and the regmap sets
use_single_write, so caching them would turn one bulk
sdw_nwrite_no_pm() into a byte-at-a-time replay of the whole image on
every regcache_sync().

Split the way you describe -- the page 0 config block cached, the two
RAM ranges volatile -- it does look like the right shape for this
device. One consequence worth flagging to whoever writes it: if the
page 0 registers end up volatile rather than cached, almost the entire
reg_defaults table is dead and should go with them.

As with the series itself: the analysis above was done with AI
assistance (Claude Opus 5). The image figures are from the shipped
binary and the driver source, so they are checkable from both.

Ville