[PATCH 0/2] Improvements on RZ/G2L MIPI DSI driver

[PATCH 0/2] Improvements on RZ/G2L MIPI DSI driver

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Hi All,

Enhance RZ/G2L MIPI DSI driver based on "Figure 34.5 Power on sequence",
on section "34.4.2.1 Reset" of the RZ/G2L hardware manual Rev.1.50 May,
2025.

As per the hardware manual, it is required to wait >= 1msec after
deasserting the CMN_RSTB signal, and writing to DSI PHY timing registers
and few LINK registers should be done before deasserting the CMN_RSTB.

Biju Das (2):
  drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init
  drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence

 .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c    | 27 +++++++++++--------
 1 file changed, 16 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Replace udelay(1) with fsleep(1000) in rzg2l_mipi_dsi_dphy_init() to
follow 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.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 a87a301326c7..e53b48e4de56 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
@@ -528,7 +528,7 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
 	if (ret < 0)
 		return ret;
 
-	udelay(1);
+	fsleep(1000);
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 12:36:00 +0000
Biju <biju.das.au@gmail.com> wrote:

> From: Biju Das <biju.das.jz@bp.renesas.com>

The commit message seems confusing to me, indicating that you only
changed fsleep, but did not change the delay? Maybe change it to
indicate that you increase the delay from 1us to 1ms, which is the real
reason of the change (why).

> 
> Replace udelay(1) with fsleep(1000) in rzg2l_mipi_dsi_dphy_init() to

Same here, the real reason of the change is to increase the delay, and
this is not obviously stated.


> follow 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.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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 a87a301326c7..e53b48e4de56 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> @@ -528,7 +528,7 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
>  	if (ret < 0)
>  		return ret;
>  
> -	udelay(1);
> +	fsleep(1000);
>  
>  	return 0;
>  }
> -- 
> 2.43.0
> 
> 


-- 
Hugo Villeneuve <hugo@hugovil.com>

RE: [PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init

[Biju Das]

Hi Hugo,

Thanks for the feedback.

> -----Original Message-----
> From: Hugo Villeneuve <hugo@hugovil.com>
> Sent: 17 March 2026 14:47
> Subject: Re: [PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init
> 
> Hi Biju,
> 
> On Tue, 17 Mar 2026 12:36:00 +0000
> Biju <biju.das.au@gmail.com> wrote:
> 
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> The commit message seems confusing to me, indicating that you only changed fsleep, but did not change
> the delay? Maybe change it to indicate that you increase the delay from 1us to 1ms, which is the real
> reason of the change (why).
> 
> >
> > Replace udelay(1) with fsleep(1000) in rzg2l_mipi_dsi_dphy_init() to
> 
> Same here, the real reason of the change is to increase the delay, and this is not obviously stated.

Agreed. I will update the commit message for increasing the delay from 1us to 1 ms to
match with the hardware manual.

Cheers,
Biju

Re: [PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 12:36:00 +0000
Biju <biju.das.au@gmail.com> wrote:

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Replace udelay(1) with fsleep(1000) in rzg2l_mipi_dsi_dphy_init() to
> follow 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.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Seems to me like this should be backported to stable branches (missing Fixes / Cc: stable tags)?


> ---
>  drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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 a87a301326c7..e53b48e4de56 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> @@ -528,7 +528,7 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
>  	if (ret < 0)
>  		return ret;
>  
> -	udelay(1);
> +	fsleep(1000);
>  
>  	return 0;
>  }
> -- 
> 2.43.0
> 
> 


-- 
Hugo Villeneuve <hugo@hugovil.com>

RE: [PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init

[Biju Das]

Hi Hugo,

> -----Original Message-----
> From: Hugo Villeneuve <hugo@hugovil.com>
> Sent: 17 March 2026 15:02
> Subject: Re: [PATCH 1/2] drm: renesas: rzg2l_mipi_dsi: Use fsleep() for 1ms delay in D-PHY init
> 
> Hi Biju,
> 
> On Tue, 17 Mar 2026 12:36:00 +0000
> Biju <biju.das.au@gmail.com> wrote:
> 
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Replace udelay(1) with fsleep(1000) in rzg2l_mipi_dsi_dphy_init() to
> > follow 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.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Seems to me like this should be backported to stable branches (missing Fixes / Cc: stable tags)?

Agreed, will add fixes/stable tags.

Cheers,
Biju

> 
> 
> > ---
> >  drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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 a87a301326c7..e53b48e4de56 100644
> > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > @@ -528,7 +528,7 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	udelay(1);
> > +	fsleep(1000);
> >
> >  	return 0;
> >  }
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Hugo Villeneuve <hugo@hugovil.com>

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

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

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@bp.renesas.com>
---
 .../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;
-
 	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)
+		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

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

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 12:36:01 +0000
Biju <biju.das.au@gmail.com> wrote:

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> 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@bp.renesas.com>

Seems to me like this should be backported to stable branches (missing Fixes / Cc: 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).


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


> +		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 <hugo@hugovil.com>

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

[Biju Das]

Hi Hugo,

Thanks for the feedback.

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com> wrote:
> 
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > 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@bp.renesas.com>
> 
> 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 <hugo@hugovil.com>

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

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 15:13:07 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Hugo,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com> wrote:
> > 
> > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > 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@bp.renesas.com>
> > 
> > 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...


> 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 <hugo@hugovil.com>
> 


-- 
Hugo Villeneuve <hugo@hugovil.com>

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

[Biju Das]

Hi Hugo,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> 
> > Hi Hugo,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com> wrote:
> > >
> > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > 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@bp.renesas.com>
> > >
> > > 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

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

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 15:45:29 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Hugo,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > 
> > > Hi Hugo,
> > >
> > > Thanks for the feedback.
> > >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com> wrote:
> > > >
> > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > >
> > > > > 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@bp.renesas.com>
> > > >
> > > > 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?

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?

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

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


-- 
Hugo Villeneuve <hugo@hugovil.com>

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

[Biju Das]

Hi Hugo,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> 
> > Hi Hugo,
> >
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > >
> > > > Hi Hugo,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > -----Original Message-----
> > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > >
> > > > > > 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@bp.renesas.com>
> > > > >
> > > > > 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);
	}

