[PATCH 0/3] ASoC: tas2783: fix stereo split and resume on a two-amp pair

[PATCH 0/3] ASoC: tas2783: fix stereo split and resume on a two-amp pair

[Ville Saarinen]

Three fixes for the SoundWire TAS2783 driver, found while bringing up an
HP OmniBook X Flip 14-kc0xxx, which carries two TAS2783 amplifiers
aggregated on one link. Two of the three are not board-specific: as far as
I can tell any machine with an aggregated TAS2783 pair is affected today.

Patch 1 makes deferred SDCA writes work. A Function answering
COMMAND_IGNORED has deferred the transaction, and regmap-sdw-mbq is
supposed to poll Entity-0 Function Status and retry -- but this driver
neither makes that status register readable nor sets the poll interval and
deadline, so the poll is skipped and the retry is instantaneous. Every
deferred write fails by construction with -ENODATA.

Patch 2 makes an aggregated pair render stereo. Both amplifiers currently
play the same channel, so right-channel content is inaudible. The SDCA
control that would select the channel, the UDMPU23 Cluster Index, turns
out not to be implemented on this part -- a genuine device read of it
returns -ENODATA -- so the split is done host-side, by claiming a single
channel per amplifier and dropping the pair out of the SoundWire core's
mirror mode. That is exposed as a boolean "RX Single Channel Switch",
off by default, so the change is opt-in from the machine's UCM profile.

The switch deliberately does not name a side. sdw_compute_slave_ports()
advances the payload offset by the popcount of ch_mask and never looks at
which bit is set, so a driver cannot choose which channel an amplifier
renders; that follows from the amplifier's position in the slave iteration
order, i.e. from the machine driver's codec order. An earlier version of
this series exposed an rt1316/rt1318-style "RX Channel Select" enum with
Left/Right values, and I am glad it did not go out: those values were
measured to be inert, and the control would have promised an ABI the bus
allocator cannot honour.

Patch 3 stops the amplifiers losing their firmware tuning on resume.
Firmware is downloaded with sdw_nwrite_no_pm(), which bypasses the regmap
cache, so the cache keeps stale reg_defaults for every firmware-owned
register; the regcache_sync() on resume then writes those defaults back
over live firmware values. On the affected machine the speakers are dead
after every system resume and stay dead until reboot.

Patch 3 is the one I would most like reviewed on its own merits. It is the
smallest of the three, it is not specific to a stereo pair, and its
symptom is severe.

The three are independent and can be taken separately, though patch 2
builds on the mbq configuration touched by patch 1.

Testing
=======

All three are running on the affected machine and the whole stack works
from a cold boot with no manual steps, including across suspend/resume.

Patch 2 was measured rather than judged by ear: a 1 kHz tone that is
left-only for its first half and right-only for its second, played as one
continuous stream, isolating each amplifier by muting the other and
capturing on the machine's internal DMIC array, with every condition
normalised against both amplifiers muted. Each run was verified from the
kernel log to have re-run hw_params with the setting under test, since the
switch is read at hw_params and a sink that never suspends will not pick
up a change. Numbers are in that patch's changelog; briefly, with the
switch on each amplifier carries one side and the other side sits within
0.4 dB of the muted floor, and with it off both amplifiers render the same
left channel. Ear tests on this hardware are unreliable -- the two
speakers are close enough together that localisation gave a confidently
wrong answer more than once.

Patch 3's mechanism was confirmed by cache-bypassing debugfs register
reads taken while the amplifiers were dead, showing all 12 firmware-owned
registers reverted to their defaults. The fix has been through one
suspend/resume cycle with the speakers still working; that confirmation is
behavioural and I have not re-read the registers after a resume on the
fixed build.

Caveats
=======

The hardware testing above was done on v7.1.6 with clang. The three
patches apply to the master commit named below and have been compile-
tested there with gcc and W=1, with no new warnings; the series adds
exactly two new external references, regcache_drop_region (EXPORT_SYMBOL_
GPL) and snd_ctl_boolean_mono_info (EXPORT_SYMBOL), established by diffing
nm -u output against the unpatched objects. They have not been run on a
master kernel. I have exactly one TAS2783 machine, so the stereo split in
patch 2 is verified on one board with two amplifiers and nothing else.

Tool disclosure, per Documentation/process/generated-content.rst
================================================================

This work was done in extended interactive sessions with Claude (Anthropic,
model claude-opus-5) acting as a coding and debugging assistant, and a
substantial amount of the analysis and of the patch text originated with
it. All three patches carry an Assisted-by tag as described in
Documentation/process/coding-assistants.rst.

