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

Hi Andrey !

I'm ok with you sending the patch !

Thx

Antoine

On 8/12/26 23:33, Andrey Golovko wrote:
> Antoine,
>
> your patch from 27 July is still the only working stereo fix for these
> boards, and it is now the only one left: Ville withdrew his series on
> 11 August and asked that the tags go to Robin and to you, and nobody
> else has posted anything. It has never been sent as a formal [PATCH],
> so as things stand it cannot be applied by anyone.
>
> I think it should be, and I would like to know how you want that to
> happen.
>
> Where it has been tested
> ========================
>
>    - your machine, ASUS ProArt PX13 HN7306EA (the original report);
>    - mine, ASUS ProArt PX13 HN7306EAC: Tested-by sent on 7 August, and
>      re-confirmed yesterday on a current broonie/sound for-next kernel --
>      speaker-test -c2 -s1 is the physically left speaker, -s2 the right,
>      with no audible level imbalance between them;
>    - Robin's board, same amp pair, where the one-channel-mask approach was
>      independently established and measured.
>
> Robin has asked for Reported-by: and Suggested-by: on the channel-mask
> patch, since his 5 August report and the follow-up measurement
> established both the approach and the positional behaviour of
> sdw_compute_slave_ports().
>
> Two options
> ===========
>
> Either you post it yourself as a proper [PATCH] -- which I would prefer,
> it is your work -- or, if you would rather not spend time on it, I am
> happy to send it with your authorship intact:
>
>    From: Antoine Monnet <[email protected]>
>    Reported-by: Robin Everaars <[email protected]>
>    Suggested-by: Robin Everaars <[email protected]>
>    Tested-by: Andrey Golovko <[email protected]>
>
> Just say which, and if the second, whether you want anything changed
> first. I will not send anything under your name without your word.
>
> One thing worth putting in the changelog
> ========================================
>
> The name_prefix -> BIT(n) mapping reads as if the bit chose the channel,
> and it does not. As Robin measured, inverting the two masks between the
> amps does not move the audio: 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 fixes mono by defeating mirror mode, and L/R
> then follows codec order in the DAI link, which on these boards happens
> to match the speakers.
>
> That is worth a sentence in the commit message so nobody later reads the
> mapping as an ABI promise. It does not make the patch less correct -- the
> split it produces is right on three machines -- but it is the honest
> description of why it works.
>
> For completeness on the SDCA-correct alternative that Pierre-Louis raised:
> Ville measured that UDMPU23 ClusterIndex is simply not implemented on his
> TAS2783 revision, answering COMMAND_IGNORED to writes and to a plain read
> alike, so that route is closed at least on that silicon. TI have not
> commented on the intended mechanism.
>
> Unrelated, but you followed it: the "no audio after s2idle resume" problem
> you reported in July is understood and patched. After S0i3 the amplifier
> comes back with PDE23 at PS3, and a data port cannot complete channel
> preparation while the Function is powered down; the power-up only ever
> happened in hw_params(), which a resumed stream never calls again. Patch:
>
>    https://lore.kernel.org/all/[email protected]/
>
> Thanks,
> Andrey