diff mbox

[1/5] arm64: dts: rockchip: Add rk3399 vop and display-subsystem

Message ID 1499875435-23944-2-git-send-email-jacob-chen@iotwrt.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jacob Chen July 12, 2017, 4:03 p.m. UTC
Add devicetree nodes for rk3399 VOP (Video Output Processors), and the
top level display-subsystem root node.

Later patches add endpoints (eDP, HDMI, MIPI, etc) that attach to the
VOPs' output ports.

Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 65 ++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

Comments

Heiko Stübner July 13, 2017, 11:34 p.m. UTC | #1
Hi Jacob,

Am Donnerstag, 13. Juli 2017, 00:03:51 CEST schrieb Jacob Chen:
> Add devicetree nodes for rk3399 VOP (Video Output Processors), and the
> top level display-subsystem root node.
> 
> Later patches add endpoints (eDP, HDMI, MIPI, etc) that attach to the
> VOPs' output ports.
> 
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 65 ++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index e795135..300e500 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1455,6 +1455,71 @@
>  		status = "disabled";
>  	};
>  
> +	vopl: vop@ff8f0000 {
> +		compatible = "rockchip,rk3399-vop-lit";
> +		reg = <0x0 0xff8f0000 0x0 0x1ffc>, <0x0 0xff8f2000 0x0 0x400>;

What is this second memory region doing? It looks like this is the meant
to cover VOP_GAMMA_LUT_ADDR, but can't you just map the whole area?
ChromeOS seems to be doing fine using the whole area as
	<0x0 0xff8f0000 0x0 0x3efc>
and I've also not seen any code changes actually mapping/using this
second area.


> +		interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
> +		clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>, <&cru DCLK_VOP1_DIV>;
> +		clock-names = "aclk_vop", "dclk_vop", "hclk_vop", "dclk_source";

While I know that this is based on my idea on handling the hdmi pll-rate
requirements, I haven't found the matching code- and dt-binding-changes
posted to a list yet. 


Heiko
Jacob Chen July 14, 2017, 1:52 a.m. UTC | #2
Hi heko,

2017-07-14 7:34 GMT+08:00 Heiko Stuebner <heiko@sntech.de>:
> Hi Jacob,
>
> Am Donnerstag, 13. Juli 2017, 00:03:51 CEST schrieb Jacob Chen:
>> Add devicetree nodes for rk3399 VOP (Video Output Processors), and the
>> top level display-subsystem root node.
>>
>> Later patches add endpoints (eDP, HDMI, MIPI, etc) that attach to the
>> VOPs' output ports.
>>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>> Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 65 ++++++++++++++++++++++++++++++++
>>  1 file changed, 65 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index e795135..300e500 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -1455,6 +1455,71 @@
>>               status = "disabled";
>>       };
>>
>> +     vopl: vop@ff8f0000 {
>> +             compatible = "rockchip,rk3399-vop-lit";
>> +             reg = <0x0 0xff8f0000 0x0 0x1ffc>, <0x0 0xff8f2000 0x0 0x400>;
>
> What is this second memory region doing? It looks like this is the meant
> to cover VOP_GAMMA_LUT_ADDR, but can't you just map the whole area?
> ChromeOS seems to be doing fine using the whole area as
>         <0x0 0xff8f0000 0x0 0x3efc>
> and I've also not seen any code changes actually mapping/using this
> second area.
>

0x1c00 - 0x200 for cabc_lut
0x2000 - 0x300 for gama_lut

My mistakes, It seems mark havn't send gamma and cabc support to upstream.
We shoud just using the whole area map.


>
>> +             interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
>> +             clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>, <&cru DCLK_VOP1_DIV>;
>> +             clock-names = "aclk_vop", "dclk_vop", "hclk_vop", "dclk_source";
>
> While I know that this is based on my idea on handling the hdmi pll-rate
> requirements, I haven't found the matching code- and dt-binding-changes
> posted to a list yet.
>

Yeah, should i collect mark's patches or remove it at first?

>
> Heiko
>
Heiko Stübner July 14, 2017, 6:33 a.m. UTC | #3
Hi Jacob,

Am Freitag, 14. Juli 2017, 09:52:30 CEST schrieb Jacob Chen:
> 2017-07-14 7:34 GMT+08:00 Heiko Stuebner <heiko@sntech.de>:
> > Am Donnerstag, 13. Juli 2017, 00:03:51 CEST schrieb Jacob Chen:
> >> Add devicetree nodes for rk3399 VOP (Video Output Processors), and the
> >> top level display-subsystem root node.
> >>
> >> Later patches add endpoints (eDP, HDMI, MIPI, etc) that attach to the
> >> VOPs' output ports.
> >>
> >> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> >> Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>
> >> ---
> >>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 65 ++++++++++++++++++++++++++++++++
> >>  1 file changed, 65 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> index e795135..300e500 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> @@ -1455,6 +1455,71 @@
> >>               status = "disabled";
> >>       };
> >>
> >> +     vopl: vop@ff8f0000 {
> >> +             compatible = "rockchip,rk3399-vop-lit";
> >> +             reg = <0x0 0xff8f0000 0x0 0x1ffc>, <0x0 0xff8f2000 0x0 0x400>;
> >
> > What is this second memory region doing? It looks like this is the meant
> > to cover VOP_GAMMA_LUT_ADDR, but can't you just map the whole area?
> > ChromeOS seems to be doing fine using the whole area as
> >         <0x0 0xff8f0000 0x0 0x3efc>
> > and I've also not seen any code changes actually mapping/using this
> > second area.
> >
> 
> 0x1c00 - 0x200 for cabc_lut
> 0x2000 - 0x300 for gama_lut
> 
> My mistakes, It seems mark havn't send gamma and cabc support to upstream.
> We shoud just using the whole area map.
> 
> 
> >
> >> +             interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
> >> +             clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>, <&cru DCLK_VOP1_DIV>;
> >> +             clock-names = "aclk_vop", "dclk_vop", "hclk_vop", "dclk_source";
> >
> > While I know that this is based on my idea on handling the hdmi pll-rate
> > requirements, I haven't found the matching code- and dt-binding-changes
> > posted to a list yet.
> >
> 
> Yeah, should i collect mark's patches or remove it at first?

As we don't know how much discussion is necessary for that, removing
dclk_source in the first iteration and re-add it once the feature.


Heiko
Heiko Stübner July 16, 2017, 5:31 p.m. UTC | #4
Hi Jacob,

Am Donnerstag, 13. Juli 2017, 00:03:51 CEST schrieb Jacob Chen:
> Add devicetree nodes for rk3399 VOP (Video Output Processors), and the
> top level display-subsystem root node.
> 
> Later patches add endpoints (eDP, HDMI, MIPI, etc) that attach to the
> VOPs' output ports.
> 
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>

you might want to reduce the number of Signed-offs a bit :-)

