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

[PATCH v4 0/7] 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.

Note: the v4 no longer has the dts and the clk patches, as those have
been merged to renesas-devel.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
Changes in v4:
- Dropped patches merged to renesas-devel
- Added new patch "dt-bindings: display: renesas,du: Add missing
  maxItems" to fix the bindings
- Add the missing maxItems to "dt-bindings: display: renesas,du: Add
  r8a779h0"
- Link to v3: https://lore.kernel.org/r/20241206-rcar-gh-dsi-v3-0-d74c2166fa15@ideasonboard.com

Changes in v3:
- Update "Write DPTSR only if there are more than one crtc" patch to
  "Write DPTSR only if the second source exists"
- Add Laurent's Rb
- Link to v2: https://lore.kernel.org/r/20241205-rcar-gh-dsi-v2-0-42471851df86@ideasonboard.com

Changes in v2:
- Add the DT binding with a new conditional block, so that we can set
  only the port@0 as required
- Drop port@1 from r8a779h0.dtsi (there's no port@1)
- Add a new patch to write DPTSR only if num_crtcs > 1
- Drop RCAR_DU_FEATURE_NO_DPTSR (not needed anymore)
- Add Cc: stable to the fix, and move it as first patch
- Added the tags from reviews
- Link to v1: https://lore.kernel.org/r/20241203-rcar-gh-dsi-v1-0-738ae1a95d2a@ideasonboard.com

---
Tomi Valkeinen (7):
      drm/rcar-du: dsi: Fix PHY lock bit check
      drm/rcar-du: Write DPTSR only if the second source exists
      dt-bindings: display: renesas,du: Add missing maxItems
      dt-bindings: display: renesas,du: Add r8a779h0
      dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779h0
      drm/rcar-du: dsi: Add r8a779h0 support
      drm/rcar-du: Add support for r8a779h0

 .../display/bridge/renesas,dsi-csi2-tx.yaml        |  1 +
 .../devicetree/bindings/display/renesas,du.yaml    | 63 ++++++++++++++++++++--
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c      | 18 +++++++
 drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c    | 24 ++++++---
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c    |  4 +-
 .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h   |  1 -
 6 files changed, 99 insertions(+), 12 deletions(-)
---
base-commit: adc218676eef25575469234709c2d87185ca223a
change-id: 20241008-rcar-gh-dsi-9c01f5deeac8

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

[PATCH v4 1/7] 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")
Cc: stable@vger.kernel.org
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 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

[PATCH v4 2/7] drm/rcar-du: Write DPTSR only if the second source exists

[Tomi Valkeinen]

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

Currently the driver always writes DPTSR when setting up the hardware.
However, writing the register is only meaningful when the second source
for a plane is used, and the register is not even documented for SoCs
that do not have the second source.

So move the write behind a condition.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> # On R-Car M3-N
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

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..1ec806c8e013 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
@@ -185,11 +185,21 @@ 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);
+	/*
+	 * DPTSR is used to select the source for the planes of a group. The
+	 * first source is chosen by writing 0 to the respective bits, and this
+	 * is always the default value of the register. In other words, writing
+	 * DPTSR is only needed if the SoC supports choosing the second source.
+	 *
+	 * The SoCs documentations seems to confirm this, as the DPTSR register
+	 * is not documented if only the first source exists on that SoC.
+	 */
+	if (rgrp->channels_mask & BIT(1)) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
 }
 
 /*

-- 
2.43.0

[PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Tomi Valkeinen]

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

The binding is missing maxItems for all renesas,cmms and renesas,vsps
properties. As the amount of cmms or vsps is always a fixed amount, set
the maxItems to match the minItems.

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

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index c5b9e6812bce..e5fbc4ffe29c 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -489,9 +489,11 @@ allOf:
 
         renesas,cmms:
           minItems: 4
+          maxItems: 4
 
         renesas,vsps:
           minItems: 4
+          maxItems: 4
 
       required:
         - clock-names
@@ -558,9 +560,11 @@ allOf:
 
         renesas,cmms:
           minItems: 3
+          maxItems: 3
 
         renesas,vsps:
           minItems: 3
+          maxItems: 3
 
       required:
         - clock-names
@@ -627,9 +631,11 @@ allOf:
 
         renesas,cmms:
           minItems: 3
+          maxItems: 3
 
         renesas,vsps:
           minItems: 3
+          maxItems: 3
 
       required:
         - clock-names
@@ -684,6 +690,7 @@ allOf:
 
         renesas,vsps:
           minItems: 1
+          maxItems: 1
 
       required:
         - clock-names
@@ -746,9 +753,11 @@ allOf:
 
         renesas,cmms:
           minItems: 2
+          maxItems: 2
 
         renesas,vsps:
           minItems: 2
+          maxItems: 2
 
       required:
         - clock-names
@@ -799,6 +808,7 @@ allOf:
 
         renesas,vsps:
           minItems: 2
+          maxItems: 2
 
       required:
         - clock-names

-- 
2.43.0

[PATCH v4 4/7] 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.

Note that we remove the requirement for two ports from the global part
of the bindings, as each conditional part defines the number of required
ports already. This came up with r8a779h0 as it's the first one that has
only one port.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../devicetree/bindings/display/renesas,du.yaml    | 53 ++++++++++++++++++++--
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index e5fbc4ffe29c..1b4cd8fac825 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
@@ -69,10 +70,6 @@ properties:
         $ref: /schemas/graph.yaml#/properties/port
         unevaluatedProperties: false
 
-    required:
-      - port@0
-      - port@1
-
     unevaluatedProperties: false
 
   renesas,cmms:
@@ -817,6 +814,54 @@ allOf:
         - reset-names
         - renesas,vsps
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,du-r8a779h0
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Functional clock
+
+        clock-names:
+          items:
+            - const: du.0
+
+        interrupts:
+          maxItems: 1
+
+        resets:
+          maxItems: 1
+
+        reset-names:
+          items:
+            - const: du.0
+
+        ports:
+          properties:
+            port@0:
+              description: DSI 0
+            port@1: false
+            port@2: false
+            port@3: false
+
+          required:
+            - port@0
+
+        renesas,vsps:
+          minItems: 1
+          maxItems: 1
+
+      required:
+        - clock-names
+        - interrupts
+        - resets
+        - reset-names
+        - renesas,vsps
+
 additionalProperties: false
 
 examples:

-- 
2.43.0

[PATCH v4 5/7] 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../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

[PATCH v4 6/7] 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 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

[PATCH v4 7/7] 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>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c   | 18 ++++++++++++++++++
 drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c |  4 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

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..7858e10839f2 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,23 @@ 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,
+	.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 +588,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_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
index 1ec806c8e013..068c106e586c 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;

-- 
2.43.0

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> The binding is missing maxItems for all renesas,cmms and renesas,vsps
> properties. As the amount of cmms or vsps is always a fixed amount, set
> the maxItems to match the minItems.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index c5b9e6812bce..e5fbc4ffe29c 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -489,9 +489,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 4
> +          maxItems: 4
>  
>          renesas,vsps:
>            minItems: 4
> +          maxItems: 4
>  
>        required:
>          - clock-names
> @@ -558,9 +560,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 3
> +          maxItems: 3
>  
>          renesas,vsps:
>            minItems: 3
> +          maxItems: 3
>  
>        required:
>          - clock-names
> @@ -627,9 +631,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 3
> +          maxItems: 3
>  
>          renesas,vsps:
>            minItems: 3
> +          maxItems: 3
>  
>        required:
>          - clock-names
> @@ -684,6 +690,7 @@ allOf:
>  
>          renesas,vsps:
>            minItems: 1
> +          maxItems: 1
>  
>        required:
>          - clock-names
> @@ -746,9 +753,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 2
> +          maxItems: 2
>  
>          renesas,vsps:
>            minItems: 2
> +          maxItems: 2
>  
>        required:
>          - clock-names
> @@ -799,6 +808,7 @@ allOf:
>  
>          renesas,vsps:
>            minItems: 2
> +          maxItems: 2
>  
>        required:
>          - clock-names

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Krzysztof Kozlowski]

On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> The binding is missing maxItems for all renesas,cmms and renesas,vsps
> properties. As the amount of cmms or vsps is always a fixed amount, set
> the maxItems to match the minItems.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)

The top level property should define widest constraints as well.

Best regards,
Krzysztof

Re: [PATCH v4 4/7] dt-bindings: display: renesas,du: Add r8a779h0

[Krzysztof Kozlowski]

On Fri, Dec 13, 2024 at 04:03:00PM +0200, Tomi Valkeinen wrote:
>  
> -    required:
> -      - port@0
> -      - port@1
> -
>      unevaluatedProperties: false
>  
>    renesas,cmms:
> @@ -817,6 +814,54 @@ allOf:
>          - reset-names
>          - renesas,vsps
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - renesas,du-r8a779h0
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Functional clock
> +
> +        clock-names:
> +          items:
> +            - const: du.0
> +
> +        interrupts:
> +          maxItems: 1
> +
> +        resets:

No minItems here...

> +          maxItems: 1
> +
> +        reset-names:
> +          items:
> +            - const: du.0
> +
> +        ports:
> +          properties:
> +            port@0:
> +              description: DSI 0
> +            port@1: false
> +            port@2: false
> +            port@3: false
> +
> +          required:
> +            - port@0
> +
> +        renesas,vsps:
> +          minItems: 1

so drop minItems here as well.

Best regards,
Krzysztof

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Laurent Pinchart]

Hi Krzysztof,

On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
> > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > 
> > The binding is missing maxItems for all renesas,cmms and renesas,vsps
> > properties. As the amount of cmms or vsps is always a fixed amount, set
> > the maxItems to match the minItems.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> 
> The top level property should define widest constraints as well.

I'm curious, why is that ? I understand why a top-level default would
make sense when it's optionally overridden by model-specific values, but
in this case there's no such default. Every SoC has its own fixed value.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Krzysztof Kozlowski]

On 16/12/2024 09:32, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
>> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>
>>> The binding is missing maxItems for all renesas,cmms and renesas,vsps
>>> properties. As the amount of cmms or vsps is always a fixed amount, set
>>> the maxItems to match the minItems.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> ---
>>>  Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>
>> The top level property should define widest constraints as well.
> 
> I'm curious, why is that ? I understand why a top-level default would
> make sense when it's optionally overridden by model-specific values, but
> in this case there's no such default. Every SoC has its own fixed value.

Because otherwise top level property does not have proper description
and we expect properties to be defined at top-level.

Best regards,
Krzysztof

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Tomi Valkeinen]

Hi,

On 16/12/2024 12:42, Krzysztof Kozlowski wrote:
> On 16/12/2024 09:32, Laurent Pinchart wrote:
>> Hi Krzysztof,
>>
>> On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
>>> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
>>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>>
>>>> The binding is missing maxItems for all renesas,cmms and renesas,vsps
>>>> properties. As the amount of cmms or vsps is always a fixed amount, set
>>>> the maxItems to match the minItems.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
>>>>   1 file changed, 10 insertions(+)
>>>
>>> The top level property should define widest constraints as well.
>>
>> I'm curious, why is that ? I understand why a top-level default would
>> make sense when it's optionally overridden by model-specific values, but
>> in this case there's no such default. Every SoC has its own fixed value.
> 
> Because otherwise top level property does not have proper description
> and we expect properties to be defined at top-level.

As we don't know what is the maximum number of items for future SoCs, 
should we then use a number that'll surely be big enough? At the moment 
the max cmms seems to be 4, so maybe 16 would be safely big enough. But 
is it then better to be extra safe, and use, say, maxItems 256?

  Tomi

Re: [PATCH v4 4/7] dt-bindings: display: renesas,du: Add r8a779h0

[Tomi Valkeinen]

Hi,

On 16/12/2024 10:00, Krzysztof Kozlowski wrote:
> On Fri, Dec 13, 2024 at 04:03:00PM +0200, Tomi Valkeinen wrote:
>>   
>> -    required:
>> -      - port@0
>> -      - port@1
>> -
>>       unevaluatedProperties: false
>>   
>>     renesas,cmms:
>> @@ -817,6 +814,54 @@ allOf:
>>           - reset-names
>>           - renesas,vsps
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - renesas,du-r8a779h0
>> +    then:
>> +      properties:
>> +        clocks:
>> +          items:
>> +            - description: Functional clock
>> +
>> +        clock-names:
>> +          items:
>> +            - const: du.0
>> +
>> +        interrupts:
>> +          maxItems: 1
>> +
>> +        resets:
> 
> No minItems here...
> 
>> +          maxItems: 1
>> +
>> +        reset-names:
>> +          items:
>> +            - const: du.0
>> +
>> +        ports:
>> +          properties:
>> +            port@0:
>> +              description: DSI 0
>> +            port@1: false
>> +            port@2: false
>> +            port@3: false
>> +
>> +          required:
>> +            - port@0
>> +
>> +        renesas,vsps:
>> +          minItems: 1
> 
> so drop minItems here as well.

Ok. I wanted to be consistent with the other vsps entries in the file, 
so I added both min and max items. But I can drop it.

  Tomi

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Laurent Pinchart]

On Mon, Dec 16, 2024 at 11:42:30AM +0100, Krzysztof Kozlowski wrote:
> On 16/12/2024 09:32, Laurent Pinchart wrote:
> > On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
> >>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>>
> >>> The binding is missing maxItems for all renesas,cmms and renesas,vsps
> >>> properties. As the amount of cmms or vsps is always a fixed amount, set
> >>> the maxItems to match the minItems.
> >>>
> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>
> >> The top level property should define widest constraints as well.
> > 
> > I'm curious, why is that ? I understand why a top-level default would
> > make sense when it's optionally overridden by model-specific values, but
> > in this case there's no such default. Every SoC has its own fixed value.
> 
> Because otherwise top level property does not have proper description
> and we expect properties to be defined at top-level.

Is it invalid YAML schema to have

  renesas,cmms:
    description: ......

with the min/maxItems in the conditional blocks ? Or did you mean, by
proper description, not just the description field ? We could have

  renesas,cmms:
    description: ......
    minItems: 1
    maxItems: 256

but I really don't see what that would bring from a documentation point
of view. Are there tools that depend on minItems and maxItems being
present at the top level ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 4/7] dt-bindings: display: renesas,du: Add r8a779h0

[Laurent Pinchart]

On Mon, Dec 16, 2024 at 12:56:09PM +0200, Tomi Valkeinen wrote:
> On 16/12/2024 10:00, Krzysztof Kozlowski wrote:
> > On Fri, Dec 13, 2024 at 04:03:00PM +0200, Tomi Valkeinen wrote:
> >>   
> >> -    required:
> >> -      - port@0
> >> -      - port@1
> >> -
> >>       unevaluatedProperties: false
> >>   
> >>     renesas,cmms:
> >> @@ -817,6 +814,54 @@ allOf:
> >>           - reset-names
> >>           - renesas,vsps
> >>   
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            enum:
> >> +              - renesas,du-r8a779h0
> >> +    then:
> >> +      properties:
> >> +        clocks:
> >> +          items:
> >> +            - description: Functional clock
> >> +
> >> +        clock-names:
> >> +          items:
> >> +            - const: du.0
> >> +
> >> +        interrupts:
> >> +          maxItems: 1
> >> +
> >> +        resets:
> > 
> > No minItems here...
> > 
> >> +          maxItems: 1
> >> +
> >> +        reset-names:
> >> +          items:
> >> +            - const: du.0
> >> +
> >> +        ports:
> >> +          properties:
> >> +            port@0:
> >> +              description: DSI 0
> >> +            port@1: false
> >> +            port@2: false
> >> +            port@3: false
> >> +
> >> +          required:
> >> +            - port@0
> >> +
> >> +        renesas,vsps:
> >> +          minItems: 1
> > 
> > so drop minItems here as well.
> 
> Ok. I wanted to be consistent with the other vsps entries in the file, 
> so I added both min and max items. But I can drop it.

I'd favour consistency with the other vsps entries, but not enough to
fight over it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Tomi Valkeinen]

Hi,

On 16/12/2024 10:32, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
>> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>
>>> The binding is missing maxItems for all renesas,cmms and renesas,vsps
>>> properties. As the amount of cmms or vsps is always a fixed amount, set
>>> the maxItems to match the minItems.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> ---
>>>   Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>
>> The top level property should define widest constraints as well.
> 
> I'm curious, why is that ? I understand why a top-level default would
> make sense when it's optionally overridden by model-specific values, but
> in this case there's no such default. Every SoC has its own fixed value.

Looking at the file, shouldn't we have minItems == maxItems for 
interrupts and resets too? Well, I guess for interrupts we could in 
theory run with just some of the interrupt lines connected. I'm not sure 
if that's the case for resets.

  Tomi

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Laurent Pinchart]

On Mon, Dec 16, 2024 at 01:02:26PM +0200, Tomi Valkeinen wrote:
> On 16/12/2024 10:32, Laurent Pinchart wrote:
> > On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
> >>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>>
> >>> The binding is missing maxItems for all renesas,cmms and renesas,vsps
> >>> properties. As the amount of cmms or vsps is always a fixed amount, set
> >>> the maxItems to match the minItems.
> >>>
> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
> >>>   1 file changed, 10 insertions(+)
> >>
> >> The top level property should define widest constraints as well.
> > 
> > I'm curious, why is that ? I understand why a top-level default would
> > make sense when it's optionally overridden by model-specific values, but
> > in this case there's no such default. Every SoC has its own fixed value.
> 
> Looking at the file, shouldn't we have minItems == maxItems for 
> interrupts and resets too? Well, I guess for interrupts we could in 
> theory run with just some of the interrupt lines connected. I'm not sure 
> if that's the case for resets.

Unless there's some magic handling of min/maxItems for those that I
wouldn't be aware of, I think it makes sense.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 3/7] dt-bindings: display: renesas,du: Add missing maxItems

[Krzysztof Kozlowski]

On 16/12/2024 11:54, Tomi Valkeinen wrote:
> Hi,
> 
> On 16/12/2024 12:42, Krzysztof Kozlowski wrote:
>> On 16/12/2024 09:32, Laurent Pinchart wrote:
>>> Hi Krzysztof,
>>>
>>> On Mon, Dec 16, 2024 at 08:58:49AM +0100, Krzysztof Kozlowski wrote:
>>>> On Fri, Dec 13, 2024 at 04:02:59PM +0200, Tomi Valkeinen wrote:
>>>>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>>>
>>>>> The binding is missing maxItems for all renesas,cmms and renesas,vsps
>>>>> properties. As the amount of cmms or vsps is always a fixed amount, set
>>>>> the maxItems to match the minItems.
>>>>>
>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/display/renesas,du.yaml | 10 ++++++++++
>>>>>   1 file changed, 10 insertions(+)
>>>>
>>>> The top level property should define widest constraints as well.
>>>
>>> I'm curious, why is that ? I understand why a top-level default would
>>> make sense when it's optionally overridden by model-specific values, but
>>> in this case there's no such default. Every SoC has its own fixed value.
>>
>> Because otherwise top level property does not have proper description
>> and we expect properties to be defined at top-level.
> 
> As we don't know what is the maximum number of items for future SoCs, 
> should we then use a number that'll surely be big enough? At the moment 
> the max cmms seems to be 4, so maybe 16 would be safely big enough. But 
> is it then better to be extra safe, and use, say, maxItems 256?

No, look at all other bindings. Widest constraints for ONLY these
devices. We do not talk about future SoCs here.

Best regards,
Krzysztof