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

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

[Biju]

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

Hi All,

Enhance the RZ/G2L MIPI DSI driver based on section "34.4.2.1 Reset" of
the RZ/G2L hardware manual Rev.1.50 May 2025. According to this section,
it is required to wait >= 1 msec after deasserting the CMN_RSTB signal,
and writing to DSI PHY timing registers and LINK registers should be done
before deasserting CMN_RSTB.
Additionally, the hardware manual suggests display timing settings should
be done after the HS clock is started.

v1->v2:
 * Updated commit header and description
 * Moved the code from rzg2l_mipi_dsi_dphy_init() to rzg2l_mipi_dsi_startup()
 * Moved the check before calling reset_control_deassert(), so that it will be
   skipped for RZ/V2H SoC
 * Added fixes patch for moving rzg2l_mipi_dsi_set_display_timing()
 * Added fixes patch for assert of CMN_RSTB signal

Biju Das (4):
  drm: renesas: rzg2l_mipi_dsi: Move rzg2l_mipi_dsi_set_display_timing()
  drm: renesas: rzg2l_mipi_dsi: Fix assert of CMN_RSTB signal
  drm: renesas: rzg2l_mipi_dsi: Fix deassert of CMN_RSTB signal
  drm: renesas: rzg2l_mipi_dsi: Increase reset delay

 .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c    | 34 +++++++++++--------
 1 file changed, 19 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] drm: renesas: rzg2l_mipi_dsi: Move rzg2l_mipi_dsi_set_display_timing()

[Biju]

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

The RZ/G2L hardware manual (Rev. 1.50, May 2025), Section 34.4.2.4
Video-Input Operation, requires display timings to be set after the
HS clock is started. Move rzg2l_mipi_dsi_set_display_timing() from
rzg2l_mipi_dsi_atomic_pre_enable() to rzg2l_mipi_dsi_atomic_enable(),
placing it after rzg2l_mipi_dsi_start_hs_clock(). Drop the unused ret
variable from rzg2l_mipi_dsi_atomic_pre_enable().

Fixes: 5ce16c169a4c ("drm: renesas: rz-du: Add atomic_pre_enable")
Fixes: 7a043f978ed1 ("drm: rcar-du: Add RZ/G2L DSI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2:
 * New patch
---
 drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 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 a87a301326c7..ff95cb9a7de5 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
@@ -1025,29 +1025,33 @@ static void rzg2l_mipi_dsi_atomic_pre_enable(struct drm_bridge *bridge,
 	const struct drm_display_mode *mode;
 	struct drm_connector *connector;
 	struct drm_crtc *crtc;
-	int ret;
 
 	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);
+	rzg2l_mipi_dsi_startup(dsi, mode);
 }
 
 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;
-- 
2.43.0

Claude review: drm: renesas: rzg2l_mipi_dsi: Move rzg2l_mipi_dsi_set_display_timing()

[Claude Code Review Bot]

Patch Review

This moves `rzg2l_mipi_dsi_set_display_timing()` from `atomic_pre_enable` to `atomic_enable`, placing it after `rzg2l_mipi_dsi_start_hs_clock()`. The hardware manual requires display timings to be set after HS clock is started.

**Issue: Silent dropping of error handling for rzg2l_mipi_dsi_startup()**

In the original code:
```c
	ret = rzg2l_mipi_dsi_startup(dsi, mode);
	if (ret < 0)
		return;
```

The patch changes this to:
```c
	rzg2l_mipi_dsi_startup(dsi, mode);
```

`rzg2l_mipi_dsi_startup()` returns an int and can fail (e.g., `pm_runtime_resume_and_get()`, `dphy_init()`, `dphy_conf_clks()` can all return errors). Silently ignoring the return value means that if startup fails, the driver will proceed to `atomic_enable` and attempt to start the HS clock on uninitialized hardware. The commit message mentions "Drop the unused ret variable" but `ret` was being *used* for error checking — it just couldn't propagate the error since the bridge callback returns void. Still, the early return prevented further damage. This should be preserved:

```c
	if (rzg2l_mipi_dsi_startup(dsi, mode) < 0)
		return;
```

**Duplicated connector/crtc/mode lookup:** The mode lookup code is now duplicated in both `atomic_pre_enable` and `atomic_enable`. Consider caching the mode pointer in `struct rzg2l_mipi_dsi` during `atomic_pre_enable` to avoid the duplication, though this is a minor style point.

Otherwise the fix itself is correct per the hardware manual requirements.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 2/4] drm: renesas: rzg2l_mipi_dsi: Fix assert of CMN_RSTB signal

