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

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:

quoted

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.