2019-06-14 16:55:49

by Heiko Stuebner

[permalink] [raw]
Subject: [PATCH 3/4] ARM: dts: rockchip: add display nodes for rk322x

From: Justin Swartz <[email protected]>

Add display_subsystem, hdmi_phy, vop, and hdmi device nodes plus
a few hdmi pinctrl entries to allow for HDMI output.

Signed-off-by: Justin Swartz <[email protected]>
[added assigned-clock settings for hdmiphy output]
Signed-off-by: Heiko Stuebner <[email protected]>
---
arch/arm/boot/dts/rk322x.dtsi | 83 +++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)

diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index da102fff96a2..148f9b5157ea 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -143,6 +143,11 @@
#clock-cells = <0>;
};

+ display_subsystem: display-subsystem {
+ compatible = "rockchip,display-subsystem";
+ ports = <&vop_out>;
+ };
+
i2s1: i2s1@100b0000 {
compatible = "rockchip,rk3228-i2s", "rockchip,rk3066-i2s";
reg = <0x100b0000 0x4000>;
@@ -529,6 +534,17 @@
status = "disabled";
};

+ hdmi_phy: hdmi-phy@12030000 {
+ compatible = "rockchip,rk3228-hdmi-phy";
+ reg = <0x12030000 0x10000>;
+ clocks = <&cru PCLK_HDMI_PHY>, <&xin24m>, <&cru DCLK_HDMI_PHY>;
+ clock-names = "sysclk", "refoclk", "refpclk";
+ #clock-cells = <0>;
+ clock-output-names = "hdmiphy_phy";
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
gpu: gpu@20000000 {
compatible = "rockchip,rk3228-mali", "arm,mali-400";
reg = <0x20000000 0x10000>;
@@ -572,6 +588,28 @@
status = "disabled";
};

+ vop: vop@20050000 {
+ compatible = "rockchip,rk3228-vop";
+ reg = <0x20050000 0x1ffc>;
+ interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_VOP>, <&cru DCLK_VOP>, <&cru HCLK_VOP>;
+ clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
+ resets = <&cru SRST_VOP_A>, <&cru SRST_VOP_H>, <&cru SRST_VOP_D>;
+ reset-names = "axi", "ahb", "dclk";
+ iommus = <&vop_mmu>;
+ status = "disabled";
+
+ vop_out: port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vop_out_hdmi: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&hdmi_in_vop>;
+ };
+ };
+ };
+
vop_mmu: iommu@20053f00 {
compatible = "rockchip,iommu";
reg = <0x20053f00 0x100>;
@@ -594,6 +632,36 @@
status = "disabled";
};

+ hdmi: hdmi@200a0000 {
+ compatible = "rockchip,rk3228-dw-hdmi";
+ reg = <0x200a0000 0x20000>;
+ reg-io-width = <4>;
+ interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+ assigned-clocks = <&cru SCLK_HDMI_PHY>;
+ assigned-clock-parents = <&hdmi_phy>;
+ clocks = <&cru SCLK_HDMI_HDCP>, <&cru PCLK_HDMI_CTRL>, <&cru SCLK_HDMI_CEC>;
+ clock-names = "isfr", "iahb", "cec";
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmii2c_xfer &hdmi_hpd &hdmi_cec>;
+ resets = <&cru SRST_HDMI_P>;
+ reset-names = "hdmi";
+ phys = <&hdmi_phy>;
+ phy-names = "hdmi";
+ rockchip,grf = <&grf>;
+ status = "disabled";
+
+ ports {
+ hdmi_in: port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ hdmi_in_vop: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vop_out_hdmi>;
+ };
+ };
+ };
+ };
+
sdmmc: dwmmc@30000000 {
compatible = "rockchip,rk3228-dw-mshc", "rockchip,rk3288-dw-mshc";
reg = <0x30000000 0x4000>;
@@ -922,6 +990,21 @@
};
};

