Re: ASoC: tas2783-sdw: no stereo channel split for two mono amps -> mono output (AMD ACP SoundWire, ASUS ProArt PX13)
Antoine Monnet <[email protected]>
| Newsgroups | org.kernel.vger.linux-sound,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
(Apologies — I sent that from the wrong identity. [email protected] and [email protected] 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