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 v2:
- Moved the bounds check directly into drm_calc_timestamping_constants()
- Changed the helper's return type to int to propagate errors
- Used u64 for intermediate duration calculations to prevent overflows
- Added drm_WARN_ON_ONCE() when durations exceed INT_MAX
- Dropped virtio-gpu specific workarounds to keep the bugfix contained
to DRM core
Notes:
Tested on 7.2.0-rc1.
drivers/gpu/drm/drm_vblank.c | 67 +++++++++++++++++++++---------------
include/drm/drm_vblank.h | 4 +--
2 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f90fb2d13..b6a342e48 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -631,44 +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;
-
- /*
- * 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;
- } else {
- drm_err(dev, "crtc %u: Can't calculate constants, dotclock =
0!\n",
- crtc->base.id);
+ if (dotclock <= 0) {
+ drm_err(dev, "crtc %u: Can't calculate constants, dotclock =
%d!\n",
+ crtc->base.id, dotclock);
+ return -EINVAL;
}
+ frame_size = (unsigned int)mode->crtc_htotal * (unsigned
int)mode->crtc_vtotal;
+
+ /*
+ * 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 (drm_WARN_ON_ONCE(dev, linedur_ns > INT_MAX) ||
+ drm_WARN_ON_ONCE(dev, framedur_ns > INT_MAX))
+ return -EINVAL;
+
vblank->linedur_ns = linedur_ns;
vblank->framedur_ns = framedur_ns;
drm_mode_copy(&vblank->hwmode, mode);
@@ -678,7 +685,10 @@ 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;
}
EXPORT_SYMBOL(drm_calc_timestamping_constants);
@@ -2221,6 +2231,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 +2250,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
