Hi Hugo,

> -----Original Message-----
> From: dri-devel <[email protected]> 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 <[email protected]> wrote:
> 
> > Hi Hugo,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: dri-devel <[email protected]> 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 <[email protected]> wrote:
> > >
> > > > From: Biju Das <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > >
> > > 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.

Currently this is optional reset, and it is no-op for RZ/V2H.

What do you think? 

Patch #1) 1 us-> 1ms  
Patch #2) Introduce SoC specific callback to handle reset_deassert followed by 
delay
Patch #3) Move the code as per this patch  

Or

Patch #1) 1 us-> 1ms
Patch #2) Use this patch as it is, with updated commit description
So that fixes tag can be applied

Cheers,
Biju

Reply via email to