Re: ASoC: tas2783-sdw: no stereo channel split for two mono amps -> mono output (AMD ACP SoundWire, ASUS ProArt PX13)

[Antoine Monnet]

(Apologies — I sent that from the wrong identity. 
monnet.antoine@xxxxxxxxx and antoine@xxxxxxxxxxxx are the same person)

Hi Andrey,

Thanks for the second-machine confirmation and for spelling out the
mechanism - I went with exactly your suggestion and keyed the channel off
the machine-assigned component prefix rather than unique_id.

I logged name_prefix against the resulting ch_mask and got tas2783-1 = 0x8
= left, tas2783-2 = 0xb = right; the split is correct by ear and by per-amp
mixer mute. Since your unit is the same HN7306EAC the prefix ordering is
identical, so this should give correct L/R for you too - a Tested-by from
the second machine would be welcome if you get a chance, but nothing needs
re-deriving.

On the ch_maps alternative you raised: since asoc_sdw_hw_params() hands
every codec the full mask for playback, dai_link->ch_maps doesn't carry a
per-amp selection there, so I took the prefix-index route. Happy to switch
to ch_maps if the machine layer is taught to fill per-amp maps first -
that's really the TI question below.

Patch below, against broonie/sound for-next.

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

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -216,6 +216,24 @@ static s32 tas_sdw_hw_params(struct 
snd_pcm_substream *substream,
      /* SoundWire specific configuration */
      snd_sdw_params_to_config(substream, params,
                               &stream_config, &port_config);
+
+     /*
+      * The two mono amps each render one channel of the stereo stream:
+      * snd_sdw_params_to_config() hands every codec the full mask for
+      * playback, so without a per-amp channel selection only one amp would
+      * output. Derive the channel from the machine-assigned component
+      * prefix rather than the SoundWire address (which is board-specific):
+      * soc_sdw_ti_amp.c maps tas2783-1/-3 to the Left speaker and
+      * tas2783-2/-4 to the Right, so odd-indexed amps take the left
+      * channel and even-indexed the right.
+      */
+     if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
+         params_channels(params) == 2 && component->name_prefix) {
+             const char *idx_str = strrchr(component->name_prefix, '-');
+             unsigned long idx;
+
+             if (idx_str && !kstrtoul(idx_str + 1, 10, &idx) && idx)
+                     port_config.ch_mask = (idx & 1) ? BIT(0) : BIT(1);
+     }
+
      /* port 1 for playback */
      if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
              port_config.num = 1;
--
2.53.0


On 7/27/26 10:48, Andrey Golovko wrote:
> Hi Antoine,
>
> Confirmed on a second machine: ASUS ProArt PX13 HN7306EAC (Ryzen AI MAX+
> 395), Ubuntu 26.04, 7.2-rc4 based kernel, same two TAS2783 at unique_id
> 0x8 / 0xB plus RT721 on SoundWire link 1.
>
> Rather than judging by ear, I used the per-amp mixer controls, which
> makes the result unambiguous. Both amps at full scale ('tas2783-1/-2 Amp
> Volume' = 20, 'Speaker Volume' = 200), all four 'Left/Right Spk[2]
> Switch' on:
>
>    speaker-test -Dpipewire -c2 -s1 (left channel only)
>      - audible
>      - 'tas2783-2 Speaker Volume' = 0  -> complete silence
>      - 'tas2783-1 Speaker Volume' = 0  -> no audible change
>
>    speaker-test -Dpipewire -c2 -s2 (right channel only)
>      - silent, with both amps unmuted at full scale
>
> So exactly your picture: one amp (tas2783-2) renders audio and it renders
> the *left* channel, the other amp contributes nothing, and the right
> channel is never reproduced. Both amps do load their own per-address
> blob here (1714-1-8.bin / 1714-1-B.bin, via the fallback naming path
> after the 0x-prefixed names miss), so this is not a case of the wrong
> configuration being downloaded.
>
> On the "proper mechanism" question: a good part of the plumbing already
> exists, and it does not need unique_id at all.
>
>   - The machine layer already knows which amp is which. In
>     sound/soc/sdw_utils/soc_sdw_ti_amp.c, asoc_sdw_ti_spk_rtd_init() maps
>     the component name prefix to a speaker widget: tas2783-1 -> "Left
>     Spk", tas2783-2 -> "Right Spk", tas2783-3/-4 -> "Left/Right Spk2".
>     That is where the four 'Left/Right Spk[2] Switch' controls on this
>     board come from.
>
>   - asoc_sdw_hw_params() (sound/soc/sdw_utils/soc_sdw_utils.c) fills
>     dai_link->ch_maps, but for playback it deliberately hands every codec
>     the full mask ("Identical data will be sent to all codecs in
>     playback"), leaving the per-amp channel selection to the amp itself.
>     acp-sdw-legacy-mach, which drives this board, uses both.
>
> So the driver could derive the channel from the same prefix index the
> DAPM routing already uses, or from its entry in dai_link->ch_maps,
> instead of hard-coding SoundWire addresses.
>
> Which raises the question for TI: on TAS2783 is the channel selection
> meant to come from the per-device .bin (in which case it is evidently
> not taking effect on this board), or is the driver expected to program a
> per-amp channel mask? Depending on the answer, either the firmware
> description or tas_sdw_hw_params() needs fixing - and in the latter case
> the amp's channel should come from the machine-level mapping rather than
> from unique_id, which as you say is board-specific.
>
> Happy to test patches on this hardware; I can also collect register
> dumps from both amps if that helps.
>
> Thanks,
> Andrey