RE: [PATCH v5 14/20] drm: renesas: rz-du: Add RZ/G3E support

[Biju Das]

Hi Tommaso,

> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> Sent: 11 June 2026 15:49
> Subject: Re: [PATCH v5 14/20] drm: renesas: rz-du: Add RZ/G3E support
> 
> Hi Laurent,
> Thanks for your review.
> 
> 
> On Thu, Jun 11, 2026 at 12:22:34AM +0300, Laurent Pinchart wrote:
> > Hi Tommaso,
> >
> > On Wed, Mar 18, 2026 at 03:45:54PM +0100, Tommaso Merciai wrote:
> > > On Fri, Feb 13, 2026 at 05:27:40PM +0100, 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.
> > > >
> > > > Depending on the selected output, the correct SMUX2 clock parent
> > > > must be chosen based on the requested duty cycle:
> > > >
> > > >  - Index 0 for LVDS -> CDIV7_DSIx_CLK (DUTY H/L=4/3, 4/7 duty
> > > > cycle)
> > > >  - Index 1 for DSI/DPAD -> CSDIV_2to16_PLLDSIx (symmetric 50% duty
> > > > cycle)
> > > >
> > > > To support this behavior, introduce the
> > > > `RZG2L_DU_FEATURE_SMUX2_DSI_CLK` feature flag and extend the
> > > > `rzg2l_du_device_info` structure to include a features field.
> > > > Also, add a new helper function `rzg2l_du_has()` to check for feature flags.
> > > >
> > > > Add support for the RZ/G3E SoC by introducing:
> > > >  - `rzg2l_du_r9a09g047_du_info` structure
> > > >  - The `renesas,r9a09g047-du` compatible string
> > > >
> > > > Additionally, introduce the missing output definitions
> > > > `RZG2L_DU_OUTPUT_LVDS{0,1}`.
> > > >
> > > > Introduce `rzg2l_du_crtc_atomic_check()` helper to store the
> > > > routes from the CRTC output to the DU outputs.
> > > >
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> > > > ---
> > > > v4->v5:
> > > >  - Fixed RG2L_DU_FEATURE_SMUX2_DSI_CLK to RZG2L_DU_FEATURE_SMUX2_DSI_CLK,
> > > >    update commit body accordingly.
> > > >  - Added features field documentation.
> > > >
> > > > v3->v4:
> > > >  - No changes.
> > > >
> > > > v2->v3:
> > > >  - No changes.
> > > >
> > > > v1->v2:
> > > >  - Instead of using clk-provider API to select the right parent clock,
> > > >    based on the outputs. Just set the correct duty cycle based on the
> > > >    output, this reflects at CPG lvl to select the right parent.
> > > >  - Updated commit message accordingly.
> > > >
> > > >  drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 48
> > > > +++++++++++++++++++  drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c
> > > > | 26 ++++++++++  drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h  |
> > > > 12 +++++
> > > >  3 files changed, 86 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > > > index 6e7aac6219be..cc35dd409e3e 100644
> > > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > > > @@ -64,11 +64,32 @@
> > > >  static void rzg2l_du_crtc_set_display_timing(struct rzg2l_du_crtc
> > > > *rcrtc)  {
> > > >  	const struct drm_display_mode *mode =
> > > > &rcrtc->crtc.state->adjusted_mode;
> > > > +	struct rzg2l_du_crtc_state *rstate =
> > > > +		to_rzg2l_crtc_state(rcrtc->crtc.state);
> > > >  	unsigned long mode_clock = mode->clock * 1000;
> > > >  	u32 ditr0, ditr1, ditr2, ditr3, ditr4, pbcr0;
> > > >  	struct rzg2l_du_device *rcdu = rcrtc->dev;
> > > >
> > > >  	clk_prepare_enable(rcrtc->rzg2l_clocks.dclk);
> > > > +
> > > > +	if (rzg2l_du_has(rcdu, RZG2L_DU_FEATURE_SMUX2_DSI_CLK)) {
> > > > +		struct clk *clk_parent;
> > > > +
> > > > +		clk_parent = clk_get_parent(rcrtc->rzg2l_clocks.dclk);
> > > > +
> > > > +		/*
> > > > +		 * Request appropriate duty cycle to let clock driver select
> > > > +		 * the correct parent:
> > > > +		 * - CDIV7_DSIx_CLK (LVDS path) has DUTY H/L=4/3, 4/7 duty cycle.
> > > > +		 * - CSDIV_2to16_PLLDSIx (DSI/RGB path) has symmetric 50% duty cycle.
> > > > +		 */
> > > > +		if (rstate->outputs == BIT(RZG2L_DU_OUTPUT_LVDS0) ||
> > > > +		    rstate->outputs == BIT(RZG2L_DU_OUTPUT_LVDS1))
> > > > +			clk_set_duty_cycle(clk_parent, 4, 7);
> > > > +		else
> > > > +			clk_set_duty_cycle(clk_parent, 1, 2);
> > > > +	}
> > > > +
> > >
> > > I’d appreciate any feedback/suggestions regarding this.
> > > Thank you in advance for your time.
> >
> > Sorry for the very late reply.
> 
> No worries, thanks for your time.
> 
> >
> > I've taken time to analyse the clock tree, and I think the way you
> > model it makes sense. As the SMUX2_DSI[01]_CLK clocks are used by the
> > LCD, LVDS and DSI blocks, I may have selected the duty cycle in the
> > LVDS and DSI drivers personally. I wonder if it would lead to simpler
> > code (you wouldn't need to implement rzg2l_du_crtc_atomic_check())
> > here for instance. In any case, it does not affect the DT bindings, so
> > it could be changed later too.
> 
> Agreed, we can change this later.

I believe you don't need to use the feature flag 
RZG2L_DU_FEATURE_SMUX2_DSI_CLK and rzg2l_du_crtc_atomic_check()
as Laurent suggested, If you move the clk_set_duty_cycle() to
LVDS bridge's attach() to make sure it calls before the
rzg2l_du_crtc_set_display_timing(), so that PLL generates right
dot clock.

Default is 50% duty cycle(for DSI and RGB) and once LVDS driver
calls clk_set_duty_cycle() it changes to 4/7 duty cycle.

Cheers,
Biju