The division of work:

  - The assistant did the register-level analysis, read the relevant core
    code, formed the hypotheses, wrote the driver changes and the
    measurement harness, and drafted the changelogs.
  - I ran everything needing root or physical access, rebooted into each
    build, and ran the acoustic measurements.
  - No single prompt produced these patches. It was iterative over roughly
    a day, and a fair number of the assistant's intermediate conclusions
    were wrong: that one amplifier was dead, that the firmware page-0
    configuration set the channel, that the Cluster Index could be made to
    work from the driver, and -- latest and most relevant to what you are
    reading -- that an enum could assign a specific side to each
    amplifier. That last one survived into a fully drafted patch whose
    changelog claimed a per-side assignment, and was caught only by
    re-reading the bus allocator and then measuring both amplifiers set to
    the same value. Each wrong turn was discarded only because it was
    measured. The dead ends around the Cluster Index are summarised in
    patch 2's changelog, because they are the reason the split is done
    host-side.

I have reviewed all three patches, I understand what they do, and I take
responsibility for them.

Ville Saarinen (3):
  ASoC: tas2783: let regmap-sdw-mbq poll for deferred transactions
  ASoC: tas2783: add RX Single Channel Switch to split a two-amp stereo
    pair
  ASoC: tas2783: drop firmware-owned registers from the regmap cache

 sound/soc/codecs/tas2783-sdw.c | 157 +++++++++++++++++++++++++++++++++
 1 file changed, 157 insertions(+)


base-commit: 06cf61899d6498b33e4b7c87d99d5bd471ccc375
-- 
2.55.0

[PATCH 1/3] ASoC: tas2783: let regmap-sdw-mbq poll for deferred transactions

[Ville Saarinen]

In SDCA, a Function answering COMMAND_IGNORED to a Control write has
deferred the transaction rather than rejected it. regmap_sdw_mbq_write()
handles that by calling regmap_sdw_mbq_poll_busy(), which waits for the
Entity-0 Function Busy bit to clear and then retries once. Neither half
of that works for this codec:

 - poll_busy only polls if ->readable_reg() accepts the Entity-0 Function
   Status address. tas2783_readable_register() answers out of
   tas2783_sdca_mbq_size(), which has no case for that address, so the
   poll is skipped and the core falls through to a bare
   fsleep(cfg.timeout_us).

 - tas2783_mbq_cfg sets only .mbq_size, so timeout_us and retry_us are
   both 0. The fallback wait is fsleep(0) and the retry is therefore
   instantaneous.

Every deferred write consequently fails by construction, returning
-ENODATA and logging "Defer on undeferrable control".

Add the Function Status register to the mbq size table so the poll can
run, and to the volatile table so a later regcache_sync() never writes
back to a status register. Mark the UDMPU23 Cluster Index deferrable and
give the mbq cfg a poll interval and a deadline.

Note the two cfg fields reach read_poll_timeout() as (sleep_us,
timeout_us), i.e. .timeout_us is the poll interval and .retry_us the
overall deadline -- the reverse of the kerneldoc on struct
regmap_sdw_mbq_cfg. The values here follow the code, which is what runs.

Developed with AI assistance. The assistant traced the -ENODATA into
regmap_sdw_mbq_poll_busy() and drafted the fix.

All hardware measurements quoted above were run by the submitter on the
affected machine. The submitter has reviewed the change, understands it
and takes responsibility for it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Ville Saarinen <wiza@saarinenkoti.fi>
---
 sound/soc/codecs/tas2783-sdw.c | 37 ++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index 3d0b11654..5a7ac6224 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -423,6 +423,13 @@ static int tas2783_sdca_mbq_size(struct device *dev, u32 reg)
 	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x01, 0):
 	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x01, 1):
 	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT25, 0x04, 0):
+	/*
+	 * Entity 0 Function Status. regmap_sdw_mbq_poll_busy() only polls the
+	 * Function Busy bit if ->readable_reg() accepts this address; without
+	 * it the core drops into a bare fsleep(cfg.timeout_us) and, with that
+	 * left at 0, retries a deferred transaction instantly and fails.
+	 */
+	case SDW_SDCA_CTL(1, 0, SDCA_CTL_ENTITY_0_FUNCTION_STATUS, 0):
 		return 1;
 
 	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT26, 0x10, 0):
@@ -505,6 +512,8 @@ static bool tas2783_volatile_register(struct device *dev, u32 reg)
 	case 0x400 ... 0x440: /* Data port 4. */
 	case 0x500 ... 0x540: /* Data port 5. */
 	case 0x800001:
+	/* Function Status is a live status word; never let the cache hold it. */
+	case SDW_SDCA_CTL(1, 0, SDCA_CTL_ENTITY_0_FUNCTION_STATUS, 0):
 		return true;
 
 	default:
@@ -525,8 +534,36 @@ static const struct regmap_config tas_regmap = {
 	.use_single_write = true,
 };
 
