[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJpqJiiR15Xxw3j+vGPUBYYLgvxuXKeigMGBb4F3w178cfA@mail.gmail.com>
Date: Tue, 24 Dec 2024 08:37:41 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Liu Ying <victor.liu@....com>
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 Tue, 24 Dec 2024 at 07:56, Liu Ying <victor.liu@....com> wrote:
>
> 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.
This is about HW description, no OS in place.
>
> 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.
3) use IO addresses to determine the block ID. I think this has been
pointed out by somebody else too, not only by me.
>
> 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
--
With best wishes
Dmitry
Powered by blists - more mailing lists