Re: [PATCH v8 1/9] dt-bindings: display: renesas,rzg2l-du: Document RZ/G3E SoC

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Geert,

On Wed, Sep 02, 2026 at 04:20:51PM +0200, Geert Uytterhoeven wrote:
> Hi all,
> 
> On Wed, 2 Sept 2026 at 16:03, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > On Wed, Sep 02, 2026 at 08:37:06AM +0200, Krzysztof Kozlowski wrote:
> > > On Fri, Aug 28, 2026 at 02:21:04PM +0200, Tommaso Merciai wrote:
> > > > The RZ/G3E Soc has 2 LCD controller (LCDC), contain a Frame Compression
> > > > Processor (FCPVD), a Video Signal Processor (VSPD), Video Signal
> > > > Processor (VSPD), and Display Unit (DU).
> > > >
> > > >  - LCDC0 supports DSI and LVDS (single or dual-channel) outputs.
> > > >  - LCDC1 supports DSI, LVDS (single-channel), and RGB outputs.
> > > >
> > > > Add new SoC-specific compatible string 'renesas,r9a09g047-du'.
> > > >
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> > > > --- a/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/renesas,rzg2l-du.yaml
> > > > @@ -21,6 +21,7 @@ properties:
> > > >            - renesas,r9a07g043u-du # RZ/G2UL
> > > >            - renesas,r9a07g044-du # RZ/G2{L,LC}
> > > >            - renesas,r9a08g046-du # RZ/G3L
> > > > +          - renesas,r9a09g047-du # RZ/G3E
> > > >            - renesas,r9a09g057-du # RZ/V2H(P)
> > > >            - renesas,r9a09g077-du # RZ/T2H
> > > >        - items:
> > > > @@ -35,25 +36,51 @@ properties:
> > > >            - const: renesas,r9a09g077-du # RZ/T2H fallback
> > > >
> > > >    reg:
> > > > -    maxItems: 1
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: du.0
> > > > +      - const: du.1
> > >
> > > du is the name of the device, thus calling items "0" and "1" is pretty
> > > pointless - indices already define that. Please drop the reg-names.
> >
> > Will drop this in v8.
> >
> > > >    interrupts:
> > > > -    maxItems: 1
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +
> > > > +  interrupt-names:
> > > > +    items:
> > > > +      - const: du.0
> > > > +      - const: du.1
> > >
> > > Same here
> >
> > Same, thanks.
> >
> > >
> > > >
> > > >    clocks:
> > > > +    minItems: 3
> > > >      items:
> > > >        - description: Main clock
> > > >        - description: Register access clock
> > > >        - description: Video clock
> > > > +      - description: Main clock for DU1
> > > > +      - description: Register access clock for DU1
> > > > +      - description: Video clock for DU1
> > > >
> > > >    clock-names:
> > > > +    minItems: 3
> > > >      items:
> > > >        - const: aclk
> > > >        - const: pclk
> > > >        - const: vclk
> > > > +      - const: aclk1
> > > > +      - const: pclk1
> > > > +      - const: vclk1
> > > >
> > > >    resets:
> > > > -    maxItems: 1
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +
> > > > +  reset-names:
> > > > +    items:
> > > > +      - const: resetn
> > > > +      - const: resetn1
> > >
> > > Drop reset-names
> >
> > For reset-names, I got the the following comment from Philipp in v7 [1].
> >
> > Dropping reset-names would force the driver back to an index-based
> > lookup, which is what that comment explicitly asked me to avoid.
> >
> > [1] https://lore.kernel.org/all/8382e2b9fd07fb1132c26e228b3899336fc1fdd4.camel@pengutronix.de/
> >
> > Philipp, Krzysztof, could you agree on which way you'd prefer?
> > I'll follow whatever you decide.
> 
> Until we get a variant with a third interrupt (or reset or reg),
> which is not related to the number of channels...

Just to make sure I follow: are you referring to variants like R-Car H3
and M3-N, where the position in the array no longer matches the channel
index? From renesas,du.yaml:

[renesas,du-r8a7795: 4 channels, two resets] [1]
  reset-names:
    items:
      - const: du.0
      - const: du.2

[renesas,du-r8a77965: 3 channels, du.2 (channel 2) does not exist] [2]
  clock-names:
    items:
      - const: du.0
      - const: du.1
      - const: du.3
  reset-names:
    items:
      - const: du.0
      - const: du.3

If so, I agree: the names are what makes these unambiguous, and an
index-based lookup could not express them.

Then on RZ/G3E we can keep:

reg-names:
    items:
      - const: du.0
      - const: du.1

interrupt-names:
    items:
      - const: du.0
      - const: du.1

Maybe make sense also for reset-names having:

reset-names:
    items:
      - const: du.0
      - const: du.1

?