[Biju]

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

The CMN_RSTB reset signal is not required for reading PHY registers in the
probe. Move reset_control_assert() from rzg2l_mipi_dsi_dphy_exit() to
rzg2l_mipi_dsi_stop(), placing it before the dphy_exit() call. Since this
reset signal is optional for RZ/V2H, the call is a no-op on that SoC.

Fixes: 2991c3f0ca86 ("drm: renesas: rz-du: mipi_dsi: Add OF data support")
Fixes: 418bb3a69e13 ("drm: rcar-du: rzg2l_mipi_dsi: Enhance device lanes check")
Cc: stable@vger.kernel.org
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2:
 * New patch
---
 drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 3 +--
 1 file changed, 1 insertion(+), 2 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 ff95cb9a7de5..d2da247abf05 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
@@ -541,8 +541,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,
@@ -822,6 +820,7 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
 
 static void rzg2l_mipi_dsi_stop(struct rzg2l_mipi_dsi *dsi)
 {
+	reset_control_assert(dsi->rstc);
 	dsi->info->dphy_exit(dsi);
 	pm_runtime_put(dsi->dev);
 }
-- 
2.43.0

Claude review: drm: renesas: rzg2l_mipi_dsi: Fix assert of CMN_RSTB signal

[Claude Code Review Bot]

Patch Review

Moves `reset_control_assert(dsi->rstc)` from `rzg2l_mipi_dsi_dphy_exit()` to `rzg2l_mipi_dsi_stop()`, placing it **before** the `dphy_exit()` call.

```c
 static void rzg2l_mipi_dsi_stop(struct rzg2l_mipi_dsi *dsi)
 {
+	reset_control_assert(dsi->rstc);
 	dsi->info->dphy_exit(dsi);
 	pm_runtime_put(dsi->dev);
 }
```

**Question on ordering:** The commit message says "placing it before the dphy_exit() call." The original code had the assert at the *end* of `dphy_exit()` — so it was after the PHY power-down sequence. Now it's *before* the PHY power-down. Is asserting CMN_RSTB before powering down the PHY (clearing EN_LDO1200, EN_BGR) the correct hardware sequence? The commit message doesn't explain *why* the assert must come before `dphy_exit()` rather than after. The hardware manual section cited is about the reset sequence, so if the manual says assert first then power down, this is fine, but it would be good to have that clarified in the commit message.

**Fixes tags:** The two `Fixes:` tags reference commits that add OF data support and lanes checking. These seem tangential — the original placement of `reset_control_assert()` in `dphy_exit()` dates back to the initial driver commit, not to these fixes-referenced commits. The `Fixes:` tags should point to the commit that originally placed the assert in the wrong location.

**RZ/V2H note:** The commit message says "Since this reset signal is optional for RZ/V2H, the call is a no-op on that SoC." This is correct — `dsi->rstc` is obtained via `devm_reset_control_get_optional_exclusive()`, so it will be NULL when not present, and `reset_control_assert(NULL)` is a no-op.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 3/4] drm: renesas: rzg2l_mipi_dsi: Fix deassert of CMN_RSTB signal

[Biju]

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

The RZ/G2L hardware manual (Rev. 1.50, May 2025), Section 34.4.2.1,
requires deasserting the CMN_RSTB signal after setting the Link
registers. Move the reset_control_deassert() call from
rzg2l_mipi_dsi_dphy_init() to rzg2l_mipi_dsi_startup(), placing it
after the Link register writes. This reset signal is optional for
RZ/V2H SoCs, so add a NULL check. Drop the unused ret variable from
rzg2l_mipi_dsi_dphy_init().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Updated commit header and description
 * Moved the code from rzg2l_mipi_dsi_dphy_init() to rzg2l_mipi_dsi_startup()
 * Moved the check before calling reset_control_deassert(), so that it will be
   skipped for RZ/V2H SoC
---
 drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 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 d2da247abf05..5020be0f72ac 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;
-
-	udelay(1);
-
 	return 0;
 }
 
@@ -809,6 +802,14 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
 	FIELD_MODIFY(DSISETR_MRPSZ, &dsisetr, RZG2L_DCS_BUF_SIZE);
 	rzg2l_mipi_dsi_link_write(dsi, DSISETR, dsisetr);
 
+	if (dsi->rstc) {
+		ret = reset_control_deassert(dsi->rstc);
+		if (ret < 0)
+			goto err_phy;
+
+		udelay(1);
+	}
+
 	return 0;
 
 err_phy:
