RE: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence

[Biju Das]

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Biju Das
> Sent: 17 March 2026 20:11
> To: Hugo Villeneuve <hugo@xxxxxxxxxxx>
> Cc: biju.das.au <biju.das.au@xxxxxxxxx>; Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime
> Ripard <mripard@xxxxxxxxxx>; Thomas Zimmermann <tzimmermann@xxxxxxx>; David Airlie
> <airlied@xxxxxxxxx>; Simona Vetter <simona@xxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>; dri-
> devel@xxxxxxxxxxxxxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Geert
> Uytterhoeven <geert+renesas@xxxxxxxxx>; Chris Brandt <Chris.Brandt@xxxxxxxxxxx>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> Subject: RE: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence
> 
> Hi Hugo,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Hugo Villeneuve
> > Sent: 17 March 2026 19:50
> > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the
> > power-on sequence
> >
> > Hi Biju,
> >
> > On Tue, 17 Mar 2026 19:37:35 +0000
> > Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> >
> > > Hi Hugo,
> > >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On
> > > > Behalf Of Hugo Villeneuve
> > > > Sent: 17 March 2026 18:52
> > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the
> > > > power-on sequence
> > > >
> > > > Hi Biju,
> > > >
> > > > On Tue, 17 Mar 2026 16:36:05 +0000 Biju Das
> > > > <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > >
> > > > > Hi Hugo,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On
> > > > > > Behalf Of Hugo Villeneuve
> > > > > > Sent: 17 March 2026 16:13
> > > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the
> > > > > > power-on sequence
> > > > > >
> > > > > > Hi Biju,
> > > > > >
> > > > > > On Tue, 17 Mar 2026 15:45:29 +0000 Biju Das
> > > > > > <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > >
> > > > > > > Hi Hugo,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx>
> > > > > > > > On Behalf Of Hugo Villeneuve
> > > > > > > > Sent: 17 March 2026 15:21
> > > > > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix
> > > > > > > > the power-on sequence
> > > > > > > >
> > > > > > > > Hi Biju,
> > > > > > > >
> > > > > > > > On Tue, 17 Mar 2026 15:13:07 +0000 Biju Das
> > > > > > > > <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > > > >
> > > > > > > > > Hi Hugo,
> > > > > > > > >
> > > > > > > > > Thanks for the feedback.
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: dri-devel
> > > > > > > > > > <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > > > > > > > > > Hugo Villeneuve
> > > > > > > > > > Sent: 17 March 2026 15:01
> > > > > > > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi:
> > > > > > > > > > Fix the power-on sequence
> > > > > > > > > >
> > > > > > > > > > Hi Biju,
> > > > > > > > > >
> > > > > > > > > > On Tue, 17 Mar 2026 12:36:01 +0000 Biju
> > > > > > > > > > <biju.das.au@xxxxxxxxx>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > > > > > > >
> > > > > > > > > > > Move reset_control_deassert() and
> > > > > > > > > > > reset_control_assert() from
> > > > > > > > > > > rzg2l_mipi_dsi_dphy_init()/rzg2l_mipi_dsi_dphy_exit(
> > > > > > > > > > > )
> > > > > > > > > > > to
> > > > > > > > > > > atomic_pre_enable() and atomic_post_disable()
> > > > > > > > > > > respectively, and move
> > > > > > > > > > > rzg2l_mipi_dsi_set_display_timing() from
> > > > > > > > > > > atomic_pre_enable() to atomic_enable(), to align
> > > > > > > > > > > with the power-on sequence described in Figure 34.5
> > > > > > > > > > > of section
> > > > > > > > > > > "34.4.2.1 Reset" of the RZ/G2L hardware manual
> > > > > > > > > > > Rev.1.50 May 2025.
> > > > > > > > > > >
> > > > > > > > > > > According to the hardware manual, LINK registers
> > > > > > > > > > > must be written before deasserting CMN_RSTB, and the
> > > > > > > > > > > 1ms delay is retained in
> > > > > > > > > > > atomic_pre_enable() after the deassert.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > > > > > >
> > > > > > > > > > Seems to me like this should be backported to stable
> > > > > > > > > > branches (missing Fixes / Cc: stable
> > > > > > tags)?
> > > > > > > > >
> > > > > > > > > OK, will add fixes/stable tags.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > ---
> > > > > > > > > > >  .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c    | 27 +++++++++++--------
> > > > > > > > > > >  1 file changed, 16 insertions(+), 11 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git
> > > > > > > > > > > a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > > > > > > > > > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > > > > > > > > > > index e53b48e4de56..9053ce037b75 100644
> > > > > > > > > > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > > > > > > > > > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > > > > > > > > > > @@ -484,7 +484,6 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi
> *dsi,
> > > > > > > > > > >  	u32 dphytim1;
> > > > > > > > > > >  	u32 dphytim2;
> > > > > > > > > > >  	u32 dphytim3;
> > > > > > > > > > > -	int ret;
> > > > > > > > > > >
> > > > > > > > > > >  	/* All DSI global operation timings are set with recommended setting */
> > > > > > > > > > >  	for (i = 0; i <
> > > > > > > > > > > ARRAY_SIZE(rzg2l_mipi_dsi_global_timings);
> > > > > > > > > > > ++i) { @@
> > > > > > > > > > > -524,12 +523,6 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
> > > > > > > > > > >  	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM2, dphytim2);
> > > > > > > > > > >  	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM3,
> > > > > > > > > > > dphytim3);
> > > > > > > > > > >
> > > > > > > > > > > -	ret = reset_control_deassert(dsi->rstc);
> > > > > > > > > > > -	if (ret < 0)
> > > > > > > > > > > -		return ret;
> > > > > > > > > > > -
> > > > > > > > > > > -	fsleep(1000);
> > > > > > > > > > > -
> > > > > > > > > > >  	return 0;
> > > > > > > > > > >  }
> > > > > > > > > > >
> > > > > > > > > > > @@ -541,8 +534,6 @@ static void
> > > > > > > > > > > rzg2l_mipi_dsi_dphy_exit(struct rzg2l_mipi_dsi *dsi)
> > > > > > > > > > >
> > > > > > > > > > >  	dphyctrl0 &= ~(DSIDPHYCTRL0_EN_LDO1200 | DSIDPHYCTRL0_EN_BGR);
> > > > > > > > > > >  	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYCTRL0,
> > > > > > > > > > > dphyctrl0);
> > > > > > > > > > > -
> > > > > > > > > > > -	reset_control_assert(dsi->rstc);
> > > > > > > > > > >  }
> > > > > > > > > > >
> > > > > > > > > > >  static int rzg2l_dphy_conf_clks(struct
> > > > > > > > > > > rzg2l_mipi_dsi *dsi, unsigned long mode_freq, @@
> > > > > > > > > > > -1030,24 +1021,37 @@ static void
> > > > > > > > > > > rzg2l_mipi_dsi_atomic_pre_enable(struct
> > > > > > > > > > drm_bridge *bridge,
> > > > > > > > > > >  	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> > > > > > > > > > >  	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> > > > > > > > > > >  	mode = &drm_atomic_get_new_crtc_state(state,
> > > > > > > > > > > crtc)->adjusted_mode;
> > > > > > > > > > > -
> > > > > > > > > >
> > > > > > > > > > This is not related to your commit message (coding style change).
> > > > > > > > >
> > > > > > > > > Ack. Will restore it.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >  	ret = rzg2l_mipi_dsi_startup(dsi, mode);
> > > > > > > > > > >  	if (ret < 0)
> > > > > > > > > > >  		return;
> > > > > > > > > > >
> > > > > > > > > > > -	rzg2l_mipi_dsi_set_display_timing(dsi, mode);
> > > > > > > > > > > +	ret = reset_control_deassert(dsi->rstc);
> > > > > > > > > > > +	if (ret < 0)
> > > > > > > > > > > +		return;
> > > > > > > > > > > +
> > > > > > > > > > > +	if (dsi->rstc)
> > > > > > > > > >
> > > > > > > > > > This seems new and not documented in the commit message? Is this a fix?
> > > > > > > > >
> > > > > > > > > RZ/V2H does not need this as it uses different IP.
> > > > > > > > > Previously
> > > > > > > > > fsleep() is in RZ/G2L specific function. I will update
> > > > > > > > > commit description for this
> > change.
> > > > > > > >
> > > > > > > > Suggestion: maybe move this to a separate patch, to facilitate review/understanding...
> > > > > > >
> > > > > > > The only way is to introduce a new callback to handle it for RZ/G2L SoC.
> > > > > > > Then we won't be able to apply fixes tag as it is not fixing anything.
> > > > > >
> > > > > > I am not sure what you mean by that callback? How a callback
> > > > > > is needed only if you split the
> > > > patch?
> > > > >
> > > > > You cannot split the patch.
> > > > >
> > > > > Before:
> > > > >   atomic_pre_enable():
> > > > >     startup()
> > > > >       dphy_init()
> > > > >         write DSIDPHYTIMx         (F) PHY timing regs
> > > > >         reset_control_deassert()  (G) deassert CMN_RSTB
> > > > >         udelay(1)                 (H)
> > > > > 	  setting below link registers
> > > > >         − TXSETR
> > > > > 	  − ULPSSETR
> > > > >         − DSISETR
> > > > >         − CLSTPTSETR
> > > > >         − LPTRNSTSETR
> > > > >
> > > > > Current patch:
> > > > >
> > > > > atomic_pre_enable():
> > > > >     startup()
> > > > >       dphy_init()
> > > > >         write DSIDPHYTIMx         (F) PHY timing regs
> > > > > 	setting below link registers
> > > > >         − TXSETR
> > > > > 	  − ULPSSETR
> > > > >         − DSISETR
> > > > >         − CLSTPTSETR
> > > > >         − LPTRNSTSETR
> > > > >
> > > > >       reset_control_deassert()  (G) deassert CMN_RSTB
> > > > >       fsleep(1000)              (H)
> > > > >
> > > > > >
> > > > > > In this original patch you test for the validity of dsi->rstc
> > > > > > to determine if you apply the delay or not. So in the case of
> > > > > > RZ/V2H, I understand that it is
> > NULL?
> > > > >
> > > > > Yes, that is correct.
> > > > >
> > > > > >
> > > > > > > Currently this is optional reset, and it is no-op for RZ/V2H.
> > > > > >
> > > > > > Does this means that the call to
> > > > > > reset_control_deassert(dsi->rstc) should not occur for
> > RZ/V2H?
> > > > >
> > > > > reset_control_deassert(dsi->rstc) will return immediately as it is null.
> > > > >
> > > > > or
> > > > >
> > > > > We could add this check instead
> > > > >
> > > > > 	if (dsi->rstc) {
> > > > > 	    ret = reset_control_deassert(dsi->rstc);
> > > > > 	    if (ret < 0)
> > > > > 		return;
> > > > >
> > > > > 	    fsleep(1000);
> > > > > 	}
> > > >
> > > > Yes, like Tommaso suggested.
> > > >
> > > > But I don't see why you cannot simply implement (split) this
> > > > change as a separate commit just after commit #1, or after commit #2?
> > > >
> > > > This seems like an optimization for RZ/V2H, so I think it doesnt
> > > > really matter if it does not go to stable branches?
> > >
> > > Previously RZ/V2H do not call reset_control_deassert(dsi->rstc) as
> > > it is called from SoC-specific function.
> >
> > Ok, so this change could be split, if you want, as commit #3. This
> > would make commit #2 easier to understand IMHO.
> 
> 
> atomic_pre_enable():
> 	startup()
> 		dphy_init() -> SoC specific
> 		write DSIDPHYTIMx         (F) PHY timing regs
> 
> The below calls are in common path. So, no way you can split.
> 	setting below link registers
> 		   − TXSETR
> 		   − ULPSSETR
> 		   − DSISETR
> 		   − CLSTPTSETR
> 		   − LPTRNSTSETR
> 	reset_control_deassert()  (G) deassert CMN_RSTB
> 	fsleep(1000)              (H)
> 
> Patch#1 1usec to 1 msec
> Patch#2 Move rzg2l_mipi_dsi_set_display_timing() to rzg2l_mipi_dsi_atomic_enable()
>         after starting hs clock.
> Patch#3 Move reset assert/deassert from rzg2l_mipi_dsi_dphy_{init, exit} to
>         rzg2l_mipi_dsi_atomic_pre_enable and rzg2l_mipi_dsi_atomic_post_disable,
>         with a guard for RZ/V2H by checking (dsi->rstc), as it is in the common path.
> 	  Like previous case assert/deassert won't be called.
> 
> Are you OK?

I will send next version like below:

Patch#1 1usec to 1 msec

Patch#2 Move rzg2l_mipi_dsi_set_display_timing() to rzg2l_mipi_dsi_atomic_enable()
         after starting hs clock.

Patch#3 Move reset_deassert from phy_init to rzg2l_mipi_dsi_atomic_pre_enable() with below change

	if (dsi->rstc) {
	    ret = reset_control_deassert(dsi->rstc);
	    if (ret < 0)
			return;
		fsleep(1000);
 	}

Patch#4 Move reset_assert from phy_exit to rzg2l_mipi_dsi_atomic_post_disable() to make
        the balanced operations.

Cheers,
Biju