[PATCH] drm/rockchip: vop2: Use drm_err_ratelimited() for wait timeouts

[PATCH] drm/rockchip: vop2: Use drm_err_ratelimited() for wait timeouts

[Hsieh Hung-En]

Replace deprecated DRM_DEV_ERROR() with drm_err_ratelimited() in the VOP2
register wait timeout paths to align with current DRM logging helpers and
avoid flooding the kernel log when timeouts repeat.

Signed-off-by: Hsieh Hung-En <hungen3108@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index f3950e8476a7..02a788a4dfdd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -2106,8 +2106,8 @@ static void rk3568_vop2_wait_for_port_mux_done(struct vop2 *vop2)
 	ret = readx_poll_timeout_atomic(rk3568_vop2_read_port_mux, vop2, port_mux_sel,
 					port_mux_sel == vop2->old_port_sel, 10, 50 * 1000);
 	if (ret)
-		DRM_DEV_ERROR(vop2->dev, "wait port_mux done timeout: 0x%x--0x%x\n",
-			      port_mux_sel, vop2->old_port_sel);
+		drm_err_ratelimited(vop2->drm, "wait port_mux done timeout: 0x%x--0x%x\n",
+				    port_mux_sel, vop2->old_port_sel);
 }
 
 static u32 rk3568_vop2_read_layer_cfg(struct vop2 *vop2)
@@ -2126,8 +2126,8 @@ static void rk3568_vop2_wait_for_layer_cfg_done(struct vop2 *vop2, u32 cfg)
 	ret = readx_poll_timeout_atomic(rk3568_vop2_read_layer_cfg, vop2, atv_layer_cfg,
 					atv_layer_cfg == cfg, 10, 50 * 1000);
 	if (ret)
-		DRM_DEV_ERROR(vop2->dev, "wait layer cfg done timeout: 0x%x--0x%x\n",
-			      atv_layer_cfg, cfg);
+		drm_err_ratelimited(vop2->drm, "wait layer cfg done timeout: 0x%x--0x%x\n",
+				    atv_layer_cfg, cfg);
 }
 
 static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
-- 
2.34.1

Claude review: drm/rockchip: vop2: Use drm_err_ratelimited() for wait timeouts

[Claude Code Review Bot]

Overall Series Review

Subject: drm/rockchip: vop2: Use drm_err_ratelimited() for wait
 timeouts
Author: Hsieh Hung-En <hungen3108@gmail.com>
Patches: 1
Reviewed: 2026-02-11T16:54:13.232428

---

This single-patch series updates error logging in the Rockchip VOP2 driver by replacing the deprecated `DRM_DEV_ERROR()` macro with `drm_err_ratelimited()`. The change is straightforward and addresses two timeout error messages in register polling functions.

**Scope**: The patch touches only error logging paths in two wait-for-done functions in `rockchip_vop2_reg.c`.

**Correctness**: The conversion appears correct but has a subtle behavioral change that should be acknowledged - rate limiting is now applied where it wasn't before.

**Rationale**: Modernizing to current DRM logging APIs and preventing log spam from repeated timeouts is reasonable.

**Concerns**:
1. The commit message claims this prevents "flooding the kernel log when timeouts repeat" - but the original `DRM_DEV_ERROR()` was NOT rate-limited, so this introduces new behavior that should be discussed
2. Missing analysis of whether rate-limiting timeout errors is actually desirable in this specific context
3. The conversion changes the first parameter from `vop2->dev` to `vop2->drm` - this changes the log device context

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Use drm_err_ratelimited() for wait timeouts

[Claude Code Review Bot]

Patch Review

**Commit Message Analysis**:
```
Replace deprecated DRM_DEV_ERROR() with drm_err_ratelimited() in the VOP2
register wait timeout paths to align with current DRM logging helpers and
avoid flooding the kernel log when timeouts repeat.
```

The commit message is concise but contains a misleading claim. The original code did NOT use rate limiting, so "avoid flooding" implies this is fixing an existing problem when it's actually introducing new behavior. The message should clarify that this adds rate limiting as an improvement, not that it maintains existing behavior.

**Technical Review**:

**Change 1: Port mux timeout logging** (lines 113-117)
```c
-		DRM_DEV_ERROR(vop2->dev, "wait port_mux done timeout: 0x%x--0x%x\n",
-			      port_mux_sel, vop2->old_port_sel);
+		drm_err_ratelimited(vop2->drm, "wait port_mux done timeout: 0x%x--0x%x\n",
+				    port_mux_sel, vop2->old_port_sel);
```

Issues:
1. **Device context change**: Switching from `vop2->dev` (struct device) to `vop2->drm` (struct drm_device) changes what device appears in the log. Is `vop2->drm` guaranteed to be non-NULL at this point in the code path?
2. **Rate limiting implications**: Timeouts here indicate hardware/synchronization failures. Rate limiting could hide repeated failures during debugging. Is this the right tradeoff?
3. **Indentation**: The new indentation appears correct (aligned with opening parenthesis).

**Change 2: Layer config timeout logging** (lines 124-128)
```c
-		DRM_DEV_ERROR(vop2->dev, "wait layer cfg done timeout: 0x%x--0x%x\n",
-			      atv_layer_cfg, cfg);
+		drm_err_ratelimited(vop2->drm, "wait layer cfg done timeout: 0x%x--0x%x\n",
+				    atv_layer_cfg, cfg);
```

Same issues as Change 1:
1. Device context switch from `vop2->dev` to `vop2->drm`
2. Rate limiting on hardware timeout errors
3. Indentation looks correct

**Missing Analysis**:

1. **Initialization verification**: The patch should verify that `vop2->drm` is initialized before these functions can be called. A quick grep would show if there are code paths where these waits happen during early initialization.

2. **Timeout frequency**: Are these timeouts expected to occur repeatedly in normal operation (suggesting rate limiting is good), or are they exceptional events that should always be logged (suggesting rate limiting is bad)?

3. **Consistency**: Are there other timeout errors in the same file still using `DRM_DEV_ERROR()`? A grep for remaining uses would show if this conversion should be more comprehensive.

**Questions for Author**:

1. Can you verify that `vop2->drm` is guaranteed to be initialized when these wait functions are called?
2. Have you observed repeated timeout errors that motivated the rate limiting, or is this purely preventative?
3. Should the commit message be updated to clarify this adds rate limiting rather than implying it was already present?
4. Are there debugging scenarios where we'd want to see every timeout occurrence, or is rate limiting always appropriate here?

**Verdict**: The mechanical conversion appears correct, but the behavioral change (adding rate limiting) and device context change need justification. The commit message should be more accurate about introducing new behavior.

**Suggested Commit Message**:
```
drm/rockchip: vop2: Use drm_err_ratelimited() for wait timeouts

Replace deprecated DRM_DEV_ERROR() with drm_err_ratelimited() in the VOP2
register wait timeout paths to align with current DRM logging helpers.

This also adds rate limiting to these error messages to prevent log spam
if hardware issues cause repeated timeouts. The timeout duration is 50ms,
so without rate limiting, a persistent issue could log many times per second.

Signed-off-by: Hsieh Hung-En <hungen3108@gmail.com>
```

---
Generated by Claude Code Patch Reviewer