+/*
+ * The amp answers COMMAND_IGNORED (-ENODATA) to a UDMPU23 Cluster Index write,
+ * which in SDCA terms means the Function deferred the transaction. Declaring it
+ * deferrable stops regmap-sdw-mbq warning about it and documents the intent;
+ * the poll-and-retry in regmap_sdw_mbq_write() runs either way.
+ */
+static bool tas2783_sdca_deferrable(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_UDMPU23,
+			  TAS2783_SDCA_CTL_UDMPU_CLUSTER, 0):
+		return true;
+
+	default:
+		return false;
+	}
+}
+
 static const struct regmap_sdw_mbq_cfg tas2783_mbq_cfg = {
 	.mbq_size = tas2783_sdca_mbq_size,
+	.deferrable = tas2783_sdca_deferrable,
+	/*
+	 * NB: regmap_sdw_mbq_poll_busy() passes these to read_poll_timeout() as
+	 * (sleep_us, timeout_us) -- i.e. timeout_us is the poll interval and
+	 * retry_us the overall deadline, the opposite of what the kerneldoc on
+	 * struct regmap_sdw_mbq_cfg says. Values below follow the code, which
+	 * is what actually runs (checked against mainline master too).
+	 */
+	.timeout_us = 1000,
+	.retry_us = 100000,
 };
 
 static s32 tas2783_digital_getvol(struct snd_kcontrol *kcontrol,
-- 
2.55.0

Re: [PATCH 1/3] ASoC: tas2783: let regmap-sdw-mbq poll for deferred transactions

[Pierre-Louis Bossart]

On 8/9/26 12:16, Ville Saarinen wrote:
> In SDCA, a Function answering COMMAND_IGNORED to a Control write has
> deferred the transaction rather than rejected it. regmap_sdw_mbq_write()
> handles that by calling regmap_sdw_mbq_poll_busy(), which waits for the
> Entity-0 Function Busy bit to clear and then retries once. Neither half
> of that works for this codec:
> 
>  - poll_busy only polls if ->readable_reg() accepts the Entity-0 Function
>    Status address. tas2783_readable_register() answers out of
>    tas2783_sdca_mbq_size(), which has no case for that address, so the
>    poll is skipped and the core falls through to a bare
>    fsleep(cfg.timeout_us).
> 
>  - tas2783_mbq_cfg sets only .mbq_size, so timeout_us and retry_us are
>    both 0. The fallback wait is fsleep(0) and the retry is therefore
>    instantaneous.
> 
> Every deferred write consequently fails by construction, returning
> -ENODATA and logging "Defer on undeferrable control".
> 
> Add the Function Status register to the mbq size table so the poll can
> run, and to the volatile table so a later regcache_sync() never writes
> back to a status register. Mark the UDMPU23 Cluster Index deferrable and
> give the mbq cfg a poll interval and a deadline.
> 
> Note the two cfg fields reach read_poll_timeout() as (sleep_us,
> timeout_us), i.e. .timeout_us is the poll interval and .retry_us the
> overall deadline -- the reverse of the kerneldoc on struct
> regmap_sdw_mbq_cfg. The values here follow the code, which is what runs.
> 
> Developed with AI assistance. The assistant traced the -ENODATA into
> regmap_sdw_mbq_poll_busy() and drafted the fix.
> 
> All hardware measurements quoted above were run by the submitter on the
> affected machine. The submitter has reviewed the change, understands it
> and takes responsibility for it.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Ville Saarinen <wiza@saarinenkoti.fi>
> ---
>  sound/soc/codecs/tas2783-sdw.c | 37 ++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
> index 3d0b11654..5a7ac6224 100644
> --- a/sound/soc/codecs/tas2783-sdw.c
> +++ b/sound/soc/codecs/tas2783-sdw.c
> @@ -423,6 +423,13 @@ static int tas2783_sdca_mbq_size(struct device *dev, u32 reg)
>  	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x01, 0):
>  	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x01, 1):
>  	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT25, 0x04, 0):
> +	/*
> +	 * Entity 0 Function Status. regmap_sdw_mbq_poll_busy() only polls the
> +	 * Function Busy bit if ->readable_reg() accepts this address; without
> +	 * it the core drops into a bare fsleep(cfg.timeout_us) and, with that
> +	 * left at 0, retries a deferred transaction instantly and fails.
> +	 */
> +	case SDW_SDCA_CTL(1, 0, SDCA_CTL_ENTITY_0_FUNCTION_STATUS, 0):
>  		return 1;

I don't think this is correct, sorry.

This point has been discussed quite extensively in the SDCA group and
the net result is that accesses to the FUNCTION_STATUS *cannot* be
deferred. You cannot e.g. get a COMMAND_IGNORED response that would set
the Function_Busy bit while trying to read the Function_Busy bit...

Put differently, deferred access is only permitted in specific cases,
and Function-level controls/status are not in that list.

See 10.2.8.1 Rules About Which SDCA Resources Can and Cannot Use
Deferred Access

Re: [PATCH 1/3] ASoC: tas2783: let regmap-sdw-mbq poll for deferred transactions

[Ville Saarinen]

Hi Bossart,

Thanks for taking the time to review this. I am a bit of a kernel noob, 
but I completely agree with you and the spec: FUNCTION_STATUS cannot be 
deferred.

I think my commit message caused a misunderstanding. The patch doesn't 
defer the status read:

The hardware is actually returning COMMAND_IGNORED for the UDMPU23 
Cluster Index write, which is the only control tas2783_sdca_deferrable() 
flags as true.

