diff mbox

[v2,01/11] dt-bindings: update the binding for Allwinner H3 TVE support

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

Commit Message

Icenowy Zheng June 4, 2017, 4:01 p.m. UTC
Allwinner H3 features a "DE2.0" and a TV Encoder.

Add device tree bindings for the following parts:
- H3 TCONs
- H3 Mixers
- The connection between H3 TCONs and H3 Mixers
- H3 TV Encoder
- H3 Display engine

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- Changed endpoint reg definition on TCONs and Mixers. Now the endpoint
  id is just the component id.
- Changed TVE and TCON1 binding -- now CLK_TVE is passed as TCON1 lcd-ch1.

 .../bindings/display/sunxi/sun4i-drm.txt           | 37 +++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

Comments

Maxime Ripard June 7, 2017, 8:45 a.m. UTC | #1
On Mon, Jun 05, 2017 at 12:01:39AM +0800, Icenowy Zheng wrote:
> Allwinner H3 features a "DE2.0" and a TV Encoder.
> 
> Add device tree bindings for the following parts:
> - H3 TCONs
> - H3 Mixers
> - The connection between H3 TCONs and H3 Mixers
> - H3 TV Encoder
> - H3 Display engine
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> Changes in v2:
> - Changed endpoint reg definition on TCONs and Mixers. Now the endpoint
>   id is just the component id.
> - Changed TVE and TCON1 binding -- now CLK_TVE is passed as TCON1 lcd-ch1.
> 
>  .../bindings/display/sunxi/sun4i-drm.txt           | 37 +++++++++++++++++++---
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index b83e6018041d..7ad164cb7dcb 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -49,7 +49,9 @@ The TV Encoder supports the composite and VGA output. It is one end of
>  the pipeline.
>  
>  Required properties:
> - - compatible: value should be "allwinner,sun4i-a10-tv-encoder".
> + - compatible: value must be either:
> +    * allwinner,sun4i-a10-tv-encoder
> +    * allwinner,sun8i-h3-tv-encoder
>   - reg: base address and size of memory-mapped region
>   - clocks: the clocks driving the TV encoder
>   - resets: phandle to the reset controller driving the encoder
> @@ -69,23 +71,27 @@ Required properties:
>     * allwinner,sun6i-a31-tcon
>     * allwinner,sun6i-a31s-tcon
>     * allwinner,sun8i-a33-tcon
> +   * allwinner,sun8i-h3-tcon
>     * allwinner,sun8i-v3s-tcon
>   - reg: base address and size of memory-mapped region
>   - interrupts: interrupt associated to this IP
>   - clocks: phandles to the clocks feeding the TCON. Three are needed:
>     - 'ahb': the interface clocks
> -   - 'tcon-ch0': The clock driving the TCON channel 0
>   - resets: phandles to the reset controllers driving the encoder
>     - "lcd": the reset line for the TCON channel 0
>  
>   - clock-names: the clock names mentioned above
>   - reset-names: the reset names mentioned above
> - - clock-output-names: Name of the pixel clock created
>  
>  - ports: A ports node with endpoint definitions as defined in
>    Documentation/devicetree/bindings/media/video-interfaces.txt. The
>    first port should be the input endpoint, the second one the output
>  
> +  In the situation of Display Engine 2.0 that the connection between
> +  the mixer and the TCON can be swapped, the input should have two
> +  endpoints. The first input endpoint should be connected to mixer0
> +  and the second should be connected to mixer1.
> +

AFAIK, this is also the case for dual-pipelines on DE1 (for example on
the A31). And this is already covered by:

"
For the input port of all components up to the TCON in the display
pipeline, if there are multiple components, the local endpoint IDs
must correspond to the index of the upstream block. For example, if
the remote endpoint is Frontend 1, then the local endpoint ID must
be 1.
"

