[PATCH RFC v2] drm/rockchip: vop2: Add clock rate mode check

[PATCH RFC v2] drm/rockchip: vop2: Add clock rate mode check

[Sebastian Reichel]

The display might offer modes, which exceed the maximum clock rate of a
video output. This usually happens for displays that offer refresh rates
above 60 Hz. This results in no picture (or a broken one) being displayed
without manual intervention. Fix this by teaching the driver about the
maximum achievable clock rates for each video port.

The information about the maximum clock rates for each video channel and
the tip about multiple pixels being processed per clock were provided by
Andy Yan and roughly checked against the information available in the
datasheet (which specifies limits like "2560x1600@60Hz with 10-bit"
instead of a specific pixel rate).

For the video ports supporting a 600 MHz input clock, there is some
logic to handle up to 4 pixels in parallel when needed resulting in
the extra multiplier.

Suggested-by: Andy Yan <andy.yan@rock-chips.com>
Link: https://lore.kernel.org/linux-rockchip/1528d788.186b.19d08ed974c.Coremail.andyshrk@163.com/
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
I've kept the RFC tag, as I'm not sure about the 4x parallel pixel
processing. IIUIC all of the video ports with a maximum of 600 MHz
input clock support it, considering they can go to 4K @ 120Hz,
which is above 1.2GHz while Andy mentioned a max. support clock rate
of 600 MHz.
---
Changes in v2:
- Link to v1: https://lore.kernel.org/r/20260217-vop2-clk-rate-check-v1-1-989b569119ba@collabora.com
- based on v7.0-rc7
- rename max_clock_rate into max_pixel_clock_rate to distinguish from
  input clock
