diff mbox series

[v2,3/9] dt-bindings: display: renesas,rzg2l-du: Document RZ/G2UL DU bindings

Message ID 20240709135152.185042-4-biju.das.jz@bp.renesas.com
State Not Applicable
Headers show
Series Add support for RZ/G2UL Display Unit | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Biju Das July 9, 2024, 1:51 p.m. UTC 
Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
SoC, but has only DPI interface.

While at it, add missing required property port@1 for RZ/G2L and RZ/V2L
SoCs. Currently there is no user for the DPI interface and hence there
won't be any ABI breakage for adding port@1 as required property for
RZ/G2L and RZ/V2L SoCs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
v1->v2:
 * Updated commit description related to non ABI breakage.
 * Added Ack from Conor.
---
 .../bindings/display/renesas,rzg2l-du.yaml    | 32 +++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)

Comments

Geert Uytterhoeven July 26, 2024, 4:03 p.m. UTC | #1 
On Tue, Jul 9, 2024 at 3:52 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
> SoC, but has only DPI interface.
>
> While at it, add missing required property port@1 for RZ/G2L and RZ/V2L
> SoCs. Currently there is no user for the DPI interface and hence there
> won't be any ABI breakage for adding port@1 as required property for
> RZ/G2L and RZ/V2L SoCs.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> v1->v2:
>  * Updated commit description related to non ABI breakage.
>  * Added Ack from Conor.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Laurent Pinchart July 27, 2024, 12:49 a.m. UTC | #2 
Hi Biju,

Thank you for the patch.

On Tue, Jul 09, 2024 at 02:51:41PM +0100, Biju Das wrote:
> Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
> SoC, but has only DPI interface.
> 
> While at it, add missing required property port@1 for RZ/G2L and RZ/V2L
> SoCs. Currently there is no user for the DPI interface and hence there
> won't be any ABI breakage for adding port@1 as required property for
> RZ/G2L and RZ/V2L SoCs.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> v1->v2:
>  * Updated commit description related to non ABI breakage.
>  * Added Ack from Conor.
> ---
>  .../bindings/display/renesas,rzg2l-du.yaml    | 32 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> index 08e5b9478051..c0fec282fa45 100644
> --- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> @@ -18,6 +18,7 @@ properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - renesas,r9a07g043u-du # RZ/G2UL
>            - renesas,r9a07g044-du # RZ/G2{L,LC}
>        - items:
>            - enum:
> @@ -60,9 +61,6 @@ properties:
>          $ref: /schemas/graph.yaml#/properties/port
>          unevaluatedProperties: false
>  
> -    required:
> -      - port@0
> -
>      unevaluatedProperties: false
>  
>    renesas,vsps:
> @@ -88,6 +86,34 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a07g043u-du
> +    then:
> +      properties:
> +        ports:
> +          properties:
> +            port@0: false
> +            port@1:
> +              description: DPI
> +
> +          required:
> +            - port@1

Why do you use port@1 for the DPI output here, and not port@0 ?

> +    else:
> +      properties:
> +        ports:
> +          properties:
> +            port@0:
> +              description: DSI
> +            port@1:
> +              description: DPI
> +
> +          required:
> +            - port@0
> +            - port@1

You're missing a blank line here.

>  examples:
>    # RZ/G2L DU
>    - |
Biju Das July 29, 2024, 9:05 a.m. UTC | #3 
Hi Laurent Pinchart,

Thanks for the feedback.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Saturday, July 27, 2024 1:50 AM
> Subject: Re: [PATCH v2 3/9] dt-bindings: display: renesas,rzg2l-du: Document RZ/G2UL DU bindings
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Tue, Jul 09, 2024 at 02:51:41PM +0100, Biju Das wrote:
> > Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
> > SoC, but has only DPI interface.
> >
> > While at it, add missing required property port@1 for RZ/G2L and
> > RZ/V2L SoCs. Currently there is no user for the DPI interface and
> > hence there won't be any ABI breakage for adding port@1 as required
> > property for RZ/G2L and RZ/V2L SoCs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > v1->v2:
> >  * Updated commit description related to non ABI breakage.
> >  * Added Ack from Conor.
> > ---
> >  .../bindings/display/renesas,rzg2l-du.yaml    | 32 +++++++++++++++++--
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > index 08e5b9478051..c0fec282fa45 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > @@ -18,6 +18,7 @@ properties:
> >    compatible:
> >      oneOf:
> >        - enum:
> > +          - renesas,r9a07g043u-du # RZ/G2UL
> >            - renesas,r9a07g044-du # RZ/G2{L,LC}
> >        - items:
> >            - enum:
> > @@ -60,9 +61,6 @@ properties:
> >          $ref: /schemas/graph.yaml#/properties/port
> >          unevaluatedProperties: false
> >
> > -    required:
> > -      - port@0
> > -
> >      unevaluatedProperties: false
> >
> >    renesas,vsps:
> > @@ -88,6 +86,34 @@ required:
> >
> >  additionalProperties: false
> >
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,r9a07g043u-du
> > +    then:
> > +      properties:
> > +        ports:
> > +          properties:
> > +            port@0: false
> > +            port@1:
> > +              description: DPI
> > +
> > +          required:
> > +            - port@1
> 
> Why do you use port@1 for the DPI output here, and not port@0 ?

