RE: [PATCH v3 1/4] dt-bindings: display: Document Renesas RZ/G2L DU bindings

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Biju Das <biju.das@bp.renesas.com>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH v3 1/4] dt-bindings: display: Document Renesas RZ/G2L DU bindings
Date: Fri, 22 Apr 2022 08:10:51 +0000	[thread overview]
Message-ID: <OS0PR01MB5922728B940A386674DAB36786F79@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdWMmkY+_O_oyKeumuOqNvw_KJaDxuiwrETc3W-kCV2MCg@mail.gmail.com>

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v3 1/4] dt-bindings: display: Document Renesas RZ/G2L
> DU bindings
> 
> Hi Biju,
> 
> On Thu, Apr 21, 2022 at 6:31 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > The RZ/G2L LCD controller is composed of Frame Compression Processor
> > (FCPVD), Video Signal Processor (VSPD), and Display Unit (DU).
> >
> > The DU module supports the following hardware features − Display
> > Parallel Interface (DPI) and MIPI LINK Video Interface − Display
> > timing master − Generates video timings − Selecting the polarity of
> > output DCLK, HSYNC, VSYNC, and DE − Supports Progressive − Input data
> > format (from VSPD): RGB888, RGB666 − Output data format: same as Input
> > data format − Supporting Full HD (1920 pixels x 1080 lines) for
> > MIPI-DSI Output − Supporting WXGA (1280 pixels x 800 lines) for
> > Parallel Output
> >
> > This patch document DU module found on RZ/G2L LCDC.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > @@ -0,0 +1,159 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +
> > +title: Renesas RZ/G2L Display Unit (DU)
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |
> > +  These DT bindings describe the Display Unit embedded in the Renesas
> > +RZ/G2L
> > +  and RZ/V2L SoCs.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,du-r9a07g044c # for RZ/G2LC compatible DU
> > +      - renesas,du-r9a07g044l # for RZ/G2L compatible DU
> 
> Please use the format "<manuf>,<soc>-<modulo>" for new bindings.
> 

OK.

> I thought there was no need to differentiate RZ/G2LC and RZ/G2L, as the
> only difference is a wiring difference due to the limited number of pins on
> the RZ/G2LC package, as per your confirmation[1]?
> Hence please just use "renesas,r9a07g044-du".

I cross checked HW manual, on the overview section(page 69) Supported
DU channels on various SoC's are as below

RZ/{G2L,V2L}
− 1 channel MIPI DSI interface or 1channel parallel output interface selectable,

RZ/G2LC
− 1 channel MIPI DSI interface

RZ/G2UL ( From RZ/G2UL hardware manual overview)
− 1 channel parallel output interface.

> 
> Do you want a family-specific compatible value ("rzg2l-"), as this IP block
> is shared by (at least) RZ/GL(C), RZ/V2L, and RZ/G2UL?

May be will conclude after the above discussion??

> 
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,du-r9a07g044c
> > +    then:
> > +      properties:
> > +        ports:
> > +          properties:
> > +            port@0:
> > +              description: DSI 0
> > +          required:
> > +            - port@0
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,du-r9a07g044l
> > +    then:
> > +      properties:
> > +        ports:
> > +          properties:
> > +            port@0:
> > +              description: DPAD 0
> > +            port@1:
> > +              description: DSI 0
> > +          required:
> > +            - port@0
> > +            - port@1
> 
> Having different port numbers for the common DSI0 output indeed complicates
> matters ;-)
> 

But we could delete as per [1] for RZ/G2LC where it supports only DSI and [2] for RZ/G2UL where it supports only DPI, right?

[1] https://github.com/renesas-rz/rz_linux-cip/blob/rz-5.10-cip1/arch/arm64/boot/dts/renesas/r9a07g044c2.dtsi#L24

[2] https://github.com/renesas-rz/rz_linux-cip/blob/rz-5.10-cip1/arch/arm64/boot/dts/renesas/r9a07g043.dtsi#L1000

Regards,
Biju