- update max clock rates to the numbers provided by Andy Yan with
  extra 4x multiplier for 4K 120Hz VPs
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  3 +++
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  1 +
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 10 ++++++++++
 3 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index a195f5c819a2..35a0edda5375 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1434,6 +1434,9 @@ static enum drm_mode_status vop2_crtc_mode_valid(struct drm_crtc *crtc,
 	if (mode->hdisplay > vp->data->max_output.width)
 		return MODE_BAD_HVALUE;
 
+	if (mode->clock > vp->data->max_pixel_clock_rate / 1000)
+		return MODE_CLOCK_HIGH;
+
 	return MODE_OK;
 }
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
index 9124191899ba..fd46913f3346 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
@@ -225,6 +225,7 @@ struct vop2_video_port_data {
 	u16 gamma_lut_len;
 	u16 cubic_lut_len;
 	struct vop_rect max_output;
+	u32 max_pixel_clock_rate;
 	const u8 pre_scan_max_dly[4];
 	unsigned int offset;
 	/**
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index f3950e8476a7..6ae3d506c476 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -559,18 +559,21 @@ static const struct vop2_video_port_data rk3568_vop_video_ports[] = {
 		.gamma_lut_len = 1024,
 		.cubic_lut_len = 9 * 9 * 9,
 		.max_output = { 4096, 2304 },
+		.max_pixel_clock_rate = 600000000U,
 		.pre_scan_max_dly = { 69, 53, 53, 42 },
 		.offset = 0xc00,
 	}, {
 		.id = 1,
 		.gamma_lut_len = 1024,
 		.max_output = { 2048, 1536 },
+		.max_pixel_clock_rate = 200000000U,
 		.pre_scan_max_dly = { 40, 40, 40, 40 },
 		.offset = 0xd00,
 	}, {
 		.id = 2,
 		.gamma_lut_len = 1024,
 		.max_output = { 1920, 1080 },
+		.max_pixel_clock_rate = 150000000U,
 		.pre_scan_max_dly = { 40, 40, 40, 40 },
 		.offset = 0xe00,
 	},
@@ -775,6 +778,7 @@ static const struct vop2_video_port_data rk3576_vop_video_ports[] = {
 		.gamma_lut_len = 1024,
 		.cubic_lut_len = 9 * 9 * 9, /* 9x9x9 */
 		.max_output = { 4096, 2304 },
+		.max_pixel_clock_rate = 600000000U * 4,
 		/* win layer_mix hdr  */
 		.pre_scan_max_dly = { 10, 8, 2, 0 },
 		.offset = 0xc00,
@@ -785,6 +789,7 @@ static const struct vop2_video_port_data rk3576_vop_video_ports[] = {
 		.gamma_lut_len = 1024,
 		.cubic_lut_len = 729, /* 9x9x9 */
 		.max_output = { 2560, 1600 },
+		.max_pixel_clock_rate = 300000000U,
 		/* win layer_mix hdr  */
 		.pre_scan_max_dly = { 10, 6, 0, 0 },
 		.offset = 0xd00,
@@ -793,6 +798,7 @@ static const struct vop2_video_port_data rk3576_vop_video_ports[] = {
 		.id = 2,
 		.gamma_lut_len = 1024,
 		.max_output = { 1920, 1080 },
+		.max_pixel_clock_rate = 150000000U,
 		/* win layer_mix hdr  */
 		.pre_scan_max_dly = { 10, 6, 0, 0 },
 		.offset = 0xe00,
@@ -1061,6 +1067,7 @@ static const struct vop2_video_port_data rk3588_vop_video_ports[] = {
 		.gamma_lut_len = 1024,
 		.cubic_lut_len = 9 * 9 * 9, /* 9x9x9 */
 		.max_output = { 4096, 2304 },
+		.max_pixel_clock_rate = 600000000U * 4,
 		/* hdr2sdr sdr2hdr hdr2hdr sdr2sdr */
 		.pre_scan_max_dly = { 76, 65, 65, 54 },
 		.offset = 0xc00,
@@ -1070,6 +1077,7 @@ static const struct vop2_video_port_data rk3588_vop_video_ports[] = {
 		.gamma_lut_len = 1024,
 		.cubic_lut_len = 729, /* 9x9x9 */
 		.max_output = { 4096, 2304 },
+		.max_pixel_clock_rate = 600000000U * 4,
 		.pre_scan_max_dly = { 76, 65, 65, 54 },
 		.offset = 0xd00,
 	}, {
@@ -1078,12 +1086,14 @@ static const struct vop2_video_port_data rk3588_vop_video_ports[] = {
 		.gamma_lut_len = 1024,
 		.cubic_lut_len = 17 * 17 * 17, /* 17x17x17 */
 		.max_output = { 4096, 2304 },
+		.max_pixel_clock_rate = 600000000U * 4,
 		.pre_scan_max_dly = { 52, 52, 52, 52 },
 		.offset = 0xe00,
 	}, {
 		.id = 3,
 		.gamma_lut_len = 1024,
 		.max_output = { 2048, 1536 },
+		.max_pixel_clock_rate = 150000000U,
 		.pre_scan_max_dly = { 52, 52, 52, 52 },
 		.offset = 0xf00,
 	},

---
base-commit: 591cd656a1bf5ea94a222af5ef2ee76df029c1d2
change-id: 20260217-vop2-clk-rate-check-268269778cac

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>

Claude review: drm/rockchip: vop2: Add clock rate mode check

[Claude Code Review Bot]

Overall Series Review

Subject: drm/rockchip: vop2: Add clock rate mode check
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Patches: 1
Reviewed: 2026-04-12T10:38:57.670124

---

This is a single RFC patch that adds pixel clock rate validation to the Rockchip VOP2 display driver's `mode_valid` callback. The approach is straightforward and correct in principle: store a per-video-port maximum pixel clock rate and reject modes that exceed it. The implementation is clean and minimal.

The main area of concern — which the author explicitly flags via the RFC tag — is the correctness of the `* 4` multiplier applied to the 600 MHz base clock for certain video ports. The rk3588 `* 4` multiplier is well-supported by the existing code comment at `rockchip_vop2_reg.c:1572` ("4 pixclk/cycle on rk3588") and the `dclk_core_rate = v_pixclk >> 2` logic. The rk3576 case is less clear.

**Verdict**: The code change itself is correct and well-structured. The numerical values need confirmation from the hardware vendor (Andy Yan), particularly for rk3576. A couple of minor issues noted below.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Add clock rate mode check

[Claude Code Review Bot]

Patch Review

**Positive aspects:**
- Straightforward approach using the existing `mode_valid` callback pattern, consistent with the existing `max_output.width` check just above.
- Naming (`max_pixel_clock_rate`) is clear and distinguishes from the VOP input clock.
- Values for rk3568 (600/200/150 MHz) and rk3588 VP3 (150 MHz) look reasonable and straightforward.

**Issues:**

1. **rk3576 VP0 multiplier inconsistency (medium concern):** The rk3576 VP0 sets `max_pixel_clock_rate = 600000000U * 4` (2.4 GHz effective), but its `pixel_rate` is set to `2` (line 785), not 4. The rk3576 `set_intf_mux` at line 1496 divides by `port_pix_rate` (which is 2), and when `crtc_clock > 600000` it applies `dclk_div = 2`. This suggests the hardware does 2 pixels per cycle, not 4, giving a max effective pixel clock of 600 MHz * 2 = 1.2 GHz. The `* 4` multiplier may be too permissive for rk3576, allowing modes the hardware cannot actually drive. This should be confirmed with Andy Yan — it might need to be `* 2` instead.

2. **rk3588 VP0-VP2 have no `pixel_rate` set (informational):** The rk3588 video ports VP0-VP2 get `max_pixel_clock_rate = 600000000U * 4` but don't set the `pixel_rate` field (defaulting to 0). The rk3588 code path uses a separate `rk3588_calc_cru_cfg()` function that hardcodes `dclk_core_rate = v_pixclk >> 2` rather than reading `pixel_rate`, so this is correct behavior — but it's worth noting the asymmetry with rk3576 for future maintainability.

3. **No guard against `max_pixel_clock_rate == 0` (minor):** If a future SoC variant is added and the developer forgets to set `max_pixel_clock_rate`, the field defaults to 0, and the check:
   ```c
   if (mode->clock > vp->data->max_pixel_clock_rate / 1000)
       return MODE_CLOCK_HIGH;
   ```
   would reject **all** modes (since `mode->clock` is always > 0). This is a fail-closed behavior which is arguably safe (you'd notice immediately), but a defensive guard like:
   ```c
   if (vp->data->max_pixel_clock_rate &&
       mode->clock > vp->data->max_pixel_clock_rate / 1000)
   ```
   would be more forgiving during bringup of new SoCs. Alternatively, a `WARN_ON(!vp->data->max_pixel_clock_rate)` during probe would catch the missing initialization early.

4. **Integer arithmetic precision (nit):** The comparison `mode->clock > vp->data->max_pixel_clock_rate / 1000` truncates via integer division. For the values used here (all multiples of 1,000,000), this is fine. But if a future value isn't evenly divisible by 1000, the truncation would make the limit slightly stricter than intended. Not an issue with current values.

5. **Commit message clarity (nit):** The commit message says "up to 4 pixels in parallel when needed resulting in the extra multiplier" but doesn't explain *which* specific ports get the multiplier and why. Since this is the core question the RFC is asking about, it would help reviewers to have a table in the commit message mapping each SoC/VP to its base clock, multiplier, and effective max pixel clock.

---
Generated by Claude Code Patch Reviewer