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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> > > Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > ---
> > > 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'

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux