[PATCH v1 0/8] Renesas V4H DSI & DP output support

[PATCH v1 0/8] Renesas V4H DSI & DP output support

[Tomi Valkeinen]

Hi,

These add support for DSI on V4H SoC (r8a779g0) and DP for Whitehawk
board.

The last patch is a hack, but needed to get the DSI working. It is still
unclear what the register write does, and as that patch is needed to get
the DSI working, this series is not ready yet. But all the rest of the
patches are ready for review.

 Tomi

Tomi Valkeinen (8):
  dt-bindings: display: renesas,du: Provide bindings for r8a779g0
  dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0
  clk: renesas: r8a779g0: Add display related clocks
  arm64: dts: renesas: r8a779g0: Add display related data
  arm64: dts: renesas: white-hawk-cpu: Add DP output support
  drm: rcar-du: Add r8a779g0 support
  drm: rcar-du: dsi: Add r8A779g0 support
  HACK: drm: rcar-du: dsi: use-extal-clk hack

 .../display/bridge/renesas,dsi-csi2-tx.yaml   |   3 +-
 .../bindings/display/renesas,du.yaml          |   2 +
 .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  |  94 ++++
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi     | 129 +++++
 drivers/clk/renesas/r8a779g0-cpg-mssr.c       |  14 +
 drivers/gpu/drm/rcar-du/rcar_du_drv.c         |  22 +
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c       | 488 ++++++++++++++----
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h  |   6 +-
 8 files changed, 651 insertions(+), 107 deletions(-)

-- 
2.34.1

[PATCH v1 1/8] dt-bindings: display: renesas,du: Provide bindings for r8a779g0

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Extend the Renesas DU display bindings to support the r8a779g0 V4H.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 Documentation/devicetree/bindings/display/renesas,du.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index b3e588022082..d4830f52c512 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -40,6 +40,7 @@ properties:
       - renesas,du-r8a77990 # for R-Car E3 compatible DU
       - renesas,du-r8a77995 # for R-Car D3 compatible DU
       - renesas,du-r8a779a0 # for R-Car V3U compatible DU
+      - renesas,du-r8a779g0 # for R-Car V4H compatible DU
 
   reg:
     maxItems: 1
@@ -762,6 +763,7 @@ allOf:
           contains:
             enum:
               - renesas,du-r8a779a0
+              - renesas,du-r8a779g0
     then:
       properties:
         clocks:
-- 
2.34.1

Re: [PATCH v1 1/8] dt-bindings: display: renesas,du: Provide bindings for r8a779g0

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:40)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DU display bindings to support the r8a779g0 V4H.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Matches my expectations, and interpretations of the datasheets.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/display/renesas,du.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index b3e588022082..d4830f52c512 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -40,6 +40,7 @@ properties:
>        - renesas,du-r8a77990 # for R-Car E3 compatible DU
>        - renesas,du-r8a77995 # for R-Car D3 compatible DU
>        - renesas,du-r8a779a0 # for R-Car V3U compatible DU
> +      - renesas,du-r8a779g0 # for R-Car V4H compatible DU
>  
>    reg:
>      maxItems: 1
> @@ -762,6 +763,7 @@ allOf:
>            contains:
>              enum:
>                - renesas,du-r8a779a0
> +              - renesas,du-r8a779g0
>      then:
>        properties:
>          clocks:
> -- 
> 2.34.1
>

Re: [PATCH v1 1/8] dt-bindings: display: renesas,du: Provide bindings for r8a779g0

[Krzysztof Kozlowski]

On 17/11/2022 13:25, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DU display bindings to support the r8a779g0 V4H.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/renesas,du.yaml | 2 ++
>  1 file changed, 2 insertions(+)


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v1 1/8] dt-bindings: display: renesas,du: Provide bindings for r8a779g0

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thu, Nov 17, 2022 at 02:25:40PM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DU display bindings to support the r8a779g0 V4H.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/display/renesas,du.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index b3e588022082..d4830f52c512 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -40,6 +40,7 @@ properties:
>        - renesas,du-r8a77990 # for R-Car E3 compatible DU
>        - renesas,du-r8a77995 # for R-Car D3 compatible DU
>        - renesas,du-r8a779a0 # for R-Car V3U compatible DU
> +      - renesas,du-r8a779g0 # for R-Car V4H compatible DU
>  
>    reg:
>      maxItems: 1
> @@ -762,6 +763,7 @@ allOf:
>            contains:
>              enum:
>                - renesas,du-r8a779a0
> +              - renesas,du-r8a779g0
>      then:
>        properties:
>          clocks:

-- 
Regards,

Laurent Pinchart

[PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Extend the Renesas DSI display bindings to support the r8a779g0 V4H.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml           | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
index afeeb967393d..bc3101f77e5a 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
@@ -11,13 +11,14 @@ maintainers:
 
 description: |
   This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
-  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
+  R-Car V3U/V4H SoC. The encoder can operate in either DSI or CSI-2 mode, with up
   to four data lanes.
 
 properties:
   compatible:
     enum:
       - renesas,r8a779a0-dsi-csi2-tx    # for V3U
+      - renesas,r8a779g0-dsi-csi2-tx    # for V4H
 
   reg:
     maxItems: 1
-- 
2.34.1

Re: [PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:41)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DSI display bindings to support the r8a779g0 V4H.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml           | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> index afeeb967393d..bc3101f77e5a 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> @@ -11,13 +11,14 @@ maintainers:
>  
>  description: |
>    This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> -  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> +  R-Car V3U/V4H SoC. The encoder can operate in either DSI or CSI-2 mode, with up

That's 81 chars I think ... so perhaps the 'up' should go 'down' a line
... but hey it's one char, I don't care too much, but I can't resist an
up/down reference :D

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

>    to four data lanes.
>  
>  properties:
>    compatible:
>      enum:
>        - renesas,r8a779a0-dsi-csi2-tx    # for V3U
> +      - renesas,r8a779g0-dsi-csi2-tx    # for V4H
>  
>    reg:
>      maxItems: 1
> -- 
> 2.34.1
>

Re: [PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Geert Uytterhoeven]

Hi Tomi,

On Thu, Nov 17, 2022 at 1:26 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>
> Extend the Renesas DSI display bindings to support the r8a779g0 V4H.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml           | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> index afeeb967393d..bc3101f77e5a 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> @@ -11,13 +11,14 @@ maintainers:
>
>  description: |
>    This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> -  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> +  R-Car V3U/V4H SoC. The encoder can operate in either DSI or CSI-2 mode, with up

Perhaps "R-Car Gen4 SoCs", so we stay within 80 chars, and don't have
to update this when the next member of the family is around the block?

Is there anything that might be SoC-specific?
If not, perhaps the time is ripe for a family-specific compatible value?

>    to four data lanes.
>
>  properties:
>    compatible:
>      enum:
>        - renesas,r8a779a0-dsi-csi2-tx    # for V3U
> +      - renesas,r8a779g0-dsi-csi2-tx    # for V4H
>
>    reg:
>      maxItems: 1

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Laurent Pinchart]

On Thu, Nov 17, 2022 at 04:14:21PM +0100, Geert Uytterhoeven wrote:
> On Thu, Nov 17, 2022 at 1:26 PM Tomi Valkeinen wrote:
> > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >
> > Extend the Renesas DSI display bindings to support the r8a779g0 V4H.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml           | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > index afeeb967393d..bc3101f77e5a 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > @@ -11,13 +11,14 @@ maintainers:
> >
> >  description: |
> >    This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> > -  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> > +  R-Car V3U/V4H SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> 
> Perhaps "R-Car Gen4 SoCs", so we stay within 80 chars, and don't have
> to update this when the next member of the family is around the block?

Sounds good.

> Is there anything that might be SoC-specific?
> If not, perhaps the time is ripe for a family-specific compatible value?

That's hard to tell, I have little visibility into what surprises other
SoCs will bring :-S

> >    to four data lanes.
> >
> >  properties:
> >    compatible:
> >      enum:
> >        - renesas,r8a779a0-dsi-csi2-tx    # for V3U
> > +      - renesas,r8a779g0-dsi-csi2-tx    # for V4H

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> >
> >    reg:
> >      maxItems: 1

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Tomi Valkeinen]

On 17/11/2022 17:14, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Thu, Nov 17, 2022 at 1:26 PM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> Extend the Renesas DSI display bindings to support the r8a779g0 V4H.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml           | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
>> index afeeb967393d..bc3101f77e5a 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
>> @@ -11,13 +11,14 @@ maintainers:
>>
>>   description: |
>>     This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
>> -  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
>> +  R-Car V3U/V4H SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> 
> Perhaps "R-Car Gen4 SoCs", so we stay within 80 chars, and don't have
> to update this when the next member of the family is around the block?

Is V3U gen 4? Or do you mean "R-Car V3U and Gen 4 SoCs"?

> Is there anything that might be SoC-specific?
> If not, perhaps the time is ripe for a family-specific compatible value?

At least v3u and v4h DSIs are slightly different. Well, the DSI IP block 
itself looks the same, but the PLL and PHY are different.

  Tomi

Re: [PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Geert Uytterhoeven]

Hi Tomi,

On Tue, Nov 22, 2022 at 9:20 AM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
> On 17/11/2022 17:14, Geert Uytterhoeven wrote:
> > On Thu, Nov 17, 2022 at 1:26 PM Tomi Valkeinen
> > <tomi.valkeinen@ideasonboard.com> wrote:
> >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>
> >> Extend the Renesas DSI display bindings to support the r8a779g0 V4H.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >> ---
> >>   .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml           | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> >> index afeeb967393d..bc3101f77e5a 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> >> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> >> @@ -11,13 +11,14 @@ maintainers:
> >>
> >>   description: |
> >>     This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> >> -  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> >> +  R-Car V3U/V4H SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> >
> > Perhaps "R-Car Gen4 SoCs", so we stay within 80 chars, and don't have
> > to update this when the next member of the family is around the block?
>
> Is V3U gen 4? Or do you mean "R-Car V3U and Gen 4 SoCs"?

Despite the name, R-Car V3U is the first member of the R-Car Gen4 family...
https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs/r-car-v3u-best-class-r-car-v3u-asil-d-system-chip-automated-driving

> > Is there anything that might be SoC-specific?
> > If not, perhaps the time is ripe for a family-specific compatible value?
>
> At least v3u and v4h DSIs are slightly different. Well, the DSI IP block
> itself looks the same, but the PLL and PHY are different.

I noticed, when I saw the dsi-csi2 driver changes.
So no family-specific compatible value is needed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 2/8] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0

[Krzysztof Kozlowski]

On 17/11/2022 13:25, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DSI display bindings to support the r8a779g0 V4H.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v1 3/8] clk: renesas: r8a779g0: Add display related clocks

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Add clocks related to display which are needed to get the DSI output
working.

Extracted from Renesas BSP tree.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/clk/renesas/r8a779g0-cpg-mssr.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/clk/renesas/r8a779g0-cpg-mssr.c b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
index c6337a408e5e..6937f1aee677 100644
--- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
@@ -145,6 +145,8 @@ static const struct cpg_core_clk r8a779g0_core_clks[] __initconst = {
 	DEF_FIXED("viobusd2",	R8A779G0_CLK_VIOBUSD2,	CLK_VIO,	2, 1),
 	DEF_FIXED("vcbus",	R8A779G0_CLK_VCBUS,	CLK_VC,		1, 1),
 	DEF_FIXED("vcbusd2",	R8A779G0_CLK_VCBUSD2,	CLK_VC,		2, 1),
+	DEF_FIXED("dsiref",	R8A779G0_CLK_DSIREF,	CLK_PLL5_DIV4,	48, 1),
+	DEF_DIV6P1("dsiext",	R8A779G0_CLK_DSIEXT,	CLK_PLL5_DIV4,	0x884),
 
 	DEF_GEN4_SDH("sd0h",	R8A779G0_CLK_SD0H,	CLK_SDSRC,	   0x870),
 	DEF_GEN4_SD("sd0",	R8A779G0_CLK_SD0,	R8A779G0_CLK_SD0H, 0x870),
@@ -161,6 +163,14 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
 	DEF_MOD("avb0",		211,	R8A779G0_CLK_S0D4_HSC),
 	DEF_MOD("avb1",		212,	R8A779G0_CLK_S0D4_HSC),
 	DEF_MOD("avb2",		213,	R8A779G0_CLK_S0D4_HSC),
+
+	DEF_MOD("dis0",			411,	R8A779G0_CLK_S0D3),
+	DEF_MOD("dsitxlink0",		415,	R8A779G0_CLK_DSIREF),
+	DEF_MOD("dsitxlink1",		416,	R8A779G0_CLK_DSIREF),
+
+	DEF_MOD("fcpvd0",		508,	R8A779G0_CLK_S0D3),
+	DEF_MOD("fcpvd1",		509,	R8A779G0_CLK_S0D3),
+
 	DEF_MOD("hscif0",	514,	R8A779G0_CLK_SASYNCPERD1),
 	DEF_MOD("hscif1",	515,	R8A779G0_CLK_SASYNCPERD1),
 	DEF_MOD("hscif2",	516,	R8A779G0_CLK_SASYNCPERD1),
@@ -193,6 +203,10 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
 	DEF_MOD("tmu3",		716,	R8A779G0_CLK_SASYNCPERD2),
 	DEF_MOD("tmu4",		717,	R8A779G0_CLK_SASYNCPERD2),
 	DEF_MOD("tpu0",		718,	R8A779G0_CLK_SASYNCPERD4),
+
+	DEF_MOD("vspd0",		830,	R8A779G0_CLK_S0D1_VIO),
+	DEF_MOD("vspd1",		831,	R8A779G0_CLK_S0D1_VIO),
+
 	DEF_MOD("wdt1:wdt0",	907,	R8A779G0_CLK_R),
 	DEF_MOD("cmt0",		910,	R8A779G0_CLK_R),
 	DEF_MOD("cmt1",		911,	R8A779G0_CLK_R),
-- 
2.34.1

Re: [PATCH v1 3/8] clk: renesas: r8a779g0: Add display related clocks

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:42)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add clocks related to display which are needed to get the DSI output
> working.
> 
> Extracted from Renesas BSP tree.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/clk/renesas/r8a779g0-cpg-mssr.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/clk/renesas/r8a779g0-cpg-mssr.c b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> index c6337a408e5e..6937f1aee677 100644
> --- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> @@ -145,6 +145,8 @@ static const struct cpg_core_clk r8a779g0_core_clks[] __initconst = {
>         DEF_FIXED("viobusd2",   R8A779G0_CLK_VIOBUSD2,  CLK_VIO,        2, 1),
>         DEF_FIXED("vcbus",      R8A779G0_CLK_VCBUS,     CLK_VC,         1, 1),
>         DEF_FIXED("vcbusd2",    R8A779G0_CLK_VCBUSD2,   CLK_VC,         2, 1),
> +       DEF_FIXED("dsiref",     R8A779G0_CLK_DSIREF,    CLK_PLL5_DIV4,  48, 1),
> +       DEF_DIV6P1("dsiext",    R8A779G0_CLK_DSIEXT,    CLK_PLL5_DIV4,  0x884),
>  
>         DEF_GEN4_SDH("sd0h",    R8A779G0_CLK_SD0H,      CLK_SDSRC,         0x870),
>         DEF_GEN4_SD("sd0",      R8A779G0_CLK_SD0,       R8A779G0_CLK_SD0H, 0x870),
> @@ -161,6 +163,14 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
>         DEF_MOD("avb0",         211,    R8A779G0_CLK_S0D4_HSC),
>         DEF_MOD("avb1",         212,    R8A779G0_CLK_S0D4_HSC),
>         DEF_MOD("avb2",         213,    R8A779G0_CLK_S0D4_HSC),
> +
> +       DEF_MOD("dis0",                 411,    R8A779G0_CLK_S0D3),

dsi0?

Oh - how curious - it's listed as dis0 in the datasheet.
Ok - so this is the DU *display* not DSI ;-)

> +       DEF_MOD("dsitxlink0",           415,    R8A779G0_CLK_DSIREF),
> +       DEF_MOD("dsitxlink1",           416,    R8A779G0_CLK_DSIREF),
> +
> +       DEF_MOD("fcpvd0",               508,    R8A779G0_CLK_S0D3),
> +       DEF_MOD("fcpvd1",               509,    R8A779G0_CLK_S0D3),
> +

checks out. I guess the fcpcs is the CSI related FCP ? Anyway, if it's
not needed it can be ignored for now.


>         DEF_MOD("hscif0",       514,    R8A779G0_CLK_SASYNCPERD1),
>         DEF_MOD("hscif1",       515,    R8A779G0_CLK_SASYNCPERD1),
>         DEF_MOD("hscif2",       516,    R8A779G0_CLK_SASYNCPERD1),
> @@ -193,6 +203,10 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
>         DEF_MOD("tmu3",         716,    R8A779G0_CLK_SASYNCPERD2),
>         DEF_MOD("tmu4",         717,    R8A779G0_CLK_SASYNCPERD2),
>         DEF_MOD("tpu0",         718,    R8A779G0_CLK_SASYNCPERD4),
> +
> +       DEF_MOD("vspd0",                830,    R8A779G0_CLK_S0D1_VIO),
> +       DEF_MOD("vspd1",                831,    R8A779G0_CLK_S0D1_VIO),
> +

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

>         DEF_MOD("wdt1:wdt0",    907,    R8A779G0_CLK_R),
>         DEF_MOD("cmt0",         910,    R8A779G0_CLK_R),
>         DEF_MOD("cmt1",         911,    R8A779G0_CLK_R),
> -- 
> 2.34.1
>

Re: [PATCH v1 3/8] clk: renesas: r8a779g0: Add display related clocks

[Laurent Pinchart]