>    The output may have multiple endpoints. The TCON has two channels,
>    usually with the first channel being used for the panels interfaces
>    (RGB, LVDS, etc.), and the second being used for the outputs that
> @@ -94,7 +100,23 @@ Required properties:
>    channel the endpoint is associated to. If that property is not
>    present, the endpoint number will be used as the channel number.
>  
> -On SoCs other than the A33 and V3s, there is one more clock required:
> +For the following compatibles:
> +   * allwinner,sun5i-a13-tcon
> +   * allwinner,sun6i-a31-tcon
> +   * allwinner,sun6i-a31s-tcon
> +   * allwinner,sun8i-a33-tcon
> +   * allwinner,sun8i-v3s-tcon
> +there is one more clock and one more property required:
> + - clocks:
> +   - 'tcon-ch0': The clock driving the TCON channel 0
> + - clock-output-names: Name of the pixel clock created
> +
> +For the following compatibles:
> +   * allwinner,sun5i-a13-tcon
> +   * allwinner,sun6i-a31-tcon
> +   * allwinner,sun6i-a31s-tcon
> +   * allwinner,sun8i-h3-tcon
> +there is one more clock required:
>     - 'tcon-ch1': The clock driving the TCON channel 1
>  
>  DRC
> @@ -189,6 +211,8 @@ supported.
>  Required properties:
>    - compatible: value must be one of:
>      * allwinner,sun8i-v3s-de2-mixer
> +    * allwinner,sun8i-h3-de2-mixer0
> +    * allwinner,sun8i-h3-de2-mixer1

Again, please explain why we need to have different compatibles
here. If it's only about the number of planes, this should be dealt
with a property, not a compatible.

Maxime
Icenowy Zheng June 7, 2017, 8:48 a.m. UTC | #2
于 2017年6月7日 GMT+08:00 下午4:45:44, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Mon, Jun 05, 2017 at 12:01:39AM +0800, Icenowy Zheng wrote:
>> Allwinner H3 features a "DE2.0" and a TV Encoder.
>> 
>> Add device tree bindings for the following parts:
>> - H3 TCONs
>> - H3 Mixers
>> - The connection between H3 TCONs and H3 Mixers
>> - H3 TV Encoder
>> - H3 Display engine
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>> Changes in v2:
>> - Changed endpoint reg definition on TCONs and Mixers. Now the
>endpoint
>>   id is just the component id.
>> - Changed TVE and TCON1 binding -- now CLK_TVE is passed as TCON1
>lcd-ch1.
>> 
>>  .../bindings/display/sunxi/sun4i-drm.txt           | 37
>+++++++++++++++++++---
>>  1 file changed, 33 insertions(+), 4 deletions(-)
>> 
>> diff --git
>a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> index b83e6018041d..7ad164cb7dcb 100644
>> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> @@ -49,7 +49,9 @@ The TV Encoder supports the composite and VGA
>output. It is one end of
>>  the pipeline.
>>  
>>  Required properties:
>> - - compatible: value should be "allwinner,sun4i-a10-tv-encoder".
>> + - compatible: value must be either:
>> +    * allwinner,sun4i-a10-tv-encoder
>> +    * allwinner,sun8i-h3-tv-encoder
>>   - reg: base address and size of memory-mapped region
>>   - clocks: the clocks driving the TV encoder
>>   - resets: phandle to the reset controller driving the encoder
>> @@ -69,23 +71,27 @@ Required properties:
>>     * allwinner,sun6i-a31-tcon
>>     * allwinner,sun6i-a31s-tcon
>>     * allwinner,sun8i-a33-tcon
>> +   * allwinner,sun8i-h3-tcon
>>     * allwinner,sun8i-v3s-tcon
>>   - reg: base address and size of memory-mapped region
>>   - interrupts: interrupt associated to this IP
>>   - clocks: phandles to the clocks feeding the TCON. Three are
>needed:
>>     - 'ahb': the interface clocks
>> -   - 'tcon-ch0': The clock driving the TCON channel 0
>>   - resets: phandles to the reset controllers driving the encoder
>>     - "lcd": the reset line for the TCON channel 0
>>  
>>   - clock-names: the clock names mentioned above
>>   - reset-names: the reset names mentioned above
>> - - clock-output-names: Name of the pixel clock created
>>  
>>  - ports: A ports node with endpoint definitions as defined in
>>    Documentation/devicetree/bindings/media/video-interfaces.txt. The
>>    first port should be the input endpoint, the second one the output
>>  
>> +  In the situation of Display Engine 2.0 that the connection between
>> +  the mixer and the TCON can be swapped, the input should have two
>> +  endpoints. The first input endpoint should be connected to mixer0
>> +  and the second should be connected to mixer1.
>> +
>
>AFAIK, this is also the case for dual-pipelines on DE1 (for example on
>the A31). And this is already covered by:
>
>"
>For the input port of all components up to the TCON in the display
>pipeline, if there are multiple components, the local endpoint IDs
>must correspond to the index of the upstream block. For example, if
>the remote endpoint is Frontend 1, then the local endpoint ID must
>be 1.
>"

