* Re: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-06-13 22:44 [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration Roman Ilin
@ 2026-06-30 12:06 ` Thorsten Leemhuis
2026-06-30 12:09 ` Thomas Zimmermann
2026-06-30 12:58 ` Thomas Zimmermann
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Thorsten Leemhuis @ 2026-06-30 12:06 UTC (permalink / raw)
To: Roman Ilin
Cc: Louis Chauvet, David Airlie, Simona Vetter, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Javier Martinez Canillas,
Dmitry Osipenko, dri-devel, virtualization, linux-kernel,
Peter Arnesen, Linux kernel regressions list
On 6/14/26 00:44, Roman Ilin 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.
Roman, what's the status here? Does the problem still happen with
7.2-rc1? Sounds like it likely will, just want to make sure before
prodding Thomas about it, who authored and committed the change that
causes this regression.
Side note: CCing Peter Arnesen, who seems to be affected as well.
Quoting a message that was CCed to the regression list, but apparently
was rejected there:
"""
Hi everyone,
I am experiencing a similar Wayland display freeze on Kernel 7.0
described in Roman Ilin's recent patch thread ("drm/vblank: Don't arm
vblank timer with invalid frame duration").
Since my laptop consistently reproduces this freeze on boot, I am
reaching out to offer my hardware for testing if you need verification
that this patch resolves the issue on the newer Ryzen AI architectures.
My System:
* CPU / iGPU: AMD Ryzen AI 9 HX
* dGPU: NVIDIA GeForce RTX 5070
* OS: Fedora 44, Kubuntu 26.04 and Nobara Linux (Fedora-based) / Wayland
* Regression Status: Works perfectly on Kernel 6.19. Fails on Kernel 7.0.
* Tested Workarounds: Kernel parameters amdgpu.dcdebugmask=0x10, 0x410,
and amdgpu.sg_display=0 do not bypass the freeze.
Please let me know if you need me to pull specific dmesg logs or similar
from the frozen state, or if there is a specific patched branch you
would like me to compile and boot to verify the fix.
Regards
Peter
"""
Peter, would be great if you could test 7.2-rc1, too. And if you really
think the problem is the same, it might be worth trying Roman's patch.
Ciao, Thorsten
> 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").
>
> Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
> return an error. enable_vblank then fails and the driver falls back to
> sending the vblank event immediately, as it did before the vblank timer
> was introduced. Valid modes are unaffected, and the timer self-heals on
> the next mode that has a sane clock.
>
> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Roman Ilin <me@romanilin.is>
> ---
> Notes:
>
> Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
>
> Open questions:
>
> This relies on the int overflow producing a negative value. The deeper
> issue is that drm_calc_timestamping_constants() truncates framedur_ns to
> int. Would you prefer to widen framedur_ns to s64, or to bound the
> interval here (e.g. reject framedur_ns above one second) so that any
> bogus interval is rejected regardless of sign?
>
> Should virtio-gpu additionally sanitize the user-supplied clock in its
> atomic_check (similar to vmwgfx for the clock==0 case) so the
> vblank-timer throttling is preserved for these resizes, instead of
> falling back to immediate events?
>
> drivers/gpu/drm/drm_vblank.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f78bf37f1..557cd0bc8 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>
> drm_calc_timestamping_constants(crtc, &crtc->mode);
>
> + /*
> + * Return an error so the driver falls back to sending vblank events
> + * when a small mode clock yields a frame duration exceeding INT_MAX.
> + */
> + if (vblank->framedur_ns <= 0)
> + return -EINVAL;
> +
> spin_lock_irqsave(&vtimer->interval_lock, flags);
> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
> spin_unlock_irqrestore(&vtimer->interval_lock, flags);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-06-30 12:06 ` Thorsten Leemhuis
@ 2026-06-30 12:09 ` Thomas Zimmermann
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Zimmermann @ 2026-06-30 12:09 UTC (permalink / raw)
To: Thorsten Leemhuis, Roman Ilin
Cc: Louis Chauvet, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Peter Arnesen,
Linux kernel regressions list
Hi,
please also see this bug report:
https://gitlab.freedesktop.org/spice/linux/vd_agent/-/work_items/52
Am 30.06.26 um 14:06 schrieb Thorsten Leemhuis:
> On 6/14/26 00:44, Roman Ilin 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.
> Roman, what's the status here? Does the problem still happen with
> 7.2-rc1? Sounds like it likely will, just want to make sure before
> prodding Thomas about it, who authored and committed the change that
> causes this regression.
>
> Side note: CCing Peter Arnesen, who seems to be affected as well.
> Quoting a message that was CCed to the regression list, but apparently
> was rejected there:
>
> """
> Hi everyone,
>
> I am experiencing a similar Wayland display freeze on Kernel 7.0
> described in Roman Ilin's recent patch thread ("drm/vblank: Don't arm
> vblank timer with invalid frame duration").
>
> Since my laptop consistently reproduces this freeze on boot, I am
> reaching out to offer my hardware for testing if you need verification
> that this patch resolves the issue on the newer Ryzen AI architectures.
>
> My System:
> * CPU / iGPU: AMD Ryzen AI 9 HX
> * dGPU: NVIDIA GeForce RTX 5070
> * OS: Fedora 44, Kubuntu 26.04 and Nobara Linux (Fedora-based) / Wayland
> * Regression Status: Works perfectly on Kernel 6.19. Fails on Kernel 7.0.
> * Tested Workarounds: Kernel parameters amdgpu.dcdebugmask=0x10, 0x410,
> and amdgpu.sg_display=0 do not bypass the freeze.
>
> Please let me know if you need me to pull specific dmesg logs or similar
> from the frozen state, or if there is a specific patched branch you
> would like me to compile and boot to verify the fix.
>
> Regards
>
> Peter
> """
>
> Peter, would be great if you could test 7.2-rc1, too. And if you really
> think the problem is the same, it might be worth trying Roman's patch.
> Ciao, Thorsten
>
>> 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").
>>
>> Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
>> return an error. enable_vblank then fails and the driver falls back to
>> sending the vblank event immediately, as it did before the vblank timer
>> was introduced. Valid modes are unaffected, and the timer self-heals on
>> the next mode that has a sane clock.
>>
>> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Roman Ilin <me@romanilin.is>
>> ---
>> Notes:
>>
>> Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
>>
>> Open questions:
>>
>> This relies on the int overflow producing a negative value. The deeper
>> issue is that drm_calc_timestamping_constants() truncates framedur_ns to
>> int. Would you prefer to widen framedur_ns to s64, or to bound the
>> interval here (e.g. reject framedur_ns above one second) so that any
>> bogus interval is rejected regardless of sign?
>>
>> Should virtio-gpu additionally sanitize the user-supplied clock in its
>> atomic_check (similar to vmwgfx for the clock==0 case) so the
>> vblank-timer throttling is preserved for these resizes, instead of
>> falling back to immediate events?
>>
>> drivers/gpu/drm/drm_vblank.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index f78bf37f1..557cd0bc8 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>>
>> drm_calc_timestamping_constants(crtc, &crtc->mode);
>>
>> + /*
>> + * Return an error so the driver falls back to sending vblank events
>> + * when a small mode clock yields a frame duration exceeding INT_MAX.
>> + */
>> + if (vblank->framedur_ns <= 0)
>> + return -EINVAL;
>> +
>> spin_lock_irqsave(&vtimer->interval_lock, flags);
>> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
>> spin_unlock_irqrestore(&vtimer->interval_lock, flags);
--
--
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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-06-13 22:44 [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration Roman Ilin
2026-06-30 12:06 ` Thorsten Leemhuis
@ 2026-06-30 12:58 ` Thomas Zimmermann
2026-07-02 15:23 ` [PATCH v2] " Roman Ilin
2026-07-02 18:10 ` [PATCH v3] " Roman Ilin
3 siblings, 0 replies; 8+ messages in thread
From: Thomas Zimmermann @ 2026-06-30 12:58 UTC (permalink / raw)
To: Roman Ilin, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Ville Syrjälä
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel
(cc Ville)
Hi,
thanks for addressing the issue.
Am 14.06.26 um 00:44 schrieb Roman Ilin:
> 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.:
'off by' as in it should be in Hz rather than kHz.
>
> 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").
>
> Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
> return an error. enable_vblank then fails and the driver falls back to
> sending the vblank event immediately, as it did before the vblank timer
> was introduced. Valid modes are unaffected, and the timer self-heals on
> the next mode that has a sane clock.
>
> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Roman Ilin <me@romanilin.is>
> ---
> Notes:
>
> Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
>
> Open questions:
>
> This relies on the int overflow producing a negative value. The deeper
> issue is that drm_calc_timestamping_constants() truncates framedur_ns to
> int. Would you prefer to widen framedur_ns to s64, or to bound the
> interval here (e.g. reject framedur_ns above one second) so that any
> bogus interval is rejected regardless of sign?
Generally speaking, I think we should not accept such a bugos mode in
the first place. But this would require changes to the mode-setting code
that are too invasive for a bug fix.
So, if anything, we should try to detect the problem in
drm_calc_timestamping_constants(). Let's make the helper return errno
codes instead of failing silently. Within the helper, let's do the
following changes:
- declare frame_size an unsigned int
- declare linedur_ns and framedur_ns of type u64
This should avoid possible overflows in the code. And before assigning
linedur_ns and framedur_ns to the vblank fields, test them against INT_MAX.
Maybe at [1]. Using drm_WARN_ON_ONCE is likely a good idea for future
debugging.
if (drm_WARN_ON_ONCE(dev, linedur_ns > INT_MAX) || drm_WARN_ON_ONCE(dev,
framedur_ns > INT_MAX))
return -EINVAL
[1]
https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_vblank.c#L662
>
> Should virtio-gpu additionally sanitize the user-supplied clock in its
> atomic_check (similar to vmwgfx for the clock==0 case) so the
> vblank-timer throttling is preserved for these resizes, instead of
> falling back to immediate events?
>
> drivers/gpu/drm/drm_vblank.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f78bf37f1..557cd0bc8 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>
> drm_calc_timestamping_constants(crtc, &crtc->mode);
>
> + /*
> + * Return an error so the driver falls back to sending vblank events
> + * when a small mode clock yields a frame duration exceeding INT_MAX.
> + */
> + if (vblank->framedur_ns <= 0)
> + return -EINVAL;
Here, you would just forward the error upwards in the call stack.
Best regards
Thomas
> +
> spin_lock_irqsave(&vtimer->interval_lock, flags);
> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
> spin_unlock_irqrestore(&vtimer->interval_lock, flags);
--
--
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)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-06-13 22:44 [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration Roman Ilin
2026-06-30 12:06 ` Thorsten Leemhuis
2026-06-30 12:58 ` Thomas Zimmermann
@ 2026-07-02 15:23 ` Roman Ilin
2026-07-02 18:10 ` [PATCH v3] " Roman Ilin
3 siblings, 0 replies; 8+ messages in thread
From: Roman Ilin @ 2026-07-02 15:23 UTC (permalink / raw)
To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Ville Syrjälä,
Thorsten Leemhuis, Peter Arnesen, Roman Ilin
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 <tzimmermann@suse.de>
Signed-off-by: Roman Ilin <me@romanilin.is>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-06-13 22:44 [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration Roman Ilin
` (2 preceding siblings ...)
2026-07-02 15:23 ` [PATCH v2] " Roman Ilin
@ 2026-07-02 18:10 ` Roman Ilin
2026-07-02 18:23 ` Roman Ilin
3 siblings, 1 reply; 8+ messages in thread
From: Roman Ilin @ 2026-07-02 18:10 UTC (permalink / raw)
To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Ville Syrjälä,
Thorsten Leemhuis, Peter Arnesen, Roman Ilin
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 <tzimmermann@suse.de>
Signed-off-by: Roman Ilin <me@romanilin.is>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-07-02 18:10 ` [PATCH v3] " Roman Ilin
@ 2026-07-02 18:23 ` Roman Ilin
2026-07-03 7:26 ` Thomas Zimmermann
0 siblings, 1 reply; 8+ messages in thread
From: Roman Ilin @ 2026-07-02 18:23 UTC (permalink / raw)
To: Thomas Zimmermann, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Ville Syrjälä,
Thorsten Leemhuis, Peter Arnesen
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 <me@romanilin.is> 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 <tzimmermann@suse.de>
> Signed-off-by: Roman Ilin <me@romanilin.is>
> ---
> 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] drm/vblank: Don't arm vblank timer with invalid frame duration
2026-07-02 18:23 ` Roman Ilin
@ 2026-07-03 7:26 ` Thomas Zimmermann
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Zimmermann @ 2026-07-03 7:26 UTC (permalink / raw)
To: Roman Ilin, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Ville Syrjälä,
Thorsten Leemhuis, Peter Arnesen
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 <me@romanilin.is> 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 <tzimmermann@suse.de>
>> Signed-off-by: Roman Ilin <me@romanilin.is>
>> ---
>> 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)
^ permalink raw reply [flat|nested] 8+ messages in thread