Re: [PATCH v7 03/19] dt-bindings: display: imx: Add i.MX8qxp Display Controller display engine

[Liu Ying <victor.liu@nxp.com>]

On 12/23/2024, Dmitry Baryshkov wrote:
> On Mon, Dec 23, 2024 at 02:41:31PM +0800, Liu Ying wrote:
>> i.MX8qxp Display Controller display engine consists of all processing units
>> that operate in a display clock domain.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>> v7:
>> * Drop DT alias documentations and add instance numbers to compatible strings.
>>   (Rob)
>>
>> v6:
>> * No change.
>>
>> v5:
>> * Document aliases.  Drop Rob's previous R-b tag. (Maxime)
>>
>> v4:
>> * Collect Rob's R-b tag.
>>
>> v3:
>> * No change.
>>
>> v2:
>> * Drop fsl,dc-*-id DT properties. (Krzysztof)
>> * Drop port property. (Krzysztof)
>> * Fix register range sizes in example.
>>
>>  .../imx/fsl,imx8qxp-dc-display-engine0.yaml   | 235 ++++++++++++++++++
>>  1 file changed, 235 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-display-engine0.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-display-engine0.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-display-engine0.yaml
>> new file mode 100644
>> index 000000000000..60d1e0a4a5dd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-display-engine0.yaml
>> @@ -0,0 +1,235 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/imx/fsl,imx8qxp-dc-display-engine0.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Freescale i.MX8qxp Display Controller Display Engine
>> +
>> +description:
>> +  All Processing Units that operate in a display clock domain. Pixel pipeline
>> +  is driven by a video timing and cannot be stalled. Implements all display
>> +  specific processing.
>> +
>> +maintainers:
>> +  - Liu Ying <victor.liu@nxp.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - fsl,imx8qxp-dc-display-engine0
>> +      - fsl,imx8qxp-dc-display-engine1
> 
> Is there actual difference between engines 0 and 1? If there is none,
> why are you artificially adding one here?

I think engine 0 and 1 have the same HW implementation, but they connect
to different ExtDsts through FrameGens.  The two compatible strings make
it possible for an operating system to figure out which engine is which
by getting the instance numbers from them.

If the two engines use a same compatible string, then either 1) use
DT aliases to get the instance number(as v5/v6 do) or 2) use OF graph to
describe the connections between FrameGens and ExtDsts.  However, in v6,
Rob doesn't like 1) because it abuses the aliases which contain the display
controller instance number, like "dc0-display-engine0"(i.MX8QM SoC has
two display controllers), and 2) is too complex because all connections
between internal devices need to be documented with OF graph.  So, I choose
to use the two compatible strings, like brcm,bcm2835-pixelvalve0.yaml does.
Thinking about 2) more, maybe the connections between pixel engine and
display engines need to be documented too, which seems to be more or less
duplicating the connections between FrameGens and ExtDsts.

                                           CF0/1/4/5
                           PE               | | | |
                                            V V V V  primary layer cross bar
                          +------------------------------------------+
                          |                                          |
4 FUs + (VS4/5 + HS4/5) =>|               LB0/1/2/3                  |
   secondary layer        |                                          |
   cross bar              +------------------------------------------+
                             |          |              |          |
                             V          V              V          V
                          +-----+    +-----+        +-----+    +-----+
                          | ED0 |    | ED4 |        | ED5 |    | ED1 |
                          +-----+    +-----+        +-----+    +-----+
-----------------------------|----------|--------------|----------|-------------
                          content     safety        safety     content
                          stream0    stream0        stream1    stream1
                             |          |              |          |
                             |  DE0     V              V    DE1   |
                             |       +-----+        +-----+       |
                              ------>| FG0 |        | FG1 |<------
                                     +-----+        +-----+
                                        |              |
                                        V              V
                                       ...            ...

