[PATCH 0/9] drm: Add DSI/DP support for Renesas r8a779h0 V4M and grey-hawk board

[PATCH 0/9] drm: Add DSI/DP support for Renesas r8a779h0 V4M and grey-hawk board

[Tomi Valkeinen]

Add everything needed to support the DSI output on Renesas r8a779h0
(V4M) SoC, and the DP output (via sn65dsi86 DSI to DP bridge) on the
Renesas grey-hawk board.

Overall the DSI and the board design is almost identical to Renesas
r8a779g0 and white-hawk board.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
Tomi Valkeinen (9):
      dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779h0
      dt-bindings: display: renesas,du: Add r8a779h0
      clk: renesas: r8a779h0: Add display clocks
      drm/rcar-du: dsi: Fix PHY lock bit check
      drm/rcar-du: dsi: Add r8a779h0 support
      drm/rcar-du: Add support for r8a779h0
      arm64: dts: renesas: gray-hawk-single: Fix indentation
      arm64: dts: renesas: r8a779h0: Add display support
      arm64: dts: renesas: gray-hawk-single: Add DisplayPort support

 .../display/bridge/renesas,dsi-csi2-tx.yaml        |   1 +
 .../devicetree/bindings/display/renesas,du.yaml    |   1 +
 .../boot/dts/renesas/r8a779h0-gray-hawk-single.dts | 119 ++++++++++++++++++---
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi          |  77 +++++++++++++
 drivers/clk/renesas/r8a779h0-cpg-mssr.c            |   4 +
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c      |  19 ++++
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h      |   1 +
 drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c    |  16 +--
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c    |   4 +-
 .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h   |   1 -
 10 files changed, 223 insertions(+), 20 deletions(-)
---
base-commit: adc218676eef25575469234709c2d87185ca223a
change-id: 20241008-rcar-gh-dsi-9c01f5deeac8

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH 1/9] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779h0

[Tomi Valkeinen]

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

Extend the Renesas DSI display bindings to support the r8a779h0 V4M.

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

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 d33026f85e19..c167795c63f6 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
@@ -19,6 +19,7 @@ properties:
     enum:
       - renesas,r8a779a0-dsi-csi2-tx    # for V3U
       - renesas,r8a779g0-dsi-csi2-tx    # for V4H
+      - renesas,r8a779h0-dsi-csi2-tx    # for V4M
 
   reg:
     maxItems: 1

-- 
2.43.0

Re: [PATCH 1/9] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779h0

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:35AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DSI display bindings to support the r8a779h0 V4M.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  .../devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml          | 1 +
>  1 file changed, 1 insertion(+)
> 
> 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 d33026f85e19..c167795c63f6 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> @@ -19,6 +19,7 @@ properties:
>      enum:
>        - renesas,r8a779a0-dsi-csi2-tx    # for V3U
>        - renesas,r8a779g0-dsi-csi2-tx    # for V4H
> +      - renesas,r8a779h0-dsi-csi2-tx    # for V4M
>  
>    reg:
>      maxItems: 1

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/9] dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779h0

[Krzysztof Kozlowski]

On Tue, Dec 03, 2024 at 10:01:35AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DSI display bindings to support the r8a779h0 V4M.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  .../devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml          | 1 +
>  1 file changed, 1 insertion(+)

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

Best regards,
Krzysztof

[PATCH 2/9] dt-bindings: display: renesas,du: Add r8a779h0

[Tomi Valkeinen]

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

Extend the Renesas DU display bindings to support the r8a779h0 V4M.

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

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index c5b9e6812bce..d369953f16f7 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -41,6 +41,7 @@ properties:
       - 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
+      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
 
   reg:
     maxItems: 1

-- 
2.43.0

Re: [PATCH 2/9] dt-bindings: display: renesas,du: Add r8a779h0

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:36AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Extend the Renesas DU display bindings to support the r8a779h0 V4M.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/renesas,du.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index c5b9e6812bce..d369953f16f7 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -41,6 +41,7 @@ properties:
>        - 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
> +      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
>  
>    reg:
>      maxItems: 1

You also need to add h0 to the g0 block in the conditional properties
below. With that,

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

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/9] dt-bindings: display: renesas,du: Add r8a779h0

[Tomi Valkeinen]

On 03/12/2024 10:19, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tue, Dec 03, 2024 at 10:01:36AM +0200, Tomi Valkeinen wrote:
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> Extend the Renesas DU display bindings to support the r8a779h0 V4M.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   Documentation/devicetree/bindings/display/renesas,du.yaml | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
>> index c5b9e6812bce..d369953f16f7 100644
>> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
>> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
>> @@ -41,6 +41,7 @@ properties:
>>         - 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
>> +      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
>>   
>>     reg:
>>       maxItems: 1
> 
> You also need to add h0 to the g0 block in the conditional properties
> below. With that,

Ah, I missed that. Thanks.

  Tomi

Re: [PATCH 2/9] dt-bindings: display: renesas,du: Add r8a779h0

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Dec 3, 2024 at 9:19 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tue, Dec 03, 2024 at 10:01:36AM +0200, Tomi Valkeinen wrote:
> > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >
> > Extend the Renesas DU display bindings to support the r8a779h0 V4M.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  Documentation/devicetree/bindings/display/renesas,du.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> > index c5b9e6812bce..d369953f16f7 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> > +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> > @@ -41,6 +41,7 @@ properties:
> >        - 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
> > +      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
> >
> >    reg:
> >      maxItems: 1
>
> You also need to add h0 to the g0 block in the conditional properties
> below. With that,

Which is not sufficient, as the DU on R-Car V4M has only a single channel,
unlike on R-Car V3U and V4H.

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 2/9] dt-bindings: display: renesas,du: Add r8a779h0

[Laurent Pinchart]

On Tue, Dec 03, 2024 at 09:38:44AM +0100, Geert Uytterhoeven wrote:
> Hi Laurent,
> 
> On Tue, Dec 3, 2024 at 9:19 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Tue, Dec 03, 2024 at 10:01:36AM +0200, Tomi Valkeinen wrote:
> > > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > >
> > > Extend the Renesas DU display bindings to support the r8a779h0 V4M.
> > >
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > > ---
> > >  Documentation/devicetree/bindings/display/renesas,du.yaml | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> > > index c5b9e6812bce..d369953f16f7 100644
> > > --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> > > +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> > > @@ -41,6 +41,7 @@ properties:
> > >        - 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
> > > +      - renesas,du-r8a779h0 # for R-Car V4M compatible DU
> > >
> > >    reg:
> > >      maxItems: 1
> >
> > You also need to add h0 to the g0 block in the conditional properties
> > below. With that,
> 
> Which is not sufficient, as the DU on R-Car V4M has only a single channel,
> unlike on R-Car V3U and V4H.

Ah, indeed, in that case the DT bindings also need to take that into
account :-)

-- 
Regards,

Laurent Pinchart

[PATCH 3/9] clk: renesas: r8a779h0: Add display clocks

[Tomi Valkeinen]

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

Add display related clocks for DU, DSI, FCPVD, and VSPD.

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