I only added FUNCTION_STATUS to mbq_size to expose it to -> 
readable_reg(). Without that, regmap_sdw_mbq_poll_busy() considers the 
status register unreadable, skips checking the busy bit for the deferred 
UDMPU23 write, and instantly fails.

My main goal was just getting audio working on my personal laptop, which 
is now resolved on my end. I will leave it at this and leave it to you 
to proceed, adapt, or handle the changes however you see fit.

Thanks again for your time,

Ville

On 8/9/26 17:05, Pierre-Louis Bossart wrote:
> On 8/9/26 12:16, Ville Saarinen wrote:
>> In SDCA, a Function answering COMMAND_IGNORED to a Control write has
>> deferred the transaction rather than rejected it. regmap_sdw_mbq_write()
>> handles that by calling regmap_sdw_mbq_poll_busy(), which waits for the
>> Entity-0 Function Busy bit to clear and then retries once. Neither half
>> of that works for this codec:
>>
>>   - poll_busy only polls if ->readable_reg() accepts the Entity-0 Function
>>     Status address. tas2783_readable_register() answers out of
>>     tas2783_sdca_mbq_size(), which has no case for that address, so the
>>     poll is skipped and the core falls through to a bare
>>     fsleep(cfg.timeout_us).
>>
>>   - tas2783_mbq_cfg sets only .mbq_size, so timeout_us and retry_us are
>>     both 0. The fallback wait is fsleep(0) and the retry is therefore
>>     instantaneous.
>>
>> Every deferred write consequently fails by construction, returning
>> -ENODATA and logging "Defer on undeferrable control".
>>
>> Add the Function Status register to the mbq size table so the poll can
>> run, and to the volatile table so a later regcache_sync() never writes
>> back to a status register. Mark the UDMPU23 Cluster Index deferrable and
>> give the mbq cfg a poll interval and a deadline.
>>
>> Note the two cfg fields reach read_poll_timeout() as (sleep_us,
>> timeout_us), i.e. .timeout_us is the poll interval and .retry_us the
>> overall deadline -- the reverse of the kerneldoc on struct
>> regmap_sdw_mbq_cfg. The values here follow the code, which is what runs.
>>
>> Developed with AI assistance. The assistant traced the -ENODATA into
>> regmap_sdw_mbq_poll_busy() and drafted the fix.
>>
>> All hardware measurements quoted above were run by the submitter on the
>> affected machine. The submitter has reviewed the change, understands it
>> and takes responsibility for it.
>>
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Ville Saarinen <wiza@saarinenkoti.fi>
>> ---
>>   sound/soc/codecs/tas2783-sdw.c | 37 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
>> index 3d0b11654..5a7ac6224 100644
>> --- a/sound/soc/codecs/tas2783-sdw.c
>> +++ b/sound/soc/codecs/tas2783-sdw.c
>> @@ -423,6 +423,13 @@ static int tas2783_sdca_mbq_size(struct device *dev, u32 reg)
>>   	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x01, 0):
>>   	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x01, 1):
>>   	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT25, 0x04, 0):
>> +	/*
>> +	 * Entity 0 Function Status. regmap_sdw_mbq_poll_busy() only polls the
>> +	 * Function Busy bit if ->readable_reg() accepts this address; without
>> +	 * it the core drops into a bare fsleep(cfg.timeout_us) and, with that
>> +	 * left at 0, retries a deferred transaction instantly and fails.
>> +	 */
>> +	case SDW_SDCA_CTL(1, 0, SDCA_CTL_ENTITY_0_FUNCTION_STATUS, 0):
>>   		return 1;
> I don't think this is correct, sorry.
>
> This point has been discussed quite extensively in the SDCA group and
> the net result is that accesses to the FUNCTION_STATUS *cannot* be
> deferred. You cannot e.g. get a COMMAND_IGNORED response that would set
> the Function_Busy bit while trying to read the Function_Busy bit...
>
> Put differently, deferred access is only permitted in specific cases,
> and Function-level controls/status are not in that list.
>
> See 10.2.8.1 Rules About Which SDCA Resources Can and Cannot Use
> Deferred Access
>

[PATCH 2/3] ASoC: tas2783: add RX Single Channel Switch to split a two-amp stereo pair

[Ville Saarinen]

A pair of TAS2783 amplifiers aggregated on one link renders the same
channel from both amplifiers, so a stereo stream is heard as the left
channel from both speakers and right-channel content is inaudible. On the
HP OmniBook X Flip 14-kc0xxx (board 8EA1) that is every stereo stream.

The SDCA control that would select the rendered channel, the UDMPU23
Cluster Index, is written only from tas2783_reg_default[] as 0x0 and
never per device -- but on this part it cannot be written at all. The
amplifier answers COMMAND_IGNORED (-ENODATA) in every state tried:
streaming, idle, and with the SDCA function confirmed actually powered on
(PDE23 Actual Power State == ON); as a 4-byte MBQ write, as a plain
single-byte write, after clearing the latched Entity-0 status bits, and
on the Next rank of a dual-ranked control. A genuine device read of the
same Control (sdw_read_no_pm) also returns -ENODATA, which is the tell:
it is not write-protected, it is not implemented.