Cheers,
Biju

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

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 16:36:05 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Hugo,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > 
> > > Hi Hugo,
> > >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > >
> > > > > Hi Hugo,
> > > > >
> > > > > Thanks for the feedback.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > >
> > > > > > > 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@bp.renesas.com>
> > > > > >
> > > > > > 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?

Hugo Villeneuve <hugo@hugovil.com>

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

[Biju Das]

Hi Hugo,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> 
> > Hi Hugo,
> >
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > >
> > > > Hi Hugo,
> > > >
> > > > > -----Original Message-----
> > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > >
> > > > > > Hi Hugo,
> > > > > >
> > > > > > Thanks for the feedback.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > >
> > > > > > > > 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@bp.renesas.com>
> > > > > > >
> > > > > > > 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.

Cheers,
Biju

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

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 19:37:35 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Hugo,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > 
> > > Hi Hugo,
> > >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > >
> > > > > Hi Hugo,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > > >
> > > > > > > Hi Hugo,
> > > > > > >
> > > > > > > Thanks for the feedback.
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > >
> > > > > > > > > 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@bp.renesas.com>
> > > > > > > >
> > > > > > > > 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.

Thank you for the clarifications.

Hugo Villeneuve <hugo@hugovil.com>

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

[Biju Das]

Hi Hugo,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> 
> > Hi Hugo,
> >
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > >
> > > > Hi Hugo,
> > > >
> > > > > -----Original Message-----
> > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > >
> > > > > > Hi Hugo,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > > > >
> > > > > > > > Hi Hugo,
> > > > > > > >
> > > > > > > > Thanks for the feedback.
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: dri-devel
> > > > > > > > > <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > >
> > > > > > > > > > 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@bp.renesas.com>
> > > > > > > > >
> > > > > > > > > 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?     

Cheers,
Biju

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

[Biju Das]

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Biju Das
> Sent: 17 March 2026 20:11
> To: Hugo Villeneuve <hugo@hugovil.com>
> Cc: biju.das.au <biju.das.au@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime
> Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie
> <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Philipp Zabel <p.zabel@pengutronix.de>; dri-
> devel@lists.freedesktop.org; linux-renesas-soc@vger.kernel.org; linux-kernel@vger.kernel.org; Geert
> Uytterhoeven <geert+renesas@glider.be>; Chris Brandt <Chris.Brandt@renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 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@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> >
> > > Hi Hugo,
> > >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > >
> > > > > Hi Hugo,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > > >
> > > > > > > Hi Hugo,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > > 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@bp.renesas.com> wrote:
> > > > > > > >
> > > > > > > > > Hi Hugo,
> > > > > > > > >
> > > > > > > > > Thanks for the feedback.
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: dri-devel
> > > > > > > > > > <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > > >
> > > > > > > > > > > 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@bp.renesas.com>
> > > > > > > > > >
> > > > > > > > > > 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

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

[Hugo Villeneuve]

Hi Biju,

On Tue, 17 Mar 2026 20:10:31 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Hugo,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > 
> > > Hi Hugo,
> > >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > >
> > > > > Hi Hugo,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > > >
> > > > > > > Hi Hugo,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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@bp.renesas.com> wrote:
> > > > > > > >
> > > > > > > > > Hi Hugo,
> > > > > > > > >
> > > > > > > > > Thanks for the feedback.
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: dri-devel
> > > > > > > > > > <dri-devel-bounces@lists.freedesktop.org> 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@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > > > >
> > > > > > > > > > > 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@bp.renesas.com>
> > > > > > > > > >
> > > > > > > > > > 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)

I do understand now, with your previous explanations, why you add the
reset guard for RZ/V2H).

I do not understand why you say "no way you can split", because
my suggestion was merely to split the change in two commits instead of
a single one, and the final source code would be 100% identical...

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

My suggestion was only... a suggestion :) Feel free to do what you
think is best, as long as it is very well documented.

Hugo Villeneuve <hugo@hugovil.com>

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

[Tommaso Merciai]

Hi Biju,
Thanks for your patch.

On Tue, Mar 17, 2026 at 12:36:01PM +0000, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> 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@bp.renesas.com>
> ---
>  .../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;
> -
>  	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)
> +		fsleep(1000);

What about?

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

	    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);
> +

Manual/Patch says that LINK registers must be written before
deasserting CMN_RSTB:

  atomic_pre_enable():
	  startup()                  (F) PHY timing regs + LINK
	  set_display_timing()       (F) writing VICH1* (LINK regs)
	  reset_control_deassert()   (G)
	  fsleep(1000)               (H)

Before this series we have:

  atomic_pre_enable():
    startup()
      dphy_init()
        write DSIDPHYTIMx         (F) PHY timing regs
        reset_control_deassert()  (G) deassert CMN_RSTB
        udelay(1)                 (H)
    set_display_timing()          (F) writing VICH1* (LINK regs)  


Moving set_display_timing() here you are setting LINK regs after
reset_control_deassert() and the sequence will be:

 atomic_pre_enable():
	 startup()		  (F) PHY timing regs + LINK
	 reset_control_deassert() (G) CMN_RSTB deassert
	 fsleep(1000)             (H) wait 1ms

 atomic_enable():
	 start_hs_clock()
	 set_display_timing()     (F) writing VICH1* (LINK regs)
	 start_video()

I think to provide the right sequence we need to just move

	reset_control_deassert(dsi->rstc);

From rzg2l_mipi_dsi_dphy_init() to rzg2l_mipi_dsi_atomic_pre_enable()
just after rzg2l_mipi_dsi_set_display_timing() call.


>  	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_dsii_stop(dsi);

rzg2l_mipi_dsi_stop() is writing DSIDPHYCTRL0 reg via dphy_exit().
I think the right order should be:

	rzg2l_mipi_dsii_stop(dsi);
	reset_control_assert(dsi->rstc);

What do you think?


Kind Regards,
Tommaso

>  }
>  
> -- 
> 2.43.0
> 

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

[Biju Das]

Hi Tommaso Merciai,

> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Sent: 17 March 2026 16:05
> Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence
> 
> Hi Biju,
> Thanks for your patch.
> 
> On Tue, Mar 17, 2026 at 12:36:01PM +0000, Biju wrote:
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > 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@bp.renesas.com>
> > ---
> >  .../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;
> > -
> >  	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)
> > +		fsleep(1000);
> 
> What about?
> 
> 	if (dsi->rstc) {
> 	    ret = reset_control_deassert(dsi->rstc);
> 	    if (ret < 0)
> 		return;
> 
> 	    fsleep(1000);
> 	}

OK.

> 
> 
> >  }
> >
> >  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);
> > +
> 
> Manual/Patch says that LINK registers must be written before deasserting CMN_RSTB:

NOT ALL.

Only these link registers.

− TXSETR
− ULPSSETR
− DSISETR
− CLSTPTSETR
− LPTRNSTSETR

> 
>   atomic_pre_enable():
> 	  startup()                  (F) PHY timing regs + LINK
> 	  set_display_timing()       (F) writing VICH1* (LINK regs)

This is not F. This is after starting HS CLK.

> 	  reset_control_deassert()   (G)
> 	  fsleep(1000)               (H)
> 
> Before this series we have:
> 
>   atomic_pre_enable():
>     startup()
>       dphy_init()
>         write DSIDPHYTIMx         (F) PHY timing regs
>         reset_control_deassert()  (G) deassert CMN_RSTB
>         udelay(1)                 (H)
>     set_display_timing()          (F) writing VICH1* (LINK regs)

This is not F. This is after starting HS CLK.

> 
> 
> Moving set_display_timing() here you are setting LINK regs after
> reset_control_deassert() and the sequence will be:
> 
>  atomic_pre_enable():
> 	 startup()		  (F) PHY timing regs + LINK
> 	 reset_control_deassert() (G) CMN_RSTB deassert
> 	 fsleep(1000)             (H) wait 1ms
> 
>  atomic_enable():
> 	 start_hs_clock()
> 	 set_display_timing()     (F) writing VICH1* (LINK regs)

This is not F. It is after starting HSCLK and it is as per hardware manual.

