Re: [PATCH v3 2/2] arm64: dts: rockchip: Add HDMI0 audio output on rock-5b

Detlev Casanova
2025-01-31T17:20:31Z

Hi Quentin,

On Friday, 31 January 2025 11:38:34 EST Quentin Schulz wrote:
> Hi Detlev,
>
> On 1/30/25 5:45 PM, Detlev Casanova wrote:
> > Use the simple-audio-card driver with the hdmi0 QP node as CODEC and
> > the i2s5 device as CPU.
> >
> > The simple-audio-card,mclk-fs value is set to 128 as it is done in
> > the downstream driver.
> >
> > The #sound-dai-cells value is set to 0 in the hdmi0 node so that it can be
> > used as an audio codec node.
> >
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> >
> > arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 1 +
> > .../boot/dts/rockchip/rk3588-rock-5b.dts | 19 +++++++++++++++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi index
> > 8cfa30837ce72..790c1c25a3e41 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> > @@ -1394,6 +1394,7 @@ hdmi0: hdmi@fde80000 {
> >
> > reset-names = "ref", "hdp";
> > rockchip,grf = <&sys_grf>;
> > rockchip,vo-grf = <&vo1_grf>;
> >
> > + #sound-dai-cells = <0>;
> >
> > status = "disabled";
> >
> > ports {
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts index
> > d597112f1d5b8..1909078538367 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > @@ -49,6 +49,21 @@ hdmi0_con_in: endpoint {
> >
> > };
> >
> > };
> >
> > + hdmi0-sound {
> > + compatible = "simple-audio-card";
> > + simple-audio-card,format = "i2s";
> > + simple-audio-card,mclk-fs = <128>;
> > + simple-audio-card,name = "hdmi0";
> > +
> > + simple-audio-card,codec {
> > + sound-dai = <&hdmi0>;
> > + };
> > +
> > + simple-audio-card,cpu {
> > + sound-dai = <&i2s5_8ch>;
> > + };
> > + };
> > +
>
> This is SoC specific and not board specific I believe. I2S5 is not
> usable anywhere else but with HDMI0 controller if I've read the TRM
> properly.
>
> Therefore, I would suggest to move this to rk3588-base.dtsi (with
> status=disabled; and when hdmi1-sound comes up, to rk3588-extra.dtsi),
> the same way we've done for RK3399 for example.

Indeed, I only could test on a Rock 5B, but I think it can be moved to the SoC
DTS.

> The only hesitation I have is that HDMI0 can use either I2S or SPDIF for
> audio, both audio controllers internal exclusive to HDMI0/1 controller.
> But the user could anyway define their own simple audio card for spdif
> or modify this one if they wanted to.

So some boards will use I2S and some SPDIF ? Or any board can be used with one
or the other ?
The disabled status makes sense as hdmi is disabled in the SoC tree as well.
So if a user wants to use SPDIF instead, they could keep hdmi0-sound disabled
and add their own simple-audio-card compatible node.

> I've tested with my suggested change and same changes than for Rock 5B
> DTS on RK3588 Tiger Haikou with speaker-test -c 2 -t wav, left is left,
> right is right :)
>
> I'm not giving my Tb here as the commit title is specifically about Rock
> 5B and I haven't tested this series on it. If you had a separate patch
> for the hdmi sound node and enabling it on Rock 5b, I would have given
> my Tb on the former and not the latter.

Thank you for testing anyway ! I will move the node and enable it in all board
dts that already enable hdmi0.

Detlev.