Then we can add struct rzg2l_du_channel_res:

struct rzg2l_du_channel_res {
	const char *reg;
	const char *aclk;
	const char *pclk;
	const char *vclk;
	const char *rst;
};

Into struct rzg2l_du_device_info in this way we can cover future cases
du-r8a77965, du-r8a7795 -like using:

static const struct rzg2l_du_channel_res rzg2l_du_r8a7795_channels_res[] = {
	[0] = { "du.0", "aclk",  "pclk",  "vclk",  "du.0" },
	[1] = { "du.1", "aclk1", "pclk1", "vclk1", "du.0" },
	[2] = { "du.2", "aclk2", "pclk2", "vclk2", "du.2" },
	[3] = { "du.3", "aclk3", "pclk3", "vclk3", "du.2" },
};

static const struct rzg2l_du_device_info rzg2l_du_r8a7795_info = {
	.channels_mask = BIT(0) | BIT(1) | BIT(2) | BIT(3),
	.channels_res = rzg2l_du_r8a7795_channels_res,
	.num_channels_res = ARRAY_SIZE(rzg2l_du_r8a7795_channels_res),
	.routes = {
		[RZG2L_DU_OUTPUT_DPAD0] = {
			.possible_crtcs = BIT(0),
			.port = 0,
		},
		[RZG2L_DU_OUTPUT_DSI0] = {
			.possible_crtcs = BIT(1) | BIT(2),
			.port = 1,
		},
		[RZG2L_DU_OUTPUT_LVDS0] = {
			.possible_crtcs = BIT(3),
			.port = 3,
		},
	},
	.mode_clock_min = 5440,
	.mode_clock_max = 87000,
};

/* Channels 0, 1 and 3: channel 2 does not exist, so [2] is a hole. */
static const struct rzg2l_du_channel_res rzg2l_du_r8a77965_channels_res[] = {
	[0] = { "du.0", "aclk",  "pclk",  "vclk",  "du.0" },
	[1] = { "du.1", "aclk1", "pclk1", "vclk1", "du.0" },
	[3] = { "du.3", "aclk3", "pclk3", "vclk3", "du.3" },
};

static const struct rzg2l_du_device_info rzg2l_du_r8a77965_info = {
	.channels_mask = BIT(0) | BIT(1) | BIT(3),
	.channels_res = rzg2l_du_r8a77965_channels_res,
	.num_channels_res = ARRAY_SIZE(rzg2l_du_r8a77965_channels_res),
	.routes = {
		/*
		 * possible_crtcs is a software channel index: hardware channel
		 * 3 is the third populated one, hence BIT(2) and not BIT(3).
		 */
		[RZG2L_DU_OUTPUT_DPAD0] = {
			.possible_crtcs = BIT(0),
			.port = 0,
		},
		[RZG2L_DU_OUTPUT_DSI0] = {
			.possible_crtcs = BIT(1),
			.port = 1,
		},
		[RZG2L_DU_OUTPUT_LVDS0] = {
			.possible_crtcs = BIT(2),
			.port = 2,
		},
	},
	.mode_clock_min = 5440,
	.mode_clock_max = 87000,
};

And for RZ/G3E we will have:

static const struct rzg2l_du_channel_res rzg2l_du_r9a09g047_channels_res[] = {
	[0] = { "du.0", "aclk",  "pclk",  "vclk",  "du.0" },
	[1] = { "du.1", "aclk1", "pclk1", "vclk1", "du.1" },
};

static const struct rzg2l_du_device_info rzg2l_du_r9a09g047_info = {
	.channels_mask = BIT(0) | BIT(1),
	.channels_res = rzg2l_du_r9a09g047_channels_res,
	.num_channels_res = ARRAY_SIZE(rzg2l_du_r9a09g047_channels_res),
	.routes = {
		[RZG2L_DU_OUTPUT_DSI0] = {
			.possible_crtcs = BIT(0) | BIT(1),
			.port = 0,
		},
		[RZG2L_DU_OUTPUT_DPAD0] = {
			.possible_crtcs = BIT(1),
			.port = 1,
		},
		[RZG2L_DU_OUTPUT_LVDS0] = {
			.possible_crtcs = BIT(0),
			.port = 2,
		},
		[RZG2L_DU_OUTPUT_LVDS1] = {
			.possible_crtcs = BIT(0) | BIT(1),
			.port = 3,
		},
	},
	.mode_clock_min = 5400,
	.mode_clock_max = 87000,
};

What do you think?

[1] https://elixir.bootlin.com/linux/v7.2/source/arch/arm64/boot/dts/renesas/r8a77951.dtsi#L3363
[2] https://elixir.bootlin.com/linux/v7.2/source/arch/arm64/boot/dts/renesas/r8a77965.dtsi#L2819

Kind regards,
Tommaso

> 
> 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