If you think it can fully cover this situation, I'm glad to remove
this paragraph, although I think overall paragraphs are easy
to be ignored.

>
>>    The output may have multiple endpoints. The TCON has two channels,
>>    usually with the first channel being used for the panels
>interfaces
>>    (RGB, LVDS, etc.), and the second being used for the outputs that
>> @@ -94,7 +100,23 @@ Required properties:
>>    channel the endpoint is associated to. If that property is not
>>    present, the endpoint number will be used as the channel number.
>>  
>> -On SoCs other than the A33 and V3s, there is one more clock
>required:
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-a33-tcon
>> +   * allwinner,sun8i-v3s-tcon
>> +there is one more clock and one more property required:
>> + - clocks:
>> +   - 'tcon-ch0': The clock driving the TCON channel 0
>> + - clock-output-names: Name of the pixel clock created
>> +
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-h3-tcon
>> +there is one more clock required:
>>     - 'tcon-ch1': The clock driving the TCON channel 1
>>  
>>  DRC
>> @@ -189,6 +211,8 @@ supported.
>>  Required properties:
>>    - compatible: value must be one of:
>>      * allwinner,sun8i-v3s-de2-mixer
>> +    * allwinner,sun8i-h3-de2-mixer0
>> +    * allwinner,sun8i-h3-de2-mixer1
>
>Again, please explain why we need to have different compatibles
>here. If it's only about the number of planes, this should be dealt
>with a property, not a compatible.

Only mixer0 has "VEP" and write-back support, at least on H3.

>
>Maxime
Maxime Ripard June 9, 2017, 4:49 p.m. UTC | #3
On Wed, Jun 07, 2017 at 04:48:50PM +0800, Icenowy Zheng wrote:
> >> @@ -189,6 +211,8 @@ supported.
> >>  Required properties:
> >>    - compatible: value must be one of:
> >>      * allwinner,sun8i-v3s-de2-mixer
> >> +    * allwinner,sun8i-h3-de2-mixer0
> >> +    * allwinner,sun8i-h3-de2-mixer1
> >
> >Again, please explain why we need to have different compatibles
> >here. If it's only about the number of planes, this should be dealt
> >with a property, not a compatible.
> 
> Only mixer0 has "VEP" and write-back support, at least on H3.

What is that VEP? writeback support can also be expressed by a
property.

Maxime
Icenowy Zheng June 9, 2017, 4:51 p.m. UTC | #4
于 2017年6月10日 GMT+08:00 上午12:49:15, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Wed, Jun 07, 2017 at 04:48:50PM +0800, Icenowy Zheng wrote:
>> >> @@ -189,6 +211,8 @@ supported.
>> >>  Required properties:
>> >>    - compatible: value must be one of:
>> >>      * allwinner,sun8i-v3s-de2-mixer
>> >> +    * allwinner,sun8i-h3-de2-mixer0
>> >> +    * allwinner,sun8i-h3-de2-mixer1
>> >
>> >Again, please explain why we need to have different compatibles
>> >here. If it's only about the number of planes, this should be dealt
>> >with a property, not a compatible.
>> 
>> Only mixer0 has "VEP" and write-back support, at least on H3.
>
>What is that VEP? writeback support can also be expressed by a
>property.

I don't know what VEP is...

But we are really facing different cores, like sun50i-a64-mmc
and sun50i-a64-emmc.

>
>Maxime
Jernej Škrabec June 9, 2017, 9:24 p.m. UTC | #5
Hi!

Dne petek, 09. junij 2017 ob 18:51:02 CEST je Icenowy Zheng napisal(a):
> 于 2017年6月10日 GMT+08:00 上午12:49:15, Maxime Ripard <maxime.ripard@free-
electrons.com> 写到:
> >On Wed, Jun 07, 2017 at 04:48:50PM +0800, Icenowy Zheng wrote:
> >> >> @@ -189,6 +211,8 @@ supported.
> >> >> 
> >> >>  Required properties:
> >> >>    - compatible: value must be one of:
> >> >>      * allwinner,sun8i-v3s-de2-mixer
> >> >> 
> >> >> +    * allwinner,sun8i-h3-de2-mixer0
> >> >> +    * allwinner,sun8i-h3-de2-mixer1
> >> >
> >> >Again, please explain why we need to have different compatibles
> >> >here. If it's only about the number of planes, this should be dealt
> >> >with a property, not a compatible.
> >> 
> >> Only mixer0 has "VEP" and write-back support, at least on H3.
> >
> >What is that VEP? writeback support can also be expressed by a
> >property.
> 
> I don't know what VEP is...

