Hi
Am 02.07.26 um 20:23 schrieb Roman Ilin:
Apologies, I accidentally fired off send-email before saving my final
changelog notes. Please ignore the changelog and notes in the original
v3 email.
The actual changes in v3 are:
- Changed the WARN_ON_ONCE to drm_dbg_kms (to avoid user-triggerable
panics).
We want to know when this happens. And user space will only be able to
trigger this once, so there's no risk of spamming the kernel log.
So this is not really a problem. drm_WARN_ON_ONCE was ok for that. You
can also use a regular DRM print macro. But instead of drm_dbg_kms()
should use drm_err_once(). But please also output linedur_ns and
framedur_ns in the error. We want to know which of them is incorrect.
You can also add more information to the error message. See [1] for the
mode-formating macros.
[1]
https://elixir.bootlin.com/linux/v7.1.2/source/include/drm/drm_modes.h#L422
- Updated drm_calc_timestamping_constants with the goto error fallback
to clear the stale state.
Also, an automated review bot pointed out an AB-BA deadlock in
drm_crtc_vblank_start_timer(). But I am leaving this out of the patch to
keep the fixes orthogonal.
Sorry for the noise.
On Jul 2, 2026, at 21:10, Roman Ilin <[email protected]> wrote:
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, modify drm_calc_timestamping_constants() to use u64 for
calculations, check for INT_MAX overflows, and return an error code.
drm_crtc_vblank_start_timer() will then propagate the error, enabling
the driver to fall back to immediate vblank events. Valid modes are
unaffected, and the timer self-heals on the next mode with a sane clock.
Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
Suggested-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Roman Ilin <[email protected]>
---
Changes in v3:
- Changed the WARN_ON_ONCE to drm_err_once
Notes:
drivers/gpu/drm/drm_vblank.c | 71 +++++++++++++++++++++++-------------
include/drm/drm_vblank.h | 4 +-
2 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f90fb2d13..629b9fcc7 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -631,42 +631,51 @@ EXPORT_SYMBOL(drm_crtc_vblank_waitqueue);
* drm_crtc_vblank_helper_get_vblank_timestamp(). They are derived from
* CRTC's true scanout timing, so they take things like panel scaling or
* other adjustments into account.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
*/
-void drm_calc_timestamping_constants(struct drm_crtc *crtc,
- const struct drm_display_mode *mode)
+int drm_calc_timestamping_constants(struct drm_crtc *crtc,
+ const struct drm_display_mode *mode)
{
struct drm_device *dev = crtc->dev;
unsigned int pipe = drm_crtc_index(crtc);
struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
- int linedur_ns = 0, framedur_ns = 0;
+ u64 linedur_ns, framedur_ns;
int dotclock = mode->crtc_clock;
+ unsigned int frame_size;
if (!drm_dev_has_vblank(dev))
- return;
+ return 0;
if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
- return;
+ return -EINVAL;
- /* Valid dotclock? */
- if (dotclock > 0) {
- int frame_size = mode->crtc_htotal * mode->crtc_vtotal;
+ if (dotclock <= 0) {
+ drm_err(dev, "crtc %u: Can't calculate constants, dotclock = %d!\n",
Please turn this into drm_err_once() because this call can actually be
triggered repeatedly from userspace.
Best regards
Thomas
+ crtc->base.id, dotclock);
+ goto error;
+ }
- /*
- * Convert scanline length in pixels and video
- * dot clock to line duration and frame duration
- * in nanoseconds:
- */
- linedur_ns = div_u64((u64) mode->crtc_htotal * 1000000, dotclock);
- framedur_ns = div_u64((u64) frame_size * 1000000, dotclock);
+ frame_size = (unsigned int)mode->crtc_htotal * (unsigned
int)mode->crtc_vtotal;
- /*
- * Fields of interlaced scanout modes are only half a frame duration.
- */
- if (mode->flags & DRM_MODE_FLAG_INTERLACE)
- framedur_ns /= 2;
- } else {
- drm_err(dev, "crtc %u: Can't calculate constants, dotclock = 0!\n",
- crtc->base.id);
+ /*
+ * Convert scanline length in pixels and video dot clock to line duration
+ * and frame duration in nanoseconds.
+ */
+ linedur_ns = div_u64((u64)mode->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 (mode->flags & DRM_MODE_FLAG_INTERLACE)
+ framedur_ns /= 2;
+
+ if (linedur_ns > INT_MAX || framedur_ns > INT_MAX) {
+ drm_dbg_kms(dev, "crtc %u: Can't calculate constants, mode clock too
small!\n",
+ crtc->base.id);
+ goto error;
}
vblank->linedur_ns = linedur_ns;
@@ -678,7 +687,16 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
crtc->base.id, mode->crtc_htotal,
mode->crtc_vtotal, mode->crtc_vdisplay);
drm_dbg_core(dev, "crtc %u: clock %d kHz framedur %d linedur %d\n",
- crtc->base.id, dotclock, framedur_ns, linedur_ns);
+ crtc->base.id, dotclock,
+ vblank->framedur_ns, vblank->linedur_ns);
+
+ return 0;
+
+error:
+ vblank->linedur_ns = 0;
+ vblank->framedur_ns = 0;
+ drm_mode_copy(&vblank->hwmode, mode);
+ return -EINVAL;
}
EXPORT_SYMBOL(drm_calc_timestamping_constants);
@@ -2221,6 +2239,7 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
unsigned long flags;
+ int ret;
if (!vtimer->crtc) {
/*
@@ -2239,7 +2258,9 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
hrtimer_try_to_cancel(&vtimer->timer);
}
- drm_calc_timestamping_constants(crtc, &crtc->mode);
+ ret = drm_calc_timestamping_constants(crtc, &crtc->mode);
+ if (ret)
+ return ret;
spin_lock_irqsave(&vtimer->interval_lock, flags);
vtimer->interval = ns_to_ktime(vblank->framedur_ns);
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 2fcef9c0f..d99772dfa 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -311,8 +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);
-void drm_calc_timestamping_constants(struct drm_crtc *crtc,
- const struct drm_display_mode *mode);
+int 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);
void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc,
u32 max_vblank_count);
--
2.54.0
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