Currently the output is based on port number and port = 1 corresponds to DPI. See [1].

For consistency, I documented bindings for RZ/G2L family DU's similar to RZ/G2{H,M,N,E} DU [2].

So please let me know, are you ok with this?

[1] https://elixir.bootlin.com/linux/v6.10.2/source/drivers/gpu/drm/renesas/rz-du/rzg2l_du_kms.c#L187

[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/renesas,du.yaml?h=next-20240729#n546

> 
> > +    else:
> > +      properties:
> > +        ports:
> > +          properties:
> > +            port@0:
> > +              description: DSI
> > +            port@1:
> > +              description: DPI
> > +
> > +          required:
> > +            - port@0
> > +            - port@1
> 
> You're missing a blank line here.

OK, will fix this'

Cheers,
Biju
Laurent Pinchart July 31, 2024, 1:47 p.m. UTC | #4 
Hi Biju,

On Mon, Jul 29, 2024 at 09:05:59AM +0000, Biju Das wrote:
> On Saturday, July 27, 2024 1:50 AM, Laurent Pinchart wrote:
> > On Tue, Jul 09, 2024 at 02:51:41PM +0100, Biju Das wrote:
> > > Document DU found in RZ/G2UL SoC. The DU block is identical to RZ/G2L
> > > SoC, but has only DPI interface.
> > >
> > > While at it, add missing required property port@1 for RZ/G2L and
> > > RZ/V2L SoCs. Currently there is no user for the DPI interface and
> > > hence there won't be any ABI breakage for adding port@1 as required
> > > property for RZ/G2L and RZ/V2L SoCs.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > > v1->v2:
> > >  * Updated commit description related to non ABI breakage.
> > >  * Added Ack from Conor.
> > > ---
> > >  .../bindings/display/renesas,rzg2l-du.yaml    | 32 +++++++++++++++++--
> > >  1 file changed, 29 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > index 08e5b9478051..c0fec282fa45 100644
> > > --- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > >    compatible:
> > >      oneOf:
> > >        - enum:
> > > +          - renesas,r9a07g043u-du # RZ/G2UL
> > >            - renesas,r9a07g044-du # RZ/G2{L,LC}
> > >        - items:
> > >            - enum:
> > > @@ -60,9 +61,6 @@ properties:
> > >          $ref: /schemas/graph.yaml#/properties/port
> > >          unevaluatedProperties: false
> > >
> > > -    required:
> > > -      - port@0
> > > -
> > >      unevaluatedProperties: false
> > >
> > >    renesas,vsps:
> > > @@ -88,6 +86,34 @@ required:
> > >
> > >  additionalProperties: false
> > >
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: renesas,r9a07g043u-du
> > > +    then:
> > > +      properties:
> > > +        ports:
> > > +          properties:
> > > +            port@0: false
> > > +            port@1:
> > > +              description: DPI
> > > +
> > > +          required:
> > > +            - port@1
> > 
> > Why do you use port@1 for the DPI output here, and not port@0 ?
> 
> Currently the output is based on port number and port = 1 corresponds to DPI. See [1].
> 
> For consistency, I documented bindings for RZ/G2L family DU's similar to RZ/G2{H,M,N,E} DU [2].
> 
> So please let me know, are you ok with this?

I won't insist strongly, but I don't think that using the port number to
indicate the output type is the best idea. In the R-Car DU driver at
least, that wouldn't have scaled. We have multiple outputs of the same
type on some SoCs. Furthemore, the same DU hardware channel number (i.e.
the offset of the registers specific to that channel in the DU register
space) is not the same across SoCs for the same output type. I recommend
numbering the ports based on the hardware number of the output (the
exact meaning of this is specific to your device, I haven't checked what
it means for RZ/G2L), not on the output type.

> [1] https://elixir.bootlin.com/linux/v6.10.2/source/drivers/gpu/drm/renesas/rz-du/rzg2l_du_kms.c#L187
> 
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/renesas,du.yaml?h=next-20240729#n546
> 
> > > +    else:
> > > +      properties:
> > > +        ports:
> > > +          properties:
> > > +            port@0:
> > > +              description: DSI
> > > +            port@1:
> > > +              description: DPI
> > > +
> > > +          required:
> > > +            - port@0
> > > +            - port@1
> > 
> > You're missing a blank line here.
> 
> OK, will fix this'
Biju Das Aug. 1, 2024, 10:53 a.m. UTC | #5 
Hi Laurent Pinchart,

Thanks for the feedback.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Wednesday, July 31, 2024 2:47 PM
> Subject: Re: [PATCH v2 3/9] dt-bindings: display: renesas,rzg2l-du: Document RZ/G2UL DU bindings
> 
> Hi Biju,
> 
> On Mon, Jul 29, 2024 at 09:05:59AM +0000, Biju Das wrote:
> > On Saturday, July 27, 2024 1:50 AM, Laurent Pinchart wrote:
> > > On Tue, Jul 09, 2024 at 02:51:41PM +0100, Biju Das wrote:
> > > > Document DU found in RZ/G2UL SoC. The DU block is identical to
> > > > RZ/G2L SoC, but has only DPI interface.
> > > >
> > > > While at it, add missing required property port@1 for RZ/G2L and
> > > > RZ/V2L SoCs. Currently there is no user for the DPI interface and
> > > > hence there won't be any ABI breakage for adding port@1 as
> > > > required property for RZ/G2L and RZ/V2L SoCs.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > ---
> > > > v1->v2:
> > > >  * Updated commit description related to non ABI breakage.
> > > >  * Added Ack from Conor.
> > > > ---
> > > >  .../bindings/display/renesas,rzg2l-du.yaml    | 32 +++++++++++++++++--
> > > >  1 file changed, 29 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > > b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > > index 08e5b9478051..c0fec282fa45 100644
> > > > ---
> > > > a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.y
> > > > +++ aml
> > > > @@ -18,6 +18,7 @@ properties:
> > > >    compatible:
> > > >      oneOf:
> > > >        - enum:
> > > > +          - renesas,r9a07g043u-du # RZ/G2UL
> > > >            - renesas,r9a07g044-du # RZ/G2{L,LC}
> > > >        - items:
> > > >            - enum:
> > > > @@ -60,9 +61,6 @@ properties:
> > > >          $ref: /schemas/graph.yaml#/properties/port
> > > >          unevaluatedProperties: false
> > > >
> > > > -    required:
> > > > -      - port@0
> > > > -
> > > >      unevaluatedProperties: false
> > > >
> > > >    renesas,vsps:
> > > > @@ -88,6 +86,34 @@ required:
> > > >
> > > >  additionalProperties: false
> > > >
> > > > +allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: renesas,r9a07g043u-du
> > > > +    then:
> > > > +      properties:
> > > > +        ports:
> > > > +          properties:
> > > > +            port@0: false
> > > > +            port@1:
> > > > +              description: DPI
> > > > +
> > > > +          required:
> > > > +            - port@1
> > >
> > > Why do you use port@1 for the DPI output here, and not port@0 ?
> >
> > Currently the output is based on port number and port = 1 corresponds to DPI. See [1].
> >
> > For consistency, I documented bindings for RZ/G2L family DU's similar to RZ/G2{H,M,N,E} DU [2].
> >
> > So please let me know, are you ok with this?
> 
> I won't insist strongly, but I don't think that using the port number to indicate the output type is
> the best idea. In the R-Car DU driver at least, that wouldn't have scaled. We have multiple outputs of
> the same type on some SoCs. Furthemore, the same DU hardware channel number (i.e.
> the offset of the registers specific to that channel in the DU register
> space) is not the same across SoCs for the same output type. I recommend numbering the ports based on
> the hardware number of the output (the exact meaning of this is specific to your device, I haven't
> checked what it means for RZ/G2L), not on the output type.

OK, will update the bindings to use port for RZ/G2UL and Ports for RZ/{G2L, V2L} as it has multiple DU outputs.
From the driver will remove using port number to indicate the output type.

-                       if (rcdu->info->routes[i].port == ep.port) {
-                               output = i;
+                       if (i == rcdu->info->routes[i].du_output) {
+                               output = rcdu->info->routes[i].du_output;
Cheers,
Biju

> 
> > [1]
> > https://elixir.bootlin.com/linux/v6.10.2/source/drivers/gpu/drm/renesa
> > s/rz-du/rzg2l_du_kms.c#L187
> >
> > [2]
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tr
> > ee/Documentation/devicetree/bindings/display/renesas,du.yaml?h=next-20
> > 240729#n546
> >
> > > > +    else:
> > > > +      properties:
> > > > +        ports:
> > > > +          properties:
> > > > +            port@0:
> > > > +              description: DSI
> > > > +            port@1:
> > > > +              description: DPI
> > > > +
> > > > +          required:
> > > > +            - port@0
> > > > +            - port@1
> > >
> > > You're missing a blank line here.
> >
> > OK, will fix this'
> 
> --
> Regards,
> 
> Laurent Pinchart
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
index 08e5b9478051..c0fec282fa45 100644
--- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
@@ -18,6 +18,7 @@  properties:
   compatible:
     oneOf:
       - enum:
+          - renesas,r9a07g043u-du # RZ/G2UL
           - renesas,r9a07g044-du # RZ/G2{L,LC}
       - items:
           - enum:
@@ -60,9 +61,6 @@  properties:
         $ref: /schemas/graph.yaml#/properties/port
         unevaluatedProperties: false
 
-    required:
-      - port@0
-
     unevaluatedProperties: false
 
   renesas,vsps:
@@ -88,6 +86,34 @@  required:
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a07g043u-du
+    then:
+      properties:
+        ports:
+          properties:
+            port@0: false
+            port@1:
+              description: DPI
+
+          required:
+            - port@1
+    else:
+      properties:
+        ports:
+          properties:
+            port@0:
+              description: DSI
+            port@1:
+              description: DPI
+
+          required:
+            - port@0
+            - port@1
 examples:
   # RZ/G2L DU
   - |