Also authorship needs to be fixed. I.e. first Signed-off for this patch
is from Mark, so in that case Mark probably also was the original author
and the patch should reflect that. General rule, topmost Signed-off is
of course from the original author.


Heiko
Jacob Chen July 17, 2017, 3:44 a.m. UTC | #5
Hi heko,

2017-07-17 1:31 GMT+08:00 Heiko Stuebner <heiko@sntech.de>:
> Hi Jacob,
>
> Am Donnerstag, 13. Juli 2017, 00:03:51 CEST schrieb Jacob Chen:
>> Add devicetree nodes for rk3399 VOP (Video Output Processors), and the
>> top level display-subsystem root node.
>>
>> Later patches add endpoints (eDP, HDMI, MIPI, etc) that attach to the
>> VOPs' output ports.
>>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>> Signed-off-by: Jacob Chen <jacob-chen@iotwrt.com>
>
> you might want to reduce the number of Signed-offs a bit :-)
>
> Also authorship needs to be fixed. I.e. first Signed-off for this patch
> is from Mark, so in that case Mark probably also was the original author
> and the patch should reflect that. General rule, topmost Signed-off is
> of course from the original author.
>

OK, get it.

>
> Heiko
>
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index e795135..300e500 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1455,6 +1455,71 @@ 
 		status = "disabled";
 	};
 
+	vopl: vop@ff8f0000 {
+		compatible = "rockchip,rk3399-vop-lit";
+		reg = <0x0 0xff8f0000 0x0 0x1ffc>, <0x0 0xff8f2000 0x0 0x400>;
+		interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_VOP1>, <&cru DCLK_VOP1>, <&cru HCLK_VOP1>, <&cru DCLK_VOP1_DIV>;
+		clock-names = "aclk_vop", "dclk_vop", "hclk_vop", "dclk_source";
+		resets = <&cru SRST_A_VOP1>, <&cru SRST_H_VOP1>, <&cru SRST_D_VOP1>;
+		reset-names = "axi", "ahb", "dclk";
+		power-domains = <&power RK3399_PD_VOPL>;
+		iommus = <&vopl_mmu>;
+		status = "disabled";
+
+		vopl_out: port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	vopl_mmu: iommu@ff8f3f00 {
+		compatible = "rockchip,iommu";
+		reg = <0x0 0xff8f3f00 0x0 0x100>;
+		interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "vopl_mmu";
+		clocks = <&cru ACLK_VOP1>, <&cru HCLK_VOP1>;
+		clock-names = "aclk", "hclk";
+		power-domains = <&power RK3399_PD_VOPL>;
+		#iommu-cells = <0>;
+		status = "disabled";
+	};
+
+	vopb: vop@ff900000 {
+		compatible = "rockchip,rk3399-vop-big";
+		reg = <0x0 0xff900000 0x0 0x1ffc>, <0x0 0xff902000 0x0 0x1000>;
+		interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>, <&cru DCLK_VOP0_DIV>;
+		clock-names = "aclk_vop", "dclk_vop", "hclk_vop", "dclk_source";
+		resets = <&cru SRST_A_VOP0>, <&cru SRST_H_VOP0>, <&cru SRST_D_VOP0>;
+		reset-names = "axi", "ahb", "dclk";
+		power-domains = <&power RK3399_PD_VOPB>;
+		iommus = <&vopb_mmu>;
+		status = "disabled";
+
+		vopb_out: port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	vopb_mmu: iommu@ff903f00 {
+		compatible = "rockchip,iommu";
+		reg = <0x0 0xff903f00 0x0 0x100>;
+		interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "vopb_mmu";
+		clocks = <&cru ACLK_VOP0>, <&cru HCLK_VOP0>;
+		clock-names = "aclk", "hclk";
+		power-domains = <&power RK3399_PD_VOPB>;
+		#iommu-cells = <0>;
+		status = "disabled";
+	};
+
+	display-subsystem {
+		compatible = "rockchip,display-subsystem";
+		ports = <&vopl_out>, <&vopb_out>;
+	};
+
 	pinctrl: pinctrl {
 		compatible = "rockchip,rk3399-pinctrl";
 		rockchip,grf = <&grf>;