Do the split on the host side instead, where it costs no device access.
sdw_compute_slave_ports() advances the payload block offset by one
channel per slave port, except that a slave whose ch_mask covers every
channel of the stream is treated as mirror mode and the offset is reset
for the next slave. snd_sdw_params_to_config() hands both amplifiers
ch_mask 0x3 on a 2-channel stream, so both were given the same block
offset -- confirmed on the wire, DP1_CHANNELEN 0x03 and DP1_OFFSETCTRL1
0x41 read identically on the two amplifiers during playback.

Claiming a single channel drops the pair out of mirror mode and the core
assigns consecutive offsets in sdw_stream_add_slave() order, i.e.
dai_link codec order. Expose that as a boolean "RX Single Channel
Switch", left off by default so an unconfigured card behaves exactly as
before and the split is opt-in from the machine's UCM profile.

Note what the control deliberately does not do. It selects *whether* an
amplifier takes one channel or mirrors the whole stream; it cannot select
*which* channel. sdw_compute_slave_ports() advances the offset by
"port_bo += bps * hweight32(p_rt->ch_mask)" -- the popcount of the mask
only, never which bit is set -- so BIT(0) and BIT(1) are indistinguishable
to the allocator and the side an amplifier renders is fixed by its
position in the slave iteration order, i.e. by the machine driver's codec
order. An earlier version of this patch exposed an "RX Channel Select"
enum with Left/Right values on the rt1316/rt1318 model; that was measured
to be inert in exactly that respect and would have promised an ABI the
bus cannot honour.

Measured on the affected machine with a 1 kHz tone that is left-only for
its first half and right-only for its second, isolating each amplifier by
muting the other and capturing on the internal DMIC array, with each
condition normalised to both amplifiers muted. Every run was verified from
the kernel log to have re-run hw_params with the new setting.

  With the switch on for both amplifiers:
    left content   +14.1 dB from amp 1,  +0.1 dB from amp 2
    right content   -0.4 dB from amp 1, +17.5 dB from amp 2

  With the switch off for both, i.e. the previous behaviour:
    left content   +14.0 dB from amp 1, +17.0 dB from amp 2
    right content    0.0 dB from amp 1,  -0.2 dB from amp 2

So with the switch on, each amplifier carries one side and the other side
sits within 0.4 dB of the muted floor; with it off, both amplifiers render
the same left channel and right-channel content is inaudible on both. The
level each amplifier produces is unchanged between the two settings
(+14.1 vs +14.0 for amp 1, +17.5 vs +17.0 for amp 2), which is the
expected signature of a change in which channel reaches an amplifier
rather than in its gain.

Developed with AI assistance. The assistant did the register-level
analysis, identified the mirror-mode reset in sdw_compute_slave_ports()
as the cause and drafted the patch. Several earlier hypotheses it
produced were wrong and were discarded only because they were measured:
most of the SDCA Cluster Index work summarised above, and the Left/Right
enum of the earlier version, whose changelog claimed a per-side
assignment that the bus allocator cannot implement.