> 	 start_video()
> 
> I think to provide the right sequence we need to just move
> 
> 	reset_control_deassert(dsi->rstc);
> 
> From rzg2l_mipi_dsi_dphy_init() to rzg2l_mipi_dsi_atomic_pre_enable()
> just after rzg2l_mipi_dsi_set_display_timing() call.
> 
> 
> >  	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_dsii_stop(dsi);
> 
> rzg2l_mipi_dsi_stop() is writing DSIDPHYCTRL0 reg via dphy_exit().
> I think the right order should be:
> 
> 	rzg2l_mipi_dsii_stop(dsi);
> 	reset_control_assert(dsi->rstc);

.atomic_pre_enable()

dsi_start()
reset_deassert()-->G

.atomic_post_disbale()

Just opposite of atomic_pre_enable()
reset_assert()-->G
dsi_stop()


Cheers,
Biju

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

[Tommaso Merciai]

Hi Biju,
Thanks for your comments.

On 3/17/26 17:16, Biju Das wrote:
> Hi Tommaso Merciai,
> 
>> -----Original Message-----
>> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
>> Sent: 17 March 2026 16:05
>> Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence
>>
>> Hi Biju,
>> Thanks for your patch.
>>
>> On Tue, Mar 17, 2026 at 12:36:01PM +0000, Biju wrote:
>>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>>
>>> 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@bp.renesas.com>
>>> ---
>>>   .../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;
>>> -
>>>   	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)
>>> +		fsleep(1000);
>>
>> What about?
>>
>> 	if (dsi->rstc) {
>> 	    ret = reset_control_deassert(dsi->rstc);
>> 	    if (ret < 0)
>> 		return;
>>
>> 	    fsleep(1000);
>> 	}
> 
> OK.
> 
>>
>>
>>>   }
>>>
>>>   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);
>>> +
>>
>> Manual/Patch says that LINK registers must be written before deasserting CMN_RSTB:
> 
> NOT ALL.
> 
> Only these link registers.
> 
> − TXSETR
> − ULPSSETR
> − DSISETR
> − CLSTPTSETR
> − LPTRNSTSETR
> 
>>
>>    atomic_pre_enable():
>> 	  startup()                  (F) PHY timing regs + LINK
>> 	  set_display_timing()       (F) writing VICH1* (LINK regs)
> 
> This is not F. This is after starting HS CLK.
> 
>> 	  reset_control_deassert()   (G)
>> 	  fsleep(1000)               (H)
>>
>> Before this series we have:
>>
>>    atomic_pre_enable():
>>      startup()
>>        dphy_init()
>>          write DSIDPHYTIMx         (F) PHY timing regs
>>          reset_control_deassert()  (G) deassert CMN_RSTB
>>          udelay(1)                 (H)
>>      set_display_timing()          (F) writing VICH1* (LINK regs)
> 
> This is not F. This is after starting HS CLK.
> 
>>
>>
>> Moving set_display_timing() here you are setting LINK regs after
>> reset_control_deassert() and the sequence will be:
>>
>>   atomic_pre_enable():
>> 	 startup()		  (F) PHY timing regs + LINK
>> 	 reset_control_deassert() (G) CMN_RSTB deassert
>> 	 fsleep(1000)             (H) wait 1ms
>>
>>   atomic_enable():
>> 	 start_hs_clock()
>> 	 set_display_timing()     (F) writing VICH1* (LINK regs)
> 
> This is not F. It is after starting HSCLK and it is as per hardware manual.
> 
>> 	 start_video()
>>
>> I think to provide the right sequence we need to just move
>>
>> 	reset_control_deassert(dsi->rstc);
>>
>>  From rzg2l_mipi_dsi_dphy_init() to rzg2l_mipi_dsi_atomic_pre_enable()
>> just after rzg2l_mipi_dsi_set_display_timing() call.
>>
>>
>>>   	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_dsii_stop(dsi);
>>
>> rzg2l_mipi_dsi_stop() is writing DSIDPHYCTRL0 reg via dphy_exit().
>> I think the right order should be:
>>
>> 	rzg2l_mipi_dsii_stop(dsi);
>> 	reset_control_assert(dsi->rstc);
> 
> .atomic_pre_enable()
> 
> dsi_start()
> reset_deassert()-->G
> 
> .atomic_post_disbale()
> 
> Just opposite of atomic_pre_enable()
> reset_assert()-->G
> dsi_stop()


You're right, please ignore me on this part.

I missed that the hardware manual only requires 
TXSETR/ULPSSETR/DSISETR/CLSTPTSETR/LPTRNSTSETR to be written before 
deasserting CMN_RSTB.

The placement of set_display_timing() in atomic_enable() is correct.

Meanwhile I've tested the series on RZ/G3E EVK.
Tested-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Kind Regards,
Tommaso

> 
> 
> Cheers,
> Biju