On Thu, Nov 17, 2022 at 02:08:57PM +0000, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-11-17 12:25:42)
> > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > 
> > Add clocks related to display which are needed to get the DSI output
> > working.
> > 
> > Extracted from Renesas BSP tree.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  drivers/clk/renesas/r8a779g0-cpg-mssr.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/clk/renesas/r8a779g0-cpg-mssr.c b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> > index c6337a408e5e..6937f1aee677 100644
> > --- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> > @@ -145,6 +145,8 @@ static const struct cpg_core_clk r8a779g0_core_clks[] __initconst = {
> >         DEF_FIXED("viobusd2",   R8A779G0_CLK_VIOBUSD2,  CLK_VIO,        2, 1),
> >         DEF_FIXED("vcbus",      R8A779G0_CLK_VCBUS,     CLK_VC,         1, 1),
> >         DEF_FIXED("vcbusd2",    R8A779G0_CLK_VCBUSD2,   CLK_VC,         2, 1),
> > +       DEF_FIXED("dsiref",     R8A779G0_CLK_DSIREF,    CLK_PLL5_DIV4,  48, 1),
> > +       DEF_DIV6P1("dsiext",    R8A779G0_CLK_DSIEXT,    CLK_PLL5_DIV4,  0x884),
> >  
> >         DEF_GEN4_SDH("sd0h",    R8A779G0_CLK_SD0H,      CLK_SDSRC,         0x870),
> >         DEF_GEN4_SD("sd0",      R8A779G0_CLK_SD0,       R8A779G0_CLK_SD0H, 0x870),
> > @@ -161,6 +163,14 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
> >         DEF_MOD("avb0",         211,    R8A779G0_CLK_S0D4_HSC),
> >         DEF_MOD("avb1",         212,    R8A779G0_CLK_S0D4_HSC),
> >         DEF_MOD("avb2",         213,    R8A779G0_CLK_S0D4_HSC),
> > +
> > +       DEF_MOD("dis0",                 411,    R8A779G0_CLK_S0D3),
> 
> dsi0?
> 
> Oh - how curious - it's listed as dis0 in the datasheet.
> Ok - so this is the DU *display* not DSI ;-)
> 
> > +       DEF_MOD("dsitxlink0",           415,    R8A779G0_CLK_DSIREF),
> > +       DEF_MOD("dsitxlink1",           416,    R8A779G0_CLK_DSIREF),
> > +
> > +       DEF_MOD("fcpvd0",               508,    R8A779G0_CLK_S0D3),
> > +       DEF_MOD("fcpvd1",               509,    R8A779G0_CLK_S0D3),
> > +
> 
> checks out. I guess the fcpcs is the CSI related FCP ? Anyway, if it's
> not needed it can be ignored for now.
> 
> 
> >         DEF_MOD("hscif0",       514,    R8A779G0_CLK_SASYNCPERD1),
> >         DEF_MOD("hscif1",       515,    R8A779G0_CLK_SASYNCPERD1),
> >         DEF_MOD("hscif2",       516,    R8A779G0_CLK_SASYNCPERD1),
> > @@ -193,6 +203,10 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] __initconst = {
> >         DEF_MOD("tmu3",         716,    R8A779G0_CLK_SASYNCPERD2),
> >         DEF_MOD("tmu4",         717,    R8A779G0_CLK_SASYNCPERD2),
> >         DEF_MOD("tpu0",         718,    R8A779G0_CLK_SASYNCPERD4),
> > +
> > +       DEF_MOD("vspd0",                830,    R8A779G0_CLK_S0D1_VIO),
> > +       DEF_MOD("vspd1",                831,    R8A779G0_CLK_S0D1_VIO),
> > +
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

I can't verify the MSTP clock parents, I assume they come from the BSP,
so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> >         DEF_MOD("wdt1:wdt0",    907,    R8A779G0_CLK_R),
> >         DEF_MOD("cmt0",         910,    R8A779G0_CLK_R),
> >         DEF_MOD("cmt1",         911,    R8A779G0_CLK_R),

-- 
Regards,

Laurent Pinchart

[PATCH v1 4/8] arm64: dts: renesas: r8a779g0: Add display related data

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Add DT nodes for components needed to get the DSI output working:
- FCPv
- VSPd
- DU
- DSI

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 129 ++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 45d8d927ad26..31d4930c5adc 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -1207,6 +1207,135 @@ prr: chipid@fff00044 {
 			compatible = "renesas,prr";
 			reg = <0 0xfff00044 0 4>;
 		};
+
+		fcpvd0: fcp@fea10000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea10000 0 0x200>;
+			clocks = <&cpg CPG_MOD 508>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			resets = <&cpg 508>;
+		};
+
+		fcpvd1: fcp@fea11000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea11000 0 0x200>;
+			clocks = <&cpg CPG_MOD 509>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			resets = <&cpg 509>;
+		};
+
+		vspd0: vsp@fea20000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea20000 0 0x5000>;
+			interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 830>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			resets = <&cpg 830>;
+
+			renesas,fcp = <&fcpvd0>;
+		};
+
+		vspd1: vsp@fea28000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea28000 0 0x5000>;
+			interrupts = <GIC_SPI 551 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 831>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			resets = <&cpg 831>;
+
+			renesas,fcp = <&fcpvd1>;
+		};
+
+		du: display@feb00000 {
+			compatible = "renesas,du-r8a779g0";
+			reg = <0 0xfeb00000 0 0x40000>;
+			interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 524 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 411>;
+			clock-names = "du.0";
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			resets = <&cpg 411>;
+			reset-names = "du.0";
+			renesas,vsps = <&vspd0 0>, <&vspd1 0>;
+
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					du_out_dsi0: endpoint {
+						remote-endpoint = <&dsi0_in>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					du_out_dsi1: endpoint {
+						remote-endpoint = <&dsi1_in>;
+					};
+				};
+			};
+		};
+
+		dsi0: dsi-encoder@fed80000 {
+			compatible = "renesas,r8a779g0-dsi-csi2-tx";
+			reg = <0 0xfed80000 0 0x10000>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 415>,
+				 <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
+				 <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
+			clock-names = "fck", "dsi", "pll";
+			resets = <&cpg 415>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					dsi0_in: endpoint {
+						remote-endpoint = <&du_out_dsi0>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+				};
+			};
+		};
+
+		dsi1: dsi-encoder@fed90000 {
+			compatible = "renesas,r8a779g0-dsi-csi2-tx";
+			reg = <0 0xfed90000 0 0x10000>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			clocks = <&cpg CPG_MOD 416>,
+				 <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
+				 <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
+			clock-names = "fck", "dsi", "pll";
+			resets = <&cpg 416>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					dsi1_in: endpoint {
+						remote-endpoint = <&du_out_dsi1>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+				};
+			};
+		};
+
 	};
 
 	timer {
-- 
2.34.1

Re: [PATCH v1 4/8] arm64: dts: renesas: r8a779g0: Add display related data

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:43)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add DT nodes for components needed to get the DSI output working:
> - FCPv
> - VSPd
> - DU
> - DSI
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 129 ++++++++++++++++++++++
>  1 file changed, 129 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> index 45d8d927ad26..31d4930c5adc 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> @@ -1207,6 +1207,135 @@ prr: chipid@fff00044 {
>                         compatible = "renesas,prr";
>                         reg = <0 0xfff00044 0 4>;
>                 };

I think these nodes are supposed to be in sort order based on the
register address in memory.

Disregarding sort order, I'll review the node contents.

I would probably s/data/nodes/ in $SUBJECT too.


> +
> +               fcpvd0: fcp@fea10000 {
> +                       compatible = "renesas,fcpv";
> +                       reg = <0 0xfea10000 0 0x200>;
> +                       clocks = <&cpg CPG_MOD 508>;
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       resets = <&cpg 508>;
> +               };
> +
> +               fcpvd1: fcp@fea11000 {
> +                       compatible = "renesas,fcpv";
> +                       reg = <0 0xfea11000 0 0x200>;
> +                       clocks = <&cpg CPG_MOD 509>;
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       resets = <&cpg 509>;
> +               };

I'm intrigued at the length of 0x200 as I only see 3 registers up to
0x0018 ..

But all existing platforms with fcpv* set 0x200 ... so lets cargo cult it up... :-)

> +
> +               vspd0: vsp@fea20000 {
> +                       compatible = "renesas,vsp2";
> +                       reg = <0 0xfea20000 0 0x5000>;

"""
Below are the base addresses of each VSP unit. VSPX has 32Kbyte address
space. VSPD has 28Kbyte address space.
"""

Hrm : 28K is 0x7000

RPf n OSD CLUT Table: H’4000 + H’0400*n to H’43fc + H’0400*n

 0x43fc+(0x400*5)
	22524	[0x57fc]

So this needs to be /at least/ 0x6000 (Would 0x5800 be odd?) and perhaps as it clearly states
28k, we should just set it to 0x7000.



> +                       interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 830>;
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       resets = <&cpg 830>;
> +
> +                       renesas,fcp = <&fcpvd0>;
> +               };
> +
> +               vspd1: vsp@fea28000 {
> +                       compatible = "renesas,vsp2";
> +                       reg = <0 0xfea28000 0 0x5000>;

Same here of course (reg = <0 0xfea28000 0 0x7000>)


> +                       interrupts = <GIC_SPI 551 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 831>;
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       resets = <&cpg 831>;
> +
> +                       renesas,fcp = <&fcpvd1>;
> +               };
> +
> +               du: display@feb00000 {
> +                       compatible = "renesas,du-r8a779g0";
> +                       reg = <0 0xfeb00000 0 0x40000>;
> +                       interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 524 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 411>;
> +                       clock-names = "du.0";
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       resets = <&cpg 411>;
> +                       reset-names = "du.0";
> +                       renesas,vsps = <&vspd0 0>, <&vspd1 0>;
> +
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               port@0 {
> +                                       reg = <0>;
> +                                       du_out_dsi0: endpoint {
> +                                               remote-endpoint = <&dsi0_in>;
> +                                       };
> +                               };
> +
> +                               port@1 {
> +                                       reg = <1>;
> +                                       du_out_dsi1: endpoint {
> +                                               remote-endpoint = <&dsi1_in>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               dsi0: dsi-encoder@fed80000 {
> +                       compatible = "renesas,r8a779g0-dsi-csi2-tx";
> +                       reg = <0 0xfed80000 0 0x10000>;
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       clocks = <&cpg CPG_MOD 415>,
> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
> +                       clock-names = "fck", "dsi", "pll";
> +                       resets = <&cpg 415>;

blank line here to separate it, and highlight that it's disabled? (Like
is done for DU?

> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               port@0 {
> +                                       reg = <0>;
> +                                       dsi0_in: endpoint {
> +                                               remote-endpoint = <&du_out_dsi0>;
> +                                       };
> +                               };
> +
> +                               port@1 {
> +                                       reg = <1>;
> +                               };
> +                       };
> +               };
> +
> +               dsi1: dsi-encoder@fed90000 {
> +                       compatible = "renesas,r8a779g0-dsi-csi2-tx";
> +                       reg = <0 0xfed90000 0 0x10000>;
> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +                       clocks = <&cpg CPG_MOD 416>,
> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
> +                       clock-names = "fck", "dsi", "pll";
> +                       resets = <&cpg 416>;

Same.

With the VSPD register ranges increased accordingly:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               port@0 {
> +                                       reg = <0>;
> +                                       dsi1_in: endpoint {
> +                                               remote-endpoint = <&du_out_dsi1>;
> +                                       };
> +                               };
> +
> +                               port@1 {
> +                                       reg = <1>;
> +                               };
> +                       };
> +               };
> +
>         };
>  
>         timer {
> -- 
> 2.34.1
>

Re: [PATCH v1 4/8] arm64: dts: renesas: r8a779g0: Add display related data

[Laurent Pinchart]

On Thu, Nov 17, 2022 at 03:03:39PM +0000, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-11-17 12:25:43)
> > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > 
> > Add DT nodes for components needed to get the DSI output working:
> > - FCPv
> > - VSPd
> > - DU
> > - DSI
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 129 ++++++++++++++++++++++
> >  1 file changed, 129 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> > index 45d8d927ad26..31d4930c5adc 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> > @@ -1207,6 +1207,135 @@ prr: chipid@fff00044 {
> >                         compatible = "renesas,prr";
> >                         reg = <0 0xfff00044 0 4>;
> >                 };
> 
> I think these nodes are supposed to be in sort order based on the
> register address in memory.
> 
> Disregarding sort order, I'll review the node contents.
> 
> I would probably s/data/nodes/ in $SUBJECT too.
> 
> > +
> > +               fcpvd0: fcp@fea10000 {
> > +                       compatible = "renesas,fcpv";
> > +                       reg = <0 0xfea10000 0 0x200>;
> > +                       clocks = <&cpg CPG_MOD 508>;
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 508>;
> > +               };
> > +
> > +               fcpvd1: fcp@fea11000 {
> > +                       compatible = "renesas,fcpv";
> > +                       reg = <0 0xfea11000 0 0x200>;
> > +                       clocks = <&cpg CPG_MOD 509>;
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 509>;
> > +               };
> 
> I'm intrigued at the length of 0x200 as I only see 3 registers up to
> 0x0018 ..
> 
> But all existing platforms with fcpv* set 0x200 ... so lets cargo cult it up... :-)
> 
> > +
> > +               vspd0: vsp@fea20000 {
> > +                       compatible = "renesas,vsp2";
> > +                       reg = <0 0xfea20000 0 0x5000>;
> 
> """
> Below are the base addresses of each VSP unit. VSPX has 32Kbyte address
> space. VSPD has 28Kbyte address space.
> """
> 
> Hrm : 28K is 0x7000
> 
> RPf n OSD CLUT Table: H’4000 + H’0400*n to H’43fc + H’0400*n
> 
>  0x43fc+(0x400*5)
> 	22524	[0x57fc]
> 
> So this needs to be /at least/ 0x6000 (Would 0x5800 be odd?) and perhaps as it clearly states
> 28k, we should just set it to 0x7000.

I'd go for 0x7000 indeed.

> > +                       interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 830>;
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 830>;
> > +
> > +                       renesas,fcp = <&fcpvd0>;
> > +               };
> > +
> > +               vspd1: vsp@fea28000 {
> > +                       compatible = "renesas,vsp2";
> > +                       reg = <0 0xfea28000 0 0x5000>;
> 
> Same here of course (reg = <0 0xfea28000 0 0x7000>)
> 
> > +                       interrupts = <GIC_SPI 551 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 831>;
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 831>;
> > +
> > +                       renesas,fcp = <&fcpvd1>;
> > +               };
> > +
> > +               du: display@feb00000 {
> > +                       compatible = "renesas,du-r8a779g0";
> > +                       reg = <0 0xfeb00000 0 0x40000>;
> > +                       interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 524 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 411>;
> > +                       clock-names = "du.0";
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 411>;
> > +                       reset-names = "du.0";
> > +                       renesas,vsps = <&vspd0 0>, <&vspd1 0>;
> > +
> > +                       status = "disabled";
> > +
> > +                       ports {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               port@0 {
> > +                                       reg = <0>;
> > +                                       du_out_dsi0: endpoint {
> > +                                               remote-endpoint = <&dsi0_in>;
> > +                                       };
> > +                               };
> > +
> > +                               port@1 {
> > +                                       reg = <1>;
> > +                                       du_out_dsi1: endpoint {
> > +                                               remote-endpoint = <&dsi1_in>;
> > +                                       };
> > +                               };
> > +                       };
> > +               };
> > +
> > +               dsi0: dsi-encoder@fed80000 {
> > +                       compatible = "renesas,r8a779g0-dsi-csi2-tx";
> > +                       reg = <0 0xfed80000 0 0x10000>;
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       clocks = <&cpg CPG_MOD 415>,
> > +                                <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
> > +                                <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
> > +                       clock-names = "fck", "dsi", "pll";
> > +                       resets = <&cpg 415>;
> 
> blank line here to separate it, and highlight that it's disabled? (Like
> is done for DU?
> 
> > +                       status = "disabled";
> > +
> > +                       ports {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               port@0 {
> > +                                       reg = <0>;
> > +                                       dsi0_in: endpoint {
> > +                                               remote-endpoint = <&du_out_dsi0>;
> > +                                       };
> > +                               };
> > +
> > +                               port@1 {
> > +                                       reg = <1>;
> > +                               };
> > +                       };
> > +               };
> > +
> > +               dsi1: dsi-encoder@fed90000 {
> > +                       compatible = "renesas,r8a779g0-dsi-csi2-tx";
> > +                       reg = <0 0xfed90000 0 0x10000>;
> > +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +                       clocks = <&cpg CPG_MOD 416>,
> > +                                <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
> > +                                <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
> > +                       clock-names = "fck", "dsi", "pll";
> > +                       resets = <&cpg 416>;
> 
> Same.
> 
> With the VSPD register ranges increased accordingly:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > +                       status = "disabled";
> > +
> > +                       ports {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               port@0 {
> > +                                       reg = <0>;
> > +                                       dsi1_in: endpoint {
> > +                                               remote-endpoint = <&du_out_dsi1>;
> > +                                       };
> > +                               };
> > +
> > +                               port@1 {
> > +                                       reg = <1>;
> > +                               };
> > +                       };
> > +               };
> > +

Extra blank line.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> >         };
> >  
> >         timer {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 4/8] arm64: dts: renesas: r8a779g0: Add display related data

[Tomi Valkeinen]

On 17/11/2022 17:03, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-11-17 12:25:43)
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> Add DT nodes for components needed to get the DSI output working:
>> - FCPv
>> - VSPd
>> - DU
>> - DSI
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 129 ++++++++++++++++++++++
>>   1 file changed, 129 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
>> index 45d8d927ad26..31d4930c5adc 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
>> @@ -1207,6 +1207,135 @@ prr: chipid@fff00044 {
>>                          compatible = "renesas,prr";
>>                          reg = <0 0xfff00044 0 4>;
>>                  };
> 
> I think these nodes are supposed to be in sort order based on the
> register address in memory.

