[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <73c80079-582a-40cb-b9e1-e20fc96fe603@nxp.com>
Date: Tue, 24 Dec 2024 13:57:31 +0800
From: Liu Ying <victor.liu@....com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
p.zabel@...gutronix.de, airlied@...il.com, simona@...ll.ch,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
festevam@...il.com, tglx@...utronix.de, vkoul@...nel.org, kishon@...nel.org,
aisheng.dong@....com, agx@...xcpu.org, u.kleine-koenig@...libre.com,
francesco@...cini.it, frank.li@....com
Subject: Re: [PATCH v7 03/19] dt-bindings: display: imx: Add i.MX8qxp Display
Controller display engine
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@....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@....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@...8b400 {
>> + 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@...8b800 {
>> + 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@...8c800 {
>> + 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
Powered by blists - more mailing lists