mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Antoine Monnet <antoine@montane.tech>
To: Andrey Golovko <andrey.golovko@gmail.com>, linux-sound@vger.kernel.org
Cc: Shenghao Ding <shenghao-ding@ti.com>, Kevin Lu <kevin-lu@ti.com>,
	Baojun Xu <baojun.xu@ti.com>, Sen Wang <sen@ti.com>,
	Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Vijendar Mukunda <Vijendar.Mukunda@amd.com>,
	Vinod Koul <vkoul@kernel.org>,
	Bard Liao <yung-chuan.liao@linux.intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>,
	linux-kernel@vger.kernel.org
Subject: Re: ASoC: tas2783-sdw: no stereo channel split for two mono amps -> mono output (AMD ACP SoundWire, ASUS ProArt PX13)
Date: Mon, 27 Jul 2026 13:21:27 +0200	[thread overview]
Message-ID: <778f017a-e1c2-4ab6-9968-6e4c6285180b@montane.tech> (raw)
In-Reply-To: <20b6c100ec50c0b9eb1dd31a338dbc9e@gmail.com>

(Apologies — I sent that from the wrong identity. 
monnet.antoine@gmail.com and antoine@montane.tech 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

  reply	other threads:[~2026-07-27 11:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-19  0:53 Antoine Monnet
2026-07-27  8:48 ` Andrey Golovko
2026-07-27 11:21   ` Antoine Monnet [this message]
2026-08-07  5:21     ` Andrey Golovko
2026-08-07  9:19       ` Pierre-Louis Bossart
2026-08-12 21:33     ` Andrey Golovko
2026-08-13  8:56       ` Ville Saarinen
2026-08-13  9:38         ` Charles Keepax
2026-08-13 14:20       ` Antoine Monnet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=778f017a-e1c2-4ab6-9968-6e4c6285180b@montane.tech \
    --to=antoine@montane.tech \
    --cc=Vijendar.Mukunda@amd.com \
    --cc=andrey.golovko@gmail.com \
    --cc=baojun.xu@ti.com \
    --cc=broonie@kernel.org \
    --cc=kevin-lu@ti.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.dev \
    --cc=sen@ti.com \
    --cc=shenghao-ding@ti.com \
    --cc=vkoul@kernel.org \
    --cc=yung-chuan.liao@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®