Ah, I didn't realize that.

> Disregarding sort order, I'll review the node contents.
> 
> I would probably s/data/nodes/ in $SUBJECT too.
> 
> 
>> +
>> +               fcpvd0: fcp@fea10000 {
>> +                       compatible = "renesas,fcpv";
>> +                       reg = <0 0xfea10000 0 0x200>;
>> +                       clocks = <&cpg CPG_MOD 508>;
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       resets = <&cpg 508>;
>> +               };
>> +
>> +               fcpvd1: fcp@fea11000 {
>> +                       compatible = "renesas,fcpv";
>> +                       reg = <0 0xfea11000 0 0x200>;
>> +                       clocks = <&cpg CPG_MOD 509>;
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       resets = <&cpg 509>;
>> +               };
> 
> I'm intrigued at the length of 0x200 as I only see 3 registers up to
> 0x0018 ..
> 
> But all existing platforms with fcpv* set 0x200 ... so lets cargo cult it up... :-)
> 
>> +
>> +               vspd0: vsp@fea20000 {
>> +                       compatible = "renesas,vsp2";
>> +                       reg = <0 0xfea20000 0 0x5000>;
> 
> """
> Below are the base addresses of each VSP unit. VSPX has 32Kbyte address
> space. VSPD has 28Kbyte address space.
> """
> 
> Hrm : 28K is 0x7000
> 
> RPf n OSD CLUT Table: H’4000 + H’0400*n to H’43fc + H’0400*n
> 
>   0x43fc+(0x400*5)
> 	22524	[0x57fc]
> 
> So this needs to be /at least/ 0x6000 (Would 0x5800 be odd?) and perhaps as it clearly states
> 28k, we should just set it to 0x7000.

Ok. These are identical to v3u, and I was just copying v3u's dts, 
assuming they're correct. We probably should fix the v3u dts files too.

> 
>> +                       interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&cpg CPG_MOD 830>;
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       resets = <&cpg 830>;
>> +
>> +                       renesas,fcp = <&fcpvd0>;
>> +               };
>> +
>> +               vspd1: vsp@fea28000 {
>> +                       compatible = "renesas,vsp2";
>> +                       reg = <0 0xfea28000 0 0x5000>;
> 
> Same here of course (reg = <0 0xfea28000 0 0x7000>)
> 
> 
>> +                       interrupts = <GIC_SPI 551 IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&cpg CPG_MOD 831>;
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       resets = <&cpg 831>;
>> +
>> +                       renesas,fcp = <&fcpvd1>;
>> +               };
>> +
>> +               du: display@feb00000 {
>> +                       compatible = "renesas,du-r8a779g0";
>> +                       reg = <0 0xfeb00000 0 0x40000>;
>> +                       interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>,
>> +                                    <GIC_SPI 524 IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&cpg CPG_MOD 411>;
>> +                       clock-names = "du.0";
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       resets = <&cpg 411>;
>> +                       reset-names = "du.0";
>> +                       renesas,vsps = <&vspd0 0>, <&vspd1 0>;
>> +
>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               port@0 {
>> +                                       reg = <0>;
>> +                                       du_out_dsi0: endpoint {
>> +                                               remote-endpoint = <&dsi0_in>;
>> +                                       };
>> +                               };
>> +
>> +                               port@1 {
>> +                                       reg = <1>;
>> +                                       du_out_dsi1: endpoint {
>> +                                               remote-endpoint = <&dsi1_in>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               dsi0: dsi-encoder@fed80000 {
>> +                       compatible = "renesas,r8a779g0-dsi-csi2-tx";
>> +                       reg = <0 0xfed80000 0 0x10000>;
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       clocks = <&cpg CPG_MOD 415>,
>> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
>> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
>> +                       clock-names = "fck", "dsi", "pll";
>> +                       resets = <&cpg 415>;
> 
> blank line here to separate it, and highlight that it's disabled? (Like
> is done for DU?

Ok.

>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               port@0 {
>> +                                       reg = <0>;
>> +                                       dsi0_in: endpoint {
>> +                                               remote-endpoint = <&du_out_dsi0>;
>> +                                       };
>> +                               };
>> +
>> +                               port@1 {
>> +                                       reg = <1>;
>> +                               };
>> +                       };
>> +               };
>> +
>> +               dsi1: dsi-encoder@fed90000 {
>> +                       compatible = "renesas,r8a779g0-dsi-csi2-tx";
>> +                       reg = <0 0xfed90000 0 0x10000>;
>> +                       power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
>> +                       clocks = <&cpg CPG_MOD 416>,
>> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
>> +                                <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
>> +                       clock-names = "fck", "dsi", "pll";
>> +                       resets = <&cpg 416>;
> 
> Same.
> 
> With the VSPD register ranges increased accordingly:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               port@0 {
>> +                                       reg = <0>;
>> +                                       dsi1_in: endpoint {
>> +                                               remote-endpoint = <&du_out_dsi1>;
>> +                                       };
>> +                               };
>> +
>> +                               port@1 {
>> +                                       reg = <1>;
>> +                               };
>> +                       };
>> +               };
>> +
>>          };
>>   
>>          timer {
>> -- 
>> 2.34.1
>>

[PATCH v1 5/8] arm64: dts: renesas: white-hawk-cpu: Add DP output support

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Add DT nodes needed for the mini DP connector. The DP is driven by
sn65dsi86, which in turn gets the pixel data from the SoC via DSI.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
index c10740aee9f6..8aab859aac7a 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
@@ -97,6 +97,15 @@ memory@600000000 {
 		reg = <0x6 0x00000000 0x1 0x00000000>;
 	};
 
+	reg_1p2v: regulator-1p2v {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-1.2V";
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	reg_1p8v: regulator-1p8v {
 		compatible = "regulator-fixed";
 		regulator-name = "fixed-1.8V";
@@ -114,6 +123,24 @@ reg_3p3v: regulator-3p3v {
 		regulator-boot-on;
 		regulator-always-on;
 	};
+
+	mini-dp-con {
+		compatible = "dp-connector";
+		label = "CN5";
+		type = "mini";
+
+		port {
+			mini_dp_con_in: endpoint {
+				remote-endpoint = <&sn65dsi86_out>;
+			};
+		};
+	};
+
+	sn65dsi86_refclk: clk-x6 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <38400000>;
+	};
 };
 
 &avb0 {
@@ -134,6 +161,23 @@ phy0: ethernet-phy@0 {
 	};
 };
 
+&dsi0 {
+	status = "okay";
+
+	ports {
+		port@1 {
+			dsi0_out: endpoint {
+				remote-endpoint = <&sn65dsi86_in>;
+				data-lanes = <1 2 3 4>;
+			};
+		};
+	};
+};
+
+&du {
+	status = "okay";
+};
+
 &extal_clk {
 	clock-frequency = <16666666>;
 };
@@ -172,6 +216,51 @@ eeprom@50 {
 	};
 };
 
