Am 30.10.26 um 15:25 schrieb Thomas Zimmermann: [...]
How about we add a dedicated mode_fixup helper for vblank validation? It would run after new CRTC's mode_fixup callback. [1] There we could try to adapt the CRTC's adjusted_mode. Since we know the bug exactly, we can try to detect it, fix it, and document it.
The attached patch implements this idea. It does work in the sense that it doesn't immediately break my testing setup.
Thorsten (or anyone else) could you please test if this patch also resolve the bug with SPICE?
-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)
From 2a76d04bfc3630c1d6bb79b3d64ecbcadf860090 Mon Sep 17 00:00:00 2001 From: Roman Ilin <[email protected]> Date: Thu, 2 Jul 2026 21:10:27 +0300 Subject: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration When a CRTC's display mode carries a too small pixel clock, drm_calc_timestamping_constants() computes a frame duration that exceeds INT_MAX. drm_vblank_crtc.framedur_ns becomes negative. drm_crtc_vblank_start_timer() then arms the vblank hrtimer with this interval, after which vblank events are no longer delivered. Pending page flips never complete and the display appears frozen. This could be triggered on virtio-gpu guests that have dynamic resolution enabled: when the SPICE agent or the X server resizes the output, it submits a mode whose pixel clock is off by a factor of 1000, e.g.: clock = 406 kHz, htotal = 3152, vtotal = 2148 framedur_ns = 3152 * 2148 * 1000000 / 406 = 16675852216 ns (~16.7 s) 16675852216 does not fit into an int and wraps to roughly -504000000. ns_to_ktime() then yields a negative interval and the timer stops working. Found by bisection, which pointed at commit a036f5fceedb ("drm/virtgpu: Use vblank timer"). That commit merely made virtio-gpu use the vblank timer and thereby exposed the pre-existing problem in the timer setup added by commit 74afeb812850 ("drm/vblank: Add vblank timer"). To fix this, introduce drm_crtc_vblank_mode_fixup() to detect and adjust the bogus pixel clock. The helper runs unconditionally at the beginning of the mode-fixup code. Drivers still have the opportunity to detect bogus adjustments in later mode-fixup steps. v5: - fix up the broken pixel clock beforehand (Ville) v4: - avoid log spamming with drm_warn_once() Fixes: 74afeb812850 ("drm/vblank: Add vblank timer") Signed-off-by: Roman Ilin <[email protected]> mode_fixup Revert "drm/vblank: Don't arm vblank timer with invalid frame duration" This reverts commit f3ca78682a4f9607444a3dafbe12ab6acdb1fa2f. --- drivers/gpu/drm/drm_atomic_helper.c | 11 +++++ drivers/gpu/drm/drm_vblank.c | 68 +++++++++++++++++++++++++++++ include/drm/drm_vblank.h | 2 + 3 files changed, 81 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9d006f98413a..6a0f04a72292 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -432,6 +432,17 @@ mode_fixup(struct drm_atomic_commit *state) continue; drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state->mode); + + if (!new_crtc_state->enable) + continue; + + ret = drm_crtc_vblank_mode_fixup(crtc, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); + if (!ret) { + drm_dbg_atomic(crtc->dev, "[CRTC:%d:%s] vblank fixup failed\n", + crtc->base.id, crtc->name); + return -EINVAL; + } } for_each_new_connector_in_state(state, connector, new_conn_state, i) { diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index f90fb2d13e42..6e826c3b6c42 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -620,6 +620,74 @@ wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc) } EXPORT_SYMBOL(drm_crtc_vblank_waitqueue); +static int drm_vblank_validate_dotclock(u16 crtc_htotal, u16 crtc_vtotal, u32 flags, int dotclock) +{ + unsigned int frame_size; + u64 linedur_ns, framedur_ns; + + if (dotclock <= 0) + return -EINVAL; + + frame_size = (unsigned int)crtc_htotal * (unsigned int)crtc_vtotal; + + /* + * Convert scanline length in pixels and video dot clock to line duration + * and frame duration in nanoseconds. + */ + linedur_ns = div_u64((u64)crtc_htotal * 1000000, dotclock); + framedur_ns = div_u64((u64)frame_size * 1000000, dotclock); + + /* + * Fields of interlaced scanout modes are only half a frame duration. + */ + if (flags & DRM_MODE_FLAG_INTERLACE) + framedur_ns /= 2; + + if (linedur_ns > INT_MAX || framedur_ns > INT_MAX) + return -ERANGE; /* dot clock too low */ + + return 0; +} + +bool drm_crtc_vblank_mode_fixup(struct drm_crtc *crtc, const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + struct drm_device *dev = crtc->dev; + unsigned int pipe = drm_crtc_index(crtc); + int dotclock = adjusted_mode->crtc_clock; + int ret; + + if (!drm_dev_has_vblank(dev)) + return true; + + if (drm_WARN_ON(dev, pipe >= dev->num_crtcs)) + return false; + + ret = drm_vblank_validate_dotclock(adjusted_mode->crtc_htotal, + adjusted_mode->crtc_vtotal, + adjusted_mode->flags, dotclock); + if (ret == -ERANGE) { + int dotclock_1000; + + /* + * SPICE' vd_agent has a bug where it specifies the dot clock in kHz + * instead of Hz. Setting up a vblank interrupt or timer for such a + * mode overflows the calculation in drm_calc_timestamping_constants(). + * An errno code of ERANGE specifically points to this problem. Try + * converting the dot clock to Hz and validate again. + */ + if (check_mul_overflow(dotclock, 1000, &dotclock_1000)) { + ret = drm_vblank_validate_dotclock(adjusted_mode->crtc_htotal, + adjusted_mode->crtc_vtotal, + adjusted_mode->flags, dotclock_1000); + if (!ret) + adjusted_mode->crtc_clock = dotclock_1000; + } + } + + return !ret; +} +EXPORT_SYMBOL(drm_crtc_vblank_mode_fixup); /** * drm_calc_timestamping_constants - calculate vblank timestamp constants diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h index 2fcef9c0f5b1..a3628c477f59 100644 --- a/include/drm/drm_vblank.h +++ b/include/drm/drm_vblank.h @@ -311,6 +311,8 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc); u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc); void drm_crtc_vblank_restore(struct drm_crtc *crtc); +bool drm_crtc_vblank_mode_fixup(struct drm_crtc *crtc, const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode); wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc); -- 2.56.0