+ hdmi {
+ hdmi_hpd: hdmi-hpd {
+ rockchip,pins = <0 RK_PB7 1 &pcfg_pull_down>;
+ };
+
+ hdmii2c_xfer: hdmii2c-xfer {
+ rockchip,pins = <0 RK_PA6 2 &pcfg_pull_none>,
+ <0 RK_PA7 2 &pcfg_pull_none>;
+ };
+
+ hdmi_cec: hdmi-cec {
+ rockchip,pins = <0 RK_PC4 1 &pcfg_pull_none>;
+ };
+ };
+
i2c0 {
i2c0_xfer: i2c0-xfer {
rockchip,pins = <0 RK_PA0 1 &pcfg_pull_none>,
--
2.20.1


2019-06-14 17:46:00

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: rockchip: add display nodes for rk322x

Quoting Heiko Stuebner (2019-06-14 09:54:53)
> From: Justin Swartz <[email protected]>
>
> Add display_subsystem, hdmi_phy, vop, and hdmi device nodes plus
> a few hdmi pinctrl entries to allow for HDMI output.
>
> Signed-off-by: Justin Swartz <[email protected]>
> [added assigned-clock settings for hdmiphy output]
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> arch/arm/boot/dts/rk322x.dtsi | 83 +++++++++++++++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
> index da102fff96a2..148f9b5157ea 100644
> --- a/arch/arm/boot/dts/rk322x.dtsi
> +++ b/arch/arm/boot/dts/rk322x.dtsi
> @@ -143,6 +143,11 @@
> #clock-cells = <0>;
> };
>
> + display_subsystem: display-subsystem {
> + compatible = "rockchip,display-subsystem";
> + ports = <&vop_out>;
> + };
> +

What is this? It doesn't have a reg property so it looks like a virtual
device. Why is it in DT?

2019-06-14 18:33:46

by Justin Swartz

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: rockchip: add display nodes for rk322x

Hi Stephen,

On 2019-06-14 19:45, Stephen Boyd wrote:

>> diff --git a/arch/arm/boot/dts/rk322x.dtsi
>> b/arch/arm/boot/dts/rk322x.dtsi
>> index da102fff96a2..148f9b5157ea 100644
>> --- a/arch/arm/boot/dts/rk322x.dtsi
>> +++ b/arch/arm/boot/dts/rk322x.dtsi
>> @@ -143,6 +143,11 @@
>> #clock-cells = <0>;
>> };
>>
>> + display_subsystem: display-subsystem {
>> + compatible = "rockchip,display-subsystem";
>> + ports = <&vop_out>;
>> + };
>> +
>
> What is this? It doesn't have a reg property so it looks like a virtual
> device. Why is it in DT?

This is a virtual device.

I assumed it would be acceptable to it find in a device tree due to
binding documentation,
"Documentation/devicetree/bindings/display/rockchip/rockchip-drm.txt,
which states:

<quote>
The Rockchip DRM master device is a virtual device needed to list all
vop devices or other display interface nodes that comprise the
graphics subsystem.
</quote>

Without the "display_subsystem" device node, the HDMI PHY and
rockchipdrmfb frame buffer device are not initialized.

Perhaps I should have included this in my commit message? :)

Regards
Justin

2019-06-14 19:35:03

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: rockchip: add display nodes for rk322x

Am Freitag, 14. Juni 2019, 20:32:35 CEST schrieb Justin Swartz:
> On 2019-06-14 19:45, Stephen Boyd wrote:
> >> diff --git a/arch/arm/boot/dts/rk322x.dtsi
> >> b/arch/arm/boot/dts/rk322x.dtsi
> >> index da102fff96a2..148f9b5157ea 100644
> >> --- a/arch/arm/boot/dts/rk322x.dtsi
> >> +++ b/arch/arm/boot/dts/rk322x.dtsi
> >> @@ -143,6 +143,11 @@
> >> #clock-cells = <0>;
> >> };
> >>
> >> + display_subsystem: display-subsystem {
> >> + compatible = "rockchip,display-subsystem";
> >> + ports = <&vop_out>;
> >> + };
> >> +
> >
> > What is this? It doesn't have a reg property so it looks like a virtual
> > device. Why is it in DT?
>
> This is a virtual device.
>
> I assumed it would be acceptable to it find in a device tree due to
> binding documentation,
> "Documentation/devicetree/bindings/display/rockchip/rockchip-drm.txt,
> which states:
>
> <quote>
> The Rockchip DRM master device is a virtual device needed to list all
> vop devices or other display interface nodes that comprise the
> graphics subsystem.
> </quote>
>
> Without the "display_subsystem" device node, the HDMI PHY and
> rockchipdrmfb frame buffer device are not initialized.
>
> Perhaps I should have included this in my commit message? :)

As Justin said, that is very much common as the root of the components
that make up the drm device and pretty much common in a lot of devicetrees
for the last 5 years and longer ;-) .

Also gpio-keys also don't have a reg property ;-) .

Heiko


2019-06-14 20:38:41

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: rockchip: add display nodes for rk322x