diff --git a/drivers/clk/renesas/r8a779h0-cpg-mssr.c b/drivers/clk/renesas/r8a779h0-cpg-mssr.c
index e20c048bfa9b..dc37e987c0e6 100644
--- a/drivers/clk/renesas/r8a779h0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779h0-cpg-mssr.c
@@ -179,6 +179,9 @@ static const struct mssr_mod_clk r8a779h0_mod_clks[] __initconst = {
 	DEF_MOD("canfd0",	328,	R8A779H0_CLK_SASYNCPERD2),
 	DEF_MOD("csi40",	331,	R8A779H0_CLK_CSI),
 	DEF_MOD("csi41",	400,	R8A779H0_CLK_CSI),
+	DEF_MOD("dis0",		411,	R8A779H0_CLK_S0D3),
+	DEF_MOD("dsitxlink0",	415,	R8A779H0_CLK_DSIREF),
+	DEF_MOD("fcpvd0",	508,	R8A779H0_CLK_S0D3),
 	DEF_MOD("hscif0",	514,	R8A779H0_CLK_SASYNCPERD1),
 	DEF_MOD("hscif1",	515,	R8A779H0_CLK_SASYNCPERD1),
 	DEF_MOD("hscif2",	516,	R8A779H0_CLK_SASYNCPERD1),
@@ -227,6 +230,7 @@ static const struct mssr_mod_clk r8a779h0_mod_clks[] __initconst = {
 	DEF_MOD("vin15",	811,	R8A779H0_CLK_S0D4_VIO),
 	DEF_MOD("vin16",	812,	R8A779H0_CLK_S0D4_VIO),
 	DEF_MOD("vin17",	813,	R8A779H0_CLK_S0D4_VIO),
+	DEF_MOD("vspd0",	830,	R8A779H0_CLK_S0D1_VIO),
 	DEF_MOD("wdt1:wdt0",	907,	R8A779H0_CLK_R),
 	DEF_MOD("cmt0",		910,	R8A779H0_CLK_R),
 	DEF_MOD("cmt1",		911,	R8A779H0_CLK_R),

-- 
2.43.0

Re: [PATCH 3/9] clk: renesas: r8a779h0: Add display clocks

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:37AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add display related clocks for DU, DSI, FCPVD, and VSPD.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/clk/renesas/r8a779h0-cpg-mssr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/clk/renesas/r8a779h0-cpg-mssr.c b/drivers/clk/renesas/r8a779h0-cpg-mssr.c
> index e20c048bfa9b..dc37e987c0e6 100644
> --- a/drivers/clk/renesas/r8a779h0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a779h0-cpg-mssr.c
> @@ -179,6 +179,9 @@ static const struct mssr_mod_clk r8a779h0_mod_clks[] __initconst = {
>  	DEF_MOD("canfd0",	328,	R8A779H0_CLK_SASYNCPERD2),
>  	DEF_MOD("csi40",	331,	R8A779H0_CLK_CSI),
>  	DEF_MOD("csi41",	400,	R8A779H0_CLK_CSI),
> +	DEF_MOD("dis0",		411,	R8A779H0_CLK_S0D3),
> +	DEF_MOD("dsitxlink0",	415,	R8A779H0_CLK_DSIREF),
> +	DEF_MOD("fcpvd0",	508,	R8A779H0_CLK_S0D3),
>  	DEF_MOD("hscif0",	514,	R8A779H0_CLK_SASYNCPERD1),
>  	DEF_MOD("hscif1",	515,	R8A779H0_CLK_SASYNCPERD1),
>  	DEF_MOD("hscif2",	516,	R8A779H0_CLK_SASYNCPERD1),
> @@ -227,6 +230,7 @@ static const struct mssr_mod_clk r8a779h0_mod_clks[] __initconst = {
>  	DEF_MOD("vin15",	811,	R8A779H0_CLK_S0D4_VIO),
>  	DEF_MOD("vin16",	812,	R8A779H0_CLK_S0D4_VIO),
>  	DEF_MOD("vin17",	813,	R8A779H0_CLK_S0D4_VIO),
> +	DEF_MOD("vspd0",	830,	R8A779H0_CLK_S0D1_VIO),

The clock names and numbers are fine. The parents are not explicitly
listed in documentation, but the VIODBUSD1 clock description (table
8.1.4a) mentions FCPVD and VSPD as target modules. This is something
that should probably be double-checked. Quite interestingly, VIOBUSD2
also mentions the same target modules, hinting as a more complex clock
tree. A similar issue is perhaps present for "dis0" too, that's a DU
clock and the DU isn't listed as a target module of S0D3.

The way we model the "module stop" bits as clocks is clearly a limiting
factor as it can't represent real clock topologies. I don't however
don't expect it to cause any functional issue here, as the devices
related to the above clocks do not depend on the clock frequency.
There's no strict need to model the real hardware clock tree if it has
no impact on software, so

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

I think it could be worth it check with Renesas what parent to use here,
and we can update the clock definitions later if needed.

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

-- 
Regards,

Laurent Pinchart

[PATCH 4/9] drm/rcar-du: dsi: Fix PHY lock bit check

[Tomi Valkeinen]

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

The driver checks for bit 16 (using CLOCKSET1_LOCK define) in CLOCKSET1
register when waiting for the PPI clock. However, the right bit to check
is bit 17 (CLOCKSET1_LOCK_PHY define). Not only that, but there's
nothing in the documents for bit 16 for V3U nor V4H.

So, fix the check to use bit 17, and drop the define for bit 16.

Fixes: 155358310f01 ("drm: rcar-du: Add R-Car DSI driver")
Fixes: 11696c5e8924 ("drm: Place Renesas drivers in a separate dir")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c      | 2 +-
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
index 2dba7c5ffd2c..92f4261305bd 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
@@ -587,7 +587,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	for (timeout = 10; timeout > 0; --timeout) {
 		if ((rcar_mipi_dsi_read(dsi, PPICLSR) & PPICLSR_STPST) &&
 		    (rcar_mipi_dsi_read(dsi, PPIDLSR) & PPIDLSR_STPST) &&
-		    (rcar_mipi_dsi_read(dsi, CLOCKSET1) & CLOCKSET1_LOCK))
+		    (rcar_mipi_dsi_read(dsi, CLOCKSET1) & CLOCKSET1_LOCK_PHY))
 			break;
 
 		usleep_range(1000, 2000);
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
index f8114d11f2d1..a6b276f1d6ee 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
@@ -142,7 +142,6 @@
 
 #define CLOCKSET1			0x101c
 #define CLOCKSET1_LOCK_PHY		(1 << 17)
-#define CLOCKSET1_LOCK			(1 << 16)
 #define CLOCKSET1_CLKSEL		(1 << 8)
 #define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
 #define CLOCKSET1_CLKINSEL_DIG		(1 << 2)

-- 
2.43.0

Re: [PATCH 4/9] drm/rcar-du: dsi: Fix PHY lock bit check

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:38AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> The driver checks for bit 16 (using CLOCKSET1_LOCK define) in CLOCKSET1
> register when waiting for the PPI clock. However, the right bit to check
> is bit 17 (CLOCKSET1_LOCK_PHY define). Not only that, but there's
> nothing in the documents for bit 16 for V3U nor V4H.
> 
> So, fix the check to use bit 17, and drop the define for bit 16.
> 
> Fixes: 155358310f01 ("drm: rcar-du: Add R-Car DSI driver")
> Fixes: 11696c5e8924 ("drm: Place Renesas drivers in a separate dir")

Should this have CC: stable ?

> Signed-off-by: Tomi Valkeiben <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c      | 2 +-
>  drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> index 2dba7c5ffd2c..92f4261305bd 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> @@ -587,7 +587,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>  	for (timeout = 10; timeout > 0; --timeout) {
>  		if ((rcar_mipi_dsi_read(dsi, PPICLSR) & PPICLSR_STPST) &&
>  		    (rcar_mipi_dsi_read(dsi, PPIDLSR) & PPIDLSR_STPST) &&
> -		    (rcar_mipi_dsi_read(dsi, CLOCKSET1) & CLOCKSET1_LOCK))
> +		    (rcar_mipi_dsi_read(dsi, CLOCKSET1) & CLOCKSET1_LOCK_PHY))
>  			break;
>  
>  		usleep_range(1000, 2000);
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> index f8114d11f2d1..a6b276f1d6ee 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> @@ -142,7 +142,6 @@
>  
>  #define CLOCKSET1			0x101c
>  #define CLOCKSET1_LOCK_PHY		(1 << 17)
> -#define CLOCKSET1_LOCK			(1 << 16)
>  #define CLOCKSET1_CLKSEL		(1 << 8)
>  #define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
>  #define CLOCKSET1_CLKINSEL_DIG		(1 << 2)

-- 
Regards,

Laurent Pinchart

[PATCH 5/9] drm/rcar-du: dsi: Add r8a779h0 support

[Tomi Valkeinen]

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

Add support for DSI on r8a779h0. As it is identical to DSI on r8a779g0,
all we need is to handle the compatible string.

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

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
index 92f4261305bd..36e902601f84 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
@@ -1081,6 +1081,8 @@ static const struct rcar_mipi_dsi_device_info v4h_data = {
 static const struct of_device_id rcar_mipi_dsi_of_table[] = {
 	{ .compatible = "renesas,r8a779a0-dsi-csi2-tx", .data = &v3u_data },
 	{ .compatible = "renesas,r8a779g0-dsi-csi2-tx", .data = &v4h_data },
+	/* DSI in r8a779h0 is identical to r8a779g0 */
+	{ .compatible = "renesas,r8a779h0-dsi-csi2-tx", .data = &v4h_data },
 	{ }
 };
 

-- 
2.43.0

Re: [PATCH 5/9] drm/rcar-du: dsi: Add r8a779h0 support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:39AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add support for DSI on r8a779h0. As it is identical to DSI on r8a779g0,
> all we need is to handle the compatible string.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> index 92f4261305bd..36e902601f84 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> @@ -1081,6 +1081,8 @@ static const struct rcar_mipi_dsi_device_info v4h_data = {
>  static const struct of_device_id rcar_mipi_dsi_of_table[] = {
>  	{ .compatible = "renesas,r8a779a0-dsi-csi2-tx", .data = &v3u_data },
>  	{ .compatible = "renesas,r8a779g0-dsi-csi2-tx", .data = &v4h_data },
> +	/* DSI in r8a779h0 is identical to r8a779g0 */
> +	{ .compatible = "renesas,r8a779h0-dsi-csi2-tx", .data = &v4h_data },
>  	{ }
>  };
>  

-- 
Regards,

Laurent Pinchart

[PATCH 6/9] drm/rcar-du: Add support for r8a779h0

[Tomi Valkeinen]

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

Add support for r8a779h0. It is very similar to r8a779g0, but has only
one output.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 19 +++++++++++++++++++
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h   |  1 +
 drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 16 ++++++++++------
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
index fb719d9aff10..afbc74e18cce 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
@@ -545,6 +545,24 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
 	.dsi_clk_mask =  BIT(1) | BIT(0),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
+	.gen = 4,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ
+		  | RCAR_DU_FEATURE_VSP1_SOURCE
+		  | RCAR_DU_FEATURE_NO_BLENDING
+		  | RCAR_DU_FEATURE_NO_DPTSR,
+	.channels_mask = BIT(0),
+	.routes = {
+		/* R8A779H0 has one MIPI DSI output. */
+		[RCAR_DU_OUTPUT_DSI0] = {
+			.possible_crtcs = BIT(0),
+			.port = 0,
+		},
+	},
+	.num_rpf = 5,
+	.dsi_clk_mask = 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 },
@@ -571,6 +589,7 @@ static const struct of_device_id rcar_du_of_table[] = {
 	{ .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 },
+	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
 	{ }
 };
 
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
index 5cfa2bb7ad93..d7004f76f735 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
@@ -32,6 +32,7 @@ struct rcar_du_device;
 #define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
 #define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
 #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
+#define RCAR_DU_FEATURE_NO_DPTSR	BIT(6)  /* V4M does not have DPTSR */
 
 #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
 
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
index 2ccd2581f544..132d930670eb 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
@@ -107,10 +107,12 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
 		 */
 		rcrtc = rcdu->crtcs;
 		num_crtcs = rcdu->num_crtcs;
-	} else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
+	} else if ((rcdu->info->gen == 3 && rgrp->num_crtcs > 1) ||
+		   rcdu->info->gen == 4) {
 		/*
 		 * On Gen3 dot clocks are setup through per-group registers,
 		 * only available when the group has two channels.
+		 * On Gen4 the registers are there for single channel too.
 		 */
 		rcrtc = &rcdu->crtcs[rgrp->index * 2];
 		num_crtcs = rgrp->num_crtcs;
@@ -185,11 +187,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
 	rcar_du_group_write(rgrp, DORCR, dorcr);
 
-	/* Apply planes to CRTCs association. */
-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_NO_DPTSR)) {
+		/* Apply planes to CRTCs association. */
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
 }
 
 /*

-- 
2.43.0

Re: [PATCH 6/9] drm/rcar-du: Add support for r8a779h0

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:40AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add support for r8a779h0. It is very similar to r8a779g0, but has only
> one output.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 19 +++++++++++++++++++
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h   |  1 +
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 16 ++++++++++------
>  3 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> index fb719d9aff10..afbc74e18cce 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> @@ -545,6 +545,24 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
>  	.dsi_clk_mask =  BIT(1) | BIT(0),
>  };
>  
> +static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
> +	.gen = 4,
> +	.features = RCAR_DU_FEATURE_CRTC_IRQ
> +		  | RCAR_DU_FEATURE_VSP1_SOURCE
> +		  | RCAR_DU_FEATURE_NO_BLENDING
> +		  | RCAR_DU_FEATURE_NO_DPTSR,
> +	.channels_mask = BIT(0),
> +	.routes = {
> +		/* R8A779H0 has one MIPI DSI output. */
> +		[RCAR_DU_OUTPUT_DSI0] = {
> +			.possible_crtcs = BIT(0),
> +			.port = 0,
> +		},
> +	},
> +	.num_rpf = 5,
> +	.dsi_clk_mask = 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 },
> @@ -571,6 +589,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>  	{ .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 },
> +	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
>  	{ }
>  };
>  
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> index 5cfa2bb7ad93..d7004f76f735 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> @@ -32,6 +32,7 @@ struct rcar_du_device;
>  #define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
>  #define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
>  #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
> +#define RCAR_DU_FEATURE_NO_DPTSR	BIT(6)  /* V4M does not have DPTSR */

