Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC

2017-05-09 Thread Maxime Ripard
1;4601;0c
On Fri, May 05, 2017 at 08:34:16PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2017年5月5日 GMT+08:00 下午8:30:35, Maxime Ripard 
>  写到:
> >On Fri, May 05, 2017 at 04:53:43PM +0800, icen...@aosc.io wrote:
> >> > > +   de2_clocks: clock@100 {
> >> > > +   compatible =
> >"allwinner,sun50i-h5-de2-clk";
> >> > 
> >> > I am a bit skeptical about this. Since the V3S only has one mixer,
> >do
> >> > the clocks
> >> > for the second one even exist?
> >> 
> >> It's described in the de_clock.c in the BSP source code, and in
> >hardware
> >> these bits can be really set (although without clock output).
> >> 
> >> So I use this compatible which has still the extra clocks.
> >
> >If it's not usable, then it shouldn't be in the code, it's basically
> >dead code.
> 
> Thus should we have one more DE2 CCU compatible without mixer1
> clocks for V3s?

If those clocks don't exist on v3s, then yes.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC

2017-05-05 Thread Icenowy Zheng


于 2017年5月5日 GMT+08:00 下午8:30:35, Maxime Ripard 
 写到:
>On Fri, May 05, 2017 at 04:53:43PM +0800, icen...@aosc.io wrote:
>> > > +   de2_clocks: clock@100 {
>> > > +   compatible =
>"allwinner,sun50i-h5-de2-clk";
>> > 
>> > I am a bit skeptical about this. Since the V3S only has one mixer,
>do
>> > the clocks
>> > for the second one even exist?
>> 
>> It's described in the de_clock.c in the BSP source code, and in
>hardware
>> these bits can be really set (although without clock output).
>> 
>> So I use this compatible which has still the extra clocks.
>
>If it's not usable, then it shouldn't be in the code, it's basically
>dead code.

Thus should we have one more DE2 CCU compatible without mixer1
clocks for V3s?



Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC

2017-05-05 Thread Maxime Ripard
On Fri, May 05, 2017 at 04:53:43PM +0800, icen...@aosc.io wrote:
> > > +   de2_clocks: clock@100 {
> > > +   compatible = "allwinner,sun50i-h5-de2-clk";
> > 
> > I am a bit skeptical about this. Since the V3S only has one mixer, do
> > the clocks
> > for the second one even exist?
> 
> It's described in the de_clock.c in the BSP source code, and in hardware
> these bits can be really set (although without clock output).
> 
> So I use this compatible which has still the extra clocks.

If it's not usable, then it shouldn't be in the code, it's basically
dead code.

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC

2017-05-05 Thread icenowy

在 2017-05-05 11:31,Chen-Yu Tsai 写道:

On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng  wrote:

Allwinner V3s SoC features a "Display Engine 2.0" with only one TCON
which have RGB LCD output.


Please also mention that it only has one mixer.

For the subject, you could just say "Add device nodes for the display 
pipeline".




Add device nodes for it as well as the TCON.

Signed-off-by: Icenowy Zheng 
---
 arch/arm/boot/dts/sun8i-v3s.dtsi | 87 


 1 file changed, 87 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi 
b/arch/arm/boot/dts/sun8i-v3s.dtsi

index 71075969e5e6..0a895179d8ae 100644
--- a/arch/arm/boot/dts/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
@@ -41,6 +41,10 @@
  */

 #include 
+#include 
+#include 
+#include 
+#include 

 / {
#address-cells = <1>;
@@ -59,6 +63,12 @@
};
};

+   de: display-engine {
+   compatible = "allwinner,sun8i-v3s-display-engine";
+   allwinner,pipelines = <&de2_mixer0>;
+   status = "disabled";
+   };
+
timer {
compatible = "arm,armv7-timer";
interrupts = IRQ_TYPE_LEVEL_LOW)>,

@@ -93,6 +103,83 @@
#size-cells = <1>;
ranges;

+   de2_clocks: clock@100 {
+   compatible = "allwinner,sun50i-h5-de2-clk";


I am a bit skeptical about this. Since the V3S only has one mixer, do 
the clocks

for the second one even exist?


It's described in the de_clock.c in the BSP source code, and in hardware
these bits can be really set (although without clock output).

So I use this compatible which has still the extra clocks.




+   reg = <0x0100 0x10>;
+   clocks = <&ccu CLK_DE>,
+<&ccu CLK_BUS_DE>;
+   clock-names = "mod",
+ "bus";
+   resets = <&ccu RST_BUS_DE>;
+   #clock-cells = <1>;
+   #reset-cells = <1>;
+   };
+
+   de2_mixer0: mixer@110 {
+   compatible = "allwinner,sun8i-v3s-de2-mixer";
+   reg = <0x0110 0x10>;
+   clocks = <&de2_clocks CLK_MIXER0>,
+<&de2_clocks CLK_BUS_MIXER0>;
+   clock-names = "mod",
+ "bus";


Nit: could you list the bus clock first?

Regards
ChenYu


+   resets = <&de2_clocks RST_MIXER0>;
+   assigned-clocks = <&de2_clocks CLK_MIXER0>;
+   assigned-clock-rates = <15000>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   mixer0_out: port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   mixer0_out_tcon0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = 
<&tcon0_in_mixer0>;

+   };
+   };
+   };
+   };
+
+   tcon0: lcd-controller@1c0c000 {
+   compatible = "allwinner,sun8i-v3s-tcon";
+   reg = <0x01c0c000 0x1000>;
+   interrupts = ;
+   clocks = <&ccu CLK_BUS_TCON0>,
+<&ccu CLK_TCON0>;
+   clock-names = "ahb",
+ "tcon-ch0";
+   clock-output-names = "tcon-pixel-clock";
+   resets = <&ccu RST_BUS_TCON0>;
+   reset-names = "lcd";
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   tcon0_in: port@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   tcon0_in_mixer0: endpoint@0 {
+   reg = <0>;
+   remote-endpoint = 
<&mixer0_out_tcon0>;

+   };
+   };
+
+   tcon0_out: port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+   };
+

Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC

2017-05-04 Thread Chen-Yu Tsai
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng  wrote:
> Allwinner V3s SoC features a "Display Engine 2.0" with only one TCON
> which have RGB LCD output.

Please also mention that it only has one mixer.

For the subject, you could just say "Add device nodes for the display pipeline".

>
> Add device nodes for it as well as the TCON.
>
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm/boot/dts/sun8i-v3s.dtsi | 87 
> 
>  1 file changed, 87 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi 
> b/arch/arm/boot/dts/sun8i-v3s.dtsi
> index 71075969e5e6..0a895179d8ae 100644
> --- a/arch/arm/boot/dts/sun8i-v3s.dtsi
> +++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
> @@ -41,6 +41,10 @@
>   */
>
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
>
>  / {
> #address-cells = <1>;
> @@ -59,6 +63,12 @@
> };
> };
>
> +   de: display-engine {
> +   compatible = "allwinner,sun8i-v3s-display-engine";
> +   allwinner,pipelines = <&de2_mixer0>;
> +   status = "disabled";
> +   };
> +
> timer {
> compatible = "arm,armv7-timer";
> interrupts =  IRQ_TYPE_LEVEL_LOW)>,
> @@ -93,6 +103,83 @@
> #size-cells = <1>;
> ranges;
>
> +   de2_clocks: clock@100 {
> +   compatible = "allwinner,sun50i-h5-de2-clk";

I am a bit skeptical about this. Since the V3S only has one mixer, do the clocks
for the second one even exist?

> +   reg = <0x0100 0x10>;
> +   clocks = <&ccu CLK_DE>,
> +<&ccu CLK_BUS_DE>;
> +   clock-names = "mod",
> + "bus";
> +   resets = <&ccu RST_BUS_DE>;
> +   #clock-cells = <1>;
> +   #reset-cells = <1>;
> +   };
> +
> +   de2_mixer0: mixer@110 {
> +   compatible = "allwinner,sun8i-v3s-de2-mixer";
> +   reg = <0x0110 0x10>;
> +   clocks = <&de2_clocks CLK_MIXER0>,
> +<&de2_clocks CLK_BUS_MIXER0>;
> +   clock-names = "mod",
> + "bus";

Nit: could you list the bus clock first?

Regards
ChenYu

> +   resets = <&de2_clocks RST_MIXER0>;
> +   assigned-clocks = <&de2_clocks CLK_MIXER0>;
> +   assigned-clock-rates = <15000>;
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   mixer0_out: port@1 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   reg = <1>;
> +
> +   mixer0_out_tcon0: endpoint@0 {
> +   reg = <0>;
> +   remote-endpoint = 
> <&tcon0_in_mixer0>;
> +   };
> +   };
> +   };
> +   };
> +
> +   tcon0: lcd-controller@1c0c000 {
> +   compatible = "allwinner,sun8i-v3s-tcon";
> +   reg = <0x01c0c000 0x1000>;
> +   interrupts = ;
> +   clocks = <&ccu CLK_BUS_TCON0>,
> +<&ccu CLK_TCON0>;
> +   clock-names = "ahb",
> + "tcon-ch0";
> +   clock-output-names = "tcon-pixel-clock";
> +   resets = <&ccu RST_BUS_TCON0>;
> +   reset-names = "lcd";
> +   status = "disabled";
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   tcon0_in: port@0 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   reg = <0>;
> +
> +   tcon0_in_mixer0: endpoint@0 {
> +   reg = <0>;
> +   remote-endpoint = 
> <&mixer0_out_tcon0>;
> +   };
> +   };
> +
> +   tcon0_out: port@1 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   reg = <1>;
> +   };
> +