-- 
2.43.0

Claude review: drm: renesas: rzg2l_mipi_dsi: Fix deassert of CMN_RSTB signal

[Claude Code Review Bot]

Patch Review

Moves `reset_control_deassert()` from `rzg2l_mipi_dsi_dphy_init()` to `rzg2l_mipi_dsi_startup()`, placing it after Link register writes. This ensures PHY timing registers and Link registers are written before deasserting CMN_RSTB, per Section 34.4.2.1.

```c
+	if (dsi->rstc) {
+		ret = reset_control_deassert(dsi->rstc);
+		if (ret < 0)
+			goto err_phy;
+
+		udelay(1);
+	}
```

**The NULL check is unnecessary but harmless.** `reset_control_deassert(NULL)` returns 0, so the `if (dsi->rstc)` guard is not needed for correctness. However, it does skip the `udelay(1)` on RZ/V2H where there's no reset to deassert, which is a minor optimization. This is fine.

**Note:** The `udelay(1)` kept here is the old value; patch 4 will change it to `fsleep(1000)`. This is fine for bisectability — the ordering fix is separated from the timing fix.

**No Fixes tag or stable Cc:** Unlike patches 1 and 2, this patch lacks `Fixes:` and `Cc: stable` tags, even though it arguably has a stronger case for them than patch 2 — it's fixing the order of operations per the hardware manual. This seems inconsistent with the rest of the series.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 4/4] drm: renesas: rzg2l_mipi_dsi: Increase reset deassertion delay to 1 msec

[Biju]

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

The RZ/G2L hardware manual (Rev. 1.50, May 2025), Section 34.4.2.1,
requires waiting more than 1 msec after deasserting the CMN_RSTB signal
before the DSI-Tx module is ready. Increase the delay from 1 usec to
1 msec by replacing udelay(1) with fsleep(1000).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Updated commit header and description.
---
 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 5020be0f72ac..715872130780 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
@@ -807,7 +807,7 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
 		if (ret < 0)
 			goto err_phy;
 
-		udelay(1);
+		fsleep(1000);
 	}
 
 	return 0;
-- 
2.43.0

Claude review: drm: renesas: rzg2l_mipi_dsi: Increase reset deassertion delay to 1 msec

[Claude Code Review Bot]

Patch Review

Changes the delay after CMN_RSTB deassert from `udelay(1)` (1 usec) to `fsleep(1000)` (1 msec), matching the hardware manual's >= 1 msec requirement.

```c
-		udelay(1);
+		fsleep(1000);
```

This is a clear fix — `udelay(1)` is 1 microsecond, three orders of magnitude less than the required 1 millisecond. Using `fsleep()` is the correct API choice here: for a 1ms delay, `fsleep` will use `usleep_range()` internally, which is appropriate for non-atomic context (this runs from `atomic_pre_enable` bridge callback, which is a process context despite the "atomic" name).

**Missing Fixes tag:** This patch fixes a real hardware timing violation (1 usec vs required 1 msec) and could reasonably carry a `Fixes:` tag pointing to the original commit that introduced the `udelay(1)`. If the driver has been working despite this, it may be that the hardware is tolerant, but it's still out of spec.

---
Generated by Claude Code Patch Reviewer

Claude review: Improvements on RZ/G2L MIPI DSI driver

[Claude Code Review Bot]

Overall Series Review

Subject: Improvements on RZ/G2L MIPI DSI driver
Author: Biju <biju.das.au@gmail.com>
Patches: 5
Reviewed: 2026-03-22T04:12:39.924933

---

This is a 4-patch series fixing the RZ/G2L MIPI DSI driver to match hardware manual requirements around the CMN_RSTB reset signal ordering and timing. The changes are well-motivated by the hardware manual (Rev. 1.50, May 2025) and the individual changes are straightforward. The series has a logical progression: move display timing (patch 1), move assert (patch 2), move deassert (patch 3), fix delay (patch 4).

**Concerns:**
1. Patch 1 silently drops error handling from `rzg2l_mipi_dsi_startup()` in `atomic_pre_enable`.
2. Patch 2 changes the shutdown ordering — asserting reset *before* dphy_exit, which may need justification.
3. Patches 1 and 2 carry `Fixes:` tags and Cc stable, but the `Fixes:` tags on patch 2 are questionable.
4. The patches don't apply to drm-next, likely due to `FIELD_MODIFY` macro changes in the base they were developed against.

---
Generated by Claude Code Patch Reviewer