Do we need a quirk ? At first glance it seems the DPTSR register is only
used for DU instances that have two channels, so a check on the number
of channels should be enough ?

>  
>  #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
>  
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> index 2ccd2581f544..132d930670eb 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> @@ -107,10 +107,12 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>  		 */
>  		rcrtc = rcdu->crtcs;
>  		num_crtcs = rcdu->num_crtcs;
> -	} else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
> +	} else if ((rcdu->info->gen == 3 && rgrp->num_crtcs > 1) ||
> +		   rcdu->info->gen == 4) {
>  		/*
>  		 * On Gen3 dot clocks are setup through per-group registers,
>  		 * only available when the group has two channels.
> +		 * On Gen4 the registers are there for single channel too.
>  		 */
>  		rcrtc = &rcdu->crtcs[rgrp->index * 2];
>  		num_crtcs = rgrp->num_crtcs;
> @@ -185,11 +187,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
>  	rcar_du_group_write(rgrp, DORCR, dorcr);
>  
> -	/* Apply planes to CRTCs association. */
> -	mutex_lock(&rgrp->lock);
> -	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> -			    rgrp->dptsr_planes);
> -	mutex_unlock(&rgrp->lock);
> +	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_NO_DPTSR)) {
> +		/* Apply planes to CRTCs association. */
> +		mutex_lock(&rgrp->lock);
> +		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> +				    rgrp->dptsr_planes);
> +		mutex_unlock(&rgrp->lock);
> +	}
>  }
>  
>  /*

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/9] drm/rcar-du: Add support for r8a779h0

[Tomi Valkeinen]

On 03/12/2024 10:56, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tue, Dec 03, 2024 at 10:01:40AM +0200, Tomi Valkeinen wrote:
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> Add support for r8a779h0. It is very similar to r8a779g0, but has only
>> one output.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 19 +++++++++++++++++++
>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h   |  1 +
>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 16 ++++++++++------
>>   3 files changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
>> index fb719d9aff10..afbc74e18cce 100644
>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
>> @@ -545,6 +545,24 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
>>   	.dsi_clk_mask =  BIT(1) | BIT(0),
>>   };
>>   
>> +static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
>> +	.gen = 4,
>> +	.features = RCAR_DU_FEATURE_CRTC_IRQ
>> +		  | RCAR_DU_FEATURE_VSP1_SOURCE
>> +		  | RCAR_DU_FEATURE_NO_BLENDING
>> +		  | RCAR_DU_FEATURE_NO_DPTSR,
>> +	.channels_mask = BIT(0),
>> +	.routes = {
>> +		/* R8A779H0 has one MIPI DSI output. */
>> +		[RCAR_DU_OUTPUT_DSI0] = {
>> +			.possible_crtcs = BIT(0),
>> +			.port = 0,
>> +		},
>> +	},
>> +	.num_rpf = 5,
>> +	.dsi_clk_mask = 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 },
>> @@ -571,6 +589,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>>   	{ .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 },
>> +	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
>>   	{ }
>>   };
>>   
>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
>> index 5cfa2bb7ad93..d7004f76f735 100644
>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
>> @@ -32,6 +32,7 @@ struct rcar_du_device;
>>   #define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
>>   #define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
>>   #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
>> +#define RCAR_DU_FEATURE_NO_DPTSR	BIT(6)  /* V4M does not have DPTSR */
> 
> Do we need a quirk ? At first glance it seems the DPTSR register is only
> used for DU instances that have two channels, so a check on the number
> of channels should be enough ?

