Hi Laurent,
Thanks for the feedback.
> Subject: Re: [PATCH v5 1/3] media: dt-bindings: media: renesas,vsp1:
> Document RZ/{G2L,V2L} VSPD bindings
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Sat, Mar 12, 2022 at 08:42:03AM +0000, Biju Das wrote:
> > Document VSPD found in RZ/G2L and RZ/V2L family SoC's. VSPD block is
> > similar to VSP2-D found on R-Car SoC's, but it does not have a version
> > register and it has 3 clocks compared to 1 clock on vsp1 and vsp2.
> >
> > This patch introduces a new compatible 'renesas,rzg2l-vsp2' to handle
> > these differences.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx>
> > ---
> > v4->v5:
> > * No change
> > v3->v4:
> > * No change
> > v2->v3:
> > * Added Rb tag from Krzysztof.
> > v1->v2:
> > * Changed compatible from vsp2-rzg2l->rzg2l-vsp2
> > RFC->v1:
> > * Updated commit description
> > * Changed compatible from vsp2-r9a07g044->vsp2-rzg2l
> > * Defined the clocks
> > * Clock max Items is based on SoC Compatible string
> > RFC:
> > *
> > ---
> > .../bindings/media/renesas,vsp1.yaml | 52 ++++++++++++++-----
> > 1 file changed, 39 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > b/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > index 990e9c1dbc43..2696a4582251 100644
> > --- a/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > @@ -19,6 +19,7 @@ properties:
> > enum:
> > - renesas,vsp1 # R-Car Gen2 and RZ/G1
> > - renesas,vsp2 # R-Car Gen3 and RZ/G2
> > + - renesas,rzg2l-vsp2 # RZ/G2L and RZ/V2L
> >
> > reg:
> > maxItems: 1
> > @@ -26,8 +27,8 @@ properties:
> > interrupts:
> > maxItems: 1
> >
> > - clocks:
> > - maxItems: 1
> > + clocks: true
> > + clock-names: true
> >
> > power-domains:
> > maxItems: 1
> > @@ -50,17 +51,42 @@ required:
> >
> > additionalProperties: false
> >
> > -if:
> > - properties:
> > - compatible:
> > - items:
> > - - const: renesas,vsp1
> > -then:
> > - properties:
> > - renesas,fcp: false
> > -else:
> > - required:
> > - - renesas,fcp
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: renesas,vsp1
> > + then:
> > + properties:
> > + renesas,fcp: false
> > + else:
> > + required:
> > + - renesas,fcp
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: renesas,rzg2l-vsp2
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: LCDC Main clock
> > + - description: LCDC Register Access Clock
> > + - description: LCDC Video Clock
>
> Shouldn't we avoid referring to LCDC in the VSP bindings ?
OK will drop prefix LCDC.
>
> > + clock-names:
> > + items:
> > + - const: du.0
>
> Similarly, I'm not sure this is a good name from the point of view of the
> VSP.
OK, will use the name 'aclk', which is Main clock for this module which is
shared with LCDC. 'du.0' is not valid any more here as we are using different
CRTC implementation for RZ/G2LC.
>
> > + - const: pclk
> > + - const: vclk
>
> I couldn't find those names in the documentation, where do they come from
HW manual (page 312) mentions about LCDC_CLK_A, LCDC_CLK_P & LCDC_CLK_D.
Detailed description is mentioned in Clock list document. Please see below.
LCDC_CLK_A M0φ PLL3 200 200 LCDC Main clock
LCDC_CLK_P ZTφ PLL3 100 100 LCDC Register Access Clock
LCDC_CLK_D M3φ SEL_PLL5_4 148.5~5.803 LCDC Video Clock
> ? Could you maybe share a DT integration example ?
Please see below,
+ fcpvd0: fcp@10880000 {
+ compatible = "renesas,fcpv";
+ reg = <0 0x10880000 0 0x10000>;
+ clocks = <&cpg CPG_MOD R9A07G044_LCDC_CLK_A>,
+ <&cpg CPG_MOD R9A07G044_LCDC_CLK_P>,
+ <&cpg CPG_MOD R9A07G044_LCDC_CLK_D>;
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_LCDC_RESET_N>;
+ };
+ vspd0: vsp@10870000 {
+ compatible = "renesas,rzg2l-vsp2";
+ reg = <0 0x10870000 0 0x10000>;
+ interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A07G044_LCDC_CLK_A>,
+ <&cpg CPG_MOD R9A07G044_LCDC_CLK_P>,
+ <&cpg CPG_MOD R9A07G044_LCDC_CLK_D>;
+ clock-names = "du.0", "pclk", "vclk";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_LCDC_RESET_N>;
+ renesas,fcp = <&fcpvd0>;
+ };
+ du: display@0x10890000 {
+ compatible = "renesas,du-r9a07g044l";
+ reg = <0 0x10890000 0 0x10000>;
+ interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A07G044_LCDC_CLK_A>,
+ <&cpg CPG_MOD R9A07G044_LCDC_CLK_P>,
+ <&cpg CPG_MOD R9A07G044_LCDC_CLK_D>;
+ clock-names = "du.0", "pclk", "vclk";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_LCDC_RESET_N>;
+ reset-names = "du.0";
+ renesas,vsps = <&vspd0 0>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ du_out_rgb: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ du_out_dsi0: endpoint {
+ };
+ };
+ };
+ };
+ dsi0: dsi@10860000 {
+ compatible = "renesas,r9a07g044-mipi-dsi";
+ reg = <0 0x10860000 0 0x10000>, /* LINK */
+ <0 0x10850000 0 0x10000>; /* DPHY */
+
+ clocks = <&cpg CPG_MOD R9A07G044_MIPI_DSI_PLLCLK>,
+ <&cpg CPG_MOD R9A07G044_MIPI_DSI_SYSCLK>,
+ <&cpg CPG_MOD R9A07G044_MIPI_DSI_ACLK>,
+ <&cpg CPG_MOD R9A07G044_MIPI_DSI_PCLK>,
+ <&cpg CPG_MOD R9A07G044_MIPI_DSI_VCLK>,
+ <&cpg CPG_MOD R9A07G044_MIPI_DSI_LPCLK>;
+ clock-names = "pllclk", "sysclk", "aclk", "pclk", "vclk", "lpclk";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_MIPI_DSI_CMN_RSTB>,
+ <&cpg R9A07G044_MIPI_DSI_ARESET_N>,
+ <&cpg R9A07G044_MIPI_DSI_PRESET_N>;
+ reset-names = "rst", "arst", "prst";
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi0_in: endpoint {
+ remote-endpoint = <&dsi0_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi0_out: endpoint {
+ remote-endpoint = <&du_out_dsi0>;
+ };
+ };
+ };
+ };
+ hdmi-out {
+ compatible = "hdmi-connector";
+ type = "d";
+
+ port {
+ hdmi_con_out: endpoint {
+ remote-endpoint = <&adv7535_out>;
+ };
+ };
+ };
+&dsi0 {
+ status = "okay";
+
+ ports {
+ port@1 {
+ dsi0_out: endpoint {
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&adv7535_in>;
+ };
+ };
+ };
+};
+&du {
+ status = "okay";
+};
+&i2c1 {
+ pinctrl-0 = <&i2c1_pins>;
+ pinctrl-names = "default";
+
+ status = "okay";
+
+ adv7535: hdmi@3d {
+ compatible = "adi,adv7535";
+ reg = <0x3d>;
+
+ avdd-supply = <®_1p8v>;
+ dvdd-supply = <®_1p8v>;
+ pvdd-supply = <®_1p8v>;
+ a2vdd-supply = <®_1p8v>;
+ v3p3-supply = <®_3p3v>;
+ v1p2-supply = <®_1p8v>;
+
+ adi,dsi-lanes = <4>;
+ adi,disable-lanes-override;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ adv7535_in: endpoint {
+ remote-endpoint = <&dsi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ adv7535_out: endpoint {
+ remote-endpoint = <&hdmi_con_out>;
+ };
+ };
+ };
+ };
Cheers,
Biju
>
> > + required:
> > + - clock-names
> > + else:
> > + properties:
> > + clocks:
> > + maxItems: 1
> >
> > examples:
> > # R8A7790 (R-Car H2) VSP1-S
>
> --
> Regards,
>
> Laurent Pinchart