VEP is probably Video Enhancement Processor (?). Although I'm not sure if that 
is really mixer specific. Icenowy, can you explain where did you get this info? 

Best regards,
Jernej

> 
> But we are really facing different cores, like sun50i-a64-mmc
> and sun50i-a64-emmc.
> 
> >Maxime
> 
> --
> 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 June 10, 2017, 3:26 p.m. UTC | #6
在 2017-06-10 05:24,Jernej Škrabec 写道:
> Hi!
> 
> Dne petek, 09. junij 2017 ob 18:51:02 CEST je Icenowy Zheng napisal(a):
>> 于 2017年6月10日 GMT+08:00 上午12:49:15, Maxime Ripard <maxime.ripard@free-
> electrons.com> 写到:
>> >On Wed, Jun 07, 2017 at 04:48:50PM +0800, Icenowy Zheng wrote:
>> >> >> @@ -189,6 +211,8 @@ supported.
>> >> >>
>> >> >>  Required properties:
>> >> >>    - compatible: value must be one of:
>> >> >>      * allwinner,sun8i-v3s-de2-mixer
>> >> >>
>> >> >> +    * allwinner,sun8i-h3-de2-mixer0
>> >> >> +    * allwinner,sun8i-h3-de2-mixer1
>> >> >
>> >> >Again, please explain why we need to have different compatibles
>> >> >here. If it's only about the number of planes, this should be dealt
>> >> >with a property, not a compatible.
>> >>
>> >> Only mixer0 has "VEP" and write-back support, at least on H3.
>> >
>> >What is that VEP? writeback support can also be expressed by a
>> >property.
>> 
>> I don't know what VEP is...
> 
> VEP is probably Video Enhancement Processor (?). Although I'm not sure 
> if that
> is really mixer specific. Icenowy, can you explain where did you get 
> this info?

In 
lichee/linux-3.4/drivers/video/sunxi/disp2/disp/de/lowlevel_sun8iw7/de_feat.c:

static const int de_is_support_vep[] = {
	/* DISP0 CH0 */
	1,
	/* DISP0 CH1 */
	0,
	/* DISP0 CH2 */
	0,
	/* DISP0 CH3 */
	0,
	/* DISP1 CH0 */
	0,
	/* DISP1 CH1 */
	0,
};

> 
> Best regards,
> Jernej
> 
>> 
>> But we are really facing different cores, like sun50i-a64-mmc
>> and sun50i-a64-emmc.
>> 
>> >Maxime
>> 
>> --
>> 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 June 13, 2017, 7:41 a.m. UTC | #7
On Sat, Jun 10, 2017 at 12:51:02AM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2017年6月10日 GMT+08:00 上午12:49:15, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
> >On Wed, Jun 07, 2017 at 04:48:50PM +0800, Icenowy Zheng wrote:
> >> >> @@ -189,6 +211,8 @@ supported.
> >> >>  Required properties:
> >> >>    - compatible: value must be one of:
> >> >>      * allwinner,sun8i-v3s-de2-mixer
> >> >> +    * allwinner,sun8i-h3-de2-mixer0
> >> >> +    * allwinner,sun8i-h3-de2-mixer1
> >> >
> >> >Again, please explain why we need to have different compatibles
> >> >here. If it's only about the number of planes, this should be dealt
> >> >with a property, not a compatible.
> >> 
> >> Only mixer0 has "VEP" and write-back support, at least on H3.
> >
> >What is that VEP? writeback support can also be expressed by a
> >property.
> 
> I don't know what VEP is...

Ok. Let's forget about it at the moment then, especially if it's
optional, we can add support for it later.

> But we are really facing different cores, like sun50i-a64-mmc
> and sun50i-a64-emmc.

Not really. The eMMC and MMC controllers are *very* different. They
behave (slightly) differently, the eMMC controller has more signals
(8-bit bus, data strobe), can run at a higher frequency. You cannot
have exactly the same driver (or rather code path) to handle
both. Therefore the need for some other compatible.