All hardware measurements quoted above were run by the submitter on the
affected machine. The submitter has reviewed the change, understands it
and takes responsibility for it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Ville Saarinen <wiza@saarinenkoti.fi>
---
 sound/soc/codecs/tas2783-sdw.c | 100 +++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index 5a7ac6224..d1addd8ae 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -96,6 +96,8 @@ struct tas2783_prv {
 	u8 rca_binaryname[64];
 	u8 dev_name[32];
 	bool hw_init;
+	/* take one channel instead of mirroring; applied at hw_params */
+	bool rx_single_ch;
 	/* wq for firmware download */
 	wait_queue_head_t fw_wait;
 	bool fw_dl_task_done;
@@ -590,6 +592,84 @@ static s32 tas2783_amp_putvol(struct snd_kcontrol *kcontrol,
 	return snd_soc_put_volsw(kcontrol, ucontrol);
 }
 
+/*
+ * UDMPU23 Cluster Index selects which channel of the incoming stream this amp
+ * renders. The driver only ever sets it from tas2783_reg_default[] as 0x0, and
+ * never per device, so in a two-amp aggregated setup both amps come up on the
+ * same cluster and render the same channel -- on the HP OmniBook X Flip 14
+ * (board 8EA1) that means both speakers play the left channel and right-channel
+ * content is inaudible. The firmware blobs do encode a per-amp channel in page-0
+ * config registers (0x80000a differs 2a/1a between the two blobs), but the SDCA
+ * cluster index overrides it, so the split has to be set here.
+ *
+ * Unlike rt1316/rt1318, the selection CANNOT be pushed to the device. The amp
+ * answers COMMAND_IGNORED (-ENODATA) to a UDMPU23 Cluster Index write in every
+ * state tried -- streaming, idle, and with the SDCA function verified actually
+ * powered on (PDE23 Actual Power State == ON), as a 4-byte MBQ write, as a
+ * plain single-byte write, after clearing the latched Entity-0 status bits, and
+ * on the Next rank of a dual-ranked control. A genuine device read of the same
+ * control (sdw_read_no_pm) also returns -ENODATA, which is the tell: the control
+ * is not merely write-protected, it is not implemented on this part. Mainline
+ * master carries no channel assignment for tas2783 either, so there is nothing
+ * upstream to backport.
+ *
+ * So the split is done on the host side instead, where it costs no device
+ * access at all. sdw_compute_slave_ports() walks the slaves of a stream and
+ * advances the payload block offset by one channel per slave port -- but only
+ * if the slave asked for fewer channels than the stream carries. A slave whose
+ * ch_mask covers every channel of the stream is treated as "mirror mode" and
+ * the offset is reset for the next slave, which is precisely what this driver
+ * used to request: snd_sdw_params_to_config() hands both amps ch_mask 0x3 on a
+ * 2-channel stream, so both were given the same block offset and both rendered
+ * the same (left) samples. Confirmed on the wire: DP1_CHANNELEN 0x03 and
+ * DP1_OFFSETCTRL1 0x41 read identically on the two amps during playback.
+ *
+ * Asking for a single channel per amp drops the pair out of mirror mode, and
+ * the core then assigns consecutive block offsets in the order the amps call
+ * sdw_stream_add_slave() -- i.e. dai_link codec order, which on this board is
+ * amp 0x9 (spk_l_endpoint, group_position 0) then amp 0xC (spk_r, position 1).
+ *
+ * Note carefully what this control can and cannot do. It selects *whether* this
+ * amp takes a single channel or mirrors the whole stream; it does NOT and cannot
+ * select *which* channel. sdw_compute_slave_ports() advances the payload offset
+ * by "port_bo += bps * hweight32(p_rt->ch_mask)" -- the popcount of the mask
+ * only, never which bit is set -- so BIT(0) and BIT(1) are indistinguishable to
+ * the allocator, and the side an amp ends up rendering is fixed by its position
+ * in the slave iteration order. Setting this switch on both amps of a pair, with
+ * the same ch_mask on both, still produces a correct stereo split; the machine
+ * driver's codec order is what assigns the sides.
+ *
+ * Off preserves the old mirror behaviour, so an unconfigured card behaves
+ * exactly as before and the split is opt-in from UCM.
+ */
+static int tas2783_rx_single_ch_get(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct tas2783_prv *tas_dev =
+		snd_soc_component_get_drvdata(component);
+
+	ucontrol->value.integer.value[0] = tas_dev->rx_single_ch;
+
+	return 0;
+}
+
+static int tas2783_rx_single_ch_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct tas2783_prv *tas_dev =
+		snd_soc_component_get_drvdata(component);
+	bool val = !!ucontrol->value.integer.value[0];
+
+	if (tas_dev->rx_single_ch == val)
+		return 0;
+
+	tas_dev->rx_single_ch = val;
+
+	return 1;
+}
+
 static const struct snd_kcontrol_new tas2783_snd_controls[] = {
 	SOC_SINGLE_RANGE_EXT_TLV("Amp Volume", TAS2783_AMP_LEVEL,
 				 1, 0, 20, 0, tas2783_amp_getvol,
@@ -597,6 +677,9 @@ static const struct snd_kcontrol_new tas2783_snd_controls[] = {
 	SOC_SINGLE_RANGE_EXT_TLV("Speaker Volume", TAS2783_DVC_LVL,
 				 0, 0, 200, 1, tas2783_digital_getvol,
 				 tas2783_digital_putvol, tas2781_dvc_tlv),
+	SOC_SINGLE_BOOL_EXT("RX Single Channel Switch", 0,
+			    tas2783_rx_single_ch_get,
+			    tas2783_rx_single_ch_put),
 };
 
 static s32 tas2783_validate_calibdata(struct tas2783_prv *tas_dev,
@@ -992,6 +1075,23 @@ static s32 tas_sdw_hw_params(struct snd_pcm_substream *substream,
 	else
 		port_config.num = 2;
 
+	/*
+	 * Claim a single channel of the stream so the bus drops this amp out of
+	 * "mirror mode" and gives it its own payload block offset instead of the
+	 * same one as its pair. Which channel that turns out to be is decided by
+	 * slave iteration order, not by the mask -- see the comment above
+	 * tas2783_rx_single_ch_get().
+	 */
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
+	    stream_config.ch_count > 1 && tas_dev->rx_single_ch) {
+		stream_config.ch_count = 1;
+		port_config.ch_mask = BIT(0);
+	}
+
+	dev_dbg(tas_dev->dev, "port %u: single_ch=%u ch_count=%d ch_mask=%#x\n",
+		port_config.num, tas_dev->rx_single_ch,
+		stream_config.ch_count, port_config.ch_mask);
+
 	ret = sdw_stream_add_slave(sdw_peripheral,
 				   &stream_config, &port_config, 1, sdw_stream);
 	if (ret)
-- 
2.55.0

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

[Ville Saarinen]

The firmware image is downloaded with sdw_nwrite_no_pm(), which writes
straight to the peripheral and bypasses the regmap cache. The cache keeps
holding the tas2783_reg_default[] entries for every register the firmware
image owns, so cache and device disagree from the moment the download
completes.

tas2783_sdca_dev_resume() then does regcache_cache_only(false) followed by
regcache_sync(), and regcache_sync() writes out every cached register. On
a system resume the peripheral stays attached and keeps its device state,
so hw_init is still set and the firmware is never re-downloaded - but the
sync stamps the stale defaults back onto the device on top of the firmware
tuning that is still live there.

On an HP OmniBook X Flip 14, which carries two aggregated TAS2783 amps,
this was measured with a cache-bypassing debugfs read taken after an
s2idle cycle: all 12 registers where the firmware image differs from the
defaults table had reverted to their default value on both amps, and the
two amps had become byte-identical over 0x800001-0x800040. Among the lost
values are the four page-0 bytes that give each amp of a stereo pair its
own configuration. The speakers are silent after resume and stay silent
until the machine is rebooted.

Drop each downloaded file's destination range from the cache once it has
been written, so the cache no longer claims to know registers the firmware
owns and regcache_sync() has nothing stale to write over them. This is
safe: a suspend deep enough for the peripheral to actually lose its state
also takes it UNATTACHED, which clears hw_init and triggers a full
firmware re-download on re-attach.

Developed with AI assistance. The assistant diagnosed the interaction
between the cache-bypassing firmware download and the resume-time
regcache_sync(), and drafted the patch.

All hardware measurements quoted above were run by the submitter on the
affected machine. The submitter has reviewed the change, understands it
and takes responsibility for it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Ville Saarinen <wiza@saarinenkoti.fi>
---
 sound/soc/codecs/tas2783-sdw.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index d1addd8ae..f79e730b0 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -912,6 +912,26 @@ static void tas2783_fw_ready(const struct firmware *fmw, void *context)
 				"FW download failed: %d", ret);
 			break;
 		}
+
+		/*
+		 * The firmware image is written with sdw_nwrite_no_pm(), which
+		 * bypasses the regmap cache. The cache therefore keeps holding
+		 * the stale reg_defaults entries for every register the
+		 * firmware owns, and the regcache_sync() done on resume writes
+		 * those defaults back out over the firmware tuning. That wipes
+		 * the per-amp configuration, including the channel assignment,
+		 * and leaves the speakers silent until the next full re-init.
+		 *
+		 * Drop the firmware-owned registers from the cache so nothing
+		 * stale can ever be synced over them. This is safe because a
+		 * suspend deep enough to lose device state also takes the
+		 * peripheral UNATTACHED, which clears hw_init and triggers a
+		 * full firmware re-download on re-attach.
+		 */
+		if (file->length)
+			regcache_drop_region(tas_dev->regmap, file->dest_addr,
+					     file->dest_addr + file->length - 1);
+
 		cur_file++;
 	}
 	mutex_unlock(&tas_dev->pde_lock);
