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

[Icenowy Zheng]

ä 2017å6æ7æ GMT+08:00 äå4:45:44, Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> åå:
>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@xxxxxxx>
>> ---
>> 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