+&i2c1 {
+	pinctrl-0 = <&i2c1_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+	clock-frequency = <400000>;
+
+	bridge@2c {
+		compatible = "ti,sn65dsi86";
+		reg = <0x2c>;
+
+		clocks = <&sn65dsi86_refclk>;
+		clock-names = "refclk";
+
+		interrupt-parent = <&intc_ex>;
+		interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+		enable-gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
+
+		vccio-supply = <&reg_1p8v>;
+		vpll-supply = <&reg_1p8v>;
+		vcca-supply = <&reg_1p2v>;
+		vcc-supply = <&reg_1p2v>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				sn65dsi86_in: endpoint {
+					remote-endpoint = <&dsi0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				sn65dsi86_out: endpoint {
+					remote-endpoint = <&mini_dp_con_in>;
+				};
+			};
+		};
+	};
+};
+
 &mmc0 {
 	pinctrl-0 = <&mmc_pins>;
 	pinctrl-1 = <&mmc_pins>;
@@ -221,6 +310,11 @@ i2c0_pins: i2c0 {
 		function = "i2c0";
 	};
 
+	i2c1_pins: i2c1 {
+		groups = "i2c1";
+		function = "i2c1";
+	};
+
 	keys_pins: keys {
 		pins = "GP_5_0", "GP_5_1", "GP_5_2";
 		bias-pull-up;
-- 
2.34.1

Re: [PATCH v1 5/8] arm64: dts: renesas: white-hawk-cpu: Add DP output support

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:44)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add DT nodes needed for the mini DP connector. The DP is driven by
> sn65dsi86, which in turn gets the pixel data from the SoC via DSI.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> ---
>  .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> index c10740aee9f6..8aab859aac7a 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> @@ -97,6 +97,15 @@ memory@600000000 {
>                 reg = <0x6 0x00000000 0x1 0x00000000>;
>         };
>  
> +       reg_1p2v: regulator-1p2v {
> +               compatible = "regulator-fixed";
> +               regulator-name = "fixed-1.2V";
> +               regulator-min-microvolt = <1200000>;
> +               regulator-max-microvolt = <1200000>;
> +               regulator-boot-on;
> +               regulator-always-on;
> +       };
> +
>         reg_1p8v: regulator-1p8v {
>                 compatible = "regulator-fixed";
>                 regulator-name = "fixed-1.8V";
> @@ -114,6 +123,24 @@ reg_3p3v: regulator-3p3v {
>                 regulator-boot-on;
>                 regulator-always-on;
>         };
> +
> +       mini-dp-con {
> +               compatible = "dp-connector";
> +               label = "CN5";
> +               type = "mini";
> +
> +               port {
> +                       mini_dp_con_in: endpoint {
> +                               remote-endpoint = <&sn65dsi86_out>;
> +                       };
> +               };
> +       };
> +
> +       sn65dsi86_refclk: clk-x6 {
> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <38400000>;
> +       };
>  };
>  
>  &avb0 {
> @@ -134,6 +161,23 @@ phy0: ethernet-phy@0 {
>         };
>  };
>  
> +&dsi0 {
> +       status = "okay";
> +
> +       ports {
> +               port@1 {
> +                       dsi0_out: endpoint {
> +                               remote-endpoint = <&sn65dsi86_in>;
> +                               data-lanes = <1 2 3 4>;
> +                       };
> +               };
> +       };
> +};
> +
> +&du {
> +       status = "okay";
> +};
> +
>  &extal_clk {
>         clock-frequency = <16666666>;
>  };
> @@ -172,6 +216,51 @@ eeprom@50 {
>         };
>  };
>  
> +&i2c1 {
> +       pinctrl-0 = <&i2c1_pins>;
> +       pinctrl-names = "default";
> +
> +       status = "okay";
> +       clock-frequency = <400000>;
> +
> +       bridge@2c {
> +               compatible = "ti,sn65dsi86";
> +               reg = <0x2c>;
> +
> +               clocks = <&sn65dsi86_refclk>;
> +               clock-names = "refclk";
> +
> +               interrupt-parent = <&intc_ex>;
> +               interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> +               enable-gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> +
> +               vccio-supply = <&reg_1p8v>;
> +               vpll-supply = <&reg_1p8v>;
> +               vcca-supply = <&reg_1p2v>;
> +               vcc-supply = <&reg_1p2v>;
> +
> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       port@0 {
> +                               reg = <0>;
> +                               sn65dsi86_in: endpoint {
> +                                       remote-endpoint = <&dsi0_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg = <1>;
> +                               sn65dsi86_out: endpoint {
> +                                       remote-endpoint = <&mini_dp_con_in>;
> +                               };
> +                       };
> +               };
> +       };
> +};
> +
>  &mmc0 {
>         pinctrl-0 = <&mmc_pins>;
>         pinctrl-1 = <&mmc_pins>;
> @@ -221,6 +310,11 @@ i2c0_pins: i2c0 {
>                 function = "i2c0";
>         };
>  
> +       i2c1_pins: i2c1 {
> +               groups = "i2c1";
> +               function = "i2c1";
> +       };
> +
>         keys_pins: keys {
>                 pins = "GP_5_0", "GP_5_1", "GP_5_2";
>                 bias-pull-up;
> -- 
> 2.34.1
>

Re: [PATCH v1 5/8] arm64: dts: renesas: white-hawk-cpu: Add DP output support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thu, Nov 17, 2022 at 02:25:44PM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add DT nodes needed for the mini DP connector. The DP is driven by
> sn65dsi86, which in turn gets the pixel data from the SoC via DSI.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> index c10740aee9f6..8aab859aac7a 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> @@ -97,6 +97,15 @@ memory@600000000 {
>  		reg = <0x6 0x00000000 0x1 0x00000000>;
>  	};
>  
> +	reg_1p2v: regulator-1p2v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "fixed-1.2V";
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
>  	reg_1p8v: regulator-1p8v {
>  		compatible = "regulator-fixed";
>  		regulator-name = "fixed-1.8V";
> @@ -114,6 +123,24 @@ reg_3p3v: regulator-3p3v {
>  		regulator-boot-on;
>  		regulator-always-on;
>  	};
> +
> +	mini-dp-con {
> +		compatible = "dp-connector";
> +		label = "CN5";
> +		type = "mini";
> +
> +		port {
> +			mini_dp_con_in: endpoint {
> +				remote-endpoint = <&sn65dsi86_out>;
> +			};
> +		};
> +	};
> +
> +	sn65dsi86_refclk: clk-x6 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <38400000>;
> +	};
>  };
>  
>  &avb0 {
> @@ -134,6 +161,23 @@ phy0: ethernet-phy@0 {
>  	};
>  };
>  
> +&dsi0 {
> +	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			dsi0_out: endpoint {
> +				remote-endpoint = <&sn65dsi86_in>;
> +				data-lanes = <1 2 3 4>;
> +			};
> +		};
> +	};
> +};
> +
> +&du {
> +	status = "okay";
> +};
> +
>  &extal_clk {
>  	clock-frequency = <16666666>;
>  };
> @@ -172,6 +216,51 @@ eeprom@50 {
>  	};
>  };
>  
> +&i2c1 {
> +	pinctrl-0 = <&i2c1_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +	clock-frequency = <400000>;
> +
> +	bridge@2c {
> +		compatible = "ti,sn65dsi86";
> +		reg = <0x2c>;
> +
> +		clocks = <&sn65dsi86_refclk>;
> +		clock-names = "refclk";
> +
> +		interrupt-parent = <&intc_ex>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		enable-gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> +
> +		vccio-supply = <&reg_1p8v>;
> +		vpll-supply = <&reg_1p8v>;
> +		vcca-supply = <&reg_1p2v>;
> +		vcc-supply = <&reg_1p2v>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				sn65dsi86_in: endpoint {
> +					remote-endpoint = <&dsi0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				sn65dsi86_out: endpoint {
> +					remote-endpoint = <&mini_dp_con_in>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
>  &mmc0 {
>  	pinctrl-0 = <&mmc_pins>;
>  	pinctrl-1 = <&mmc_pins>;
> @@ -221,6 +310,11 @@ i2c0_pins: i2c0 {
>  		function = "i2c0";
>  	};
>  
> +	i2c1_pins: i2c1 {
> +		groups = "i2c1";
> +		function = "i2c1";
> +	};
> +
>  	keys_pins: keys {
>  		pins = "GP_5_0", "GP_5_1", "GP_5_2";
>  		bias-pull-up;

-- 
Regards,

Laurent Pinchart

[PATCH v1 6/8] drm: rcar-du: Add r8a779g0 support

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Add support for DU on r8a779g0, which is identical to DU on r8a779a0.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index d003e8d9e7a2..b1761d4ec4e5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -524,6 +524,27 @@ static const struct rcar_du_device_info rcar_du_r8a779a0_info = {
 	.dsi_clk_mask =  BIT(1) | BIT(0),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
+	.gen = 3,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ
+		  | RCAR_DU_FEATURE_VSP1_SOURCE
+		  | RCAR_DU_FEATURE_NO_BLENDING,
+	.channels_mask = BIT(1) | BIT(0),
+	.routes = {
+		/* R8A779G0 has two MIPI DSI outputs. */
+		[RCAR_DU_OUTPUT_DSI0] = {
+			.possible_crtcs = BIT(0),
+			.port = 0,
+		},
+		[RCAR_DU_OUTPUT_DSI1] = {
+			.possible_crtcs = BIT(1),
+			.port = 1,
+		},
+	},
+	.num_rpf = 5,
+	.dsi_clk_mask =  BIT(1) | BIT(0),
+};
+
 static const struct of_device_id rcar_du_of_table[] = {
 	{ .compatible = "renesas,du-r8a7742", .data = &rcar_du_r8a7790_info },
 	{ .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
@@ -549,6 +570,7 @@ static const struct of_device_id rcar_du_of_table[] = {
 	{ .compatible = "renesas,du-r8a77990", .data = &rcar_du_r8a7799x_info },
 	{ .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info },
 	{ .compatible = "renesas,du-r8a779a0", .data = &rcar_du_r8a779a0_info },
+	{ .compatible = "renesas,du-r8a779g0", .data = &rcar_du_r8a779g0_info },
 	{ }
 };
 
-- 
2.34.1

Re: [PATCH v1 6/8] drm: rcar-du: Add r8a779g0 support

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:45)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add support for DU on r8a779g0, which is identical to DU on r8a779a0.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index d003e8d9e7a2..b1761d4ec4e5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -524,6 +524,27 @@ static const struct rcar_du_device_info rcar_du_r8a779a0_info = {
>         .dsi_clk_mask =  BIT(1) | BIT(0),
>  };
>  
> +static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
> +       .gen = 3,

Given that this is the V4H ... I wonder if this should be bumped
already. I guess that has knock on effects through the driver though...

Aside from that, Which may need more work to handle correctly:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> +       .features = RCAR_DU_FEATURE_CRTC_IRQ
> +                 | RCAR_DU_FEATURE_VSP1_SOURCE
> +                 | RCAR_DU_FEATURE_NO_BLENDING,
> +       .channels_mask = BIT(1) | BIT(0),
> +       .routes = {
> +               /* R8A779G0 has two MIPI DSI outputs. */
> +               [RCAR_DU_OUTPUT_DSI0] = {
> +                       .possible_crtcs = BIT(0),
> +                       .port = 0,
> +               },
> +               [RCAR_DU_OUTPUT_DSI1] = {
> +                       .possible_crtcs = BIT(1),
> +                       .port = 1,
> +               },
> +       },
> +       .num_rpf = 5,
> +       .dsi_clk_mask =  BIT(1) | BIT(0),
> +};
> +
>  static const struct of_device_id rcar_du_of_table[] = {
>         { .compatible = "renesas,du-r8a7742", .data = &rcar_du_r8a7790_info },
>         { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
> @@ -549,6 +570,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>         { .compatible = "renesas,du-r8a77990", .data = &rcar_du_r8a7799x_info },
>         { .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info },
>         { .compatible = "renesas,du-r8a779a0", .data = &rcar_du_r8a779a0_info },
> +       { .compatible = "renesas,du-r8a779g0", .data = &rcar_du_r8a779g0_info },
>         { }
>  };
>  
> -- 
> 2.34.1
>

Re: [PATCH v1 6/8] drm: rcar-du: Add r8a779g0 support

[Laurent Pinchart]

On Thu, Nov 17, 2022 at 03:08:38PM +0000, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-11-17 12:25:45)
> > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > 
> > Add support for DU on r8a779g0, which is identical to DU on r8a779a0.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > index d003e8d9e7a2..b1761d4ec4e5 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -524,6 +524,27 @@ static const struct rcar_du_device_info rcar_du_r8a779a0_info = {
> >         .dsi_clk_mask =  BIT(1) | BIT(0),
> >  };
> >  
> > +static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
> > +       .gen = 3,
> 
> Given that this is the V4H ... I wonder if this should be bumped
> already. I guess that has knock on effects through the driver though...

rcar_du_group_setup_didsr() would need to be fixed to test gen >= 3
instead of gen == 3. That seems to be the only problematic location. It
could thus fairly easily be done in v2, but we can also delay it.

> Aside from that, Which may need more work to handle correctly:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > +       .features = RCAR_DU_FEATURE_CRTC_IRQ
> > +                 | RCAR_DU_FEATURE_VSP1_SOURCE
> > +                 | RCAR_DU_FEATURE_NO_BLENDING,
> > +       .channels_mask = BIT(1) | BIT(0),
> > +       .routes = {
> > +               /* R8A779G0 has two MIPI DSI outputs. */
> > +               [RCAR_DU_OUTPUT_DSI0] = {
> > +                       .possible_crtcs = BIT(0),
> > +                       .port = 0,
> > +               },
> > +               [RCAR_DU_OUTPUT_DSI1] = {
> > +                       .possible_crtcs = BIT(1),
> > +                       .port = 1,
> > +               },
> > +       },
> > +       .num_rpf = 5,
> > +       .dsi_clk_mask =  BIT(1) | BIT(0),
> > +};
> > +
> >  static const struct of_device_id rcar_du_of_table[] = {
> >         { .compatible = "renesas,du-r8a7742", .data = &rcar_du_r8a7790_info },
> >         { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
> > @@ -549,6 +570,7 @@ static const struct of_device_id rcar_du_of_table[] = {
> >         { .compatible = "renesas,du-r8a77990", .data = &rcar_du_r8a7799x_info },
> >         { .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info },
> >         { .compatible = "renesas,du-r8a779a0", .data = &rcar_du_r8a779a0_info },
> > +       { .compatible = "renesas,du-r8a779g0", .data = &rcar_du_r8a779g0_info },
> >         { }
> >  };
> >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 6/8] drm: rcar-du: Add r8a779g0 support

[Tomi Valkeinen]

On 22/11/2022 05:05, Laurent Pinchart wrote:
> On Thu, Nov 17, 2022 at 03:08:38PM +0000, Kieran Bingham wrote:
>> Quoting Tomi Valkeinen (2022-11-17 12:25:45)
>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>
>>> Add support for DU on r8a779g0, which is identical to DU on r8a779a0.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> ---
>>>   drivers/gpu/drm/rcar-du/rcar_du_drv.c | 22 ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>> index d003e8d9e7a2..b1761d4ec4e5 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>> @@ -524,6 +524,27 @@ static const struct rcar_du_device_info rcar_du_r8a779a0_info = {
>>>          .dsi_clk_mask =  BIT(1) | BIT(0),
>>>   };
>>>   
>>> +static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
>>> +       .gen = 3,
>>
>> Given that this is the V4H ... I wonder if this should be bumped
>> already. I guess that has knock on effects through the driver though...
> 
> rcar_du_group_setup_didsr() would need to be fixed to test gen >= 3
> instead of gen == 3. That seems to be the only problematic location. It
> could thus fairly easily be done in v2, but we can also delay it.

Ok, I can fix that here.

  Tomi

[PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Add DSI support for r8a779g0. The main differences to r8a779a0 are in
the PLL and PHTW setups.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c      | 484 +++++++++++++++----
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
 2 files changed, 384 insertions(+), 106 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index a7f2b7f66a17..723c35726c38 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -9,6 +9,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -28,6 +29,20 @@
 #include "rcar_mipi_dsi.h"
 #include "rcar_mipi_dsi_regs.h"
 
+#define MHZ(v) ((v) * 1000000u)
+
+enum rcar_mipi_dsi_hw_model {
+	RCAR_DSI_R8A779A0,
+	RCAR_DSI_R8A779G0,
+};
+
+struct rcar_mipi_dsi_device_info {
+	enum rcar_mipi_dsi_hw_model model;
+	const struct dsi_clk_config *clk_cfg;
+	u8 clockset2_m_offset;
+	u8 clockset2_n_offset;
+};
+
 struct rcar_mipi_dsi {
 	struct device *dev;
 	const struct rcar_mipi_dsi_device_info *info;
@@ -50,6 +65,17 @@ struct rcar_mipi_dsi {
 	unsigned int lanes;
 };
 
+struct dsi_setup_info {
+	unsigned long hsfreq;
+	u16 hsfreqrange;
+
+	unsigned long fout;
+	u16 m;
+	u16 n;
+	u16 vclk_divider;
+	const struct dsi_clk_config *clkset;
+};
+
 static inline struct rcar_mipi_dsi *
 bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
 {
@@ -62,22 +88,6 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
 	return container_of(host, struct rcar_mipi_dsi, host);
 }
 
-static const u32 phtw[] = {
-	0x01020114, 0x01600115, /* General testing */
-	0x01030116, 0x0102011d, /* General testing */
-	0x011101a4, 0x018601a4, /* 1Gbps testing */
-	0x014201a0, 0x010001a3, /* 1Gbps testing */
-	0x0101011f,		/* 1Gbps testing */
-};
-
-static const u32 phtw2[] = {
-	0x010c0130, 0x010c0140, /* General testing */
-	0x010c0150, 0x010c0180, /* General testing */
-	0x010c0190,
-	0x010a0160, 0x010a0170,
-	0x01800164, 0x01800174,	/* 1Gbps testing */
-};
-
 static const u32 hsfreqrange_table[][2] = {
 	{ 80000000U,   0x00 }, { 90000000U,   0x10 }, { 100000000U,  0x20 },
 	{ 110000000U,  0x30 }, { 120000000U,  0x01 }, { 130000000U,  0x11 },
@@ -103,24 +113,53 @@ static const u32 hsfreqrange_table[][2] = {
 	{ /* sentinel */ },
 };
 
-struct vco_cntrl_value {
+struct dsi_clk_config {
 	u32 min_freq;
 	u32 max_freq;
-	u16 value;
+	u8 vco_cntrl;
+	u8 cpbias_cntrl;
+	u8 gmp_cntrl;
+	u8 int_cntrl;
+	u8 prop_cntrl;
 };
 
-static const struct vco_cntrl_value vco_cntrl_table[] = {
-	{ .min_freq = 40000000U,   .max_freq = 55000000U,   .value = 0x3f },
-	{ .min_freq = 52500000U,   .max_freq = 80000000U,   .value = 0x39 },
-	{ .min_freq = 80000000U,   .max_freq = 110000000U,  .value = 0x2f },
-	{ .min_freq = 105000000U,  .max_freq = 160000000U,  .value = 0x29 },
-	{ .min_freq = 160000000U,  .max_freq = 220000000U,  .value = 0x1f },
-	{ .min_freq = 210000000U,  .max_freq = 320000000U,  .value = 0x19 },
-	{ .min_freq = 320000000U,  .max_freq = 440000000U,  .value = 0x0f },
-	{ .min_freq = 420000000U,  .max_freq = 660000000U,  .value = 0x09 },
-	{ .min_freq = 630000000U,  .max_freq = 1149000000U, .value = 0x03 },
-	{ .min_freq = 1100000000U, .max_freq = 1152000000U, .value = 0x01 },
-	{ .min_freq = 1150000000U, .max_freq = 1250000000U, .value = 0x01 },
+static const struct dsi_clk_config dsi_clk_cfg_r8a779a0[] = {
+	{   40000000u,   55000000u, 0x3f, 0x10, 0x01, 0x00, 0x0b },
+	{   52500000u,   80000000u, 0x39, 0x10, 0x01, 0x00, 0x0b },
+	{   80000000u,  110000000u, 0x2f, 0x10, 0x01, 0x00, 0x0b },
+	{  105000000u,  160000000u, 0x29, 0x10, 0x01, 0x00, 0x0b },
+	{  160000000u,  220000000u, 0x1f, 0x10, 0x01, 0x00, 0x0b },
+	{  210000000u,  320000000u, 0x19, 0x10, 0x01, 0x00, 0x0b },
+	{  320000000u,  440000000u, 0x0f, 0x10, 0x01, 0x00, 0x0b },
+	{  420000000u,  660000000u, 0x09, 0x10, 0x01, 0x00, 0x0b },
+	{  630000000u, 1149000000u, 0x03, 0x10, 0x01, 0x00, 0x0b },
+	{ 1100000000u, 1152000000u, 0x01, 0x10, 0x01, 0x00, 0x0b },
+	{ 1150000000u, 1250000000u, 0x01, 0x10, 0x01, 0x00, 0x0c },
+	{ /* sentinel */ },
+};
+
+static const struct dsi_clk_config dsi_clk_cfg_r8a779g0[] = {
+	{   40000000u,   45310000u, 0x2b, 0x00, 0x00, 0x08, 0x0a },
+	{   45310000u,   54660000u, 0x28, 0x00, 0x00, 0x08, 0x0a },
+	{   54660000u,   62500000u, 0x28, 0x00, 0x00, 0x08, 0x0a },
+	{   62500000u,   75000000u, 0x27, 0x00, 0x00, 0x08, 0x0a },
+	{   75000000u,   90630000u, 0x23, 0x00, 0x00, 0x08, 0x0a },
+	{   90630000u,  109370000u, 0x20, 0x00, 0x00, 0x08, 0x0a },
+	{  109370000u,  125000000u, 0x20, 0x00, 0x00, 0x08, 0x0a },
+	{  125000000u,  150000000u, 0x1f, 0x00, 0x00, 0x08, 0x0a },
+	{  150000000u,  181250000u, 0x1b, 0x00, 0x00, 0x08, 0x0a },
+	{  181250000u,  218750000u, 0x18, 0x00, 0x00, 0x08, 0x0a },
+	{  218750000u,  250000000u, 0x18, 0x00, 0x00, 0x08, 0x0a },
+	{  250000000u,  300000000u, 0x17, 0x00, 0x00, 0x08, 0x0a },
+	{  300000000u,  362500000u, 0x13, 0x00, 0x00, 0x08, 0x0a },
+	{  362500000u,  455480000u, 0x10, 0x00, 0x00, 0x08, 0x0a },
+	{  455480000u,  500000000u, 0x10, 0x00, 0x00, 0x08, 0x0a },
+	{  500000000u,  600000000u, 0x0f, 0x00, 0x00, 0x08, 0x0a },
+	{  600000000u,  725000000u, 0x0b, 0x00, 0x00, 0x08, 0x0a },
+	{  725000000u,  875000000u, 0x08, 0x00, 0x00, 0x08, 0x0a },
+	{  875000000u, 1000000000u, 0x08, 0x00, 0x00, 0x08, 0x0a },
+	{ 1000000000u, 1200000000u, 0x07, 0x00, 0x00, 0x08, 0x0a },
+	{ 1200000000u, 1250000000u, 0x03, 0x00, 0x00, 0x08, 0x0a },
 	{ /* sentinel */ },
 };
 
@@ -144,7 +183,7 @@ static void rcar_mipi_dsi_set(struct rcar_mipi_dsi *dsi, u32 reg, u32 set)
 	rcar_mipi_dsi_write(dsi, reg, rcar_mipi_dsi_read(dsi, reg) | set);
 }
 
-static int rcar_mipi_dsi_phtw_test(struct rcar_mipi_dsi *dsi, u32 phtw)
+static int rcar_mipi_dsi_write_phtw(struct rcar_mipi_dsi *dsi, u32 phtw)
 {
 	u32 status;
 	int ret;
@@ -163,32 +202,231 @@ static int rcar_mipi_dsi_phtw_test(struct rcar_mipi_dsi *dsi, u32 phtw)
 	return ret;
 }
 
+static int rcar_mipi_dsi_write_phtw_arr(struct rcar_mipi_dsi *dsi,
+					const u32 *phtw, unsigned int size)
+{
+	for (unsigned int i = 0; i < size; i++) {
+		int ret = rcar_mipi_dsi_write_phtw(dsi, phtw[i]);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+#define WRITE_PHTW(...)                                               \
+	({                                                            \
+		static const u32 phtw[] = { __VA_ARGS__ };            \
+		int ret;                                              \
+		ret = rcar_mipi_dsi_write_phtw_arr(dsi, phtw,         \
+						   ARRAY_SIZE(phtw)); \
+		ret;                                                  \
+	})
+
+static int rcar_mipi_dsi_init_phtw_v3u(struct rcar_mipi_dsi *dsi)
+{
+	return WRITE_PHTW(0x01020114, 0x01600115, 0x01030116, 0x0102011d,
+			  0x011101a4, 0x018601a4, 0x014201a0, 0x010001a3,
+			  0x0101011f);
+}
+
+static int rcar_mipi_dsi_post_init_phtw_v3u(struct rcar_mipi_dsi *dsi)
+{
+	return WRITE_PHTW(0x010c0130, 0x010c0140, 0x010c0150, 0x010c0180,
+			  0x010c0190, 0x010a0160, 0x010a0170, 0x01800164,
+			  0x01800174);
+}
+
+static int rcar_mipi_dsi_init_phtw_v4h(struct rcar_mipi_dsi *dsi,
+				       const struct dsi_setup_info *setup_info)
+{
+	int ret;
+
+	if (setup_info->hsfreq < MHZ(450)) {
+		ret = WRITE_PHTW(0x01010100, 0x011b01ac);
+		if (ret)
+			return ret;
+	}
+
+	ret = WRITE_PHTW(0x01010100, 0x01030173, 0x01000174, 0x01500175,
+			 0x01030176, 0x01040166, 0x010201ad);
+	if (ret)
+		return ret;
+
+	if (setup_info->hsfreq <= MHZ(1000))
+		ret = WRITE_PHTW(0x01020100, 0x01910170, 0x01020171,
+				 0x01110172);
+	else if (setup_info->hsfreq <= MHZ(1500))
+		ret = WRITE_PHTW(0x01020100, 0x01980170, 0x01030171,
+				 0x01100172);
+	else if (setup_info->hsfreq <= MHZ(2500))
+		ret = WRITE_PHTW(0x01020100, 0x0144016b, 0x01000172);
+	else
+		return -EINVAL;
+
+	if (ret)
+		return ret;
+
+	if (dsi->lanes <= 1) {
+		ret = WRITE_PHTW(0x01070100, 0x010e010b);
+		if (ret)
+			return ret;
+	}
+
+	if (dsi->lanes <= 2) {
+		ret = WRITE_PHTW(0x01090100, 0x010e010b);
+		if (ret)
+			return ret;
+	}
+
+	if (dsi->lanes <= 3) {
+		ret = WRITE_PHTW(0x010b0100, 0x010e010b);
+		if (ret)
+			return ret;
+	}
+
+	if (setup_info->hsfreq <= MHZ(1500)) {
+		ret = WRITE_PHTW(0x01010100, 0x01c0016e);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int
+rcar_mipi_dsi_post_init_phtw_v4h(struct rcar_mipi_dsi *dsi,
+				 const struct dsi_setup_info *setup_info)
+{
+	u32 status;
+	int ret;
+
+	if (setup_info->hsfreq <= MHZ(1500)) {
+		WRITE_PHTW(0x01020100, 0x00000180);
+
+		ret = read_poll_timeout(rcar_mipi_dsi_read, status,
+					status & PHTR_TEST, 2000, 10000, false,
+					dsi, PHTR);
+		if (ret < 0) {
+			dev_err(dsi->dev, "failed to test PHTR\n");
+			return ret;
+		}
+
+		WRITE_PHTW(0x01010100, 0x0100016e);
+	}
+
+	return 0;
+}
+
 /* -----------------------------------------------------------------------------
  * Hardware Setup
  */
 
-struct dsi_setup_info {
-	unsigned long fout;
-	u16 vco_cntrl;
-	u16 prop_cntrl;
-	u16 hsfreqrange;
-	u16 div;
-	unsigned int m;
-	unsigned int n;
-};
+static void rcar_mipi_dsi_pll_calc_r8a779a0(struct rcar_mipi_dsi *dsi,
+					    struct clk *clk,
+					    unsigned long fout_target,
+					    struct dsi_setup_info *setup_info)
+{
+	unsigned int best_err = -1;
+	unsigned long fin;
+
+	fin = clk_get_rate(clk);
+
+	for (unsigned int n = 3; n <= 8; n++) {
+		unsigned long fpfd;
+
+		fpfd = fin / n;
+
+		if (fpfd < MHZ(2) || fpfd > MHZ(8))
+			continue;
+
+		for (unsigned int m = 64; m <= 625; m++) {
+			unsigned int err;
+			u64 fout;
+
+			fout = (u64)fpfd * m;
+
+			if (fout < MHZ(320) || fout > MHZ(1250))
+				continue;
+
+			fout = div64_u64(fout, setup_info->vclk_divider);
+
+			if (fout < setup_info->clkset->min_freq ||
+			    fout > setup_info->clkset->max_freq)
+				continue;
+
+			err = abs((long)(fout - fout_target) * 10000 /
+				  (long)fout_target);
+
+			if (err < best_err) {
+				setup_info->m = m;
+				setup_info->n = n;
+				setup_info->fout = (unsigned long)fout;
+				best_err = err;
+
+				if (err == 0)
+					return;
+			}
+		}
+	}
+}
+
+static void rcar_mipi_dsi_pll_calc_r8a779g0(struct rcar_mipi_dsi *dsi,
+					    struct clk *clk,
+					    unsigned long fout_target,
+					    struct dsi_setup_info *setup_info)
+{
+	unsigned int best_err = -1;
+	unsigned long fin;
+
+	fin = clk_get_rate(clk);
+
+	for (unsigned int n = 1; n <= 8; n++) {
+		unsigned long fpfd;
+
+		fpfd = fin / n;
+
+		if (fpfd < MHZ(8) || fpfd > MHZ(24))
+			continue;
+
+		for (unsigned int m = 167; m <= 1000; m++) {
+			unsigned int err;
+			u64 fout;
+
+			fout = div64_u64((u64)fpfd * m, 2);
+
+			if (fout < MHZ(2000) || fout > MHZ(4000))
+				continue;
+
+			fout = div64_u64(fout, setup_info->vclk_divider);
+
+			if (fout < setup_info->clkset->min_freq ||
+			    fout > setup_info->clkset->max_freq)
+				continue;
+
+			err = abs((long)(fout - fout_target) * 10000 /
+				  (long)fout_target);
+			if (err < best_err) {
+				setup_info->m = m;
+				setup_info->n = n;
+				setup_info->fout = (unsigned long)fout;
+				best_err = err;
+
+				if (err == 0)
+					return;
+			}
+		}
+	}
+}
 
 static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
 					  struct clk *clk, unsigned long target,
 					  struct dsi_setup_info *setup_info)
 {
 
-	const struct vco_cntrl_value *vco_cntrl;
+	const struct dsi_clk_config *clkset;
 	unsigned long fout_target;
-	unsigned long fin, fout;
-	unsigned long hsfreq;
-	unsigned int best_err = -1;
-	unsigned int divider;
-	unsigned int n;
 	unsigned int i;
 	unsigned int err;
 
@@ -198,70 +436,53 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
 	 */
 	fout_target = target * mipi_dsi_pixel_format_to_bpp(dsi->format)
 		    / (2 * dsi->lanes);
-	if (fout_target < 40000000 || fout_target > 1250000000)
+	if (fout_target < MHZ(40) || fout_target > MHZ(1250))
 		return;
 
 	/* Find vco_cntrl */
-	for (vco_cntrl = vco_cntrl_table; vco_cntrl->min_freq != 0; vco_cntrl++) {
-		if (fout_target > vco_cntrl->min_freq &&
-		    fout_target <= vco_cntrl->max_freq) {
-			setup_info->vco_cntrl = vco_cntrl->value;
-			if (fout_target >= 1150000000)
-				setup_info->prop_cntrl = 0x0c;
-			else
-				setup_info->prop_cntrl = 0x0b;
+	for (clkset = dsi->info->clk_cfg; clkset->min_freq != 0; clkset++) {
+		if (fout_target > clkset->min_freq &&
+		    fout_target <= clkset->max_freq) {
+			setup_info->clkset = clkset;
 			break;
 		}
 	}
 
-	/* Add divider */
-	setup_info->div = (setup_info->vco_cntrl & 0x30) >> 4;
+	switch (dsi->info->model) {
+	case RCAR_DSI_R8A779A0:
+		setup_info->vclk_divider = 1 << ((clkset->vco_cntrl >> 4) & 0x3);
+		rcar_mipi_dsi_pll_calc_r8a779a0(dsi, clk, fout_target, setup_info);
+		break;
+
+	case RCAR_DSI_R8A779G0:
+		setup_info->vclk_divider = 1 << (((clkset->vco_cntrl >> 3) & 0x7) + 1);
+		rcar_mipi_dsi_pll_calc_r8a779g0(dsi, clk, fout_target, setup_info);
+		break;
+
+	default:
+		return;
+	}
 
 	/* Find hsfreqrange */
-	hsfreq = fout_target * 2;
+	setup_info->hsfreq = setup_info->fout * 2;
 	for (i = 0; i < ARRAY_SIZE(hsfreqrange_table); i++) {
-		if (hsfreqrange_table[i][0] >= hsfreq) {
+		if (hsfreqrange_table[i][0] >= setup_info->hsfreq) {
 			setup_info->hsfreqrange = hsfreqrange_table[i][1];
 			break;
 		}
 	}
 
-	/*
-	 * Calculate n and m for PLL clock
-	 * Following the HW manual the ranges of n and m are
-	 * n = [3-8] and m = [64-625]
-	 */
-	fin = clk_get_rate(clk);
-	divider = 1 << setup_info->div;
-	for (n = 3; n < 9; n++) {
-		unsigned long fpfd;
-		unsigned int m;
-
-		fpfd = fin / n;
-
-		for (m = 64; m < 626; m++) {
-			fout = fpfd * m / divider;
-			err = abs((long)(fout - fout_target) * 10000 /
-				  (long)fout_target);
-			if (err < best_err) {
-				setup_info->m = m - 2;
-				setup_info->n = n - 1;
-				setup_info->fout = fout;
-				best_err = err;
-				if (err == 0)
-					goto done;
-			}
-		}
-	}
+	err = abs((long)(setup_info->fout - fout_target) * 10000 / (long)fout_target);
 
-done:
 	dev_dbg(dsi->dev,
-		"%pC %lu Hz -> Fout %lu Hz (target %lu Hz, error %d.%02u%%), PLL M/N/DIV %u/%u/%u\n",
-		clk, fin, setup_info->fout, fout_target, best_err / 100,
-		best_err % 100, setup_info->m, setup_info->n, setup_info->div);
+		"Fout = %u * %lu / (2 * %u * %u) = %lu (target %lu Hz, error %d.%02u%%)\n",
+		setup_info->m, clk_get_rate(clk), setup_info->n, setup_info->vclk_divider,
+		setup_info->fout, fout_target,
+		err / 100, err % 100);
+
 	dev_dbg(dsi->dev,
 		"vco_cntrl = 0x%x\tprop_cntrl = 0x%x\thsfreqrange = 0x%x\n",
-		setup_info->vco_cntrl, setup_info->prop_cntrl,
+		clkset->vco_cntrl, clkset->prop_cntrl,
 		setup_info->hsfreqrange);
 }
 
@@ -324,7 +545,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 {
 	struct dsi_setup_info setup_info = {};
 	unsigned int timeout;
-	int ret, i;
+	int ret;
 	int dsi_format;
 	u32 phy_setup;
 	u32 clockset2, clockset3;
@@ -360,10 +581,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	phy_setup |= PHYSETUP_HSFREQRANGE(setup_info.hsfreqrange);
 	rcar_mipi_dsi_write(dsi, PHYSETUP, phy_setup);
 
-	for (i = 0; i < ARRAY_SIZE(phtw); i++) {
-		ret = rcar_mipi_dsi_phtw_test(dsi, phtw[i]);
+	switch (dsi->info->model) {
+	case RCAR_DSI_R8A779A0:
+		ret = rcar_mipi_dsi_init_phtw_v3u(dsi);
+		if (ret < 0)
+			return ret;
+		break;
+
+	case RCAR_DSI_R8A779G0:
+		ret = rcar_mipi_dsi_init_phtw_v4h(dsi, &setup_info);
 		if (ret < 0)
 			return ret;
+		break;
+
+	default:
+		return -ENODEV;
 	}
 
 	/* PLL Clock Setting */
@@ -371,12 +603,13 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	rcar_mipi_dsi_set(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
 	rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
 
-	clockset2 = CLOCKSET2_M(setup_info.m) | CLOCKSET2_N(setup_info.n)
-		  | CLOCKSET2_VCO_CNTRL(setup_info.vco_cntrl);
-	clockset3 = CLOCKSET3_PROP_CNTRL(setup_info.prop_cntrl)
-		  | CLOCKSET3_INT_CNTRL(0)
-		  | CLOCKSET3_CPBIAS_CNTRL(0x10)
-		  | CLOCKSET3_GMP_CNTRL(1);
+	clockset2 = CLOCKSET2_M(setup_info.m - dsi->info->clockset2_m_offset)
+		  | CLOCKSET2_N(setup_info.n - dsi->info->clockset2_n_offset)
+		  | CLOCKSET2_VCO_CNTRL(setup_info.clkset->vco_cntrl);
+	clockset3 = CLOCKSET3_PROP_CNTRL(setup_info.clkset->prop_cntrl)
+		  | CLOCKSET3_INT_CNTRL(setup_info.clkset->int_cntrl)
+		  | CLOCKSET3_CPBIAS_CNTRL(setup_info.clkset->cpbias_cntrl)
+		  | CLOCKSET3_GMP_CNTRL(setup_info.clkset->gmp_cntrl);
 	rcar_mipi_dsi_write(dsi, CLOCKSET2, clockset2);
 	rcar_mipi_dsi_write(dsi, CLOCKSET3, clockset3);
 
@@ -407,10 +640,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 		return -ETIMEDOUT;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(phtw2); i++) {
-		ret = rcar_mipi_dsi_phtw_test(dsi, phtw2[i]);
+	switch (dsi->info->model) {
+	case RCAR_DSI_R8A779A0:
+		ret = rcar_mipi_dsi_post_init_phtw_v3u(dsi);
 		if (ret < 0)
 			return ret;
+		break;
+
+	case RCAR_DSI_R8A779G0:
+		ret = rcar_mipi_dsi_post_init_phtw_v4h(dsi, &setup_info);
+		if (ret < 0)
+			return ret;
+		break;
+
+	default:
+		return -ENODEV;
 	}
 
 	/* Enable DOT clock */
@@ -427,8 +671,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 		dev_warn(dsi->dev, "unsupported format");
 		return -EINVAL;
 	}
-	vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
-		|  VCLKSET_LANE(dsi->lanes - 1);
+
+	vclkset |= VCLKSET_COLOR_RGB | VCLKSET_LANE(dsi->lanes - 1);
+
+	switch (dsi->info->model) {
+	case RCAR_DSI_R8A779A0:
+		vclkset |= VCLKSET_DIV_R8A779A0(__ffs(setup_info.vclk_divider));
+		break;
+
+	case RCAR_DSI_R8A779G0:
+		vclkset |= VCLKSET_DIV_R8A779G0(__ffs(setup_info.vclk_divider) - 1);
+		break;
+
+	default:
+		return -ENODEV;
+	}
 
 	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
 
@@ -841,8 +1098,25 @@ static int rcar_mipi_dsi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct rcar_mipi_dsi_device_info r8a779a0_data = {
+	.model = RCAR_DSI_R8A779A0,
+	.clk_cfg = dsi_clk_cfg_r8a779a0,
+	.clockset2_m_offset = 2,
+	.clockset2_n_offset = 1,
+
+};
+
+static const struct rcar_mipi_dsi_device_info r8a779g0_data = {
+	.model = RCAR_DSI_R8A779G0,
+	.clk_cfg = dsi_clk_cfg_r8a779g0,
+	.clockset2_m_offset = 0,
+	.clockset2_n_offset = 1,
+
+};
+
 static const struct of_device_id rcar_mipi_dsi_of_table[] = {
-	{ .compatible = "renesas,r8a779a0-dsi-csi2-tx" },
+	{ .compatible = "renesas,r8a779a0-dsi-csi2-tx", .data = &r8a779a0_data },
+	{ .compatible = "renesas,r8a779g0-dsi-csi2-tx", .data = &r8a779g0_data },
 	{ }
 };
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h
index 2eaca54636f3..608851340acf 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h
@@ -122,7 +122,8 @@
 #define VCLKSET_CKEN			(1 << 16)
 #define VCLKSET_COLOR_RGB		(0 << 8)
 #define VCLKSET_COLOR_YCC		(1 << 8)
-#define VCLKSET_DIV(x)			(((x) & 0x3) << 4)
+#define VCLKSET_DIV_R8A779A0(x)		(((x) & 0x3) << 4)
+#define VCLKSET_DIV_R8A779G0(x)		(((x) & 0x7) << 4)
 #define VCLKSET_BPP_16			(0 << 2)
 #define VCLKSET_BPP_18			(1 << 2)
 #define VCLKSET_BPP_18L			(2 << 2)
@@ -166,6 +167,9 @@
 #define PHTW_CWEN			(1 << 8)
 #define PHTW_TESTDIN_CODE(x)		(((x) & 0xff) << 0)
 
+#define PHTR				0x1038
+#define PHTR_TEST			(1 << 16)
+
 #define PHTC				0x103c
 #define PHTC_TESTCLR			(1 << 0)
 
-- 
2.34.1

Re: [PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 12:25:46)
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add DSI support for r8a779g0. The main differences to r8a779a0 are in
> the PLL and PHTW setups.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c      | 484 +++++++++++++++----
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
>  2 files changed, 384 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index a7f2b7f66a17..723c35726c38 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -9,6 +9,7 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -28,6 +29,20 @@
>  #include "rcar_mipi_dsi.h"
>  #include "rcar_mipi_dsi_regs.h"
>  
> +#define MHZ(v) ((v) * 1000000u)
> +
> +enum rcar_mipi_dsi_hw_model {
> +       RCAR_DSI_R8A779A0,
> +       RCAR_DSI_R8A779G0,
> +};
> +
> +struct rcar_mipi_dsi_device_info {
> +       enum rcar_mipi_dsi_hw_model model;
> +       const struct dsi_clk_config *clk_cfg;
> +       u8 clockset2_m_offset;
> +       u8 clockset2_n_offset;
> +};
> +
>  struct rcar_mipi_dsi {
>         struct device *dev;
>         const struct rcar_mipi_dsi_device_info *info;
> @@ -50,6 +65,17 @@ struct rcar_mipi_dsi {
>         unsigned int lanes;
>  };
>  
> +struct dsi_setup_info {
> +       unsigned long hsfreq;
> +       u16 hsfreqrange;
> +
> +       unsigned long fout;
> +       u16 m;
> +       u16 n;
> +       u16 vclk_divider;
> +       const struct dsi_clk_config *clkset;
> +};
> +
>  static inline struct rcar_mipi_dsi *
>  bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
>  {
> @@ -62,22 +88,6 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
>         return container_of(host, struct rcar_mipi_dsi, host);
>  }
>  
> -static const u32 phtw[] = {
> -       0x01020114, 0x01600115, /* General testing */
> -       0x01030116, 0x0102011d, /* General testing */
> -       0x011101a4, 0x018601a4, /* 1Gbps testing */
> -       0x014201a0, 0x010001a3, /* 1Gbps testing */
> -       0x0101011f,             /* 1Gbps testing */
> -};
> -
> -static const u32 phtw2[] = {
> -       0x010c0130, 0x010c0140, /* General testing */
> -       0x010c0150, 0x010c0180, /* General testing */
> -       0x010c0190,
> -       0x010a0160, 0x010a0170,
> -       0x01800164, 0x01800174, /* 1Gbps testing */
> -};
> -
>  static const u32 hsfreqrange_table[][2] = {
>         { 80000000U,   0x00 }, { 90000000U,   0x10 }, { 100000000U,  0x20 },
>         { 110000000U,  0x30 }, { 120000000U,  0x01 }, { 130000000U,  0x11 },
> @@ -103,24 +113,53 @@ static const u32 hsfreqrange_table[][2] = {
>         { /* sentinel */ },
>  };
>  
> -struct vco_cntrl_value {
> +struct dsi_clk_config {
>         u32 min_freq;
>         u32 max_freq;
> -       u16 value;
> +       u8 vco_cntrl;
> +       u8 cpbias_cntrl;
> +       u8 gmp_cntrl;
> +       u8 int_cntrl;
> +       u8 prop_cntrl;
>  };
>  
> -static const struct vco_cntrl_value vco_cntrl_table[] = {
> -       { .min_freq = 40000000U,   .max_freq = 55000000U,   .value = 0x3f },
> -       { .min_freq = 52500000U,   .max_freq = 80000000U,   .value = 0x39 },
> -       { .min_freq = 80000000U,   .max_freq = 110000000U,  .value = 0x2f },
> -       { .min_freq = 105000000U,  .max_freq = 160000000U,  .value = 0x29 },
> -       { .min_freq = 160000000U,  .max_freq = 220000000U,  .value = 0x1f },
> -       { .min_freq = 210000000U,  .max_freq = 320000000U,  .value = 0x19 },
> -       { .min_freq = 320000000U,  .max_freq = 440000000U,  .value = 0x0f },
> -       { .min_freq = 420000000U,  .max_freq = 660000000U,  .value = 0x09 },
> -       { .min_freq = 630000000U,  .max_freq = 1149000000U, .value = 0x03 },
> -       { .min_freq = 1100000000U, .max_freq = 1152000000U, .value = 0x01 },
> -       { .min_freq = 1150000000U, .max_freq = 1250000000U, .value = 0x01 },
> +static const struct dsi_clk_config dsi_clk_cfg_r8a779a0[] = {
> +       {   40000000u,   55000000u, 0x3f, 0x10, 0x01, 0x00, 0x0b },
> +       {   52500000u,   80000000u, 0x39, 0x10, 0x01, 0x00, 0x0b },
> +       {   80000000u,  110000000u, 0x2f, 0x10, 0x01, 0x00, 0x0b },
> +       {  105000000u,  160000000u, 0x29, 0x10, 0x01, 0x00, 0x0b },
> +       {  160000000u,  220000000u, 0x1f, 0x10, 0x01, 0x00, 0x0b },
> +       {  210000000u,  320000000u, 0x19, 0x10, 0x01, 0x00, 0x0b },
> +       {  320000000u,  440000000u, 0x0f, 0x10, 0x01, 0x00, 0x0b },
> +       {  420000000u,  660000000u, 0x09, 0x10, 0x01, 0x00, 0x0b },
> +       {  630000000u, 1149000000u, 0x03, 0x10, 0x01, 0x00, 0x0b },
> +       { 1100000000u, 1152000000u, 0x01, 0x10, 0x01, 0x00, 0x0b },
> +       { 1150000000u, 1250000000u, 0x01, 0x10, 0x01, 0x00, 0x0c },

Sigh ... is it this one 0x0c value that means we need to keep all these
entries repeated ? :-S

If it weren't for that, it seems we could keep just two sets of
> +       u8 cpbias_cntrl;
> +       u8 gmp_cntrl;
> +       u8 int_cntrl;
> +       u8 prop_cntrl;

One for each of the 9a0, and the 9g0...

But this is fine, and I guess the implication is there may be other
future differences to come in other platforms.

It could be refactored then when we have more visibility.


> +       { /* sentinel */ },
> +};
> +
> +static const struct dsi_clk_config dsi_clk_cfg_r8a779g0[] = {
> +       {   40000000u,   45310000u, 0x2b, 0x00, 0x00, 0x08, 0x0a },
> +       {   45310000u,   54660000u, 0x28, 0x00, 0x00, 0x08, 0x0a },
> +       {   54660000u,   62500000u, 0x28, 0x00, 0x00, 0x08, 0x0a },
> +       {   62500000u,   75000000u, 0x27, 0x00, 0x00, 0x08, 0x0a },
> +       {   75000000u,   90630000u, 0x23, 0x00, 0x00, 0x08, 0x0a },
> +       {   90630000u,  109370000u, 0x20, 0x00, 0x00, 0x08, 0x0a },
> +       {  109370000u,  125000000u, 0x20, 0x00, 0x00, 0x08, 0x0a },
> +       {  125000000u,  150000000u, 0x1f, 0x00, 0x00, 0x08, 0x0a },
> +       {  150000000u,  181250000u, 0x1b, 0x00, 0x00, 0x08, 0x0a },
> +       {  181250000u,  218750000u, 0x18, 0x00, 0x00, 0x08, 0x0a },
> +       {  218750000u,  250000000u, 0x18, 0x00, 0x00, 0x08, 0x0a },
> +       {  250000000u,  300000000u, 0x17, 0x00, 0x00, 0x08, 0x0a },
> +       {  300000000u,  362500000u, 0x13, 0x00, 0x00, 0x08, 0x0a },
> +       {  362500000u,  455480000u, 0x10, 0x00, 0x00, 0x08, 0x0a },
> +       {  455480000u,  500000000u, 0x10, 0x00, 0x00, 0x08, 0x0a },
> +       {  500000000u,  600000000u, 0x0f, 0x00, 0x00, 0x08, 0x0a },
> +       {  600000000u,  725000000u, 0x0b, 0x00, 0x00, 0x08, 0x0a },
> +       {  725000000u,  875000000u, 0x08, 0x00, 0x00, 0x08, 0x0a },
> +       {  875000000u, 1000000000u, 0x08, 0x00, 0x00, 0x08, 0x0a },
> +       { 1000000000u, 1200000000u, 0x07, 0x00, 0x00, 0x08, 0x0a },
> +       { 1200000000u, 1250000000u, 0x03, 0x00, 0x00, 0x08, 0x0a },
>         { /* sentinel */ },
>  };
>  
> @@ -144,7 +183,7 @@ static void rcar_mipi_dsi_set(struct rcar_mipi_dsi *dsi, u32 reg, u32 set)
>         rcar_mipi_dsi_write(dsi, reg, rcar_mipi_dsi_read(dsi, reg) | set);
>  }
>  
> -static int rcar_mipi_dsi_phtw_test(struct rcar_mipi_dsi *dsi, u32 phtw)
> +static int rcar_mipi_dsi_write_phtw(struct rcar_mipi_dsi *dsi, u32 phtw)
>  {
>         u32 status;
>         int ret;
> @@ -163,32 +202,231 @@ static int rcar_mipi_dsi_phtw_test(struct rcar_mipi_dsi *dsi, u32 phtw)
>         return ret;
>  }
>  
> +static int rcar_mipi_dsi_write_phtw_arr(struct rcar_mipi_dsi *dsi,
> +                                       const u32 *phtw, unsigned int size)
> +{
> +       for (unsigned int i = 0; i < size; i++) {
> +               int ret = rcar_mipi_dsi_write_phtw(dsi, phtw[i]);
> +
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +#define WRITE_PHTW(...)                                               \
> +       ({                                                            \
> +               static const u32 phtw[] = { __VA_ARGS__ };            \
> +               int ret;                                              \
> +               ret = rcar_mipi_dsi_write_phtw_arr(dsi, phtw,         \
> +                                                  ARRAY_SIZE(phtw)); \
> +               ret;                                                  \
> +       })
> +
> +static int rcar_mipi_dsi_init_phtw_v3u(struct rcar_mipi_dsi *dsi)
> +{
> +       return WRITE_PHTW(0x01020114, 0x01600115, 0x01030116, 0x0102011d,
> +                         0x011101a4, 0x018601a4, 0x014201a0, 0x010001a3,
> +                         0x0101011f);
> +}
> +
> +static int rcar_mipi_dsi_post_init_phtw_v3u(struct rcar_mipi_dsi *dsi)
> +{
> +       return WRITE_PHTW(0x010c0130, 0x010c0140, 0x010c0150, 0x010c0180,
> +                         0x010c0190, 0x010a0160, 0x010a0170, 0x01800164,
> +                         0x01800174);
> +}
> +
> +static int rcar_mipi_dsi_init_phtw_v4h(struct rcar_mipi_dsi *dsi,
> +                                      const struct dsi_setup_info *setup_info)
> +{
> +       int ret;
> +
> +       if (setup_info->hsfreq < MHZ(450)) {
> +               ret = WRITE_PHTW(0x01010100, 0x011b01ac);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       ret = WRITE_PHTW(0x01010100, 0x01030173, 0x01000174, 0x01500175,
> +                        0x01030176, 0x01040166, 0x010201ad);

Ok - I can see this at 67.3.5.

> +       if (ret)
> +               return ret;
> +
> +       if (setup_info->hsfreq <= MHZ(1000))
> +               ret = WRITE_PHTW(0x01020100, 0x01910170, 0x01020171,
> +                                0x01110172);

The <=1Gbps reads:

<=1Gbps

Set PHTW=H’01020100
Set PHTW=H’01010172
Set PHTW=H’01570170
Set PHTW=H’01060171
Set PHTW=H’01110172

Is it clear why theres a difference here?

> +       else if (setup_info->hsfreq <= MHZ(1500))
> +               ret = WRITE_PHTW(0x01020100, 0x01980170, 0x01030171,
> +                                0x01100172);

And here, it states the following which differs:

1Gbps < and <=1.5Gbps
Set PHTW=H’01020100
Set PHTW=H’01000172
Set PHTW=H’01200170
Set PHTW=H’01090171
Set PHTW=H’01100172 ?

> +       else if (setup_info->hsfreq <= MHZ(2500))
> +               ret = WRITE_PHTW(0x01020100, 0x0144016b, 0x01000172);

And I believe this is Case 2 on the right...

Set PHTW=H’01020100
Set PHTW=H’0104016B
Set PHTW=H’01000172

Which has differences in the middle write ?

I expect I'm missing something obvious there as there are lots of key
differences. So perhaps those are just 'examples'.

> +       else
> +               return -EINVAL;
> +
> +       if (ret)
> +               return ret;
> +
> +       if (dsi->lanes <= 1) {
> +               ret = WRITE_PHTW(0x01070100, 0x010e010b);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (dsi->lanes <= 2) {
> +               ret = WRITE_PHTW(0x01090100, 0x010e010b);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (dsi->lanes <= 3) {
> +               ret = WRITE_PHTW(0x010b0100, 0x010e010b);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (setup_info->hsfreq <= MHZ(1500)) {
> +               ret = WRITE_PHTW(0x01010100, 0x01c0016e);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int
> +rcar_mipi_dsi_post_init_phtw_v4h(struct rcar_mipi_dsi *dsi,
> +                                const struct dsi_setup_info *setup_info)
> +{
> +       u32 status;
> +       int ret;
> +
> +       if (setup_info->hsfreq <= MHZ(1500)) {
> +               WRITE_PHTW(0x01020100, 0x00000180);
> +
> +               ret = read_poll_timeout(rcar_mipi_dsi_read, status,
> +                                       status & PHTR_TEST, 2000, 10000, false,
> +                                       dsi, PHTR);
> +               if (ret < 0) {
> +                       dev_err(dsi->dev, "failed to test PHTR\n");
> +                       return ret;
> +               }
> +
> +               WRITE_PHTW(0x01010100, 0x0100016e);
> +       }
> +
> +       return 0;
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Hardware Setup
>   */
>  
> -struct dsi_setup_info {
> -       unsigned long fout;
> -       u16 vco_cntrl;
> -       u16 prop_cntrl;
> -       u16 hsfreqrange;
> -       u16 div;
> -       unsigned int m;
> -       unsigned int n;
> -};
> +static void rcar_mipi_dsi_pll_calc_r8a779a0(struct rcar_mipi_dsi *dsi,
> +                                           struct clk *clk,
> +                                           unsigned long fout_target,
> +                                           struct dsi_setup_info *setup_info)
> +{
> +       unsigned int best_err = -1;
> +       unsigned long fin;
> +
> +       fin = clk_get_rate(clk);
> +
> +       for (unsigned int n = 3; n <= 8; n++) {
> +               unsigned long fpfd;
> +
> +               fpfd = fin / n;
> +
> +               if (fpfd < MHZ(2) || fpfd > MHZ(8))
> +                       continue;
> +
> +               for (unsigned int m = 64; m <= 625; m++) {
> +                       unsigned int err;
> +                       u64 fout;
> +
> +                       fout = (u64)fpfd * m;
> +
> +                       if (fout < MHZ(320) || fout > MHZ(1250))
> +                               continue;
> +
> +                       fout = div64_u64(fout, setup_info->vclk_divider);
> +
> +                       if (fout < setup_info->clkset->min_freq ||
> +                           fout > setup_info->clkset->max_freq)
> +                               continue;
> +
> +                       err = abs((long)(fout - fout_target) * 10000 /
> +                                 (long)fout_target);
> +
> +                       if (err < best_err) {
> +                               setup_info->m = m;
> +                               setup_info->n = n;
> +                               setup_info->fout = (unsigned long)fout;
> +                               best_err = err;
> +
> +                               if (err == 0)
> +                                       return;
> +                       }
> +               }
> +       }
> +}
> +
> +static void rcar_mipi_dsi_pll_calc_r8a779g0(struct rcar_mipi_dsi *dsi,
> +                                           struct clk *clk,
> +                                           unsigned long fout_target,
> +                                           struct dsi_setup_info *setup_info)
> +{
> +       unsigned int best_err = -1;
> +       unsigned long fin;
> +
> +       fin = clk_get_rate(clk);
> +
> +       for (unsigned int n = 1; n <= 8; n++) {
> +               unsigned long fpfd;
> +
> +               fpfd = fin / n;
> +
> +               if (fpfd < MHZ(8) || fpfd > MHZ(24))
> +                       continue;
> +
> +               for (unsigned int m = 167; m <= 1000; m++) {
> +                       unsigned int err;
> +                       u64 fout;
> +
> +                       fout = div64_u64((u64)fpfd * m, 2);
> +
> +                       if (fout < MHZ(2000) || fout > MHZ(4000))
> +                               continue;
> +
> +                       fout = div64_u64(fout, setup_info->vclk_divider);
> +
> +                       if (fout < setup_info->clkset->min_freq ||
> +                           fout > setup_info->clkset->max_freq)
> +                               continue;
> +
> +                       err = abs((long)(fout - fout_target) * 10000 /
> +                                 (long)fout_target);
> +                       if (err < best_err) {
> +                               setup_info->m = m;
> +                               setup_info->n = n;
> +                               setup_info->fout = (unsigned long)fout;
> +                               best_err = err;
> +
> +                               if (err == 0)
> +                                       return;
> +                       }
> +               }
> +       }
> +}
>  
>  static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
>                                           struct clk *clk, unsigned long target,
>                                           struct dsi_setup_info *setup_info)
>  {
>  
> -       const struct vco_cntrl_value *vco_cntrl;
> +       const struct dsi_clk_config *clkset;
>         unsigned long fout_target;
> -       unsigned long fin, fout;
> -       unsigned long hsfreq;
> -       unsigned int best_err = -1;
> -       unsigned int divider;
> -       unsigned int n;
>         unsigned int i;
>         unsigned int err;
>  
> @@ -198,70 +436,53 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
>          */
>         fout_target = target * mipi_dsi_pixel_format_to_bpp(dsi->format)
>                     / (2 * dsi->lanes);
> -       if (fout_target < 40000000 || fout_target > 1250000000)
> +       if (fout_target < MHZ(40) || fout_target > MHZ(1250))
>                 return;
>  
>         /* Find vco_cntrl */
> -       for (vco_cntrl = vco_cntrl_table; vco_cntrl->min_freq != 0; vco_cntrl++) {
> -               if (fout_target > vco_cntrl->min_freq &&
> -                   fout_target <= vco_cntrl->max_freq) {
> -                       setup_info->vco_cntrl = vco_cntrl->value;
> -                       if (fout_target >= 1150000000)
> -                               setup_info->prop_cntrl = 0x0c;
> -                       else
> -                               setup_info->prop_cntrl = 0x0b;
> +       for (clkset = dsi->info->clk_cfg; clkset->min_freq != 0; clkset++) {
> +               if (fout_target > clkset->min_freq &&
> +                   fout_target <= clkset->max_freq) {
> +                       setup_info->clkset = clkset;
>                         break;
>                 }
>         }
>  
> -       /* Add divider */
> -       setup_info->div = (setup_info->vco_cntrl & 0x30) >> 4;
> +       switch (dsi->info->model) {
> +       case RCAR_DSI_R8A779A0:
> +               setup_info->vclk_divider = 1 << ((clkset->vco_cntrl >> 4) & 0x3);
> +               rcar_mipi_dsi_pll_calc_r8a779a0(dsi, clk, fout_target, setup_info);
> +               break;
> +
> +       case RCAR_DSI_R8A779G0:
> +               setup_info->vclk_divider = 1 << (((clkset->vco_cntrl >> 3) & 0x7) + 1);
> +               rcar_mipi_dsi_pll_calc_r8a779g0(dsi, clk, fout_target, setup_info);
> +               break;
> +
> +       default:
> +               return;
> +       }
>  
>         /* Find hsfreqrange */
> -       hsfreq = fout_target * 2;
> +       setup_info->hsfreq = setup_info->fout * 2;
>         for (i = 0; i < ARRAY_SIZE(hsfreqrange_table); i++) {
> -               if (hsfreqrange_table[i][0] >= hsfreq) {
> +               if (hsfreqrange_table[i][0] >= setup_info->hsfreq) {
>                         setup_info->hsfreqrange = hsfreqrange_table[i][1];
>                         break;
>                 }
>         }
>  
> -       /*
> -        * Calculate n and m for PLL clock
> -        * Following the HW manual the ranges of n and m are
> -        * n = [3-8] and m = [64-625]
> -        */
> -       fin = clk_get_rate(clk);
> -       divider = 1 << setup_info->div;
> -       for (n = 3; n < 9; n++) {
> -               unsigned long fpfd;
> -               unsigned int m;
> -
> -               fpfd = fin / n;
> -
> -               for (m = 64; m < 626; m++) {
> -                       fout = fpfd * m / divider;
> -                       err = abs((long)(fout - fout_target) * 10000 /
> -                                 (long)fout_target);
> -                       if (err < best_err) {
> -                               setup_info->m = m - 2;
> -                               setup_info->n = n - 1;
> -                               setup_info->fout = fout;
> -                               best_err = err;
> -                               if (err == 0)
> -                                       goto done;
> -                       }
> -               }
> -       }
> +       err = abs((long)(setup_info->fout - fout_target) * 10000 / (long)fout_target);
>  
> -done:
>         dev_dbg(dsi->dev,
> -               "%pC %lu Hz -> Fout %lu Hz (target %lu Hz, error %d.%02u%%), PLL M/N/DIV %u/%u/%u\n",
> -               clk, fin, setup_info->fout, fout_target, best_err / 100,
> -               best_err % 100, setup_info->m, setup_info->n, setup_info->div);
> +               "Fout = %u * %lu / (2 * %u * %u) = %lu (target %lu Hz, error %d.%02u%%)\n",
> +               setup_info->m, clk_get_rate(clk), setup_info->n, setup_info->vclk_divider,
> +               setup_info->fout, fout_target,
> +               err / 100, err % 100);
> +
>         dev_dbg(dsi->dev,
>                 "vco_cntrl = 0x%x\tprop_cntrl = 0x%x\thsfreqrange = 0x%x\n",
> -               setup_info->vco_cntrl, setup_info->prop_cntrl,
> +               clkset->vco_cntrl, clkset->prop_cntrl,
>                 setup_info->hsfreqrange);
>  }
>  
> @@ -324,7 +545,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>  {
>         struct dsi_setup_info setup_info = {};
>         unsigned int timeout;
> -       int ret, i;
> +       int ret;
>         int dsi_format;
>         u32 phy_setup;
>         u32 clockset2, clockset3;
> @@ -360,10 +581,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>         phy_setup |= PHYSETUP_HSFREQRANGE(setup_info.hsfreqrange);
>         rcar_mipi_dsi_write(dsi, PHYSETUP, phy_setup);
>  
> -       for (i = 0; i < ARRAY_SIZE(phtw); i++) {
> -               ret = rcar_mipi_dsi_phtw_test(dsi, phtw[i]);
> +       switch (dsi->info->model) {
> +       case RCAR_DSI_R8A779A0:
> +               ret = rcar_mipi_dsi_init_phtw_v3u(dsi);
> +               if (ret < 0)
> +                       return ret;
> +               break;
> +
> +       case RCAR_DSI_R8A779G0:
> +               ret = rcar_mipi_dsi_init_phtw_v4h(dsi, &setup_info);
>                 if (ret < 0)
>                         return ret;
> +               break;
> +
> +       default:
> +               return -ENODEV;
>         }
>  
>         /* PLL Clock Setting */
> @@ -371,12 +603,13 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>         rcar_mipi_dsi_set(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
>         rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
>  
> -       clockset2 = CLOCKSET2_M(setup_info.m) | CLOCKSET2_N(setup_info.n)
> -                 | CLOCKSET2_VCO_CNTRL(setup_info.vco_cntrl);
> -       clockset3 = CLOCKSET3_PROP_CNTRL(setup_info.prop_cntrl)
> -                 | CLOCKSET3_INT_CNTRL(0)
> -                 | CLOCKSET3_CPBIAS_CNTRL(0x10)
> -                 | CLOCKSET3_GMP_CNTRL(1);
> +       clockset2 = CLOCKSET2_M(setup_info.m - dsi->info->clockset2_m_offset)
> +                 | CLOCKSET2_N(setup_info.n - dsi->info->clockset2_n_offset)
> +                 | CLOCKSET2_VCO_CNTRL(setup_info.clkset->vco_cntrl);
> +       clockset3 = CLOCKSET3_PROP_CNTRL(setup_info.clkset->prop_cntrl)
> +                 | CLOCKSET3_INT_CNTRL(setup_info.clkset->int_cntrl)
> +                 | CLOCKSET3_CPBIAS_CNTRL(setup_info.clkset->cpbias_cntrl)
> +                 | CLOCKSET3_GMP_CNTRL(setup_info.clkset->gmp_cntrl);
>         rcar_mipi_dsi_write(dsi, CLOCKSET2, clockset2);
>         rcar_mipi_dsi_write(dsi, CLOCKSET3, clockset3);
>  
> @@ -407,10 +640,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>                 return -ETIMEDOUT;
>         }
>  
> -       for (i = 0; i < ARRAY_SIZE(phtw2); i++) {
> -               ret = rcar_mipi_dsi_phtw_test(dsi, phtw2[i]);
> +       switch (dsi->info->model) {
> +       case RCAR_DSI_R8A779A0:
> +               ret = rcar_mipi_dsi_post_init_phtw_v3u(dsi);
>                 if (ret < 0)
>                         return ret;
> +               break;
> +
> +       case RCAR_DSI_R8A779G0:
> +               ret = rcar_mipi_dsi_post_init_phtw_v4h(dsi, &setup_info);
> +               if (ret < 0)
> +                       return ret;
> +               break;
> +
> +       default:
> +               return -ENODEV;
>         }
>  
>         /* Enable DOT clock */
> @@ -427,8 +671,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>                 dev_warn(dsi->dev, "unsupported format");
>                 return -EINVAL;
>         }
> -       vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
> -               |  VCLKSET_LANE(dsi->lanes - 1);
> +
> +       vclkset |= VCLKSET_COLOR_RGB | VCLKSET_LANE(dsi->lanes - 1);
> +
> +       switch (dsi->info->model) {
> +       case RCAR_DSI_R8A779A0:
> +               vclkset |= VCLKSET_DIV_R8A779A0(__ffs(setup_info.vclk_divider));
> +               break;
> +
> +       case RCAR_DSI_R8A779G0:
> +               vclkset |= VCLKSET_DIV_R8A779G0(__ffs(setup_info.vclk_divider) - 1);

Why is this a -1 here ? Seems an odd difference compared to the A0.

> +               break;
> +
> +       default:
> +               return -ENODEV;
> +       }
>  
>         rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
>  
> @@ -841,8 +1098,25 @@ static int rcar_mipi_dsi_remove(struct platform_device *pdev)
>         return 0;
>  }
>  
> +static const struct rcar_mipi_dsi_device_info r8a779a0_data = {
> +       .model = RCAR_DSI_R8A779A0,
> +       .clk_cfg = dsi_clk_cfg_r8a779a0,
> +       .clockset2_m_offset = 2,
> +       .clockset2_n_offset = 1,
> +
> +};
> +
> +static const struct rcar_mipi_dsi_device_info r8a779g0_data = {
> +       .model = RCAR_DSI_R8A779G0,
> +       .clk_cfg = dsi_clk_cfg_r8a779g0,
> +       .clockset2_m_offset = 0,
> +       .clockset2_n_offset = 1,
> +
> +};
> +
>  static const struct of_device_id rcar_mipi_dsi_of_table[] = {
> -       { .compatible = "renesas,r8a779a0-dsi-csi2-tx" },
> +       { .compatible = "renesas,r8a779a0-dsi-csi2-tx", .data = &r8a779a0_data },
> +       { .compatible = "renesas,r8a779g0-dsi-csi2-tx", .data = &r8a779g0_data },
>         { }
>  };
>  
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h
> index 2eaca54636f3..608851340acf 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h
> @@ -122,7 +122,8 @@
>  #define VCLKSET_CKEN                   (1 << 16)
>  #define VCLKSET_COLOR_RGB              (0 << 8)
>  #define VCLKSET_COLOR_YCC              (1 << 8)
> -#define VCLKSET_DIV(x)                 (((x) & 0x3) << 4)
> +#define VCLKSET_DIV_R8A779A0(x)                (((x) & 0x3) << 4)
> +#define VCLKSET_DIV_R8A779G0(x)                (((x) & 0x7) << 4)
>  #define VCLKSET_BPP_16                 (0 << 2)
>  #define VCLKSET_BPP_18                 (1 << 2)
>  #define VCLKSET_BPP_18L                        (2 << 2)
> @@ -166,6 +167,9 @@
>  #define PHTW_CWEN                      (1 << 8)
>  #define PHTW_TESTDIN_CODE(x)           (((x) & 0xff) << 0)
>  
> +#define PHTR                           0x1038
> +#define PHTR_TEST                      (1 << 16)
> +
>  #define PHTC                           0x103c
>  #define PHTC_TESTCLR                   (1 << 0)
>  
> -- 
> 2.34.1
>

Re: [PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Tomi Valkeinen]

On 17/11/2022 17:46, Kieran Bingham wrote:
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (setup_info->hsfreq <= MHZ(1000))
>> +               ret = WRITE_PHTW(0x01020100, 0x01910170, 0x01020171,
>> +                                0x01110172);
> The <=1Gbps reads:
> 
> <=1Gbps
> 
> Set PHTW=H’01020100
> Set PHTW=H’01010172
> Set PHTW=H’01570170
> Set PHTW=H’01060171
> Set PHTW=H’01110172
> 
> Is it clear why theres a difference here?

What doc is that? Mine (R19UH0172EJ0054 Rev.0.54) says:

Set PHTW=H’01020100
Set PHTW=H’01910170
Set PHTW=H’01020171
Set PHTW=H’01110172

  Tomi

Re: [PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-11-17 15:49:36)
> On 17/11/2022 17:46, Kieran Bingham wrote:
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       if (setup_info->hsfreq <= MHZ(1000))
> >> +               ret = WRITE_PHTW(0x01020100, 0x01910170, 0x01020171,
> >> +                                0x01110172);
> > The <=1Gbps reads:
> > 
> > <=1Gbps
> > 
> > Set PHTW=H’01020100
> > Set PHTW=H’01010172
> > Set PHTW=H’01570170
> > Set PHTW=H’01060171
> > Set PHTW=H’01110172
> > 
> > Is it clear why theres a difference here?
> 
> What doc is that? Mine (R19UH0172EJ0054 Rev.0.54) says:

Hrm ... mine is R19UH0172EJ0051 ... So I guess I'm on an outdated
datasheet.

Which leaves me pondering if I should just wave my hand over my eyes and
add a tag ... (Or see if I can dig up the later datasheet) ;-)

--
Kieran


> 
> Set PHTW=H’01020100
> Set PHTW=H’01910170
> Set PHTW=H’01020171
> Set PHTW=H’01110172
> 
>   Tomi
>

Re: [PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Tomi Valkeinen]

On 17/11/2022 17:46, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-11-17 12:25:46)
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> Add DSI support for r8a779g0. The main differences to r8a779a0 are in
>> the PLL and PHTW setups.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c      | 484 +++++++++++++++----
>>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
>>   2 files changed, 384 insertions(+), 106 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> index a7f2b7f66a17..723c35726c38 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/delay.h>
>>   #include <linux/io.h>
>>   #include <linux/iopoll.h>
>> +#include <linux/math64.h>
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>>   #include <linux/of_device.h>
>> @@ -28,6 +29,20 @@
>>   #include "rcar_mipi_dsi.h"
>>   #include "rcar_mipi_dsi_regs.h"
>>   
>> +#define MHZ(v) ((v) * 1000000u)
>> +
>> +enum rcar_mipi_dsi_hw_model {
>> +       RCAR_DSI_R8A779A0,
>> +       RCAR_DSI_R8A779G0,
>> +};
>> +
>> +struct rcar_mipi_dsi_device_info {
>> +       enum rcar_mipi_dsi_hw_model model;
>> +       const struct dsi_clk_config *clk_cfg;
>> +       u8 clockset2_m_offset;
>> +       u8 clockset2_n_offset;
>> +};
>> +
>>   struct rcar_mipi_dsi {
>>          struct device *dev;
>>          const struct rcar_mipi_dsi_device_info *info;
>> @@ -50,6 +65,17 @@ struct rcar_mipi_dsi {
>>          unsigned int lanes;
>>   };
>>   
>> +struct dsi_setup_info {
>> +       unsigned long hsfreq;
>> +       u16 hsfreqrange;
>> +
>> +       unsigned long fout;
>> +       u16 m;
>> +       u16 n;
>> +       u16 vclk_divider;
>> +       const struct dsi_clk_config *clkset;
>> +};
>> +
>>   static inline struct rcar_mipi_dsi *
>>   bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
>>   {
>> @@ -62,22 +88,6 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
>>          return container_of(host, struct rcar_mipi_dsi, host);
>>   }
>>   
>> -static const u32 phtw[] = {
>> -       0x01020114, 0x01600115, /* General testing */
>> -       0x01030116, 0x0102011d, /* General testing */
>> -       0x011101a4, 0x018601a4, /* 1Gbps testing */
>> -       0x014201a0, 0x010001a3, /* 1Gbps testing */
>> -       0x0101011f,             /* 1Gbps testing */
>> -};
>> -
>> -static const u32 phtw2[] = {
>> -       0x010c0130, 0x010c0140, /* General testing */
>> -       0x010c0150, 0x010c0180, /* General testing */
>> -       0x010c0190,
>> -       0x010a0160, 0x010a0170,
>> -       0x01800164, 0x01800174, /* 1Gbps testing */
>> -};
>> -
>>   static const u32 hsfreqrange_table[][2] = {
>>          { 80000000U,   0x00 }, { 90000000U,   0x10 }, { 100000000U,  0x20 },
>>          { 110000000U,  0x30 }, { 120000000U,  0x01 }, { 130000000U,  0x11 },
>> @@ -103,24 +113,53 @@ static const u32 hsfreqrange_table[][2] = {
>>          { /* sentinel */ },
>>   };
>>   
>> -struct vco_cntrl_value {
>> +struct dsi_clk_config {
>>          u32 min_freq;
>>          u32 max_freq;
>> -       u16 value;
>> +       u8 vco_cntrl;
>> +       u8 cpbias_cntrl;
>> +       u8 gmp_cntrl;
>> +       u8 int_cntrl;
>> +       u8 prop_cntrl;
>>   };
>>   
>> -static const struct vco_cntrl_value vco_cntrl_table[] = {
>> -       { .min_freq = 40000000U,   .max_freq = 55000000U,   .value = 0x3f },
>> -       { .min_freq = 52500000U,   .max_freq = 80000000U,   .value = 0x39 },
>> -       { .min_freq = 80000000U,   .max_freq = 110000000U,  .value = 0x2f },
>> -       { .min_freq = 105000000U,  .max_freq = 160000000U,  .value = 0x29 },
>> -       { .min_freq = 160000000U,  .max_freq = 220000000U,  .value = 0x1f },
>> -       { .min_freq = 210000000U,  .max_freq = 320000000U,  .value = 0x19 },
>> -       { .min_freq = 320000000U,  .max_freq = 440000000U,  .value = 0x0f },
>> -       { .min_freq = 420000000U,  .max_freq = 660000000U,  .value = 0x09 },
>> -       { .min_freq = 630000000U,  .max_freq = 1149000000U, .value = 0x03 },
>> -       { .min_freq = 1100000000U, .max_freq = 1152000000U, .value = 0x01 },
>> -       { .min_freq = 1150000000U, .max_freq = 1250000000U, .value = 0x01 },
>> +static const struct dsi_clk_config dsi_clk_cfg_r8a779a0[] = {
>> +       {   40000000u,   55000000u, 0x3f, 0x10, 0x01, 0x00, 0x0b },
>> +       {   52500000u,   80000000u, 0x39, 0x10, 0x01, 0x00, 0x0b },
>> +       {   80000000u,  110000000u, 0x2f, 0x10, 0x01, 0x00, 0x0b },
>> +       {  105000000u,  160000000u, 0x29, 0x10, 0x01, 0x00, 0x0b },
>> +       {  160000000u,  220000000u, 0x1f, 0x10, 0x01, 0x00, 0x0b },
>> +       {  210000000u,  320000000u, 0x19, 0x10, 0x01, 0x00, 0x0b },
>> +       {  320000000u,  440000000u, 0x0f, 0x10, 0x01, 0x00, 0x0b },
>> +       {  420000000u,  660000000u, 0x09, 0x10, 0x01, 0x00, 0x0b },
>> +       {  630000000u, 1149000000u, 0x03, 0x10, 0x01, 0x00, 0x0b },
>> +       { 1100000000u, 1152000000u, 0x01, 0x10, 0x01, 0x00, 0x0b },
>> +       { 1150000000u, 1250000000u, 0x01, 0x10, 0x01, 0x00, 0x0c },
> 
> Sigh ... is it this one 0x0c value that means we need to keep all these
> entries repeated ? :-S
> 
> If it weren't for that, it seems we could keep just two sets of
>> +       u8 cpbias_cntrl;
>> +       u8 gmp_cntrl;
>> +       u8 int_cntrl;
>> +       u8 prop_cntrl;
> 
> One for each of the 9a0, and the 9g0...
> 
> But this is fine, and I guess the implication is there may be other
> future differences to come in other platforms.
> 
> It could be refactored then when we have more visibility.

Yes, it's not so nice. And afaiu some of these values should really be 
solved dynamically in the code. But the docs list these tables and don't 
explain how to come up with those values, so... I think having these 
tables is the safest way.

>> @@ -427,8 +671,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>>                  dev_warn(dsi->dev, "unsupported format");
>>                  return -EINVAL;
>>          }
>> -       vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
>> -               |  VCLKSET_LANE(dsi->lanes - 1);
>> +
>> +       vclkset |= VCLKSET_COLOR_RGB | VCLKSET_LANE(dsi->lanes - 1);
>> +
>> +       switch (dsi->info->model) {
>> +       case RCAR_DSI_R8A779A0:
>> +               vclkset |= VCLKSET_DIV_R8A779A0(__ffs(setup_info.vclk_divider));
>> +               break;
>> +
>> +       case RCAR_DSI_R8A779G0:
>> +               vclkset |= VCLKSET_DIV_R8A779G0(__ffs(setup_info.vclk_divider) - 1);
> 
> Why is this a -1 here ? Seems an odd difference compared to the A0.

You need to ask the HW designers =). That's how the register is on G0. 
Field value of 0 means divided by 2.

  Tomi

Re: [PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Laurent Pinchart]

On Tue, Nov 22, 2022 at 10:50:30AM +0200, Tomi Valkeinen wrote:
> On 17/11/2022 17:46, Kieran Bingham wrote:
> > Quoting Tomi Valkeinen (2022-11-17 12:25:46)
> >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>
> >> Add DSI support for r8a779g0. The main differences to r8a779a0 are in
> >> the PLL and PHTW setups.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >> ---
> >>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c      | 484 +++++++++++++++----
> >>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
> >>   2 files changed, 384 insertions(+), 106 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> >> index a7f2b7f66a17..723c35726c38 100644
> >> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> >> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> >> @@ -9,6 +9,7 @@
> >>   #include <linux/delay.h>
> >>   #include <linux/io.h>
> >>   #include <linux/iopoll.h>
> >> +#include <linux/math64.h>
> >>   #include <linux/module.h>
> >>   #include <linux/of.h>
> >>   #include <linux/of_device.h>
> >> @@ -28,6 +29,20 @@
> >>   #include "rcar_mipi_dsi.h"
> >>   #include "rcar_mipi_dsi_regs.h"
> >>   
> >> +#define MHZ(v) ((v) * 1000000u)
> >> +
> >> +enum rcar_mipi_dsi_hw_model {
> >> +       RCAR_DSI_R8A779A0,
> >> +       RCAR_DSI_R8A779G0,
> >> +};
> >> +
> >> +struct rcar_mipi_dsi_device_info {
> >> +       enum rcar_mipi_dsi_hw_model model;
> >> +       const struct dsi_clk_config *clk_cfg;
> >> +       u8 clockset2_m_offset;
> >> +       u8 clockset2_n_offset;
> >> +};
> >> +
> >>   struct rcar_mipi_dsi {
> >>          struct device *dev;
> >>          const struct rcar_mipi_dsi_device_info *info;
> >> @@ -50,6 +65,17 @@ struct rcar_mipi_dsi {
> >>          unsigned int lanes;
> >>   };
> >>   
> >> +struct dsi_setup_info {
> >> +       unsigned long hsfreq;
> >> +       u16 hsfreqrange;
> >> +
> >> +       unsigned long fout;
> >> +       u16 m;
> >> +       u16 n;
> >> +       u16 vclk_divider;
> >> +       const struct dsi_clk_config *clkset;
> >> +};
> >> +
> >>   static inline struct rcar_mipi_dsi *
> >>   bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
> >>   {
> >> @@ -62,22 +88,6 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
> >>          return container_of(host, struct rcar_mipi_dsi, host);
> >>   }
> >>   
> >> -static const u32 phtw[] = {
> >> -       0x01020114, 0x01600115, /* General testing */
> >> -       0x01030116, 0x0102011d, /* General testing */
> >> -       0x011101a4, 0x018601a4, /* 1Gbps testing */
> >> -       0x014201a0, 0x010001a3, /* 1Gbps testing */
> >> -       0x0101011f,             /* 1Gbps testing */
> >> -};
> >> -
> >> -static const u32 phtw2[] = {
> >> -       0x010c0130, 0x010c0140, /* General testing */
> >> -       0x010c0150, 0x010c0180, /* General testing */
> >> -       0x010c0190,
> >> -       0x010a0160, 0x010a0170,
> >> -       0x01800164, 0x01800174, /* 1Gbps testing */
> >> -};
> >> -
> >>   static const u32 hsfreqrange_table[][2] = {
> >>          { 80000000U,   0x00 }, { 90000000U,   0x10 }, { 100000000U,  0x20 },
> >>          { 110000000U,  0x30 }, { 120000000U,  0x01 }, { 130000000U,  0x11 },
> >> @@ -103,24 +113,53 @@ static const u32 hsfreqrange_table[][2] = {
> >>          { /* sentinel */ },
> >>   };
> >>   
> >> -struct vco_cntrl_value {
> >> +struct dsi_clk_config {
> >>          u32 min_freq;
> >>          u32 max_freq;
> >> -       u16 value;
> >> +       u8 vco_cntrl;
> >> +       u8 cpbias_cntrl;
> >> +       u8 gmp_cntrl;
> >> +       u8 int_cntrl;
> >> +       u8 prop_cntrl;
> >>   };
> >>   
> >> -static const struct vco_cntrl_value vco_cntrl_table[] = {
> >> -       { .min_freq = 40000000U,   .max_freq = 55000000U,   .value = 0x3f },
> >> -       { .min_freq = 52500000U,   .max_freq = 80000000U,   .value = 0x39 },
> >> -       { .min_freq = 80000000U,   .max_freq = 110000000U,  .value = 0x2f },
> >> -       { .min_freq = 105000000U,  .max_freq = 160000000U,  .value = 0x29 },
> >> -       { .min_freq = 160000000U,  .max_freq = 220000000U,  .value = 0x1f },
> >> -       { .min_freq = 210000000U,  .max_freq = 320000000U,  .value = 0x19 },
> >> -       { .min_freq = 320000000U,  .max_freq = 440000000U,  .value = 0x0f },
> >> -       { .min_freq = 420000000U,  .max_freq = 660000000U,  .value = 0x09 },
> >> -       { .min_freq = 630000000U,  .max_freq = 1149000000U, .value = 0x03 },
> >> -       { .min_freq = 1100000000U, .max_freq = 1152000000U, .value = 0x01 },
> >> -       { .min_freq = 1150000000U, .max_freq = 1250000000U, .value = 0x01 },
> >> +static const struct dsi_clk_config dsi_clk_cfg_r8a779a0[] = {
> >> +       {   40000000u,   55000000u, 0x3f, 0x10, 0x01, 0x00, 0x0b },
> >> +       {   52500000u,   80000000u, 0x39, 0x10, 0x01, 0x00, 0x0b },
> >> +       {   80000000u,  110000000u, 0x2f, 0x10, 0x01, 0x00, 0x0b },
> >> +       {  105000000u,  160000000u, 0x29, 0x10, 0x01, 0x00, 0x0b },
> >> +       {  160000000u,  220000000u, 0x1f, 0x10, 0x01, 0x00, 0x0b },
> >> +       {  210000000u,  320000000u, 0x19, 0x10, 0x01, 0x00, 0x0b },
> >> +       {  320000000u,  440000000u, 0x0f, 0x10, 0x01, 0x00, 0x0b },
> >> +       {  420000000u,  660000000u, 0x09, 0x10, 0x01, 0x00, 0x0b },
> >> +       {  630000000u, 1149000000u, 0x03, 0x10, 0x01, 0x00, 0x0b },
> >> +       { 1100000000u, 1152000000u, 0x01, 0x10, 0x01, 0x00, 0x0b },
> >> +       { 1150000000u, 1250000000u, 0x01, 0x10, 0x01, 0x00, 0x0c },
> > 
> > Sigh ... is it this one 0x0c value that means we need to keep all these
> > entries repeated ? :-S
> > 
> > If it weren't for that, it seems we could keep just two sets of
> >> +       u8 cpbias_cntrl;
> >> +       u8 gmp_cntrl;
> >> +       u8 int_cntrl;
> >> +       u8 prop_cntrl;
> > 
> > One for each of the 9a0, and the 9g0...
> > 
> > But this is fine, and I guess the implication is there may be other
> > future differences to come in other platforms.
> > 
> > It could be refactored then when we have more visibility.
> 
> Yes, it's not so nice. And afaiu some of these values should really be 
> solved dynamically in the code. But the docs list these tables and don't 
> explain how to come up with those values, so... I think having these 
> tables is the safest way.

We could drop the cpbias_cntrl, gmp_cntrl and int_cntrl fields and set
them based on the IP version.

> >> @@ -427,8 +671,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
> >>                  dev_warn(dsi->dev, "unsupported format");
> >>                  return -EINVAL;
> >>          }
> >> -       vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
> >> -               |  VCLKSET_LANE(dsi->lanes - 1);
> >> +
> >> +       vclkset |= VCLKSET_COLOR_RGB | VCLKSET_LANE(dsi->lanes - 1);
> >> +
> >> +       switch (dsi->info->model) {
> >> +       case RCAR_DSI_R8A779A0:
> >> +               vclkset |= VCLKSET_DIV_R8A779A0(__ffs(setup_info.vclk_divider));
> >> +               break;
> >> +
> >> +       case RCAR_DSI_R8A779G0:
> >> +               vclkset |= VCLKSET_DIV_R8A779G0(__ffs(setup_info.vclk_divider) - 1);
> > 
> > Why is this a -1 here ? Seems an odd difference compared to the A0.
> 
> You need to ask the HW designers =). That's how the register is on G0. 
> Field value of 0 means divided by 2.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 7/8] drm: rcar-du: dsi: Add r8A779g0 support

[Tomi Valkeinen]

On 29/11/2022 02:43, Laurent Pinchart wrote:
> On Tue, Nov 22, 2022 at 10:50:30AM +0200, Tomi Valkeinen wrote:
>> On 17/11/2022 17:46, Kieran Bingham wrote:
>>> Quoting Tomi Valkeinen (2022-11-17 12:25:46)
>>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>>
>>>> Add DSI support for r8a779g0. The main differences to r8a779a0 are in
>>>> the PLL and PHTW setups.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>> ---
>>>>    drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c      | 484 +++++++++++++++----
>>>>    drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
>>>>    2 files changed, 384 insertions(+), 106 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>>>> index a7f2b7f66a17..723c35726c38 100644
>>>> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>>>> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>>>> @@ -9,6 +9,7 @@
>>>>    #include <linux/delay.h>
>>>>    #include <linux/io.h>
>>>>    #include <linux/iopoll.h>
>>>> +#include <linux/math64.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/of.h>
>>>>    #include <linux/of_device.h>
>>>> @@ -28,6 +29,20 @@
>>>>    #include "rcar_mipi_dsi.h"
>>>>    #include "rcar_mipi_dsi_regs.h"
>>>>    
>>>> +#define MHZ(v) ((v) * 1000000u)
>>>> +
>>>> +enum rcar_mipi_dsi_hw_model {
>>>> +       RCAR_DSI_R8A779A0,
>>>> +       RCAR_DSI_R8A779G0,
>>>> +};
>>>> +
>>>> +struct rcar_mipi_dsi_device_info {
>>>> +       enum rcar_mipi_dsi_hw_model model;
>>>> +       const struct dsi_clk_config *clk_cfg;
>>>> +       u8 clockset2_m_offset;
>>>> +       u8 clockset2_n_offset;
>>>> +};
>>>> +
>>>>    struct rcar_mipi_dsi {
>>>>           struct device *dev;
>>>>           const struct rcar_mipi_dsi_device_info *info;
>>>> @@ -50,6 +65,17 @@ struct rcar_mipi_dsi {
>>>>           unsigned int lanes;
>>>>    };
>>>>    
>>>> +struct dsi_setup_info {
>>>> +       unsigned long hsfreq;
>>>> +       u16 hsfreqrange;
>>>> +
>>>> +       unsigned long fout;
>>>> +       u16 m;
>>>> +       u16 n;
>>>> +       u16 vclk_divider;
>>>> +       const struct dsi_clk_config *clkset;
>>>> +};
>>>> +
>>>>    static inline struct rcar_mipi_dsi *
>>>>    bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
>>>>    {
>>>> @@ -62,22 +88,6 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
>>>>           return container_of(host, struct rcar_mipi_dsi, host);
>>>>    }
>>>>    
>>>> -static const u32 phtw[] = {
>>>> -       0x01020114, 0x01600115, /* General testing */
>>>> -       0x01030116, 0x0102011d, /* General testing */
>>>> -       0x011101a4, 0x018601a4, /* 1Gbps testing */
>>>> -       0x014201a0, 0x010001a3, /* 1Gbps testing */
>>>> -       0x0101011f,             /* 1Gbps testing */
>>>> -};
>>>> -
>>>> -static const u32 phtw2[] = {
>>>> -       0x010c0130, 0x010c0140, /* General testing */
>>>> -       0x010c0150, 0x010c0180, /* General testing */
>>>> -       0x010c0190,
>>>> -       0x010a0160, 0x010a0170,
>>>> -       0x01800164, 0x01800174, /* 1Gbps testing */
>>>> -};
>>>> -
>>>>    static const u32 hsfreqrange_table[][2] = {
>>>>           { 80000000U,   0x00 }, { 90000000U,   0x10 }, { 100000000U,  0x20 },
>>>>           { 110000000U,  0x30 }, { 120000000U,  0x01 }, { 130000000U,  0x11 },
>>>> @@ -103,24 +113,53 @@ static const u32 hsfreqrange_table[][2] = {
>>>>           { /* sentinel */ },
>>>>    };
>>>>    
>>>> -struct vco_cntrl_value {
>>>> +struct dsi_clk_config {
>>>>           u32 min_freq;
>>>>           u32 max_freq;
>>>> -       u16 value;
>>>> +       u8 vco_cntrl;
>>>> +       u8 cpbias_cntrl;
>>>> +       u8 gmp_cntrl;
>>>> +       u8 int_cntrl;
>>>> +       u8 prop_cntrl;
>>>>    };
>>>>    
>>>> -static const struct vco_cntrl_value vco_cntrl_table[] = {
>>>> -       { .min_freq = 40000000U,   .max_freq = 55000000U,   .value = 0x3f },
>>>> -       { .min_freq = 52500000U,   .max_freq = 80000000U,   .value = 0x39 },
>>>> -       { .min_freq = 80000000U,   .max_freq = 110000000U,  .value = 0x2f },
>>>> -       { .min_freq = 105000000U,  .max_freq = 160000000U,  .value = 0x29 },
>>>> -       { .min_freq = 160000000U,  .max_freq = 220000000U,  .value = 0x1f },
>>>> -       { .min_freq = 210000000U,  .max_freq = 320000000U,  .value = 0x19 },
>>>> -       { .min_freq = 320000000U,  .max_freq = 440000000U,  .value = 0x0f },
>>>> -       { .min_freq = 420000000U,  .max_freq = 660000000U,  .value = 0x09 },
>>>> -       { .min_freq = 630000000U,  .max_freq = 1149000000U, .value = 0x03 },
>>>> -       { .min_freq = 1100000000U, .max_freq = 1152000000U, .value = 0x01 },
>>>> -       { .min_freq = 1150000000U, .max_freq = 1250000000U, .value = 0x01 },
>>>> +static const struct dsi_clk_config dsi_clk_cfg_r8a779a0[] = {
>>>> +       {   40000000u,   55000000u, 0x3f, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {   52500000u,   80000000u, 0x39, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {   80000000u,  110000000u, 0x2f, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {  105000000u,  160000000u, 0x29, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {  160000000u,  220000000u, 0x1f, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {  210000000u,  320000000u, 0x19, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {  320000000u,  440000000u, 0x0f, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {  420000000u,  660000000u, 0x09, 0x10, 0x01, 0x00, 0x0b },
>>>> +       {  630000000u, 1149000000u, 0x03, 0x10, 0x01, 0x00, 0x0b },
>>>> +       { 1100000000u, 1152000000u, 0x01, 0x10, 0x01, 0x00, 0x0b },
>>>> +       { 1150000000u, 1250000000u, 0x01, 0x10, 0x01, 0x00, 0x0c },
>>>
>>> Sigh ... is it this one 0x0c value that means we need to keep all these
>>> entries repeated ? :-S
>>>
>>> If it weren't for that, it seems we could keep just two sets of
>>>> +       u8 cpbias_cntrl;
>>>> +       u8 gmp_cntrl;
>>>> +       u8 int_cntrl;
>>>> +       u8 prop_cntrl;
>>>
>>> One for each of the 9a0, and the 9g0...
>>>
>>> But this is fine, and I guess the implication is there may be other
>>> future differences to come in other platforms.
>>>
>>> It could be refactored then when we have more visibility.
>>
>> Yes, it's not so nice. And afaiu some of these values should really be
>> solved dynamically in the code. But the docs list these tables and don't
>> explain how to come up with those values, so... I think having these
>> tables is the safest way.
> 
> We could drop the cpbias_cntrl, gmp_cntrl and int_cntrl fields and set
> them based on the IP version.

We could, but I have no idea what those do, and don't know if there may 
be a case (for a future SoC or if the optimal values are updated for 
current ones) where multiple values are used on a single soc. So I 
thought that it's better to keep them aligned to the HW docs (i.e. 
together in a table).

  Tomi

[PATCH v1 8/8] HACK: drm: rcar-du: dsi: use-extal-clk hack

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Renesas BSP kernel does this for Whitehawk board. It is not clear what
it does, as the bits are marked reserved in the SoC documentation.

Do not merge.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 723c35726c38..c264cb689664 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -598,6 +598,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 		return -ENODEV;
 	}
 
+	/* XXX HACK for Whitehawk board. "use_extal_clk" from BSP Kernel. */
+	if (dsi->info->model == RCAR_DSI_R8A779G0)
+		rcar_mipi_dsi_set(dsi, CLOCKSET1, 0x0100000C);
+
 	/* PLL Clock Setting */
 	rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
 	rcar_mipi_dsi_set(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
-- 
2.34.1

Re: [PATCH v1 0/8] Renesas V4H DSI & DP output support

[Tomi Valkeinen]

On 17/11/2022 14:25, Tomi Valkeinen wrote:
> Hi,
> 
> These add support for DSI on V4H SoC (r8a779g0) and DP for Whitehawk
> board.
> 
> The last patch is a hack, but needed to get the DSI working. It is still
> unclear what the register write does, and as that patch is needed to get
> the DSI working, this series is not ready yet. But all the rest of the
> patches are ready for review.

And I forgot to mention, these are based on:

git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git master
(2c980642d64882b4e373b0317dd7bd45c1c34d80)

  Tomi

> 
>   Tomi
> 
> Tomi Valkeinen (8):
>    dt-bindings: display: renesas,du: Provide bindings for r8a779g0
>    dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0
>    clk: renesas: r8a779g0: Add display related clocks
>    arm64: dts: renesas: r8a779g0: Add display related data
>    arm64: dts: renesas: white-hawk-cpu: Add DP output support
>    drm: rcar-du: Add r8a779g0 support
>    drm: rcar-du: dsi: Add r8A779g0 support
>    HACK: drm: rcar-du: dsi: use-extal-clk hack
> 
>   .../display/bridge/renesas,dsi-csi2-tx.yaml   |   3 +-
>   .../bindings/display/renesas,du.yaml          |   2 +
>   .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  |  94 ++++
>   arch/arm64/boot/dts/renesas/r8a779g0.dtsi     | 129 +++++
>   drivers/clk/renesas/r8a779g0-cpg-mssr.c       |  14 +
>   drivers/gpu/drm/rcar-du/rcar_du_drv.c         |  22 +
>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c       | 488 ++++++++++++++----
>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h  |   6 +-
>   8 files changed, 651 insertions(+), 107 deletions(-)
>