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). - 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", > + 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 >
