diff mbox

[RFC,10/11] ARM: sun8i: h3: add display engine pipeline for TVE

Message ID 20170517164354.16399-11-icenowy@aosc.io (mailing list archive)
State New, archived
Headers show

Commit Message

Icenowy Zheng May 17, 2017, 4:43 p.m. UTC
As we have already the support for the TV encoder on Allwinner H3, add
the display engine pipeline device tree nodes to its DTSI file.

The H5 pipeline has some differences and will be enabled later.

The currently-unused mixer0 and tcon0 are also needed, for the
completement of the pipeline.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 189 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 189 insertions(+)

Comments

Jernej Škrabec May 17, 2017, 8:19 p.m. UTC | #1
Hi,

Dne sreda, 17. maj 2017 ob 18:43:53 CEST je Icenowy Zheng napisal(a):
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
> 
> The H5 pipeline has some differences and will be enabled later.
> 
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> b/arch/arm/boot/dts/sun8i-h3.dtsi index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
> 
>  #include "sunxi-h3-h5.dtsi"
> +#include <dt-bindings/clock/sun8i-de2.h>
> +#include <dt-bindings/reset/sun8i-de2.h>
> 
>  / {
>  	cpus {
> @@ -72,6 +74,193 @@
>  		};
>  	};
> 
> +	de: display-engine {
> +		compatible = "allwinner,sun8i-h3-display-engine";
> +		allwinner,pipelines = <&mixer0>,
> +				      <&mixer1>;
> +		status = "disabled";
> +	};
> +
> +	soc {
> +		display_clocks: clock@1000000 {
> +			compatible = "allwinner,sun8i-a83t-de2-clk";
> +			reg = <0x01000000 0x100000>;
> +			clocks = <&ccu CLK_BUS_DE>,
> +				 <&ccu CLK_DE>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&ccu RST_BUS_DE>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			assigned-clocks = <&ccu CLK_DE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +			assigned-clock-rates = <432000000>;
> +		};
> +
> +		mixer0: mixer@1100000 {
> +			compatible = "allwinner,sun8i-h3-de2-mixer0";
> +			reg = <0x01100000 0x100000>;
> +			clocks = <&display_clocks CLK_BUS_MIXER0>,
> +				 <&display_clocks CLK_MIXER0>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&display_clocks RST_MIXER0>;
> +			status = "disabled";
> +
> +			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>;
> +					};
> +
> +					mixer0_out_tcon1: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&tcon1_in_mixer0>;
> +					};
> +				};
> +			};
> +		};
> +
> +		mixer1: mixer@1200000 {
> +			compatible = "allwinner,sun8i-h3-de2-mixer1";
> +			reg = <0x01200000 0x100000>;
> +			clocks = <&display_clocks CLK_BUS_MIXER1>,
> +				 <&display_clocks CLK_MIXER1>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&display_clocks RST_WB>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				mixer1_out: port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +
> +					mixer1_out_tcon1: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&tcon1_in_mixer1>;
> +					};
> +
> +					mixer1_out_tcon0: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&tcon0_in_mixer1>;
> +					};
> +				};
> +			};
> +		};
> +
> +		tcon0: lcd-controller@1c0c000 {
> +			compatible = "allwinner,sun8i-h3-tcon0";
> +			reg = <0x01c0c000 0x1000>;
> +			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_TCON0>,
> +				 <&ccu CLK_TCON0>;
> +			clock-names = "ahb",
> +				      "tcon-ch1";
> +			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_in_mixer1: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&mixer1_out_tcon0>;
> +					};
> +				};
> +			};
> +		};
> +
> +		tcon1: lcd-controller@1c0d000 {
> +			compatible = "allwinner,sun8i-h3-tcon1";
> +			reg = <0x01c0d000 0x1000>;
> +			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_TCON1>;
> +			clock-names = "ahb";
> +			resets = <&ccu RST_BUS_TCON1>;
> +			reset-names = "lcd";
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				tcon1_in: port@0 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <0>;
> +
> +					tcon1_in_mixer1: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&mixer1_out_tcon1>;
> +					};
> +
> +					tcon1_in_mixer0: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&mixer0_out_tcon1>;
> +					};
> +				};
> +
> +				tcon1_out: port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +
> +					tcon1_out_tve0: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&tve0_in_tcon1>;
> +					};
> +				};
> +			};
> +		};
> +
> +		tve0: tv-encoder@1e00000 {
> +			compatible = "allwinner,sun8i-h3-tv-encoder";
> +			reg = <0x01e00000 0x1000>;
> +			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_TVE>;
> +			status = "disabled";
> +
> +			assigned-clocks = <&ccu CLK_TVE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +			assigned-clock-rates = <216000000>;
> +
> +			port {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				tve0_in_tcon1: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&tcon1_out_tve0>;
> +				};

I think there should be out endpoint to composite connector (compatible: 
"composite-video-connector").

Best regards,
Jernej

> +			};
> +		};
> +	};
> +
>  	timer {
>  		compatible = "arm,armv7-timer";
>  		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | 
IRQ_TYPE_LEVEL_LOW)>,
> --
> 2.12.2
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Maxime Ripard May 19, 2017, 6:06 p.m. UTC | #2
On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
> 
> The H5 pipeline has some differences and will be enabled later.
> 
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 189 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
>  
>  #include "sunxi-h3-h5.dtsi"
> +#include <dt-bindings/clock/sun8i-de2.h>
> +#include <dt-bindings/reset/sun8i-de2.h>
>  
>  / {
>  	cpus {
> @@ -72,6 +74,193 @@
>  		};
>  	};
>  
> +	de: display-engine {
> +		compatible = "allwinner,sun8i-h3-display-engine";
> +		allwinner,pipelines = <&mixer0>,
> +				      <&mixer1>;
> +		status = "disabled";
> +	};
> +
> +	soc {
> +		display_clocks: clock@1000000 {
> +			compatible = "allwinner,sun8i-a83t-de2-clk";
> +			reg = <0x01000000 0x100000>;
> +			clocks = <&ccu CLK_BUS_DE>,
> +				 <&ccu CLK_DE>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&ccu RST_BUS_DE>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			assigned-clocks = <&ccu CLK_DE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +			assigned-clock-rates = <432000000>;

This shouldn't be set in the DT, but evaluated at runtime when calling
clk_set_rate.

> +		tve0: tv-encoder@1e00000 {
> +			compatible = "allwinner,sun8i-h3-tv-encoder";
> +			reg = <0x01e00000 0x1000>;
> +			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_TVE>;
> +			status = "disabled";
> +
> +			assigned-clocks = <&ccu CLK_TVE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;

Same thing here. clk_set_rate should just do the right thing.

> +			assigned-clock-rates = <216000000>;

And why are you setting it in the driver and in the DT?

Maxime
Icenowy Zheng May 19, 2017, 6:10 p.m. UTC | #3
于 2017年5月20日 GMT+08:00 上午2:06:16, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
>> As we have already the support for the TV encoder on Allwinner H3,
>add
>> the display engine pipeline device tree nodes to its DTSI file.
>> 
>> The H5 pipeline has some differences and will be enabled later.
>> 
>> The currently-unused mixer0 and tcon0 are also needed, for the
>> completement of the pipeline.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 189 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index b36f9f423c39..20172ef92415 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -41,6 +41,8 @@
>>   */
>>  
>>  #include "sunxi-h3-h5.dtsi"
>> +#include <dt-bindings/clock/sun8i-de2.h>
>> +#include <dt-bindings/reset/sun8i-de2.h>
>>  
>>  / {
>>  	cpus {
>> @@ -72,6 +74,193 @@
>>  		};
>>  	};
>>  
>> +	de: display-engine {
>> +		compatible = "allwinner,sun8i-h3-display-engine";
>> +		allwinner,pipelines = <&mixer0>,
>> +				      <&mixer1>;
>> +		status = "disabled";
>> +	};
>> +
>> +	soc {
>> +		display_clocks: clock@1000000 {
>> +			compatible = "allwinner,sun8i-a83t-de2-clk";
>> +			reg = <0x01000000 0x100000>;
>> +			clocks = <&ccu CLK_BUS_DE>,
>> +				 <&ccu CLK_DE>;
>> +			clock-names = "bus",
>> +				      "mod";
>> +			resets = <&ccu RST_BUS_DE>;
>> +			#clock-cells = <1>;
>> +			#reset-cells = <1>;
>> +			assigned-clocks = <&ccu CLK_DE>;
>> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
>> +			assigned-clock-rates = <432000000>;
>
>This shouldn't be set in the DT, but evaluated at runtime when calling
>clk_set_rate.

Nope, DE2 clock doesn't need evalution, as the clock is decoupled with
DE2 mixers' output signal. (Although it seems that SoCs with larger
plane size will use higher clock.)

And setting it to 432MHz is also needed for properly 216MHz clock to TVE.

>
>> +		tve0: tv-encoder@1e00000 {
>> +			compatible = "allwinner,sun8i-h3-tv-encoder";
>> +			reg = <0x01e00000 0x1000>;
>> +			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
>> +			clock-names = "bus", "mod";
>> +			resets = <&ccu RST_BUS_TVE>;
>> +			status = "disabled";
>> +
>> +			assigned-clocks = <&ccu CLK_TVE>;
>> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
>
>Same thing here. clk_set_rate should just do the right thing.
>
>> +			assigned-clock-rates = <216000000>;
>
>And why are you setting it in the driver and in the DT?
>
>Maxime
Chen-Yu Tsai May 24, 2017, 5:24 a.m. UTC | #4
On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io> wrote:
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
>
> The H5 pipeline has some differences and will be enabled later.
>
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 189 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
>
>  #include "sunxi-h3-h5.dtsi"
> +#include <dt-bindings/clock/sun8i-de2.h>
> +#include <dt-bindings/reset/sun8i-de2.h>
>
>  / {
>         cpus {
> @@ -72,6 +74,193 @@
>                 };
>         };
>
> +       de: display-engine {
> +               compatible = "allwinner,sun8i-h3-display-engine";
> +               allwinner,pipelines = <&mixer0>,
> +                                     <&mixer1>;
> +               status = "disabled";
> +       };
> +
> +       soc {
> +               display_clocks: clock@1000000 {
> +                       compatible = "allwinner,sun8i-a83t-de2-clk";
> +                       reg = <0x01000000 0x100000>;
> +                       clocks = <&ccu CLK_BUS_DE>,
> +                                <&ccu CLK_DE>;
> +                       clock-names = "bus",
> +                                     "mod";
> +                       resets = <&ccu RST_BUS_DE>;
> +                       #clock-cells = <1>;
> +                       #reset-cells = <1>;
> +                       assigned-clocks = <&ccu CLK_DE>;
> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +                       assigned-clock-rates = <432000000>;
> +               };
> +
> +               mixer0: mixer@1100000 {
> +                       compatible = "allwinner,sun8i-h3-de2-mixer0";
> +                       reg = <0x01100000 0x100000>;
> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
> +                                <&display_clocks CLK_MIXER0>;
> +                       clock-names = "bus",
> +                                     "mod";
> +                       resets = <&display_clocks RST_MIXER0>;
> +                       status = "disabled";
> +
> +                       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>;
> +                                       };
> +
> +                                       mixer0_out_tcon1: endpoint@1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&tcon1_in_mixer0>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               mixer1: mixer@1200000 {
> +                       compatible = "allwinner,sun8i-h3-de2-mixer1";
> +                       reg = <0x01200000 0x100000>;
> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
> +                                <&display_clocks CLK_MIXER1>;
> +                       clock-names = "bus",
> +                                     "mod";
> +                       resets = <&display_clocks RST_WB>;
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               mixer1_out: port@1 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <1>;
> +
> +                                       mixer1_out_tcon1: endpoint@0 {
> +                                               reg = <0>;

I would prefer if you could stick to the numbering scheme we're using for
Display Engine 1.0, as in endpoint 0 links to component 0 of whatever type.

We're probably going to stick to that for the R40's incredibly complicated
pipeline. I don't want to have any outliers unless absolutely necessary.

ChenYu

> +                                               remote-endpoint = <&tcon1_in_mixer1>;
> +                                       };
> +
> +                                       mixer1_out_tcon0: endpoint@1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&tcon0_in_mixer1>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               tcon0: lcd-controller@1c0c000 {
> +                       compatible = "allwinner,sun8i-h3-tcon0";
> +                       reg = <0x01c0c000 0x1000>;
> +                       interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_TCON0>,
> +                                <&ccu CLK_TCON0>;
> +                       clock-names = "ahb",
> +                                     "tcon-ch1";
> +                       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_in_mixer1: endpoint@1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&mixer1_out_tcon0>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               tcon1: lcd-controller@1c0d000 {
> +                       compatible = "allwinner,sun8i-h3-tcon1";
> +                       reg = <0x01c0d000 0x1000>;
> +                       interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_TCON1>;
> +                       clock-names = "ahb";
> +                       resets = <&ccu RST_BUS_TCON1>;
> +                       reset-names = "lcd";
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               tcon1_in: port@0 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <0>;
> +
> +                                       tcon1_in_mixer1: endpoint@0 {
> +                                               reg = <0>;
> +                                               remote-endpoint = <&mixer1_out_tcon1>;
> +                                       };
> +
> +                                       tcon1_in_mixer0: endpoint@1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&mixer0_out_tcon1>;
> +                                       };
> +                               };
> +
> +                               tcon1_out: port@1 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <1>;
> +
> +                                       tcon1_out_tve0: endpoint@1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&tve0_in_tcon1>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               tve0: tv-encoder@1e00000 {
> +                       compatible = "allwinner,sun8i-h3-tv-encoder";
> +                       reg = <0x01e00000 0x1000>;
> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
> +                       clock-names = "bus", "mod";
> +                       resets = <&ccu RST_BUS_TVE>;
> +                       status = "disabled";
> +
> +                       assigned-clocks = <&ccu CLK_TVE>;
> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +                       assigned-clock-rates = <216000000>;
> +
> +                       port {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               tve0_in_tcon1: endpoint@0 {
> +                                       reg = <0>;
> +                                       remote-endpoint = <&tcon1_out_tve0>;
> +                               };
> +                       };
> +               };
> +       };
> +
>         timer {
>                 compatible = "arm,armv7-timer";
>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> --
> 2.12.2
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Icenowy Zheng May 24, 2017, 5:28 a.m. UTC | #5
于 2017年5月24日 GMT+08:00 下午1:24:29, Chen-Yu Tsai <wens@csie.org> 写到:
>On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io>
>wrote:
>> As we have already the support for the TV encoder on Allwinner H3,
>add
>> the display engine pipeline device tree nodes to its DTSI file.
>>
>> The H5 pipeline has some differences and will be enabled later.
>>
>> The currently-unused mixer0 and tcon0 are also needed, for the
>> completement of the pipeline.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 189 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index b36f9f423c39..20172ef92415 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -41,6 +41,8 @@
>>   */
>>
>>  #include "sunxi-h3-h5.dtsi"
>> +#include <dt-bindings/clock/sun8i-de2.h>
>> +#include <dt-bindings/reset/sun8i-de2.h>
>>
>>  / {
>>         cpus {
>> @@ -72,6 +74,193 @@
>>                 };
>>         };
>>
>> +       de: display-engine {
>> +               compatible = "allwinner,sun8i-h3-display-engine";
>> +               allwinner,pipelines = <&mixer0>,
>> +                                     <&mixer1>;
>> +               status = "disabled";
>> +       };
>> +
>> +       soc {
>> +               display_clocks: clock@1000000 {
>> +                       compatible = "allwinner,sun8i-a83t-de2-clk";
>> +                       reg = <0x01000000 0x100000>;
>> +                       clocks = <&ccu CLK_BUS_DE>,
>> +                                <&ccu CLK_DE>;
>> +                       clock-names = "bus",
>> +                                     "mod";
>> +                       resets = <&ccu RST_BUS_DE>;
>> +                       #clock-cells = <1>;
>> +                       #reset-cells = <1>;
>> +                       assigned-clocks = <&ccu CLK_DE>;
>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>> +                       assigned-clock-rates = <432000000>;
>> +               };
>> +
>> +               mixer0: mixer@1100000 {
>> +                       compatible = "allwinner,sun8i-h3-de2-mixer0";
>> +                       reg = <0x01100000 0x100000>;
>> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
>> +                                <&display_clocks CLK_MIXER0>;
>> +                       clock-names = "bus",
>> +                                     "mod";
>> +                       resets = <&display_clocks RST_MIXER0>;
>> +                       status = "disabled";
>> +
>> +                       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>;
>> +                                       };
>> +
>> +                                       mixer0_out_tcon1: endpoint@1
>{
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&tcon1_in_mixer0>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               mixer1: mixer@1200000 {
>> +                       compatible = "allwinner,sun8i-h3-de2-mixer1";
>> +                       reg = <0x01200000 0x100000>;
>> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
>> +                                <&display_clocks CLK_MIXER1>;
>> +                       clock-names = "bus",
>> +                                     "mod";
>> +                       resets = <&display_clocks RST_WB>;
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               mixer1_out: port@1 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <1>;
>> +
>> +                                       mixer1_out_tcon1: endpoint@0
>{
>> +                                               reg = <0>;
>
>I would prefer if you could stick to the numbering scheme we're using
>for
>Display Engine 1.0, as in endpoint 0 links to component 0 of whatever
>type.

If we keep this we will need a ugly id property in mixer node,
otherwise we cannot know which TCON to be bind.

>
>We're probably going to stick to that for the R40's incredibly
>complicated
>pipeline. I don't want to have any outliers unless absolutely
>necessary.
>
>ChenYu
>
>> +                                               remote-endpoint =
><&tcon1_in_mixer1>;
>> +                                       };
>> +
>> +                                       mixer1_out_tcon0: endpoint@1
>{
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&tcon0_in_mixer1>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               tcon0: lcd-controller@1c0c000 {
>> +                       compatible = "allwinner,sun8i-h3-tcon0";
>> +                       reg = <0x01c0c000 0x1000>;
>> +                       interrupts = <GIC_SPI 86
>IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&ccu CLK_BUS_TCON0>,
>> +                                <&ccu CLK_TCON0>;
>> +                       clock-names = "ahb",
>> +                                     "tcon-ch1";
>> +                       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_in_mixer1: endpoint@1 {
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&mixer1_out_tcon0>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               tcon1: lcd-controller@1c0d000 {
>> +                       compatible = "allwinner,sun8i-h3-tcon1";
>> +                       reg = <0x01c0d000 0x1000>;
>> +                       interrupts = <GIC_SPI 87
>IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&ccu CLK_BUS_TCON1>;
>> +                       clock-names = "ahb";
>> +                       resets = <&ccu RST_BUS_TCON1>;
>> +                       reset-names = "lcd";
>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               tcon1_in: port@0 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <0>;
>> +
>> +                                       tcon1_in_mixer1: endpoint@0 {
>> +                                               reg = <0>;
>> +                                               remote-endpoint =
><&mixer1_out_tcon1>;
>> +                                       };
>> +
>> +                                       tcon1_in_mixer0: endpoint@1 {
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&mixer0_out_tcon1>;
>> +                                       };
>> +                               };
>> +
>> +                               tcon1_out: port@1 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <1>;
>> +
>> +                                       tcon1_out_tve0: endpoint@1 {
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&tve0_in_tcon1>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               tve0: tv-encoder@1e00000 {
>> +                       compatible = "allwinner,sun8i-h3-tv-encoder";
>> +                       reg = <0x01e00000 0x1000>;
>> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
>> +                       clock-names = "bus", "mod";
>> +                       resets = <&ccu RST_BUS_TVE>;
>> +                       status = "disabled";
>> +
>> +                       assigned-clocks = <&ccu CLK_TVE>;
>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>> +                       assigned-clock-rates = <216000000>;
>> +
>> +                       port {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               tve0_in_tcon1: endpoint@0 {
>> +                                       reg = <0>;
>> +                                       remote-endpoint =
><&tcon1_out_tve0>;
>> +                               };
>> +                       };
>> +               };
>> +       };
>> +
>>         timer {
>>                 compatible = "arm,armv7-timer";
>>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>IRQ_TYPE_LEVEL_LOW)>,
>> --
>> 2.12.2
>>
>> --
>> You received this message because you are subscribed to the Google
>Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it,
>send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
Chen-Yu Tsai May 24, 2017, 5:34 a.m. UTC | #6
On Wed, May 24, 2017 at 1:28 PM, Icenowy Zheng <icenowy@aosc.io> wrote:
>
>
> 于 2017年5月24日 GMT+08:00 下午1:24:29, Chen-Yu Tsai <wens@csie.org> 写到:
>>On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io>
>>wrote:
>>> As we have already the support for the TV encoder on Allwinner H3,
>>add
>>> the display engine pipeline device tree nodes to its DTSI file.
>>>
>>> The H5 pipeline has some differences and will be enabled later.
>>>
>>> The currently-unused mixer0 and tcon0 are also needed, for the
>>> completement of the pipeline.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>> ---
>>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>>++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 189 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> index b36f9f423c39..20172ef92415 100644
>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> @@ -41,6 +41,8 @@
>>>   */
>>>
>>>  #include "sunxi-h3-h5.dtsi"
>>> +#include <dt-bindings/clock/sun8i-de2.h>
>>> +#include <dt-bindings/reset/sun8i-de2.h>
>>>
>>>  / {
>>>         cpus {
>>> @@ -72,6 +74,193 @@
>>>                 };
>>>         };
>>>
>>> +       de: display-engine {
>>> +               compatible = "allwinner,sun8i-h3-display-engine";
>>> +               allwinner,pipelines = <&mixer0>,
>>> +                                     <&mixer1>;
>>> +               status = "disabled";
>>> +       };
>>> +
>>> +       soc {
>>> +               display_clocks: clock@1000000 {
>>> +                       compatible = "allwinner,sun8i-a83t-de2-clk";
>>> +                       reg = <0x01000000 0x100000>;
>>> +                       clocks = <&ccu CLK_BUS_DE>,
>>> +                                <&ccu CLK_DE>;
>>> +                       clock-names = "bus",
>>> +                                     "mod";
>>> +                       resets = <&ccu RST_BUS_DE>;
>>> +                       #clock-cells = <1>;
>>> +                       #reset-cells = <1>;
>>> +                       assigned-clocks = <&ccu CLK_DE>;
>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>> +                       assigned-clock-rates = <432000000>;
>>> +               };
>>> +
>>> +               mixer0: mixer@1100000 {
>>> +                       compatible = "allwinner,sun8i-h3-de2-mixer0";
>>> +                       reg = <0x01100000 0x100000>;
>>> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
>>> +                                <&display_clocks CLK_MIXER0>;
>>> +                       clock-names = "bus",
>>> +                                     "mod";
>>> +                       resets = <&display_clocks RST_MIXER0>;
>>> +                       status = "disabled";
>>> +
>>> +                       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>;
>>> +                                       };
>>> +
>>> +                                       mixer0_out_tcon1: endpoint@1
>>{
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&tcon1_in_mixer0>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               mixer1: mixer@1200000 {
>>> +                       compatible = "allwinner,sun8i-h3-de2-mixer1";
>>> +                       reg = <0x01200000 0x100000>;
>>> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
>>> +                                <&display_clocks CLK_MIXER1>;
>>> +                       clock-names = "bus",
>>> +                                     "mod";
>>> +                       resets = <&display_clocks RST_WB>;
>>> +
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               mixer1_out: port@1 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <1>;
>>> +
>>> +                                       mixer1_out_tcon1: endpoint@0
>>{
>>> +                                               reg = <0>;
>>
>>I would prefer if you could stick to the numbering scheme we're using
>>for
>>Display Engine 1.0, as in endpoint 0 links to component 0 of whatever
>>type.
>
> If we keep this we will need a ugly id property in mixer node,
> otherwise we cannot know which TCON to be bind.

Why? You can simply change the logic in your driver from:

    if (remote_endpoint.id) { continue; }

to:

    if (local_endpoint.id != remote_endpoint.id) { continue; }

I don't see the need for any ID property in the mixer node in this case.
The ID is already encoded into the endpoint IDs.

ChenYu

>>
>>We're probably going to stick to that for the R40's incredibly
>>complicated
>>pipeline. I don't want to have any outliers unless absolutely
>>necessary.
>>
>>ChenYu
>>
>>> +                                               remote-endpoint =
>><&tcon1_in_mixer1>;
>>> +                                       };
>>> +
>>> +                                       mixer1_out_tcon0: endpoint@1
>>{
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&tcon0_in_mixer1>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               tcon0: lcd-controller@1c0c000 {
>>> +                       compatible = "allwinner,sun8i-h3-tcon0";
>>> +                       reg = <0x01c0c000 0x1000>;
>>> +                       interrupts = <GIC_SPI 86
>>IRQ_TYPE_LEVEL_HIGH>;
>>> +                       clocks = <&ccu CLK_BUS_TCON0>,
>>> +                                <&ccu CLK_TCON0>;
>>> +                       clock-names = "ahb",
>>> +                                     "tcon-ch1";
>>> +                       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_in_mixer1: endpoint@1 {
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&mixer1_out_tcon0>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               tcon1: lcd-controller@1c0d000 {
>>> +                       compatible = "allwinner,sun8i-h3-tcon1";
>>> +                       reg = <0x01c0d000 0x1000>;
>>> +                       interrupts = <GIC_SPI 87
>>IRQ_TYPE_LEVEL_HIGH>;
>>> +                       clocks = <&ccu CLK_BUS_TCON1>;
>>> +                       clock-names = "ahb";
>>> +                       resets = <&ccu RST_BUS_TCON1>;
>>> +                       reset-names = "lcd";
>>> +                       status = "disabled";
>>> +
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               tcon1_in: port@0 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <0>;
>>> +
>>> +                                       tcon1_in_mixer1: endpoint@0 {
>>> +                                               reg = <0>;
>>> +                                               remote-endpoint =
>><&mixer1_out_tcon1>;
>>> +                                       };
>>> +
>>> +                                       tcon1_in_mixer0: endpoint@1 {
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&mixer0_out_tcon1>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               tcon1_out: port@1 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <1>;
>>> +
>>> +                                       tcon1_out_tve0: endpoint@1 {
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&tve0_in_tcon1>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               tve0: tv-encoder@1e00000 {
>>> +                       compatible = "allwinner,sun8i-h3-tv-encoder";
>>> +                       reg = <0x01e00000 0x1000>;
>>> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
>>> +                       clock-names = "bus", "mod";
>>> +                       resets = <&ccu RST_BUS_TVE>;
>>> +                       status = "disabled";
>>> +
>>> +                       assigned-clocks = <&ccu CLK_TVE>;
>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>> +                       assigned-clock-rates = <216000000>;
>>> +
>>> +                       port {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               tve0_in_tcon1: endpoint@0 {
>>> +                                       reg = <0>;
>>> +                                       remote-endpoint =
>><&tcon1_out_tve0>;
>>> +                               };
>>> +                       };
>>> +               };
>>> +       };
>>> +
>>>         timer {
>>>                 compatible = "arm,armv7-timer";
>>>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>>IRQ_TYPE_LEVEL_LOW)>,
>>> --
>>> 2.12.2
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>Groups "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>send an email to linux-sunxi+unsubscribe@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Icenowy Zheng May 24, 2017, 5:36 a.m. UTC | #7
于 2017年5月24日 GMT+08:00 下午1:34:58, Chen-Yu Tsai <wens@csie.org> 写到:
>On Wed, May 24, 2017 at 1:28 PM, Icenowy Zheng <icenowy@aosc.io> wrote:
>>
>>
>> 于 2017年5月24日 GMT+08:00 下午1:24:29, Chen-Yu Tsai <wens@csie.org> 写到:
>>>On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io>
>>>wrote:
>>>> As we have already the support for the TV encoder on Allwinner H3,
>>>add
>>>> the display engine pipeline device tree nodes to its DTSI file.
>>>>
>>>> The H5 pipeline has some differences and will be enabled later.
>>>>
>>>> The currently-unused mixer0 and tcon0 are also needed, for the
>>>> completement of the pipeline.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>>> ---
>>>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>>>++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 189 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> index b36f9f423c39..20172ef92415 100644
>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> @@ -41,6 +41,8 @@
>>>>   */
>>>>
>>>>  #include "sunxi-h3-h5.dtsi"
>>>> +#include <dt-bindings/clock/sun8i-de2.h>
>>>> +#include <dt-bindings/reset/sun8i-de2.h>
>>>>
>>>>  / {
>>>>         cpus {
>>>> @@ -72,6 +74,193 @@
>>>>                 };
>>>>         };
>>>>
>>>> +       de: display-engine {
>>>> +               compatible = "allwinner,sun8i-h3-display-engine";
>>>> +               allwinner,pipelines = <&mixer0>,
>>>> +                                     <&mixer1>;
>>>> +               status = "disabled";
>>>> +       };
>>>> +
>>>> +       soc {
>>>> +               display_clocks: clock@1000000 {
>>>> +                       compatible =
>"allwinner,sun8i-a83t-de2-clk";
>>>> +                       reg = <0x01000000 0x100000>;
>>>> +                       clocks = <&ccu CLK_BUS_DE>,
>>>> +                                <&ccu CLK_DE>;
>>>> +                       clock-names = "bus",
>>>> +                                     "mod";
>>>> +                       resets = <&ccu RST_BUS_DE>;
>>>> +                       #clock-cells = <1>;
>>>> +                       #reset-cells = <1>;
>>>> +                       assigned-clocks = <&ccu CLK_DE>;
>>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>>> +                       assigned-clock-rates = <432000000>;
>>>> +               };
>>>> +
>>>> +               mixer0: mixer@1100000 {
>>>> +                       compatible =
>"allwinner,sun8i-h3-de2-mixer0";
>>>> +                       reg = <0x01100000 0x100000>;
>>>> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
>>>> +                                <&display_clocks CLK_MIXER0>;
>>>> +                       clock-names = "bus",
>>>> +                                     "mod";
>>>> +                       resets = <&display_clocks RST_MIXER0>;
>>>> +                       status = "disabled";
>>>> +
>>>> +                       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>;
>>>> +                                       };
>>>> +
>>>> +                                       mixer0_out_tcon1:
>endpoint@1
>>>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&tcon1_in_mixer0>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               mixer1: mixer@1200000 {
>>>> +                       compatible =
>"allwinner,sun8i-h3-de2-mixer1";
>>>> +                       reg = <0x01200000 0x100000>;
>>>> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
>>>> +                                <&display_clocks CLK_MIXER1>;
>>>> +                       clock-names = "bus",
>>>> +                                     "mod";
>>>> +                       resets = <&display_clocks RST_WB>;
>>>> +
>>>> +                       ports {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               mixer1_out: port@1 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <1>;
>>>> +
>>>> +                                       mixer1_out_tcon1:
>endpoint@0
>>>{
>>>> +                                               reg = <0>;
>>>
>>>I would prefer if you could stick to the numbering scheme we're using
>>>for
>>>Display Engine 1.0, as in endpoint 0 links to component 0 of whatever
>>>type.
>>
>> If we keep this we will need a ugly id property in mixer node,
>> otherwise we cannot know which TCON to be bind.
>
>Why? You can simply change the logic in your driver from:
>
>    if (remote_endpoint.id) { continue; }
>
>to:
>
>    if (local_endpoint.id != remote_endpoint.id) { continue; }

Thanks, I forgot that there's two endpoint ids...

So silly I am.

>
>I don't see the need for any ID property in the mixer node in this
>case.
>The ID is already encoded into the endpoint IDs.
>
>ChenYu
>
>>>
>>>We're probably going to stick to that for the R40's incredibly
>>>complicated
>>>pipeline. I don't want to have any outliers unless absolutely
>>>necessary.
>>>
>>>ChenYu
>>>
>>>> +                                               remote-endpoint =
>>><&tcon1_in_mixer1>;
>>>> +                                       };
>>>> +
>>>> +                                       mixer1_out_tcon0:
>endpoint@1
>>>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&tcon0_in_mixer1>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               tcon0: lcd-controller@1c0c000 {
>>>> +                       compatible = "allwinner,sun8i-h3-tcon0";
>>>> +                       reg = <0x01c0c000 0x1000>;
>>>> +                       interrupts = <GIC_SPI 86
>>>IRQ_TYPE_LEVEL_HIGH>;
>>>> +                       clocks = <&ccu CLK_BUS_TCON0>,
>>>> +                                <&ccu CLK_TCON0>;
>>>> +                       clock-names = "ahb",
>>>> +                                     "tcon-ch1";
>>>> +                       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_in_mixer1: endpoint@1
>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&mixer1_out_tcon0>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               tcon1: lcd-controller@1c0d000 {
>>>> +                       compatible = "allwinner,sun8i-h3-tcon1";
>>>> +                       reg = <0x01c0d000 0x1000>;
>>>> +                       interrupts = <GIC_SPI 87
>>>IRQ_TYPE_LEVEL_HIGH>;
>>>> +                       clocks = <&ccu CLK_BUS_TCON1>;
>>>> +                       clock-names = "ahb";
>>>> +                       resets = <&ccu RST_BUS_TCON1>;
>>>> +                       reset-names = "lcd";
>>>> +                       status = "disabled";
>>>> +
>>>> +                       ports {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               tcon1_in: port@0 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <0>;
>>>> +
>>>> +                                       tcon1_in_mixer1: endpoint@0
>{
>>>> +                                               reg = <0>;
>>>> +                                               remote-endpoint =
>>><&mixer1_out_tcon1>;
>>>> +                                       };
>>>> +
>>>> +                                       tcon1_in_mixer0: endpoint@1
>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&mixer0_out_tcon1>;
>>>> +                                       };
>>>> +                               };
>>>> +
>>>> +                               tcon1_out: port@1 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <1>;
>>>> +
>>>> +                                       tcon1_out_tve0: endpoint@1
>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&tve0_in_tcon1>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               tve0: tv-encoder@1e00000 {
>>>> +                       compatible =
>"allwinner,sun8i-h3-tv-encoder";
>>>> +                       reg = <0x01e00000 0x1000>;
>>>> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu
>CLK_TVE>;
>>>> +                       clock-names = "bus", "mod";
>>>> +                       resets = <&ccu RST_BUS_TVE>;
>>>> +                       status = "disabled";
>>>> +
>>>> +                       assigned-clocks = <&ccu CLK_TVE>;
>>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>>> +                       assigned-clock-rates = <216000000>;
>>>> +
>>>> +                       port {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               tve0_in_tcon1: endpoint@0 {
>>>> +                                       reg = <0>;
>>>> +                                       remote-endpoint =
>>><&tcon1_out_tve0>;
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +
>>>>         timer {
>>>>                 compatible = "arm,armv7-timer";
>>>>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>>>IRQ_TYPE_LEVEL_LOW)>,
>>>> --
>>>> 2.12.2
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>Groups "linux-sunxi" group.
>>>> To unsubscribe from this group and stop receiving emails from it,
>>>send an email to linux-sunxi+unsubscribe@googlegroups.com.
>>>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google
>Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it,
>send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
Maxime Ripard May 24, 2017, 8:19 a.m. UTC | #8
On Sat, May 20, 2017 at 02:10:35AM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2017年5月20日 GMT+08:00 上午2:06:16, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
> >On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
> >> As we have already the support for the TV encoder on Allwinner H3,
> >add
> >> the display engine pipeline device tree nodes to its DTSI file.
> >> 
> >> The H5 pipeline has some differences and will be enabled later.
> >> 
> >> The currently-unused mixer0 and tcon0 are also needed, for the
> >> completement of the pipeline.
> >> 
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> ---
> >>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
> >++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 189 insertions(+)
> >> 
> >> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> >b/arch/arm/boot/dts/sun8i-h3.dtsi
> >> index b36f9f423c39..20172ef92415 100644
> >> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> >> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> >> @@ -41,6 +41,8 @@
> >>   */
> >>  
> >>  #include "sunxi-h3-h5.dtsi"
> >> +#include <dt-bindings/clock/sun8i-de2.h>
> >> +#include <dt-bindings/reset/sun8i-de2.h>
> >>  
> >>  / {
> >>  	cpus {
> >> @@ -72,6 +74,193 @@
> >>  		};
> >>  	};
> >>  
> >> +	de: display-engine {
> >> +		compatible = "allwinner,sun8i-h3-display-engine";
> >> +		allwinner,pipelines = <&mixer0>,
> >> +				      <&mixer1>;
> >> +		status = "disabled";
> >> +	};
> >> +
> >> +	soc {
> >> +		display_clocks: clock@1000000 {
> >> +			compatible = "allwinner,sun8i-a83t-de2-clk";
> >> +			reg = <0x01000000 0x100000>;
> >> +			clocks = <&ccu CLK_BUS_DE>,
> >> +				 <&ccu CLK_DE>;
> >> +			clock-names = "bus",
> >> +				      "mod";
> >> +			resets = <&ccu RST_BUS_DE>;
> >> +			#clock-cells = <1>;
> >> +			#reset-cells = <1>;
> >> +			assigned-clocks = <&ccu CLK_DE>;
> >> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> >> +			assigned-clock-rates = <432000000>;
> >
> >This shouldn't be set in the DT, but evaluated at runtime when calling
> >clk_set_rate.
> 
> Nope, DE2 clock doesn't need evalution, as the clock is decoupled with
> DE2 mixers' output signal. (Although it seems that SoCs with larger
> plane size will use higher clock.)

So it's the display engine that needs that clock to operate properly?
This is the wrong DT node to set that value then. It should be in the
mixer node, or even better in the mixers' driver.

> And setting it to 432MHz is also needed for properly 216MHz clock to
> TVE.

Just like the parenthood, this can and should be evaluated at runtime.

Maxime
diff mbox

Patch

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index b36f9f423c39..20172ef92415 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -41,6 +41,8 @@ 
  */
 
 #include "sunxi-h3-h5.dtsi"
+#include <dt-bindings/clock/sun8i-de2.h>
+#include <dt-bindings/reset/sun8i-de2.h>
 
 / {
 	cpus {
@@ -72,6 +74,193 @@ 
 		};
 	};
 
+	de: display-engine {
+		compatible = "allwinner,sun8i-h3-display-engine";
+		allwinner,pipelines = <&mixer0>,
+				      <&mixer1>;
+		status = "disabled";
+	};
+
+	soc {
+		display_clocks: clock@1000000 {
+			compatible = "allwinner,sun8i-a83t-de2-clk";
+			reg = <0x01000000 0x100000>;
+			clocks = <&ccu CLK_BUS_DE>,
+				 <&ccu CLK_DE>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&ccu RST_BUS_DE>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			assigned-clocks = <&ccu CLK_DE>;
+			assigned-clock-parents = <&ccu CLK_PLL_DE>;
+			assigned-clock-rates = <432000000>;
+		};
+
+		mixer0: mixer@1100000 {
+			compatible = "allwinner,sun8i-h3-de2-mixer0";
+			reg = <0x01100000 0x100000>;
+			clocks = <&display_clocks CLK_BUS_MIXER0>,
+				 <&display_clocks CLK_MIXER0>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&display_clocks RST_MIXER0>;
+			status = "disabled";
+
+			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>;
+					};
+
+					mixer0_out_tcon1: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&tcon1_in_mixer0>;
+					};
+				};
+			};
+		};
+
+		mixer1: mixer@1200000 {
+			compatible = "allwinner,sun8i-h3-de2-mixer1";
+			reg = <0x01200000 0x100000>;
+			clocks = <&display_clocks CLK_BUS_MIXER1>,
+				 <&display_clocks CLK_MIXER1>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&display_clocks RST_WB>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				mixer1_out: port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					mixer1_out_tcon1: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&tcon1_in_mixer1>;
+					};
+
+					mixer1_out_tcon0: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&tcon0_in_mixer1>;
+					};
+				};
+			};
+		};
+
+		tcon0: lcd-controller@1c0c000 {
+			compatible = "allwinner,sun8i-h3-tcon0";
+			reg = <0x01c0c000 0x1000>;
+			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_TCON0>,
+				 <&ccu CLK_TCON0>;
+			clock-names = "ahb",
+				      "tcon-ch1";
+			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_in_mixer1: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&mixer1_out_tcon0>;
+					};
+				};
+			};
+		};
+
+		tcon1: lcd-controller@1c0d000 {
+			compatible = "allwinner,sun8i-h3-tcon1";
+			reg = <0x01c0d000 0x1000>;
+			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_TCON1>;
+			clock-names = "ahb";
+			resets = <&ccu RST_BUS_TCON1>;
+			reset-names = "lcd";
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tcon1_in: port@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+
+					tcon1_in_mixer1: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&mixer1_out_tcon1>;
+					};
+
+					tcon1_in_mixer0: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&mixer0_out_tcon1>;
+					};
+				};
+
+				tcon1_out: port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					tcon1_out_tve0: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&tve0_in_tcon1>;
+					};
+				};
+			};
+		};
+
+		tve0: tv-encoder@1e00000 {
+			compatible = "allwinner,sun8i-h3-tv-encoder";
+			reg = <0x01e00000 0x1000>;
+			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_TVE>;
+			status = "disabled";
+
+			assigned-clocks = <&ccu CLK_TVE>;
+			assigned-clock-parents = <&ccu CLK_PLL_DE>;
+			assigned-clock-rates = <216000000>;
+
+			port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tve0_in_tcon1: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&tcon1_out_tve0>;
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv7-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,