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.

Cheers,
Biju

> 
> 
> > +           fsleep(1000);
> >  }
> >
> >  static void rzg2l_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
> >                                      struct drm_atomic_state *state)  {
> >     struct rzg2l_mipi_dsi *dsi = bridge_to_rzg2l_mipi_dsi(bridge);
> > +   const struct drm_display_mode *mode;
> > +   struct drm_connector *connector;
> > +   struct drm_crtc *crtc;
> >     int ret;
> >
> >     ret = rzg2l_mipi_dsi_start_hs_clock(dsi);
> >     if (ret < 0)
> >             goto err_stop;
> >
> > +   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;
> > +
> > +   rzg2l_mipi_dsi_set_display_timing(dsi, mode);
> > +
> >     ret = rzg2l_mipi_dsi_start_video(dsi);
> >     if (ret < 0)
> >             goto err_stop_clock;
> > @@ -1074,6 +1078,7 @@ static void
> > rzg2l_mipi_dsi_atomic_post_disable(struct drm_bridge *bridge,  {
> >     struct rzg2l_mipi_dsi *dsi = bridge_to_rzg2l_mipi_dsi(bridge);
> >
> > +   reset_control_assert(dsi->rstc);
> >     rzg2l_mipi_dsi_stop(dsi);
> >  }
> >
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Hugo Villeneuve <[email protected]>

Reply via email to