This is not the case for the mixer here. The thing that differs
between instances is not behaviour but data. And we provide most of
the data through DT.

Maxime
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index b83e6018041d..7ad164cb7dcb 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -49,7 +49,9 @@  The TV Encoder supports the composite and VGA output. It is one end of
 the pipeline.
 
 Required properties:
- - compatible: value should be "allwinner,sun4i-a10-tv-encoder".
+ - compatible: value must be either:
+    * allwinner,sun4i-a10-tv-encoder
+    * allwinner,sun8i-h3-tv-encoder
  - reg: base address and size of memory-mapped region
  - clocks: the clocks driving the TV encoder
  - resets: phandle to the reset controller driving the encoder
@@ -69,23 +71,27 @@  Required properties:
    * allwinner,sun6i-a31-tcon
    * allwinner,sun6i-a31s-tcon
    * allwinner,sun8i-a33-tcon
+   * allwinner,sun8i-h3-tcon
    * allwinner,sun8i-v3s-tcon
  - reg: base address and size of memory-mapped region
  - interrupts: interrupt associated to this IP
  - clocks: phandles to the clocks feeding the TCON. Three are needed:
    - 'ahb': the interface clocks
-   - 'tcon-ch0': The clock driving the TCON channel 0
  - resets: phandles to the reset controllers driving the encoder
    - "lcd": the reset line for the TCON channel 0
 
  - clock-names: the clock names mentioned above
  - reset-names: the reset names mentioned above
- - clock-output-names: Name of the pixel clock created
 
 - ports: A ports node with endpoint definitions as defined in
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
   first port should be the input endpoint, the second one the output
 
+  In the situation of Display Engine 2.0 that the connection between
+  the mixer and the TCON can be swapped, the input should have two
+  endpoints. The first input endpoint should be connected to mixer0
+  and the second should be connected to mixer1.
+
   The output may have multiple endpoints. The TCON has two channels,
   usually with the first channel being used for the panels interfaces
   (RGB, LVDS, etc.), and the second being used for the outputs that
@@ -94,7 +100,23 @@  Required properties:
   channel the endpoint is associated to. If that property is not
   present, the endpoint number will be used as the channel number.
 
-On SoCs other than the A33 and V3s, there is one more clock required:
+For the following compatibles:
+   * allwinner,sun5i-a13-tcon
+   * allwinner,sun6i-a31-tcon
+   * allwinner,sun6i-a31s-tcon
+   * allwinner,sun8i-a33-tcon
+   * allwinner,sun8i-v3s-tcon
+there is one more clock and one more property required:
+ - clocks:
+   - 'tcon-ch0': The clock driving the TCON channel 0
+ - clock-output-names: Name of the pixel clock created
+
+For the following compatibles:
+   * allwinner,sun5i-a13-tcon
+   * allwinner,sun6i-a31-tcon
+   * allwinner,sun6i-a31s-tcon
+   * allwinner,sun8i-h3-tcon
+there is one more clock required:
    - 'tcon-ch1': The clock driving the TCON channel 1
 
 DRC
@@ -189,6 +211,8 @@  supported.
 Required properties:
   - compatible: value must be one of:
     * allwinner,sun8i-v3s-de2-mixer
+    * allwinner,sun8i-h3-de2-mixer0
+    * allwinner,sun8i-h3-de2-mixer1
   - reg: base address and size of the memory-mapped region.
   - clocks: phandles to the clocks feeding the mixer
     * bus: the mixer interface clock
@@ -200,6 +224,10 @@  Required properties:
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
   first port should be the input endpoints, the second one the output
 
+  In the situation of Display Engine 2.0 that the connection between
+  the mixer and the TCON can be swapped, the output should have two
+  endpoints. The first should be connected to TCON0 and the second
+  should be connected to TCON1.
 
 Display Engine Pipeline
 -----------------------
@@ -215,6 +243,7 @@  Required properties:
     * allwinner,sun6i-a31-display-engine
     * allwinner,sun6i-a31s-display-engine
     * allwinner,sun8i-a33-display-engine
+    * allwinner,sun8i-h3-display-engine
     * allwinner,sun8i-v3s-display-engine
 
   - allwinner,pipelines: list of phandle to the display engine