Quoting Heiko Stuebner (2019-06-14 12:33:12)
> Am Freitag, 14. Juni 2019, 20:32:35 CEST schrieb Justin Swartz:
> > On 2019-06-14 19:45, Stephen Boyd wrote:
> > >> diff --git a/arch/arm/boot/dts/rk322x.dtsi
> > >> b/arch/arm/boot/dts/rk322x.dtsi
> > >> index da102fff96a2..148f9b5157ea 100644
> > >> --- a/arch/arm/boot/dts/rk322x.dtsi
> > >> +++ b/arch/arm/boot/dts/rk322x.dtsi
> > >> @@ -143,6 +143,11 @@
> > >> #clock-cells = <0>;
> > >> };
> > >>
> > >> + display_subsystem: display-subsystem {
> > >> + compatible = "rockchip,display-subsystem";
> > >> + ports = <&vop_out>;
> > >> + };
> > >> +
> > >
> > > What is this? It doesn't have a reg property so it looks like a virtual
> > > device. Why is it in DT?
> >
> > This is a virtual device.
> >
> > I assumed it would be acceptable to it find in a device tree due to
> > binding documentation,
> > "Documentation/devicetree/bindings/display/rockchip/rockchip-drm.txt,
> > which states:
> >
> > <quote>
> > The Rockchip DRM master device is a virtual device needed to list all
> > vop devices or other display interface nodes that comprise the
> > graphics subsystem.
> > </quote>
> >
> > Without the "display_subsystem" device node, the HDMI PHY and
> > rockchipdrmfb frame buffer device are not initialized.
> >
> > Perhaps I should have included this in my commit message? :)
>
> As Justin said, that is very much common as the root of the components
> that make up the drm device and pretty much common in a lot of devicetrees
> for the last 5 years and longer ;-) .
>
> Also gpio-keys also don't have a reg property ;-) .
>

Do you have a SoC node? If so, this virtual device should live in the
root, away from the nodes that have reg properties and are thus in the
SoC node.

2019-06-14 23:06:16

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: rockchip: add display nodes for rk322x

Stephen,

Am Freitag, 14. Juni 2019, 22:36:09 CEST schrieb Stephen Boyd:
> Quoting Heiko Stuebner (2019-06-14 12:33:12)
> > Am Freitag, 14. Juni 2019, 20:32:35 CEST schrieb Justin Swartz:
> > > On 2019-06-14 19:45, Stephen Boyd wrote:
> > > >> diff --git a/arch/arm/boot/dts/rk322x.dtsi
> > > >> b/arch/arm/boot/dts/rk322x.dtsi
> > > >> index da102fff96a2..148f9b5157ea 100644
> > > >> --- a/arch/arm/boot/dts/rk322x.dtsi
> > > >> +++ b/arch/arm/boot/dts/rk322x.dtsi
> > > >> @@ -143,6 +143,11 @@
> > > >> #clock-cells = <0>;
> > > >> };
> > > >>
> > > >> + display_subsystem: display-subsystem {
> > > >> + compatible = "rockchip,display-subsystem";
> > > >> + ports = <&vop_out>;
> > > >> + };
> > > >> +
> > > >
> > > > What is this? It doesn't have a reg property so it looks like a virtual
> > > > device. Why is it in DT?
> > >
> > > This is a virtual device.
> > >
> > > I assumed it would be acceptable to it find in a device tree due to
> > > binding documentation,
> > > "Documentation/devicetree/bindings/display/rockchip/rockchip-drm.txt,
> > > which states:
> > >
> > > <quote>
> > > The Rockchip DRM master device is a virtual device needed to list all
> > > vop devices or other display interface nodes that comprise the
> > > graphics subsystem.
> > > </quote>
> > >
> > > Without the "display_subsystem" device node, the HDMI PHY and
> > > rockchipdrmfb frame buffer device are not initialized.
> > >
> > > Perhaps I should have included this in my commit message? :)
> >
> > As Justin said, that is very much common as the root of the components
> > that make up the drm device and pretty much common in a lot of devicetrees
> > for the last 5 years and longer ;-) .
> >
> > Also gpio-keys also don't have a reg property ;-) .
> >
>
> Do you have a SoC node? If so, this virtual device should live in the
> root, away from the nodes that have reg properties and are thus in the
> SoC node.

no we don't have any soc node and that display-subsystem is really the
same on _all_ currently supported Rockchip socs ... has gone through
dt-reviews numerous times for multiple socs, so I'm somewhat
confident that we're not doing something terrible wrong.

Heiko