> 
>> +
>> +  reg:
>> +    maxItems: 2
>> +
>> +  reg-names:
>> +    items:
>> +      - const: top
>> +      - const: cfg
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 3
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: shdload
>> +      - const: framecomplete
>> +      - const: seqcomplete
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
>> +
>> +  ranges: true
>> +
>> +patternProperties:
>> +  "^dither@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - fsl,imx8qxp-dc-dither0
>> +          - fsl,imx8qxp-dc-dither1
>> +
>> +  "^framegen@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - fsl,imx8qxp-dc-framegen0
>> +          - fsl,imx8qxp-dc-framegen1
>> +
>> +  "^gammacor@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - fsl,imx8qxp-dc-gammacor0
>> +          - fsl,imx8qxp-dc-gammacor1
>> +
>> +  "^matrix@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - fsl,imx8qxp-dc-matrix0
>> +          - fsl,imx8qxp-dc-matrix1
>> +
>> +  "^signature@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - fsl,imx8qxp-dc-signature0
>> +          - fsl,imx8qxp-dc-signature1
>> +
>> +  "^tcon@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - fsl,imx8qxp-dc-tcon0
>> +          - fsl,imx8qxp-dc-tcon1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - interrupts
>> +  - interrupt-names
>> +  - power-domains
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - ranges
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: fsl,imx8qxp-dc-display-engine0
>> +    then:
>> +      patternProperties:
>> +        "^dither@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-dither0
>> +
>> +        "^framegen@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-framegen0
>> +
>> +        "^gammacor@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-gammacor0
>> +
>> +        "^matrix@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-matrix0
>> +
>> +        "^signature@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-signature0
>> +
>> +        "^tcon@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-tcon0
>> +    else:
>> +      patternProperties:
>> +        "^dither@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-dither1
>> +
>> +        "^framegen@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-framegen1
>> +
>> +        "^gammacor@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-gammacor1
>> +
>> +        "^matrix@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-matrix1
>> +
>> +        "^signature@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-signature1
>> +
>> +        "^tcon@[0-9a-f]+$":
>> +          properties:
>> +            compatible:
>> +              const: fsl,imx8qxp-dc-tcon1
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/imx8-lpcg.h>
>> +    #include <dt-bindings/firmware/imx/rsrc.h>
>> +
>> +    display-engine@5618b400 {
>> +        compatible = "fsl,imx8qxp-dc-display-engine0";
>> +        reg = <0x5618b400 0x14>, <0x5618b800 0x1c00>;
>> +        reg-names = "top", "cfg";
>> +        interrupt-parent = <&dc0_intc>;
>> +        interrupts = <15>, <16>, <17>;
>> +        interrupt-names = "shdload", "framecomplete", "seqcomplete";
>> +        power-domains = <&pd IMX_SC_R_DC_0_PLL_0>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +        ranges;
>> +
>> +        framegen@5618b800 {
>> +            compatible = "fsl,imx8qxp-dc-framegen0";
>> +            reg = <0x5618b800 0x98>;
>> +            clocks = <&dc0_disp_lpcg IMX_LPCG_CLK_0>;
>> +            interrupt-parent = <&dc0_intc>;
>> +            interrupts = <18>, <19>, <20>, <21>, <41>, <42>, <43>, <44>;
>> +            interrupt-names = "int0", "int1", "int2", "int3",
>> +                              "primsync_on", "primsync_off",
>> +                              "secsync_on", "secsync_off";
>> +        };
>> +
>> +        tcon@5618c800 {
>> +            compatible = "fsl,imx8qxp-dc-tcon0";
>> +            reg = <0x5618c800 0x588>;
>> +
>> +            port {
>> +                dc0_disp0_dc0_pixel_combiner_ch0: endpoint {
>> +                    remote-endpoint = <&dc0_pixel_combiner_ch0_dc0_disp0>;
>> +                };
>> +            };
>> +        };
>> +    };
>> -- 
>> 2.34.1
>>
> 

-- 
Regards,
Liu Ying