Linux-Rockchip Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64
@ 2020-10-05 14:03 Katsuhiro Suzuki
  2020-10-06  9:51 ` Johan Jonker
  0 siblings, 1 reply; 2+ messages in thread
From: Katsuhiro Suzuki @ 2020-10-05 14:03 UTC (permalink / raw)
  To: Heiko Stuebner, linux-rockchip
  Cc: Katsuhiro Suzuki, linux-kernel, linux-arm-kernel

This patch adds 'disabled' SPDIF sound node and related settings
of SPDIF for rk3399-rockpro64.

RockPro64 has output pins for SPDIF Tx. But RK3399 does not have
enough DMA channel for enabling SPDIF tx. Current settings are:

  - I2S0     (Req num 0, 1): Enabled : Output to 40pins header CON40
  - I2S1     (Req num 2, 3): Enabled : Output to ES8316 on board
  - I2S2     (Req num 4, 5): Enabled : Output to internal HDMI core
  - SPDIF Tx (Req num 7)   : Disabled: Output to connector J10

If users want to enable ALL sound I/Os, we need 7 DMA channels for
it. But unfortunately, RK3399 has only 6 DMA channels. So users have
to choose from the following:

  - Disable one of I2S (Ex. I2S0) and enable SPDIF tx
  - Keep enable I2S0/1/2 and disable SPDIF tx

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>

---

Changes in v3:
  - Refine commit description why adding disabled node

Changes in v2:
  - Remove redundant status property
---
 .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..58097245994a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
 		dais = <&i2s1_p0>;
 	};
 
+	sound-dit {
+		compatible = "audio-graph-card";
+		label = "rockchip,rk3399";
+		dais = <&spdif_p0>;
+	};
+
+	spdif-dit {
+		compatible = "linux,spdif-dit";
+		#sound-dai-cells = <0>;
+
+		port {
+			dit_p0_0: endpoint {
+				remote-endpoint = <&spdif_p0_0>;
+			};
+		};
+	};
+
 	vcc12v_dcin: vcc12v-dcin {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc12v_dcin";
@@ -698,6 +715,16 @@ &sdhci {
 	status = "okay";
 };
 
+&spdif {
+	pinctrl-0 = <&spdif_bus_1>;
+
+	spdif_p0: port {
+		spdif_p0_0: endpoint {
+			remote-endpoint = <&dit_p0_0>;
+		};
+	};
+};
+
 &spi1 {
 	status = "okay";
 
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64
  2020-10-05 14:03 [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64 Katsuhiro Suzuki
@ 2020-10-06  9:51 ` Johan Jonker
  0 siblings, 0 replies; 2+ messages in thread
From: Johan Jonker @ 2020-10-06  9:51 UTC (permalink / raw)
  To: Katsuhiro Suzuki, Heiko Stuebner, linux-rockchip
  Cc: linux-kernel, linux-arm-kernel

Hi Katsuhiro, Heiko,

Question for the maintainer:
Should we add a SPDIF node if the connector is not physical on a board,
only a header?

Thanks Katsuhiro for the "aplay -l" screen print.

**** List of PLAYBACK Hardware Devices ****
card 0: hdmisound [hdmi-sound], device 0: ff8a0000.i2s-i2s-hifi
i2s-hifi-0 [ff8a0000.i2s-i2s-hifi i2s-hifi-0]
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 1: rockchiprk3399 [rockchip,rk3399], device 0: ff890000.i2s-ES8316
HiFi ES8316 HiFi-0 [ff890000.i2s-ES8316 HiFi ES8316 HiFi-0]
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 2: rockchiprk339_1 [rockchip,rk3399], device 0:
ff870000.spdif-dit-hifi dit-hifi-0 [ff870000.spdif-dit-hifi dit-hifi-0]
  Subdevices: 1/1
  Subdevice #0: subdevice #0


On 10/5/20 4:03 PM, Katsuhiro Suzuki wrote:
> This patch adds 'disabled' SPDIF sound node and related settings
> of SPDIF for rk3399-rockpro64.
> 
> RockPro64 has output pins for SPDIF Tx. But RK3399 does not have
> enough DMA channel for enabling SPDIF tx. Current settings are:
> 
>   - I2S0     (Req num 0, 1): Enabled : Output to 40pins header CON40
>   - I2S1     (Req num 2, 3): Enabled : Output to ES8316 on board
>   - I2S2     (Req num 4, 5): Enabled : Output to internal HDMI core
>   - SPDIF Tx (Req num 7)   : Disabled: Output to connector J10
> 
> If users want to enable ALL sound I/Os, we need 7 DMA channels for
> it. But unfortunately, RK3399 has only 6 DMA channels. So users have
> to choose from the following:
> 
>   - Disable one of I2S (Ex. I2S0) and enable SPDIF tx
>   - Keep enable I2S0/1/2 and disable SPDIF tx
> 
> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> 
> ---
> 
> Changes in v3:
>   - Refine commit description why adding disabled node
> 
> Changes in v2:
>   - Remove redundant status property
> ---
>  .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> index 6e553ff47534..58097245994a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
> @@ -76,6 +76,23 @@ sound {
>  		dais = <&i2s1_p0>;
>  	};
>  


	hdmi_sound: hdmi-sound {
		compatible = "simple-audio-card";
		simple-audio-card,name = "hdmi-sound";

Maybe rename to "HDMI"?

[..]
	};

	sound {
		compatible = "audio-graph-card";
		label = "rockchip,rk3399";

Maybe change this to "ES8316" to prevent confusion?

		dais = <&i2s1_p0>;
	};


> +	sound-dit {
> +		compatible = "audio-graph-card"
> +		label = "rockchip,rk3399";

This would be the second sound card with the same label.
It seems that aplay/linux? adds "-1" to it and removes the comma, so we get:

hdmisound
rockchiprk3399
rockchiprk339_1

Shouldn't we label it with something that reflect the function/output.
Shouldn't we standardize to SPDIF, HDMI and Analog similar to rk3318/rk3328?
Make a shorter label without spaces or special chars, so that chars
don't get removed?

Proposal:

HDMI
ES8316
SPDIF

> +		dais = <&spdif_p0>;

Maybe disable too?

The "sound-dit" node is standard enabled and will start some process
with a dia in a node that is disabled.


> +	};
> +
> +	spdif-dit {
> +		compatible = "linux,spdif-dit";
> +		#sound-dai-cells = <0>;

Maybe disable too?

> +
> +		port {
> +			dit_p0_0: endpoint {

> +				remote-endpoint = <&spdif_p0_0>;

This also points to something that's disabled.

> +			};
> +		};
> +	};
> +
>  	vcc12v_dcin: vcc12v-dcin {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc12v_dcin";
> @@ -698,6 +715,16 @@ &sdhci {
>  	status = "okay";
>  };
>  
> +&spdif {
> +	pinctrl-0 = <&spdif_bus_1>;

This node is disabled.

> +
> +	spdif_p0: port {
> +		spdif_p0_0: endpoint {
> +			remote-endpoint = <&dit_p0_0>;
> +		};
> +	};
> +};
> +
>  &spi1 {
>  	status = "okay";
>  
> 


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-05 14:03 [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64 Katsuhiro Suzuki
2020-10-06  9:51 ` Johan Jonker

Linux-Rockchip Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rockchip/0 linux-rockchip/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rockchip linux-rockchip/ https://lore.kernel.org/linux-rockchip \
		linux-rockchip@lists.infradead.org
	public-inbox-index linux-rockchip

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-rockchip


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git