What do you mean with "DPTSR register is only used for DU instances that 
have two channels"? The upstream code sets it for all SoCs, doesn't it, 
without any checks?

Most of the SoCs seem to have two channels, but r8a77970 has one. 
However, I don't have docs for that one. It could be that it does not 
have DPTSR register, and indeed we could use the num_crtcs > 1 check there.

  Tomi

> 
>>   
>>   #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
>>   
>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
>> index 2ccd2581f544..132d930670eb 100644
>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
>> @@ -107,10 +107,12 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>>   		 */
>>   		rcrtc = rcdu->crtcs;
>>   		num_crtcs = rcdu->num_crtcs;
>> -	} else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
>> +	} else if ((rcdu->info->gen == 3 && rgrp->num_crtcs > 1) ||
>> +		   rcdu->info->gen == 4) {
>>   		/*
>>   		 * On Gen3 dot clocks are setup through per-group registers,
>>   		 * only available when the group has two channels.
>> +		 * On Gen4 the registers are there for single channel too.
>>   		 */
>>   		rcrtc = &rcdu->crtcs[rgrp->index * 2];
>>   		num_crtcs = rgrp->num_crtcs;
>> @@ -185,11 +187,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>   		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
>>   	rcar_du_group_write(rgrp, DORCR, dorcr);
>>   
>> -	/* Apply planes to CRTCs association. */
>> -	mutex_lock(&rgrp->lock);
>> -	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
>> -			    rgrp->dptsr_planes);
>> -	mutex_unlock(&rgrp->lock);
>> +	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_NO_DPTSR)) {
>> +		/* Apply planes to CRTCs association. */
>> +		mutex_lock(&rgrp->lock);
>> +		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
>> +				    rgrp->dptsr_planes);
>> +		mutex_unlock(&rgrp->lock);
>> +	}
>>   }
>>   
>>   /*
> 

Re: [PATCH 6/9] drm/rcar-du: Add support for r8a779h0

[Laurent Pinchart]

On Tue, Dec 03, 2024 at 11:22:15AM +0200, Tomi Valkeinen wrote:
> On 03/12/2024 10:56, Laurent Pinchart wrote:
> > On Tue, Dec 03, 2024 at 10:01:40AM +0200, Tomi Valkeinen wrote:
> >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>
> >> Add support for r8a779h0. It is very similar to r8a779g0, but has only
> >> one output.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >> ---
> >>   drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 19 +++++++++++++++++++
> >>   drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h   |  1 +
> >>   drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 16 ++++++++++------
> >>   3 files changed, 30 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> >> index fb719d9aff10..afbc74e18cce 100644
> >> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> >> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> >> @@ -545,6 +545,24 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
> >>   	.dsi_clk_mask =  BIT(1) | BIT(0),
> >>   };
> >>   
> >> +static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
> >> +	.gen = 4,
> >> +	.features = RCAR_DU_FEATURE_CRTC_IRQ
> >> +		  | RCAR_DU_FEATURE_VSP1_SOURCE
> >> +		  | RCAR_DU_FEATURE_NO_BLENDING
> >> +		  | RCAR_DU_FEATURE_NO_DPTSR,
> >> +	.channels_mask = BIT(0),
> >> +	.routes = {
> >> +		/* R8A779H0 has one MIPI DSI output. */
> >> +		[RCAR_DU_OUTPUT_DSI0] = {
> >> +			.possible_crtcs = BIT(0),
> >> +			.port = 0,
> >> +		},
> >> +	},
> >> +	.num_rpf = 5,
> >> +	.dsi_clk_mask = 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 },
> >> @@ -571,6 +589,7 @@ static const struct of_device_id rcar_du_of_table[] = {
> >>   	{ .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 },
> >> +	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
> >>   	{ }
> >>   };
> >>   
> >> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> >> index 5cfa2bb7ad93..d7004f76f735 100644
> >> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> >> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> >> @@ -32,6 +32,7 @@ struct rcar_du_device;
> >>   #define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
> >>   #define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
> >>   #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
> >> +#define RCAR_DU_FEATURE_NO_DPTSR	BIT(6)  /* V4M does not have DPTSR */
> > 
> > Do we need a quirk ? At first glance it seems the DPTSR register is only
> > used for DU instances that have two channels, so a check on the number
> > of channels should be enough ?
> 
> What do you mean with "DPTSR register is only used for DU instances that 
> have two channels"? The upstream code sets it for all SoCs, doesn't it, 
> without any checks?