-- 
2.55.0

Re: [PATCH 0/3] ASoC: tas2783: fix stereo split and resume on a two-amp pair

[Ville Saarinen]

I posted this series a few hours ago without having found the existing
work on exactly these bugs. That was my mistake: the threads were easy to
find and I did not look before sending. Apologies to those of you who have
been through this already. Adding Pierre-Louis, Robin, Antoine and Andrey
to Cc, and setting out below how the series relates to what is already
done, since a good part of it is not new.

Prior work I should have cited
==============================

  Robin Everaars, [BUG] every amp on the link selects the same channel
  https://lore.kernel.org/all/20260805183517.8665-1-robineveraars@pm.me/

  Antoine Monnet, no stereo channel split for two mono amps -> mono output
  https://lore.kernel.org/all/29e8c08b-9475-4aba-bce0-6d4a45a26d3b@gmail.com/

  Antoine Monnet, calibration firmware not re-downloaded after s2idle resume
  https://lore.kernel.org/all/c66ae00a-e878-4af0-a05a-272e9574eaa5@montane.tech/

  Andrey Golovko, ASoC: tas2783-sdw: drop stale regcache on uninitialized
  re-attach -- applied as b627da430357

  Andrey Golovko, port prepare never completes after S0i3
  https://lore.kernel.org/all/b1bc21c8a403fe15e742b6a6ff30b27f@gmail.com/

Everything above is on an ASUS ProArt PX13 HN7306EAC. My machine is an HP
OmniBook X Flip 14-kc0xxx (AMD Strix Point, ACP 7.2, two TAS2783 plus an
rt712-sdca on one link), so at least the reports now span two different
platforms and three different machines.

Patch 2 (RX Single Channel Switch): mostly not new
==================================================

The central finding in my changelog -- that sdw_compute_slave_ports()
advances the payload offset by hweight32(ch_mask) and never looks at which
bit is set, so a one-channel mask defeats mirror mode while L/R follows
slave iteration order rather than the mask value -- was published by Robin
before I sent, and Andrey restated it precisely in the 08-07 message. I
reached it independently, which is worth exactly nothing in terms of
credit; it is Robin's result and I should have cited it.

