Re: [PATCH v9 05/14] dt-bindings: display: bridge: Add i.MX8qm/qxp display pixel link binding

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

On Tue, 2022-06-14 at 14:11 -0600, Rob Herring wrote:
> On Sat, Jun 11, 2022 at 10:14:12PM +0800, Liu Ying wrote:
> > This patch adds bindings for i.MX8qm/qxp display pixel link.
> > 
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> > v8->v9:
> > * Add 'fsl,dc-id' and 'fsl,dc-stream-id' properties. (Laurent)
> 
> Why? Isn't the graph sufficient for determining the connections?
> That's 
> what it is for.

'fsl,dc-id' determines the SCU resource IMX_SC_R_DC_{0,1} used by the
pixel link. 'fsl,dc-stream-id' determines the SCU control codes, like
IMX_SC_C_PXL_LINK_MST{1,2}_ADDR.

The pixel combiner stands between the Display Controller(DPU) and the
pixel link. I doubt it'll be good to go through the graph in pixel link
driver to get DPU's output port index to set 'dc-stream-id'. 

The graph doesn't tell 'dc-id' unless DPU's or pixel link's OF alias
ids are used. It looks that people don't like the idea of using the
pixel link's OF alias id in pixel link driver to set 'dc-id'(and 'dc-
stream-id'), just like previous versions do.  

> 
> > * Drop Rob's R-b tag.
> > 
> > v7->v8:
> > * No change.
> > 
> > v6->v7:
> > * No change.
> > 
> > v5->v6:
> > * No change.
> > 
> > v4->v5:
> > * No change.
> > 
> > v3->v4:
> > * No change.
> > 
> > v2->v3:
> > * Add Rob's R-b tag.
> > 
> > v1->v2:
> > * Use graph schema. (Laurent)
> > * Require all four pixel link output ports. (Laurent)
> > * Mention pixel link is accessed via SCU firmware. (Rob)
> > 
> >  .../bridge/fsl,imx8qxp-pixel-link.yaml        | 144
> > ++++++++++++++++++
> >  1 file changed, 144 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-
> > link.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-
> > pixel-link.yaml
> > b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-
> > pixel-link.yaml
> > new file mode 100644
> > index 000000000000..38ecc7926fad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-
> > pixel-link.yaml
> > @@ -0,0 +1,144 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fdisplay%2Fbridge%2Ffsl%2Cimx8qxp-pixel-link.yaml%23&amp;data=05%7C01%7Cvictor.liu%40nxp.com%7Cc8ba0853ca6446514fb408da4e421ec0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637908343136669762%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ZLdEqO84HZMwOIjyo5t%2Fy%2BlcfliLr3J5mV2akOxPU5c%3D&amp;reserved=0
> > +$schema: 
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Cvictor.liu%40nxp.com%7Cc8ba0853ca6446514fb408da4e421ec0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637908343136669762%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=yL5Xi%2BWz4aX3yniTy8Jq0EhB%2FbLz59QOaCQpxP0AmDM%3D&amp;reserved=0
> > +
> > +title: Freescale i.MX8qm/qxp Display Pixel Link
> > +
> > +maintainers:
> > +  - Liu Ying <victor.liu@nxp.com>
> > +
> > +description: |
> > +  The Freescale i.MX8qm/qxp Display Pixel Link(DPL) forms a
> > standard
> > +  asynchronous linkage between pixel sources(display controller or
> > +  camera module) and pixel consumers(imaging or displays).
> > +  It consists of two distinct functions, a pixel transfer function
> > and a
> > +  control interface.  Multiple pixel channels can exist per one
> > control channel.
> > +  This binding documentation is only for pixel links whose pixel
> > sources are
> > +  display controllers.
> > +
> > +  The i.MX8qm/qxp Display Pixel Link is accessed via System
> > Controller Unit(SCU)
> > +  firmware.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,imx8qm-dc-pixel-link
> > +      - fsl,imx8qxp-dc-pixel-link
> > +
> > +  fsl,dc-id:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    description: |
> > +      u8 value representing the display controller index that the
> > pixel link
> > +      connects to.
> > +
> > +  fsl,dc-stream-id:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    description: |
> > +      u8 value representing the display controller stream index
> > that the pixel
> > +      link connects to.
> > +    enum: [0, 1]
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: The pixel link input port node from upstream
> > video source.
> > +
> > +    patternProperties:
> > +      "^port@[1-4]$":
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: The pixel link output port node to downstream
> > bridge.
> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +      - port@2
> > +      - port@3
> > +      - port@4
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,imx8qxp-dc-pixel-link
> > +    then:
> > +      properties:
> > +        fsl,dc-id:
> > +          const: 0
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,imx8qm-dc-pixel-link
> > +    then:
> > +      properties:
> > +        fsl,dc-id:
> > +          enum: [0, 1]
> > +
> > +required:
> > +  - compatible
> > +  - fsl,dc-id
> > +  - fsl,dc-stream-id
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    dc0-pixel-link0 {
> > +        compatible = "fsl,imx8qxp-dc-pixel-link";
> > +        fsl,dc-id = /bits/ 8 <0>;
> > +        fsl,dc-stream-id = /bits/ 8 <0>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            /* from dc0 pixel combiner channel0 */
> > +            port@0 {
> > +                reg = <0>;
> > +
> > +                dc0_pixel_link0_dc0_pixel_combiner_ch0: endpoint {
> > +                    remote-endpoint =
> > <&dc0_pixel_combiner_ch0_dc0_pixel_link0>;
> 
> Isn't dc0 and link0 here the same information (if you get the port 
> number from the remote end).

The remote end is the pixel combiner's channel@0 port@1.
Then, from the pixel combiner, we can reach DPU's output port.

Granted that dc0 and link0 here is the same information, but it seems
to be hard to get the 'dc-id' and 'dc-stream-id' information in the
pixel link driver.

Do you mean that we can parse the name of
'dcX_pixel_linkY_dc0_pixel_combiner_ch0' directly to get 'X' and 'Y' in
the pixel link driver? How??

Regards,
Liu Ying