DPTSR is one of those registers that controls features shared between
channels, in this specific case plane assignment to DU channels. The
default register value (i.e. all 0's) splits resources between the
channels. For DU groups with a single channel, there's no need for
resource assignment. Logically speaking, the all 0's register value as
documented in instances that have two channels would assign all the
resources that exist in the single-channel group to the single channel.
When computing the DPTSR value, the driver will (or at least should)
therefore always come up with 0x00000000. Writing that to the register
should be a no-op.

It's not clear if the register is present or not when the group has a
single channel. Some datasheets document the register is not being
applicable. Writing to it has never caused issues, so we may be dealing
with the hardware just ignoring writes to a non-implemented register, or
the register may be there, with only 0x00000000 being a meaningful
value. This being said, some people are concerned about writes to
registers that are not documented as present, as they could possibly
cause issues. Safety certification of the driver could be impacted.
We've updated the DU driver over the past few years to avoid those
writes for this reason.

TL;DR: yes, the DU driver writes to DPTSR for DU groups with a single
channel, but that seem it could be wrong, and we could fix it for all
single-channel groups in one go without introducing this feature bit. I
can test a patch on a M3 board that has a single channel in the second
group.

> Most of the SoCs seem to have two channels, but r8a77970 has one. 
> However, I don't have docs for that one. It could be that it does not 
> have DPTSR register, and indeed we could use the num_crtcs > 1 check there.
> 
> >>   #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
> >>   
> >> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> >> index 2ccd2581f544..132d930670eb 100644
> >> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> >> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> >> @@ -107,10 +107,12 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
> >>   		 */
> >>   		rcrtc = rcdu->crtcs;
> >>   		num_crtcs = rcdu->num_crtcs;
> >> -	} else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
> >> +	} else if ((rcdu->info->gen == 3 && rgrp->num_crtcs > 1) ||
> >> +		   rcdu->info->gen == 4) {
> >>   		/*
> >>   		 * On Gen3 dot clocks are setup through per-group registers,
> >>   		 * only available when the group has two channels.
> >> +		 * On Gen4 the registers are there for single channel too.
> >>   		 */
> >>   		rcrtc = &rcdu->crtcs[rgrp->index * 2];
> >>   		num_crtcs = rgrp->num_crtcs;
> >> @@ -185,11 +187,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
> >>   		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> >>   	rcar_du_group_write(rgrp, DORCR, dorcr);
> >>   
> >> -	/* Apply planes to CRTCs association. */
> >> -	mutex_lock(&rgrp->lock);
> >> -	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> >> -			    rgrp->dptsr_planes);
> >> -	mutex_unlock(&rgrp->lock);
> >> +	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_NO_DPTSR)) {
> >> +		/* Apply planes to CRTCs association. */
> >> +		mutex_lock(&rgrp->lock);
> >> +		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> >> +				    rgrp->dptsr_planes);
> >> +		mutex_unlock(&rgrp->lock);
> >> +	}
> >>   }
> >>   
> >>   /*

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/9] drm/rcar-du: Add support for r8a779h0

[Tomi Valkeinen]

Hi Laurent,

On 03/12/2024 12:48, Laurent Pinchart wrote:
> On Tue, Dec 03, 2024 at 11:22:15AM +0200, Tomi Valkeinen wrote:
>> On 03/12/2024 10:56, Laurent Pinchart wrote:
>>> On Tue, Dec 03, 2024 at 10:01:40AM +0200, Tomi Valkeinen wrote:
>>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>>
>>>> Add support for r8a779h0. It is very similar to r8a779g0, but has only
>>>> one output.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>> ---
>>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 19 +++++++++++++++++++
>>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h   |  1 +
>>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 16 ++++++++++------
>>>>    3 files changed, 30 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
>>>> index fb719d9aff10..afbc74e18cce 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
>>>> @@ -545,6 +545,24 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
>>>>    	.dsi_clk_mask =  BIT(1) | BIT(0),
>>>>    };
>>>>    
>>>> +static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
>>>> +	.gen = 4,
>>>> +	.features = RCAR_DU_FEATURE_CRTC_IRQ
>>>> +		  | RCAR_DU_FEATURE_VSP1_SOURCE
>>>> +		  | RCAR_DU_FEATURE_NO_BLENDING
>>>> +		  | RCAR_DU_FEATURE_NO_DPTSR,
>>>> +	.channels_mask = BIT(0),
>>>> +	.routes = {
>>>> +		/* R8A779H0 has one MIPI DSI output. */
>>>> +		[RCAR_DU_OUTPUT_DSI0] = {
>>>> +			.possible_crtcs = BIT(0),
>>>> +			.port = 0,
>>>> +		},
>>>> +	},
>>>> +	.num_rpf = 5,
>>>> +	.dsi_clk_mask = 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 },
>>>> @@ -571,6 +589,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>>>>    	{ .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 },
>>>> +	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
>>>>    	{ }
>>>>    };
>>>>    
>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
>>>> index 5cfa2bb7ad93..d7004f76f735 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
>>>> @@ -32,6 +32,7 @@ struct rcar_du_device;
>>>>    #define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
>>>>    #define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
>>>>    #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
>>>> +#define RCAR_DU_FEATURE_NO_DPTSR	BIT(6)  /* V4M does not have DPTSR */
>>>
>>> Do we need a quirk ? At first glance it seems the DPTSR register is only
>>> used for DU instances that have two channels, so a check on the number
>>> of channels should be enough ?
>>
>> What do you mean with "DPTSR register is only used for DU instances that
>> have two channels"? The upstream code sets it for all SoCs, doesn't it,
>> without any checks?
> 
> DPTSR is one of those registers that controls features shared between
> channels, in this specific case plane assignment to DU channels. The
> default register value (i.e. all 0's) splits resources between the
> channels. For DU groups with a single channel, there's no need for
> resource assignment. Logically speaking, the all 0's register value as
> documented in instances that have two channels would assign all the
> resources that exist in the single-channel group to the single channel.
> When computing the DPTSR value, the driver will (or at least should)
> therefore always come up with 0x00000000. Writing that to the register
> should be a no-op.
> 
> It's not clear if the register is present or not when the group has a
> single channel. Some datasheets document the register is not being
> applicable. Writing to it has never caused issues, so we may be dealing
> with the hardware just ignoring writes to a non-implemented register, or
> the register may be there, with only 0x00000000 being a meaningful
> value. This being said, some people are concerned about writes to
> registers that are not documented as present, as they could possibly
> cause issues. Safety certification of the driver could be impacted.
> We've updated the DU driver over the past few years to avoid those
> writes for this reason.
> 
> TL;DR: yes, the DU driver writes to DPTSR for DU groups with a single
> channel, but that seem it could be wrong, and we could fix it for all
> single-channel groups in one go without introducing this feature bit. I
> can test a patch on a M3 board that has a single channel in the second
> group.

Do you have docs for r8a77970? Is the register there?

Do you want me to change the series to use the number of channels here, 
or shall we go with the current version and change it later if we're 
confident that the change works?

  Tomi

>> Most of the SoCs seem to have two channels, but r8a77970 has one.
>> However, I don't have docs for that one. It could be that it does not
>> have DPTSR register, and indeed we could use the num_crtcs > 1 check there.
>>
>>>>    #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
>>>>    
>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
>>>> index 2ccd2581f544..132d930670eb 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
>>>> @@ -107,10 +107,12 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
>>>>    		 */
>>>>    		rcrtc = rcdu->crtcs;
>>>>    		num_crtcs = rcdu->num_crtcs;
>>>> -	} else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
>>>> +	} else if ((rcdu->info->gen == 3 && rgrp->num_crtcs > 1) ||
>>>> +		   rcdu->info->gen == 4) {
>>>>    		/*
>>>>    		 * On Gen3 dot clocks are setup through per-group registers,
>>>>    		 * only available when the group has two channels.
>>>> +		 * On Gen4 the registers are there for single channel too.
>>>>    		 */
>>>>    		rcrtc = &rcdu->crtcs[rgrp->index * 2];
>>>>    		num_crtcs = rgrp->num_crtcs;
>>>> @@ -185,11 +187,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>>>    		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
>>>>    	rcar_du_group_write(rgrp, DORCR, dorcr);
>>>>    
>>>> -	/* Apply planes to CRTCs association. */
>>>> -	mutex_lock(&rgrp->lock);
>>>> -	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
>>>> -			    rgrp->dptsr_planes);
>>>> -	mutex_unlock(&rgrp->lock);
>>>> +	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_NO_DPTSR)) {
>>>> +		/* Apply planes to CRTCs association. */
>>>> +		mutex_lock(&rgrp->lock);
>>>> +		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
>>>> +				    rgrp->dptsr_planes);
>>>> +		mutex_unlock(&rgrp->lock);
>>>> +	}
>>>>    }
>>>>    
>>>>    /*
> 

Re: [PATCH 6/9] drm/rcar-du: Add support for r8a779h0

[Laurent Pinchart]

On Thu, Dec 05, 2024 at 07:41:09AM +0200, Tomi Valkeinen wrote:
> On 03/12/2024 12:48, Laurent Pinchart wrote:
> > On Tue, Dec 03, 2024 at 11:22:15AM +0200, Tomi Valkeinen wrote:
> >> On 03/12/2024 10:56, Laurent Pinchart wrote:
> >>> On Tue, Dec 03, 2024 at 10:01:40AM +0200, Tomi Valkeinen wrote:
> >>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>>>
> >>>> Add support for r8a779h0. It is very similar to r8a779g0, but has only
> >>>> one output.
> >>>>
> >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>>> ---
> >>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 19 +++++++++++++++++++
> >>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h   |  1 +
> >>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 16 ++++++++++------
> >>>>    3 files changed, 30 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> >>>> index fb719d9aff10..afbc74e18cce 100644
> >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> >>>> @@ -545,6 +545,24 @@ static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
> >>>>    	.dsi_clk_mask =  BIT(1) | BIT(0),
> >>>>    };
> >>>>    
> >>>> +static const struct rcar_du_device_info rcar_du_r8a779h0_info = {
> >>>> +	.gen = 4,
> >>>> +	.features = RCAR_DU_FEATURE_CRTC_IRQ
> >>>> +		  | RCAR_DU_FEATURE_VSP1_SOURCE
> >>>> +		  | RCAR_DU_FEATURE_NO_BLENDING
> >>>> +		  | RCAR_DU_FEATURE_NO_DPTSR,
> >>>> +	.channels_mask = BIT(0),
> >>>> +	.routes = {
> >>>> +		/* R8A779H0 has one MIPI DSI output. */
> >>>> +		[RCAR_DU_OUTPUT_DSI0] = {
> >>>> +			.possible_crtcs = BIT(0),
> >>>> +			.port = 0,
> >>>> +		},
> >>>> +	},
> >>>> +	.num_rpf = 5,
> >>>> +	.dsi_clk_mask = 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 },
> >>>> @@ -571,6 +589,7 @@ static const struct of_device_id rcar_du_of_table[] = {
> >>>>    	{ .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 },
> >>>> +	{ .compatible = "renesas,du-r8a779h0", .data = &rcar_du_r8a779h0_info },
> >>>>    	{ }
> >>>>    };
> >>>>    
> >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> >>>> index 5cfa2bb7ad93..d7004f76f735 100644
> >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.h
> >>>> @@ -32,6 +32,7 @@ struct rcar_du_device;
> >>>>    #define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
> >>>>    #define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
> >>>>    #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
> >>>> +#define RCAR_DU_FEATURE_NO_DPTSR	BIT(6)  /* V4M does not have DPTSR */
> >>>
> >>> Do we need a quirk ? At first glance it seems the DPTSR register is only
> >>> used for DU instances that have two channels, so a check on the number
> >>> of channels should be enough ?
> >>
> >> What do you mean with "DPTSR register is only used for DU instances that
> >> have two channels"? The upstream code sets it for all SoCs, doesn't it,
> >> without any checks?
> > 
> > DPTSR is one of those registers that controls features shared between
> > channels, in this specific case plane assignment to DU channels. The
> > default register value (i.e. all 0's) splits resources between the
> > channels. For DU groups with a single channel, there's no need for
> > resource assignment. Logically speaking, the all 0's register value as
> > documented in instances that have two channels would assign all the
> > resources that exist in the single-channel group to the single channel.
> > When computing the DPTSR value, the driver will (or at least should)
> > therefore always come up with 0x00000000. Writing that to the register
> > should be a no-op.
> > 
> > It's not clear if the register is present or not when the group has a
> > single channel. Some datasheets document the register is not being
> > applicable. Writing to it has never caused issues, so we may be dealing
> > with the hardware just ignoring writes to a non-implemented register, or
> > the register may be there, with only 0x00000000 being a meaningful
> > value. This being said, some people are concerned about writes to
> > registers that are not documented as present, as they could possibly
> > cause issues. Safety certification of the driver could be impacted.
> > We've updated the DU driver over the past few years to avoid those
> > writes for this reason.
> > 
> > TL;DR: yes, the DU driver writes to DPTSR for DU groups with a single
> > channel, but that seem it could be wrong, and we could fix it for all
> > single-channel groups in one go without introducing this feature bit. I
> > can test a patch on a M3 board that has a single channel in the second
> > group.
> 
> Do you have docs for r8a77970? Is the register there?

According to the Gen3 documentation the register isn't preent in V3M.

> Do you want me to change the series to use the number of channels here, 
> or shall we go with the current version and change it later if we're 
> confident that the change works?

The change is easy so I'd like to do it now. It should be split to a
separate patch. I'll test it on Gen3 hardware right away.

> >> Most of the SoCs seem to have two channels, but r8a77970 has one.
> >> However, I don't have docs for that one. It could be that it does not
> >> have DPTSR register, and indeed we could use the num_crtcs > 1 check there.
> >>
> >>>>    #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
> >>>>    
> >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> >>>> index 2ccd2581f544..132d930670eb 100644
> >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> >>>> @@ -107,10 +107,12 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp)
> >>>>    		 */
> >>>>    		rcrtc = rcdu->crtcs;
> >>>>    		num_crtcs = rcdu->num_crtcs;
> >>>> -	} else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
> >>>> +	} else if ((rcdu->info->gen == 3 && rgrp->num_crtcs > 1) ||
> >>>> +		   rcdu->info->gen == 4) {
> >>>>    		/*
> >>>>    		 * On Gen3 dot clocks are setup through per-group registers,
> >>>>    		 * only available when the group has two channels.
> >>>> +		 * On Gen4 the registers are there for single channel too.
> >>>>    		 */
> >>>>    		rcrtc = &rcdu->crtcs[rgrp->index * 2];
> >>>>    		num_crtcs = rgrp->num_crtcs;
> >>>> @@ -185,11 +187,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
> >>>>    		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> >>>>    	rcar_du_group_write(rgrp, DORCR, dorcr);
> >>>>    
> >>>> -	/* Apply planes to CRTCs association. */
> >>>> -	mutex_lock(&rgrp->lock);
> >>>> -	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> >>>> -			    rgrp->dptsr_planes);
> >>>> -	mutex_unlock(&rgrp->lock);
> >>>> +	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_NO_DPTSR)) {
> >>>> +		/* Apply planes to CRTCs association. */
> >>>> +		mutex_lock(&rgrp->lock);
> >>>> +		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> >>>> +				    rgrp->dptsr_planes);
> >>>> +		mutex_unlock(&rgrp->lock);
> >>>> +	}
> >>>>    }
> >>>>    
> >>>>    /*

-- 
Regards,

Laurent Pinchart

[PATCH 7/9] arm64: dts: renesas: gray-hawk-single: Fix indentation

[Tomi Valkeinen]

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

Fix the indent on the two regulators.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 .../boot/dts/renesas/r8a779h0-gray-hawk-single.dts | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
index 9a1917b87f61..057f959d67b3 100644
--- a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
+++ b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
@@ -127,21 +127,21 @@ memory@480000000 {
 	};
 
 	reg_1p8v: regulator-1p8v {
-			compatible = "regulator-fixed";
-			regulator-name = "fixed-1.8V";
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-boot-on;
-			regulator-always-on;
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-1.8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
 	};
 
 	reg_3p3v: regulator-3p3v {
-			compatible = "regulator-fixed";
-			regulator-name = "fixed-3.3V";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-boot-on;
-			regulator-always-on;
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
 	};
 
 	sound_mux: sound-mux {

-- 
2.43.0

Re: [PATCH 7/9] arm64: dts: renesas: gray-hawk-single: Fix indentation

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:41AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Fix the indent on the two regulators.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  .../boot/dts/renesas/r8a779h0-gray-hawk-single.dts | 24 +++++++++++-----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
> index 9a1917b87f61..057f959d67b3 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
> @@ -127,21 +127,21 @@ memory@480000000 {
>  	};
>  
>  	reg_1p8v: regulator-1p8v {
> -			compatible = "regulator-fixed";
> -			regulator-name = "fixed-1.8V";
> -			regulator-min-microvolt = <1800000>;
> -			regulator-max-microvolt = <1800000>;
> -			regulator-boot-on;
> -			regulator-always-on;
> +		compatible = "regulator-fixed";
> +		regulator-name = "fixed-1.8V";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-boot-on;
> +		regulator-always-on;
>  	};
>  
>  	reg_3p3v: regulator-3p3v {
> -			compatible = "regulator-fixed";
> -			regulator-name = "fixed-3.3V";
> -			regulator-min-microvolt = <3300000>;
> -			regulator-max-microvolt = <3300000>;
> -			regulator-boot-on;
> -			regulator-always-on;
> +		compatible = "regulator-fixed";
> +		regulator-name = "fixed-3.3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;
>  	};
>  
>  	sound_mux: sound-mux {

-- 
Regards,

Laurent Pinchart

[PATCH 8/9] arm64: dts: renesas: r8a779h0: Add display support

[Tomi Valkeinen]

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

Add the device nodes for supporting DU and DSI.

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

diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
index 12d8be3fd579..82df6ee98afb 100644
--- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
@@ -1828,6 +1828,54 @@ csi41isp1: endpoint {
 			};
 		};
 
+		fcpvd0: fcp@fea10000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea10000 0 0x200>;
+			clocks = <&cpg CPG_MOD 508>;
+			power-domains = <&sysc R8A779H0_PD_C4>;
+			resets = <&cpg 508>;
+		};
+
+		vspd0: vsp@fea20000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea20000 0 0x8000>;
+			interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 830>;
+			power-domains = <&sysc R8A779H0_PD_C4>;
+			resets = <&cpg 830>;
+			renesas,fcp = <&fcpvd0>;
+		};
+
+		du: display@feb00000 {
+			compatible = "renesas,du-r8a779h0";
+			reg = <0 0xfeb00000 0 0x40000>;
+			interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 411>;
+			clock-names = "du.0";
+			power-domains = <&sysc R8A779H0_PD_C4>;
+			resets = <&cpg 411>;
+			reset-names = "du.0";
+			renesas,vsps = <&vspd0 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>;
+				};
+			};
+		};
+
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779h0-isp",
 				     "renesas,rcar-gen4-isp";
@@ -1996,6 +2044,35 @@ isp1vin15: endpoint {
 			};
 		};
 
+		dsi0: dsi-encoder@fed80000 {
+			compatible = "renesas,r8a779h0-dsi-csi2-tx";
+			reg = <0 0xfed80000 0 0x10000>;
+			clocks = <&cpg CPG_MOD 415>,
+				 <&cpg CPG_CORE R8A779H0_CLK_DSIEXT>,
+				 <&cpg CPG_CORE R8A779H0_CLK_DSIREF>;
+			clock-names = "fck", "dsi", "pll";
+			power-domains = <&sysc R8A779H0_PD_C4>;
+			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>;
+				};
+			};
+		};
+
 		prr: chipid@fff00044 {
 			compatible = "renesas,prr";
 			reg = <0 0xfff00044 0 4>;

-- 
2.43.0

Re: [PATCH 8/9] arm64: dts: renesas: r8a779h0: Add display support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:42AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add the device nodes for supporting DU and DSI.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 77 +++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> index 12d8be3fd579..82df6ee98afb 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> @@ -1828,6 +1828,54 @@ csi41isp1: endpoint {
>  			};
>  		};
>  
> +		fcpvd0: fcp@fea10000 {
> +			compatible = "renesas,fcpv";
> +			reg = <0 0xfea10000 0 0x200>;
> +			clocks = <&cpg CPG_MOD 508>;
> +			power-domains = <&sysc R8A779H0_PD_C4>;
> +			resets = <&cpg 508>;
> +		};
> +
> +		vspd0: vsp@fea20000 {
> +			compatible = "renesas,vsp2";
> +			reg = <0 0xfea20000 0 0x8000>;
> +			interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;

The documentation lists this interrupt as being LevelSensitive and
Negative. I wouldn't expect the VSP to work at all with a wrong polarity
in DT, so the level may get inverted somewhere.

> +			clocks = <&cpg CPG_MOD 830>;
> +			power-domains = <&sysc R8A779H0_PD_C4>;
> +			resets = <&cpg 830>;
> +			renesas,fcp = <&fcpvd0>;
> +		};
> +
> +		du: display@feb00000 {
> +			compatible = "renesas,du-r8a779h0";
> +			reg = <0 0xfeb00000 0 0x40000>;
> +			interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 411>;
> +			clock-names = "du.0";
> +			power-domains = <&sysc R8A779H0_PD_C4>;
> +			resets = <&cpg 411>;
> +			reset-names = "du.0";
> +			renesas,vsps = <&vspd0 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>;
> +				};

On V4M the DU has a single channel, so there should be a single port.

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

> +			};
> +		};
> +
>  		isp0: isp@fed00000 {
>  			compatible = "renesas,r8a779h0-isp",
>  				     "renesas,rcar-gen4-isp";
> @@ -1996,6 +2044,35 @@ isp1vin15: endpoint {
>  			};
>  		};
>  
> +		dsi0: dsi-encoder@fed80000 {
> +			compatible = "renesas,r8a779h0-dsi-csi2-tx";
> +			reg = <0 0xfed80000 0 0x10000>;
> +			clocks = <&cpg CPG_MOD 415>,
> +				 <&cpg CPG_CORE R8A779H0_CLK_DSIEXT>,
> +				 <&cpg CPG_CORE R8A779H0_CLK_DSIREF>;
> +			clock-names = "fck", "dsi", "pll";
> +			power-domains = <&sysc R8A779H0_PD_C4>;
> +			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>;
> +				};
> +			};
> +		};
> +
>  		prr: chipid@fff00044 {
>  			compatible = "renesas,prr";
>  			reg = <0 0xfff00044 0 4>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 8/9] arm64: dts: renesas: r8a779h0: Add display support

[Tomi Valkeinen]

Hi,

On 03/12/2024 11:37, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tue, Dec 03, 2024 at 10:01:42AM +0200, Tomi Valkeinen wrote:
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> Add the device nodes for supporting DU and DSI.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 77 +++++++++++++++++++++++++++++++
>>   1 file changed, 77 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
>> index 12d8be3fd579..82df6ee98afb 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
>> @@ -1828,6 +1828,54 @@ csi41isp1: endpoint {
>>   			};
>>   		};
>>   
>> +		fcpvd0: fcp@fea10000 {
>> +			compatible = "renesas,fcpv";
>> +			reg = <0 0xfea10000 0 0x200>;
>> +			clocks = <&cpg CPG_MOD 508>;
>> +			power-domains = <&sysc R8A779H0_PD_C4>;
>> +			resets = <&cpg 508>;
>> +		};
>> +
>> +		vspd0: vsp@fea20000 {
>> +			compatible = "renesas,vsp2";
>> +			reg = <0 0xfea20000 0 0x8000>;
>> +			interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
> 
> The documentation lists this interrupt as being LevelSensitive and
> Negative. I wouldn't expect the VSP to work at all with a wrong polarity
> in DT, so the level may get inverted somewhere.

Indeed... It's the same for V4H. And it also has IRQ_TYPE_LEVEL_HIGH in 
the dts. I tried changing it to LOW on V4H, but:

genirq: Setting trigger mode 8 for irq 91 failed
vsp1 fea20000.vsp: failed to request IRQ

I didn't dig further yet.

>> +			clocks = <&cpg CPG_MOD 830>;
>> +			power-domains = <&sysc R8A779H0_PD_C4>;
>> +			resets = <&cpg 830>;
>> +			renesas,fcp = <&fcpvd0>;
>> +		};
>> +
>> +		du: display@feb00000 {
>> +			compatible = "renesas,du-r8a779h0";
>> +			reg = <0 0xfeb00000 0 0x40000>;
>> +			interrupts = <GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cpg CPG_MOD 411>;
>> +			clock-names = "du.0";
>> +			power-domains = <&sysc R8A779H0_PD_C4>;
>> +			resets = <&cpg 411>;
>> +			reset-names = "du.0";
>> +			renesas,vsps = <&vspd0 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>;
>> +				};
> 
> On V4M the DU has a single channel, so there should be a single port.

Yep, I'll drop.

  Tomi

> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
>> +			};
>> +		};
>> +
>>   		isp0: isp@fed00000 {
>>   			compatible = "renesas,r8a779h0-isp",
>>   				     "renesas,rcar-gen4-isp";
>> @@ -1996,6 +2044,35 @@ isp1vin15: endpoint {
>>   			};
>>   		};
>>   
>> +		dsi0: dsi-encoder@fed80000 {
>> +			compatible = "renesas,r8a779h0-dsi-csi2-tx";
>> +			reg = <0 0xfed80000 0 0x10000>;
>> +			clocks = <&cpg CPG_MOD 415>,
>> +				 <&cpg CPG_CORE R8A779H0_CLK_DSIEXT>,
>> +				 <&cpg CPG_CORE R8A779H0_CLK_DSIREF>;
>> +			clock-names = "fck", "dsi", "pll";
>> +			power-domains = <&sysc R8A779H0_PD_C4>;
>> +			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>;
>> +				};
>> +			};
>> +		};
>> +
>>   		prr: chipid@fff00044 {
>>   			compatible = "renesas,prr";
>>   			reg = <0 0xfff00044 0 4>;
> 

Re: [PATCH 8/9] arm64: dts: renesas: r8a779h0: Add display support

[Geert Uytterhoeven]

Hi Tomi,

On Wed, Dec 4, 2024 at 5:04 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
> On 03/12/2024 11:37, Laurent Pinchart wrote:
> > On Tue, Dec 03, 2024 at 10:01:42AM +0200, Tomi Valkeinen wrote:
> >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>
> >> Add the device nodes for supporting DU and DSI.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >> ---
> >>   arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 77 +++++++++++++++++++++++++++++++
> >>   1 file changed, 77 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> >> index 12d8be3fd579..82df6ee98afb 100644
> >> --- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> >> +++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> >> @@ -1828,6 +1828,54 @@ csi41isp1: endpoint {
> >>                      };
> >>              };
> >>
> >> +            fcpvd0: fcp@fea10000 {
> >> +                    compatible = "renesas,fcpv";
> >> +                    reg = <0 0xfea10000 0 0x200>;
> >> +                    clocks = <&cpg CPG_MOD 508>;
> >> +                    power-domains = <&sysc R8A779H0_PD_C4>;
> >> +                    resets = <&cpg 508>;
> >> +            };
> >> +
> >> +            vspd0: vsp@fea20000 {
> >> +                    compatible = "renesas,vsp2";
> >> +                    reg = <0 0xfea20000 0 0x8000>;
> >> +                    interrupts = <GIC_SPI 546 IRQ_TYPE_LEVEL_HIGH>;
> >
> > The documentation lists this interrupt as being LevelSensitive and
> > Negative. I wouldn't expect the VSP to work at all with a wrong polarity
> > in DT, so the level may get inverted somewhere.
>
> Indeed... It's the same for V4H. And it also has IRQ_TYPE_LEVEL_HIGH in
> the dts. I tried changing it to LOW on V4H, but:
>
> genirq: Setting trigger mode 8 for irq 91 failed
> vsp1 fea20000.vsp: failed to request IRQ
>
> I didn't dig further yet.

Yeah, I don't think the GIC supports anything but IRQ_TYPE_LEVEL_HIGH.
Which brings us to the two ISP nodes on R-Car V4H and V4M, both using
IRQ_TYPE_LEVEL_LOW.
Niklas: looks like drivers/media/platform/renesas/rcar-isp.c doesn't
actually use the IRQ, so I guess that's how this could slip in?

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

[PATCH 9/9] arm64: dts: renesas: gray-hawk-single: Add DisplayPort support

[Tomi Valkeinen]

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

Add support for the mini DP output on the Gray Hawk board.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 .../boot/dts/renesas/r8a779h0-gray-hawk-single.dts | 95 ++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
index 057f959d67b3..7cdf07b6dde6 100644
--- a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
+++ b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
@@ -59,6 +59,12 @@ chosen {
 		stdout-path = "serial0:921600n8";
 	};
 
+	sn65dsi86_refclk: clk-x6 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <38400000>;
+	};
+
 	keys {
 		compatible = "gpio-keys";
 
@@ -126,6 +132,27 @@ memory@480000000 {
 		reg = <0x4 0x80000000 0x1 0x80000000>;
 	};
 
+	mini-dp-con {
+		compatible = "dp-connector";
+		label = "CN5";
+		type = "mini";
+
+		port {
+			mini_dp_con_in: endpoint {
+				remote-endpoint = <&sn65dsi86_out0>;
+			};
+		};
+	};
+
+	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";
@@ -200,6 +227,24 @@ channel1 {
 	};
 };
 
+&dsi0 {
+	status = "okay";
+
+	ports {
+		port@1 {
+			reg = <1>;
+			dsi0_out: endpoint {
+				remote-endpoint = <&sn65dsi86_in0>;
+				data-lanes = <1 2 3 4>;
+			};
+		};
+	};
+};
+
+&du {
+	status = "okay";
+};
+
 &extal_clk {
 	clock-frequency = <16666666>;
 };
@@ -269,6 +314,51 @@ eeprom@53 {
 	};
 };
 
+&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_in0: endpoint {
+					remote-endpoint = <&dsi0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				sn65dsi86_out0: endpoint {
+					remote-endpoint = <&mini_dp_con_in>;
+				};
+			};
+		};
+	};
+};
+
 &i2c3 {
 	pinctrl-0 = <&i2c3_pins>;
 	pinctrl-names = "default";
@@ -361,6 +451,11 @@ i2c0_pins: i2c0 {
 		function = "i2c0";
 	};
 
+	i2c1_pins: i2c1 {
+		groups = "i2c1";
+		function = "i2c1";
+	};
+
 	i2c3_pins: i2c3 {
 		groups = "i2c3";
 		function = "i2c3";

-- 
2.43.0

Re: [PATCH 9/9] arm64: dts: renesas: gray-hawk-single: Add DisplayPort support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:43AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Add support for the mini DP output on the Gray Hawk board.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Assuming this has passed the DT checks,

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

> ---
>  .../boot/dts/renesas/r8a779h0-gray-hawk-single.dts | 95 ++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
> index 057f959d67b3..7cdf07b6dde6 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a779h0-gray-hawk-single.dts
> @@ -59,6 +59,12 @@ chosen {
>  		stdout-path = "serial0:921600n8";
>  	};
>  
> +	sn65dsi86_refclk: clk-x6 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <38400000>;
> +	};
> +
>  	keys {
>  		compatible = "gpio-keys";
>  
> @@ -126,6 +132,27 @@ memory@480000000 {
>  		reg = <0x4 0x80000000 0x1 0x80000000>;
>  	};
>  
> +	mini-dp-con {
> +		compatible = "dp-connector";
> +		label = "CN5";
> +		type = "mini";
> +
> +		port {
> +			mini_dp_con_in: endpoint {
> +				remote-endpoint = <&sn65dsi86_out0>;
> +			};
> +		};
> +	};
> +
> +	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";
> @@ -200,6 +227,24 @@ channel1 {
>  	};
>  };
>  
> +&dsi0 {
> +	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			reg = <1>;
> +			dsi0_out: endpoint {
> +				remote-endpoint = <&sn65dsi86_in0>;
> +				data-lanes = <1 2 3 4>;
> +			};
> +		};
> +	};
> +};
> +
> +&du {
> +	status = "okay";
> +};
> +
>  &extal_clk {
>  	clock-frequency = <16666666>;
>  };
> @@ -269,6 +314,51 @@ eeprom@53 {
>  	};
>  };
>  
> +&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_in0: endpoint {
> +					remote-endpoint = <&dsi0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				sn65dsi86_out0: endpoint {
> +					remote-endpoint = <&mini_dp_con_in>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
>  &i2c3 {
>  	pinctrl-0 = <&i2c3_pins>;
>  	pinctrl-names = "default";
> @@ -361,6 +451,11 @@ i2c0_pins: i2c0 {
>  		function = "i2c0";
>  	};
>  
> +	i2c1_pins: i2c1 {
> +		groups = "i2c1";
> +		function = "i2c1";
> +	};
> +
>  	i2c3_pins: i2c3 {
>  		groups = "i2c3";
>  		function = "i2c3";

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/9] drm: Add DSI/DP support for Renesas r8a779h0 V4M and grey-hawk board

[Geert Uytterhoeven]

Hi Tomi,

On Tue, Dec 3, 2024 at 9:02 AM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
> Add everything needed to support the DSI output on Renesas r8a779h0
> (V4M) SoC, and the DP output (via sn65dsi86 DSI to DP bridge) on the
> Renesas grey-hawk board.
>
> Overall the DSI and the board design is almost identical to Renesas
> r8a779g0 and white-hawk board.

Thanks for your series!

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Woot, SoB tags for cover letters ;-)

Works fine up to 2560x1440 (I don't have a 4K display).

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

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 0/9] drm: Add DSI/DP support for Renesas r8a779h0 V4M and grey-hawk board

[Tomi Valkeinen]

On 04/12/2024 10:00, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Tue, Dec 3, 2024 at 9:02 AM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>> Add everything needed to support the DSI output on Renesas r8a779h0
>> (V4M) SoC, and the DP output (via sn65dsi86 DSI to DP bridge) on the
>> Renesas grey-hawk board.
>>
>> Overall the DSI and the board design is almost identical to Renesas
>> r8a779g0 and white-hawk board.
> 
> Thanks for your series!
> 
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Woot, SoB tags for cover letters ;-)

b4 seems to add those by default...

> Works fine up to 2560x1440 (I don't have a 4K display).
> 
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

  Tomi