Antoine's patch derives the per-amp mask from name_prefix. Mine exposes a
boolean control, off by default, and leaves the decision to the machine's
UCM profile. The honest difference is narrow: Antoine's works with no
userspace change on boards where the prefix order matches the speakers,
mine needs a UCM cset but does not encode a side in the driver at all,
which was my reaction to the same "the bit does not pick the channel"
problem. I do not think mine is obviously better and I am happy to drop it
in favour of Antoine's, or to rebase whatever is useful in it on top.

One thing that may be worth keeping either way is the naming. Andrey's
note that the name_prefix -> BIT(n) mapping "reads as if the bit picks the
channel" is the same objection that made me rename my own control: an
earlier version of this patch was an rt1316-style "RX Channel Select" enum
with Left/Right values, and those values measured inert, exactly as the
allocator predicts. A control that names a side is an ABI promise the bus
cannot keep.

A data point for the UDMPU23 ClusterIndex question
==================================================

Pierre-Louis, in the 08-07 message you suggested experimenting with
non-zero cluster indices per amp, and asked TI to comment on whether the
index is the right place for this. I have measurements on that, and they
are discouraging on this part.

SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_UDMPU23, TAS2783_SDCA_CTL_UDMPU_CLUSTER, 0)
cannot be written at all here. The amplifier answers COMMAND_IGNORED
(-ENODATA) in every state I tried: streaming, idle, and with the SDCA
function confirmed powered on via PDE23 Actual Power State == ON; as a
4-byte MBQ write, as a plain single-byte write, after clearing the latched
Entity-0 status bits, and on the Next rank of the dual-ranked control.

The tell is that a genuine device read of the same Control with
sdw_read_no_pm() also returns -ENODATA. It is not write-protected and it
is not a ranking problem: on this device the Control is not implemented,
even though tas2783_reg_default[] carries an entry for it as 0x0. That
does not settle what the SDCA spec intends, and another TAS2783 revision
may well implement it -- but on this silicon the ClusterIndex route is
closed, which is why I did the split at the port level despite your point
that this is not what SDCA designs are supposed to do.

I would still like to hear TI on the intended mechanism. If the answer is
that PostureNumber is the right control and the Posture Table is supposed
to come from platform firmware, then none of the host-side approaches in
these threads is the real fix and it would be good to know that before one
of them lands.

Patch 3 (regcache): narrower than I described, and possibly still needed
========================================================================

I based this series on torvalds master, which does not yet carry Andrey's
b627da430357, so my changelog describes a bug that is already partly
fixed. Correcting that:

b627da430357 replaces the regcache_sync() in tas_update_status() with
regcache_drop_region(regmap, 0, UINT_MAX) on the uninitialized re-attach
path. That covers the case where the device lost power, went UNATTACHED
and cleared hw_init -- which is the case I measured.

What it does not cover is tas2783_sdca_dev_resume(), which still calls
regcache_sync() unconditionally (tas2783-sdw.c:1099 in broonie/for-next).
On a resume where the peripheral stayed attached and hw_init was never
cleared, that sync still writes stale reg_defaults over every
firmware-owned register, because the firmware is downloaded with
sdw_nwrite_no_pm() and the cache never saw those values. My patch drops
the firmware-owned regions from the cache at download time, which closes
that path too.

I want to be clear about the limits of my evidence: my measurement was on
v7.1.6, which predates b627da430357, so what I actually observed may have
been the UNATTACHED path that is now fixed. The residual dev_resume() path
is a code reading, not something I have measured in isolation on a tree
that already has Andrey's fix. I will test that properly and report back
rather than asking anyone to take the patch on this basis.

Patch 1 (deferred MBQ transactions)
===================================

I did not find prior coverage of this one. tas_regmap does not make
Entity-0 Function Status readable and does not set the mbq poll interval
or deadline, so regmap-sdw-mbq's retry for a Function answering
COMMAND_IGNORED never polls and every deferred write fails with -ENODATA
by construction. It may be relevant to the "port prepare never completes
after S0i3" thread; I have not tried to reproduce that symptom.

What I will do next
===================

Unless anyone would rather I did otherwise:

 - respin against broonie/sound for-next rather than master;
 - carry Link:/Reported-by: tags for Robin's and Antoine's reports;
 - drop or rework patch 2 depending on what happens with Antoine's;
 - hold patch 3 until I have measured the dev_resume() path on a tree
   containing b627da430357;
 - keep patch 1 as the one piece I believe is unencumbered.

Robin, Antoine, Andrey -- if you would like Reported-by: or Suggested-by:
on any of this, say so and I will add it; I did not want to attach your
names to a series you have not seen.

One disclosure that applies to this mail as much as to the patches: I work
on this with Claude (Anthropic, claude-opus-5) as an assistant, and a
substantial part of the analysis above, including the register-level
ClusterIndex work, originated with it. The cover letter has the full
statement. The measurements are mine, run on my hardware, and I take
responsibility for the claims either way.

Thanks, and sorry again for the duplicated effort.

Ville