Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC
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年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
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 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
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>; > + }; > +