linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
@ 2025-09-04 14:56 Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 1/4] drm/vblank: Add vblank timer Thomas Zimmermann
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
  To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
	simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
  Cc: dri-devel, linux-hyperv, Thomas Zimmermann

Compositors often depend on vblanks to limit their display-update
rate. Without, they see vblank events ASAP, which breaks the rate-
limit feature. This creates high CPU overhead. It is especially a
problem with virtual devices with fast framebuffer access.

The series moves vkms' vblank timer to DRM and converts the hyperv
DRM driver. An earlier version of this series contains examples of
other updated drivers. In principle, any DRM driver without vblank
hardware can use the timer.

The series has been motivated by a recent discussion about hypervdrm [1]
and other long-standing bug reports. [2][3]

v3:
- fix deadlock (Ville, Lyude)
v2:
- rework interfaces

[1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
[2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
[3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606


Thomas Zimmermann (4):
  drm/vblank: Add vblank timer
  drm/vblank: Add CRTC helpers for simple use cases
  drm/vkms: Convert to DRM's vblank timer
  drm/hypervdrm: Use vblank timer

 Documentation/gpu/drm-kms-helpers.rst       |  12 ++
 drivers/gpu/drm/Makefile                    |   3 +-
 drivers/gpu/drm/drm_vblank.c                | 161 +++++++++++++++++-
 drivers/gpu/drm/drm_vblank_helper.c         | 176 ++++++++++++++++++++
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  11 ++
 drivers/gpu/drm/vkms/vkms_crtc.c            |  83 +--------
 drivers/gpu/drm/vkms/vkms_drv.h             |   2 -
 include/drm/drm_modeset_helper_vtables.h    |  12 ++
 include/drm/drm_vblank.h                    |  32 ++++
 include/drm/drm_vblank_helper.h             |  56 +++++++
 10 files changed, 467 insertions(+), 81 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
 create mode 100644 include/drm/drm_vblank_helper.h

-- 
2.50.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/4] drm/vblank: Add vblank timer
  2025-09-04 14:56 [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Thomas Zimmermann
@ 2025-09-04 14:56 ` Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 2/4] drm/vblank: Add CRTC helpers for simple use cases Thomas Zimmermann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
  To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
	simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
  Cc: dri-devel, linux-hyperv, Thomas Zimmermann

The vblank timer simulates a vblank interrupt for hardware without
support. Rate-limits the display update frequency.

DRM drivers for hardware without vblank support apply display updates
ASAP. A vblank event informs DRM clients of the completed update.
Userspace compositors immediately schedule the next update, which
creates significant load on virtualization outputs. Display updates
are usually fast on virtualization outputs, as their framebuffers are
in regular system memory and there's no hardware vblank interrupt to
throttle the update rate.

The vblank timer is a HR timer that signals the vblank in software.
It limits the update frequency of a DRM driver similar to a hardware
vblank interrupt. The timer is not synchronized to the actual vblank
interval of the display.

The code has been adopted from vkms, which added the funtionality
in commit 3a0709928b17 ("drm/vkms: Add vblank events simulated by
hrtimers").

The new implementation is part of the existing vblank support,
which sets up the timer automatically. Drivers only have to start
and cancel the vblank timer as part of enabling and disabling the
CRTC. The new vblank helper library provides callbacks for struct
drm_crtc_funcs.

The standard way for handling vblank is to call drm_crtc_handle_vblank().
Drivers that require additional processing, such as vkms, can init
handle_vblank_timeout in struct drm_crtc_helper_funcs to refer to
their timeout handler.

There's a possible deadlock between drm_crtc_handle_vblank() and
hrtimer_cancel(). [1] The implementation avoids to call hrtimer_cancel()
directly and instead signals to the timer function to not restart
itself.

v3:
- avoid deadlock when cancelling timer (Ville, Lyude)
v2:
- implement vblank timer entirely in vblank helpers
- downgrade overrun warning to debug
- fix docs

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Link: https://lore.kernel.org/all/20250510094757.4174662-1-zengheng4@huawei.com/ # [1]
---
 Documentation/gpu/drm-kms-helpers.rst    |  12 ++
 drivers/gpu/drm/Makefile                 |   3 +-
 drivers/gpu/drm/drm_vblank.c             | 161 ++++++++++++++++++++++-
 drivers/gpu/drm/drm_vblank_helper.c      |  96 ++++++++++++++
 include/drm/drm_modeset_helper_vtables.h |  12 ++
 include/drm/drm_vblank.h                 |  32 +++++
 include/drm/drm_vblank_helper.h          |  33 +++++
 7 files changed, 346 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
 create mode 100644 include/drm/drm_vblank_helper.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 5139705089f2..781129f78b06 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -92,6 +92,18 @@ GEM Atomic Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_gem_atomic_helper.c
    :export:
 
+VBLANK Helper Reference
+-----------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_vblank_helper.c
+   :doc: overview
+
+.. kernel-doc:: include/drm/drm_vblank_helper.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_vblank_helper.c
+   :export:
+
 Simple KMS Helper Reference
 ===========================
 
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 4dafbdc8f86a..5ba4ffdb8055 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -150,7 +150,8 @@ drm_kms_helper-y := \
 	drm_plane_helper.o \
 	drm_probe_helper.o \
 	drm_self_refresh_helper.o \
-	drm_simple_kms_helper.o
+	drm_simple_kms_helper.o \
+	drm_vblank_helper.o
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 46f59883183d..364f5ba969d0 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -136,8 +136,17 @@
  * vblanks after a timer has expired, which can be configured through the
  * ``vblankoffdelay`` module parameter.
  *
- * Drivers for hardware without support for vertical-blanking interrupts
- * must not call drm_vblank_init(). For such drivers, atomic helpers will
+ * Drivers for hardware without support for vertical-blanking interrupts can
+ * use DRM vblank timers to send vblank events at the rate of the current
+ * display mode's refresh. While not synchronized to the hardware's
+ * vertical-blanking regions, the timer helps DRM clients and compositors to
+ * adapt their update cycle to the display output. Drivers should set up
+ * vblanking as usual, but call drm_crtc_vblank_start_timer() and
+ * drm_crtc_vblank_cancel_timer() as part of their atomic mode setting.
+ * See also DRM vblank helpers for more information.
+ *
+ * Drivers without support for vertical-blanking interrupts nor timers must
+ * not call drm_vblank_init(). For these drivers, atomic helpers will
  * automatically generate fake vblank events as part of the display update.
  * This functionality also can be controlled by the driver by enabling and
  * disabling struct drm_crtc_state.no_vblank.
@@ -508,6 +517,9 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
 	drm_WARN_ON(dev, READ_ONCE(vblank->enabled) &&
 		    drm_core_check_feature(dev, DRIVER_MODESET));
 
+	if (vblank->vblank_timer.crtc)
+		hrtimer_cancel(&vblank->vblank_timer.timer);
+
 	drm_vblank_destroy_worker(vblank);
 	timer_delete_sync(&vblank->disable_timer);
 }
@@ -2162,3 +2174,148 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
 	return ret;
 }
 
+/*
+ * VBLANK timer
+ */
+
+static enum hrtimer_restart drm_vblank_timer_function(struct hrtimer *timer)
+{
+	struct drm_vblank_crtc_timer *vtimer =
+		container_of(timer, struct drm_vblank_crtc_timer, timer);
+	struct drm_crtc *crtc = vtimer->crtc;
+	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
+	struct drm_device *dev = crtc->dev;
+	unsigned long flags;
+	ktime_t interval;
+	u64 ret_overrun;
+	bool succ;
+
+	spin_lock_irqsave(&vtimer->interval_lock, flags);
+	interval = vtimer->interval;
+	spin_unlock_irqrestore(&vtimer->interval_lock, flags);
+
+	if (!interval)
+		return HRTIMER_NORESTART;
+
+	ret_overrun = hrtimer_forward_now(&vtimer->timer, interval);
+	if (ret_overrun != 1)
+		drm_dbg_vbl(dev, "vblank timer overrun\n");
+
+	if (crtc_funcs->handle_vblank_timeout)
+		succ = crtc_funcs->handle_vblank_timeout(crtc);
+	else
+		succ = drm_crtc_handle_vblank(crtc);
+	if (!succ)
+		return HRTIMER_NORESTART;
+
+	return HRTIMER_RESTART;
+}
+
+/**
+ * drm_crtc_vblank_start_timer - Starts the vblank timer on the given CRTC
+ * @crtc: the CRTC
+ *
+ * Drivers should call this function from their CRTC's enable_vblank
+ * function to start a vblank timer. The timer will fire after the duration
+ * of a full frame. drm_crtc_vblank_cancel_timer() disables a running timer.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+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;
+
+	if (!vtimer->crtc) {
+		/*
+		 * Set up the data structures on the first invocation.
+		 */
+		vtimer->crtc = crtc;
+		spin_lock_init(&vtimer->interval_lock);
+		hrtimer_setup(&vtimer->timer, drm_vblank_timer_function,
+			      CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	} else {
+		/*
+		 * Timer should not be active. If it is, wait for the
+		 * previous cancel operations to finish.
+		 */
+		while (hrtimer_active(&vtimer->timer))
+			hrtimer_try_to_cancel(&vtimer->timer);
+	}
+
+	drm_calc_timestamping_constants(crtc, &crtc->mode);
+
+	spin_lock_irqsave(&vtimer->interval_lock, flags);
+	vtimer->interval = ns_to_ktime(vblank->framedur_ns);
+	spin_unlock_irqrestore(&vtimer->interval_lock, flags);
+
+	hrtimer_start(&vtimer->timer, vtimer->interval, HRTIMER_MODE_REL);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_crtc_vblank_start_timer);
+
+/**
+ * drm_crtc_vblank_start_timer - Cancels the given CRTC's vblank timer
+ * @crtc: the CRTC
+ *
+ * Drivers should call this function from their CRTC's disable_vblank
+ * function to stop a vblank timer.
+ */
+void drm_crtc_vblank_cancel_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;
+
+	/*
+	 * Calling hrtimer_cancel() can result in a deadlock with DRM's
+	 * vblank_time_lime_lock and hrtimers' softirq_expiry_lock. So
+	 * clear interval and indicate cancellation. The timer function
+	 * will cancel itself on the next invocation.
+	 */
+
+	spin_lock_irqsave(&vtimer->interval_lock, flags);
+	vtimer->interval = 0;
+	spin_unlock_irqrestore(&vtimer->interval_lock, flags);
+
+	hrtimer_try_to_cancel(&vtimer->timer);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_cancel_timer);
+
+/**
+ * drm_crtc_vblank_get_vblank_timeout - Returns the vblank timeout
+ * @crtc: The CRTC
+ * @vblank_time: Returns the next vblank timestamp
+ *
+ * The helper drm_crtc_vblank_get_vblank_timeout() returns the next vblank
+ * timestamp of the CRTC's vblank timer according to the timer's expiry
+ * time.
+ */
+void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time)
+{
+	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+	struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
+
+	if (!READ_ONCE(vblank->enabled)) {
+		*vblank_time = ktime_get();
+		return;
+	}
+
+	*vblank_time = READ_ONCE(vtimer->timer.node.expires);
+
+	if (drm_WARN_ON(crtc->dev, !ktime_compare(*vblank_time, vblank->time)))
+		return; /* Already expired */
+
+	/*
+	 * To prevent races we roll the hrtimer forward before we do any
+	 * interrupt processing - this is how real hw works (the interrupt
+	 * is only generated after all the vblank registers are updated)
+	 * and what the vblank core expects. Therefore we need to always
+	 * correct the timestamp by one frame.
+	 */
+	*vblank_time = ktime_sub(*vblank_time, vtimer->interval);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_get_vblank_timeout);
diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
new file mode 100644
index 000000000000..f94d1e706191
--- /dev/null
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: MIT
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_print.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
+
+/**
+ * DOC: overview
+ *
+ * The vblank helper library provides functions for supporting vertical
+ * blanking in DRM drivers.
+ *
+ * For vblank timers, several callback implementations are available.
+ * Drivers enable support for vblank timers by setting the vblank callbacks
+ * in struct &drm_crtc_funcs to the helpers provided by this library. The
+ * initializer macro DRM_CRTC_VBLANK_TIMER_FUNCS does this conveniently.
+ *
+ * Once the driver enables vblank support with drm_vblank_init(), each
+ * CRTC's vblank timer fires according to the programmed display mode. By
+ * default, the vblank timer invokes drm_crtc_handle_vblank(). Drivers with
+ * more specific requirements can set their own handler function in
+ * struct &drm_crtc_helper_funcs.handle_vblank_timeout.
+ */
+
+/*
+ * VBLANK timer
+ */
+
+/**
+ * drm_crtc_vblank_helper_enable_vblank_timer - Implements struct &drm_crtc_funcs.enable_vblank
+ * @crtc: The CRTC
+ *
+ * The helper drm_crtc_vblank_helper_enable_vblank_timer() implements
+ * enable_vblank of struct drm_crtc_helper_funcs for CRTCs that require
+ * a VBLANK timer. It sets up the timer on the first invocation. The
+ * started timer expires after the current frame duration. See struct
+ * &drm_vblank_crtc.framedur_ns.
+ *
+ * See also struct &drm_crtc_helper_funcs.enable_vblank.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_crtc_vblank_helper_enable_vblank_timer(struct drm_crtc *crtc)
+{
+	return drm_crtc_vblank_start_timer(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_helper_enable_vblank_timer);
+
+/**
+ * drm_crtc_vblank_helper_disable_vblank_timer - Implements struct &drm_crtc_funcs.disable_vblank
+ * @crtc: The CRTC
+ *
+ * The helper drm_crtc_vblank_helper_disable_vblank_timer() implements
+ * disable_vblank of struct drm_crtc_funcs for CRTCs that require a
+ * VBLANK timer.
+ *
+ * See also struct &drm_crtc_helper_funcs.disable_vblank.
+ */
+void drm_crtc_vblank_helper_disable_vblank_timer(struct drm_crtc *crtc)
+{
+	drm_crtc_vblank_cancel_timer(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_helper_disable_vblank_timer);
+
+/**
+ * drm_crtc_vblank_helper_get_vblank_timestamp_from_timer -
+ *	Implements struct &drm_crtc_funcs.get_vblank_timestamp
+ * @crtc: The CRTC
+ * @max_error: Maximum acceptable error
+ * @vblank_time: Returns the next vblank timestamp
+ * @in_vblank_irq: True is called from drm_crtc_handle_vblank()
+ *
+ * The helper drm_crtc_helper_get_vblank_timestamp_from_timer() implements
+ * get_vblank_timestamp of struct drm_crtc_funcs for CRTCs that require a
+ * VBLANK timer. It returns the timestamp according to the timer's expiry
+ * time.
+ *
+ * See also struct &drm_crtc_funcs.get_vblank_timestamp.
+ *
+ * Returns:
+ * True on success, or false otherwise.
+ */
+bool drm_crtc_vblank_helper_get_vblank_timestamp_from_timer(struct drm_crtc *crtc,
+							    int *max_error,
+							    ktime_t *vblank_time,
+							    bool in_vblank_irq)
+{
+	drm_crtc_vblank_get_vblank_timeout(crtc, vblank_time);
+
+	return true;
+}
+EXPORT_SYMBOL(drm_crtc_vblank_helper_get_vblank_timestamp_from_timer);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index ce7c7aeac887..fe32854b7ffe 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -490,6 +490,18 @@ struct drm_crtc_helper_funcs {
 				     bool in_vblank_irq, int *vpos, int *hpos,
 				     ktime_t *stime, ktime_t *etime,
 				     const struct drm_display_mode *mode);
+
+	/**
+	 * @handle_vblank_timeout: Handles timeouts of the vblank timer.
+	 *
+	 * Called by CRTC's the vblank timer on each timeout. Semantics is
+	 * equivalient to drm_crtc_handle_vblank(). Implementations should
+	 * invoke drm_crtc_handle_vblank() as part of processing the timeout.
+	 *
+	 * This callback is optional. If unset, the vblank timer invokes
+	 * drm_crtc_handle_vblank() directly.
+	 */
+	bool (*handle_vblank_timeout)(struct drm_crtc *crtc);
 };
 
 /**
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 151ab1e85b1b..ffa564d79638 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -25,6 +25,7 @@
 #define _DRM_VBLANK_H_
 
 #include <linux/seqlock.h>
+#include <linux/hrtimer.h>
 #include <linux/idr.h>
 #include <linux/poll.h>
 #include <linux/kthread.h>
@@ -103,6 +104,28 @@ struct drm_vblank_crtc_config {
 	bool disable_immediate;
 };
 
+/**
+ * struct drm_vblank_crtc_timer - vblank timer for a CRTC
+ */
+struct drm_vblank_crtc_timer {
+	/**
+	 * @timer: The vblank's high-resolution timer
+	 */
+	struct hrtimer timer;
+	/**
+	 * @interval_lock: Protects @interval
+	 */
+	spinlock_t interval_lock;
+	/**
+	 * @interval: Duration between two vblanks
+	 */
+	ktime_t interval;
+	/**
+	 * @crtc: The timer's CRTC
+	 */
+	struct drm_crtc *crtc;
+};
+
 /**
  * struct drm_vblank_crtc - vblank tracking for a CRTC
  *
@@ -254,6 +277,11 @@ struct drm_vblank_crtc {
 	 * cancelled.
 	 */
 	wait_queue_head_t work_wait_queue;
+
+	/**
+	 * @vblank_timer: Holds the state of the vblank timer
+	 */
+	struct drm_vblank_crtc_timer vblank_timer;
 };
 
 struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
@@ -290,6 +318,10 @@ 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);
 
+int drm_crtc_vblank_start_timer(struct drm_crtc *crtc);
+void drm_crtc_vblank_cancel_timer(struct drm_crtc *crtc);
+void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time);
+
 /*
  * Helpers for struct drm_crtc_funcs
  */
diff --git a/include/drm/drm_vblank_helper.h b/include/drm/drm_vblank_helper.h
new file mode 100644
index 000000000000..74a971d0cfba
--- /dev/null
+++ b/include/drm/drm_vblank_helper.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _DRM_VBLANK_HELPER_H_
+#define _DRM_VBLANK_HELPER_H_
+
+#include <linux/hrtimer_types.h>
+#include <linux/types.h>
+
+struct drm_crtc;
+
+/*
+ * VBLANK timer
+ */
+
+int drm_crtc_vblank_helper_enable_vblank_timer(struct drm_crtc *crtc);
+void drm_crtc_vblank_helper_disable_vblank_timer(struct drm_crtc *crtc);
+bool drm_crtc_vblank_helper_get_vblank_timestamp_from_timer(struct drm_crtc *crtc,
+							    int *max_error,
+							    ktime_t *vblank_time,
+							    bool in_vblank_irq);
+
+/**
+ * DRM_CRTC_VBLANK_TIMER_FUNCS - Default implementation for VBLANK timers
+ *
+ * This macro initializes struct &drm_crtc_funcs to default helpers for
+ * VBLANK timers.
+ */
+#define DRM_CRTC_VBLANK_TIMER_FUNCS \
+	.enable_vblank = drm_crtc_vblank_helper_enable_vblank_timer, \
+	.disable_vblank = drm_crtc_vblank_helper_disable_vblank_timer, \
+	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp_from_timer
+
+#endif
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/4] drm/vblank: Add CRTC helpers for simple use cases
  2025-09-04 14:56 [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 1/4] drm/vblank: Add vblank timer Thomas Zimmermann
@ 2025-09-04 14:56 ` Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 3/4] drm/vkms: Convert to DRM's vblank timer Thomas Zimmermann
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
  To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
	simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
  Cc: dri-devel, linux-hyperv, Thomas Zimmermann

Implement atomic_flush, atomic_enable and atomic_disable of struct
drm_crtc_helper_funcs for vblank handling. Driver with no further
requirements can use these functions instead of adding their own.
Also simplifies the use of vblank timers.

The code has been adopted from vkms, which added the funtionality
in commit 3a0709928b17 ("drm/vkms: Add vblank events simulated by
hrtimers").

v3:
- mention vkms (Javier)
v2:
- fix docs

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_vblank_helper.c | 80 +++++++++++++++++++++++++++++
 include/drm/drm_vblank_helper.h     | 23 +++++++++
 2 files changed, 103 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
index f94d1e706191..a04a6ba1b0ca 100644
--- a/drivers/gpu/drm/drm_vblank_helper.c
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_modeset_helper_vtables.h>
@@ -17,6 +18,12 @@
  * Drivers enable support for vblank timers by setting the vblank callbacks
  * in struct &drm_crtc_funcs to the helpers provided by this library. The
  * initializer macro DRM_CRTC_VBLANK_TIMER_FUNCS does this conveniently.
+ * The driver further has to send the VBLANK event from its atomic_flush
+ * callback and control vblank from the CRTC's atomic_enable and atomic_disable
+ * callbacks. The callbacks are located in struct &drm_crtc_helper_funcs.
+ * The vblank helper library provides implementations of these callbacks
+ * for drivers without further requirements. The initializer macro
+ * DRM_CRTC_HELPER_VBLANK_FUNCS sets them coveniently.
  *
  * Once the driver enables vblank support with drm_vblank_init(), each
  * CRTC's vblank timer fires according to the programmed display mode. By
@@ -25,6 +32,79 @@
  * struct &drm_crtc_helper_funcs.handle_vblank_timeout.
  */
 
+/*
+ * VBLANK helpers
+ */
+
+/**
+ * drm_crtc_vblank_atomic_flush -
+ *	Implements struct &drm_crtc_helper_funcs.atomic_flush
+ * @crtc: The CRTC
+ * @state: The atomic state to apply
+ *
+ * The helper drm_crtc_vblank_atomic_flush() implements atomic_flush of
+ * struct drm_crtc_helper_funcs for CRTCs that only need to send out a
+ * VBLANK event.
+ *
+ * See also struct &drm_crtc_helper_funcs.atomic_flush.
+ */
+void drm_crtc_vblank_atomic_flush(struct drm_crtc *crtc,
+				  struct drm_atomic_state *state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct drm_pending_vblank_event *event;
+
+	spin_lock_irq(&dev->event_lock);
+
+	event = crtc_state->event;
+	crtc_state->event = NULL;
+
+	if (event) {
+		if (drm_crtc_vblank_get(crtc) == 0)
+			drm_crtc_arm_vblank_event(crtc, event);
+		else
+			drm_crtc_send_vblank_event(crtc, event);
+	}
+
+	spin_unlock_irq(&dev->event_lock);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_flush);
+
+/**
+ * drm_crtc_vblank_atomic_enable - Implements struct &drm_crtc_helper_funcs.atomic_enable
+ * @crtc: The CRTC
+ * @state: The atomic state
+ *
+ * The helper drm_crtc_vblank_atomic_enable() implements atomic_enable
+ * of struct drm_crtc_helper_funcs for CRTCs the only need to enable VBLANKs.
+ *
+ * See also struct &drm_crtc_helper_funcs.atomic_enable.
+ */
+void drm_crtc_vblank_atomic_enable(struct drm_crtc *crtc,
+				   struct drm_atomic_state *state)
+{
+	drm_crtc_vblank_on(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_enable);
+
+/**
+ * drm_crtc_vblank_atomic_disable - Implements struct &drm_crtc_helper_funcs.atomic_disable
+ * @crtc: The CRTC
+ * @state: The atomic state
+ *
+ * The helper drm_crtc_vblank_atomic_disable() implements atomic_disable
+ * of struct drm_crtc_helper_funcs for CRTCs the only need to disable VBLANKs.
+ *
+ * See also struct &drm_crtc_funcs.atomic_disable.
+ */
+void drm_crtc_vblank_atomic_disable(struct drm_crtc *crtc,
+				    struct drm_atomic_state *state)
+{
+	drm_crtc_vblank_off(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_disable);
+
 /*
  * VBLANK timer
  */
diff --git a/include/drm/drm_vblank_helper.h b/include/drm/drm_vblank_helper.h
index 74a971d0cfba..fcd8a9b35846 100644
--- a/include/drm/drm_vblank_helper.h
+++ b/include/drm/drm_vblank_helper.h
@@ -6,8 +6,31 @@
 #include <linux/hrtimer_types.h>
 #include <linux/types.h>
 
+struct drm_atomic_state;
 struct drm_crtc;
 
+/*
+ * VBLANK helpers
+ */
+
+void drm_crtc_vblank_atomic_flush(struct drm_crtc *crtc,
+				  struct drm_atomic_state *state);
+void drm_crtc_vblank_atomic_enable(struct drm_crtc *crtc,
+				   struct drm_atomic_state *state);
+void drm_crtc_vblank_atomic_disable(struct drm_crtc *crtc,
+				    struct drm_atomic_state *crtc_state);
+
+/**
+ * DRM_CRTC_HELPER_VBLANK_FUNCS - Default implementation for VBLANK helpers
+ *
+ * This macro initializes struct &drm_crtc_helper_funcs to default helpers
+ * for VBLANK handling.
+ */
+#define DRM_CRTC_HELPER_VBLANK_FUNCS \
+	.atomic_flush = drm_crtc_vblank_atomic_flush, \
+	.atomic_enable = drm_crtc_vblank_atomic_enable, \
+	.atomic_disable = drm_crtc_vblank_atomic_disable
+
 /*
  * VBLANK timer
  */
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/4] drm/vkms: Convert to DRM's vblank timer
  2025-09-04 14:56 [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 1/4] drm/vblank: Add vblank timer Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 2/4] drm/vblank: Add CRTC helpers for simple use cases Thomas Zimmermann
@ 2025-09-04 14:56 ` Thomas Zimmermann
  2025-09-04 14:56 ` [PATCH v3 4/4] drm/hypervdrm: Use " Thomas Zimmermann
  2025-09-05  5:35 ` [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Michael Kelley
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
  To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
	simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
  Cc: dri-devel, linux-hyperv, Thomas Zimmermann

Replace vkms' vblank timer with the DRM implementation. The DRM
code is identical in concept, but differs in implementation.

Vblank timers are covered in vblank helpers and initializer macros,
so remove the corresponding hrtimer in struct vkms_output. The
vblank timer calls vkms' custom timeout code via handle_vblank_timeout
in struct drm_crtc_helper_funcs.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 83 +++-----------------------------
 drivers/gpu/drm/vkms/vkms_drv.h  |  2 -
 2 files changed, 7 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index e60573e0f3e9..bd79f24686dc 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -7,25 +7,18 @@
 #include <drm/drm_managed.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
 
 #include "vkms_drv.h"
 
-static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
+static bool vkms_crtc_handle_vblank_timeout(struct drm_crtc *crtc)
 {
-	struct vkms_output *output = container_of(timer, struct vkms_output,
-						  vblank_hrtimer);
-	struct drm_crtc *crtc = &output->crtc;
+	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
 	struct vkms_crtc_state *state;
-	u64 ret_overrun;
 	bool ret, fence_cookie;
 
 	fence_cookie = dma_fence_begin_signalling();
 
-	ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
-					  output->period_ns);
-	if (ret_overrun != 1)
-		pr_warn("%s: vblank timer overrun\n", __func__);
-
 	spin_lock(&output->lock);
 	ret = drm_crtc_handle_vblank(crtc);
 	if (!ret)
@@ -57,55 +50,6 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 
 	dma_fence_end_signalling(fence_cookie);
 
-	return HRTIMER_RESTART;
-}
-
-static int vkms_enable_vblank(struct drm_crtc *crtc)
-{
-	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
-	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-
-	hrtimer_setup(&out->vblank_hrtimer, &vkms_vblank_simulate, CLOCK_MONOTONIC,
-		      HRTIMER_MODE_REL);
-	out->period_ns = ktime_set(0, vblank->framedur_ns);
-	hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_REL);
-
-	return 0;
-}
-
-static void vkms_disable_vblank(struct drm_crtc *crtc)
-{
-	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-
-	hrtimer_cancel(&out->vblank_hrtimer);
-}
-
-static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
-				      int *max_error, ktime_t *vblank_time,
-				      bool in_vblank_irq)
-{
-	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
-	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
-
-	if (!READ_ONCE(vblank->enabled)) {
-		*vblank_time = ktime_get();
-		return true;
-	}
-
-	*vblank_time = READ_ONCE(output->vblank_hrtimer.node.expires);
-
-	if (WARN_ON(*vblank_time == vblank->time))
-		return true;
-
-	/*
-	 * To prevent races we roll the hrtimer forward before we do any
-	 * interrupt processing - this is how real hw works (the interrupt is
-	 * only generated after all the vblank registers are updated) and what
-	 * the vblank core expects. Therefore we need to always correct the
-	 * timestampe by one frame.
-	 */
-	*vblank_time -= output->period_ns;
-
 	return true;
 }
 
@@ -159,9 +103,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 	.reset                  = vkms_atomic_crtc_reset,
 	.atomic_duplicate_state = vkms_atomic_crtc_duplicate_state,
 	.atomic_destroy_state   = vkms_atomic_crtc_destroy_state,
-	.enable_vblank		= vkms_enable_vblank,
-	.disable_vblank		= vkms_disable_vblank,
-	.get_vblank_timestamp	= vkms_get_vblank_timestamp,
+	DRM_CRTC_VBLANK_TIMER_FUNCS,
 	.get_crc_sources	= vkms_get_crc_sources,
 	.set_crc_source		= vkms_set_crc_source,
 	.verify_crc_source	= vkms_verify_crc_source,
@@ -213,18 +155,6 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 	return 0;
 }
 
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
-				    struct drm_atomic_state *state)
-{
-	drm_crtc_vblank_on(crtc);
-}
-
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
-				     struct drm_atomic_state *state)
-{
-	drm_crtc_vblank_off(crtc);
-}
-
 static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
 				   struct drm_atomic_state *state)
 	__acquires(&vkms_output->lock)
@@ -265,8 +195,9 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
 	.atomic_check	= vkms_crtc_atomic_check,
 	.atomic_begin	= vkms_crtc_atomic_begin,
 	.atomic_flush	= vkms_crtc_atomic_flush,
-	.atomic_enable	= vkms_crtc_atomic_enable,
-	.atomic_disable	= vkms_crtc_atomic_disable,
+	.atomic_enable	= drm_crtc_vblank_atomic_enable,
+	.atomic_disable	= drm_crtc_vblank_atomic_disable,
+	.handle_vblank_timeout = vkms_crtc_handle_vblank_timeout,
 };
 
 struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *primary,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8013c31efe3b..fb9711e1c6fb 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -215,8 +215,6 @@ struct vkms_output {
 	struct drm_crtc crtc;
 	struct drm_writeback_connector wb_connector;
 	struct drm_encoder wb_encoder;
-	struct hrtimer vblank_hrtimer;
-	ktime_t period_ns;
 	struct workqueue_struct *composer_workq;
 	spinlock_t lock;
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/4] drm/hypervdrm: Use vblank timer
  2025-09-04 14:56 [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2025-09-04 14:56 ` [PATCH v3 3/4] drm/vkms: Convert to DRM's vblank timer Thomas Zimmermann
@ 2025-09-04 14:56 ` Thomas Zimmermann
  2025-09-05  5:35 ` [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Michael Kelley
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
  To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
	simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
  Cc: dri-devel, linux-hyperv, Thomas Zimmermann

HyperV's virtual hardware does not provide vblank interrupts. Use a
vblank timer to simulate the interrupt. Rate-limits the display's
update frequency to the display-mode settings. Avoids excessive CPU
overhead with compositors that do not rate-limit their output.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 945b9482bcb3..6e6eb1c12a68 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -19,6 +19,8 @@
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_panic.h>
 #include <drm/drm_plane.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
 
 #include "hyperv_drm.h"
 
@@ -111,11 +113,15 @@ static void hyperv_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 				crtc_state->mode.hdisplay,
 				crtc_state->mode.vdisplay,
 				plane_state->fb->pitches[0]);
+
+	drm_crtc_vblank_on(crtc);
 }
 
 static const struct drm_crtc_helper_funcs hyperv_crtc_helper_funcs = {
 	.atomic_check = drm_crtc_helper_atomic_check,
+	.atomic_flush = drm_crtc_vblank_atomic_flush,
 	.atomic_enable = hyperv_crtc_helper_atomic_enable,
+	.atomic_disable = drm_crtc_vblank_atomic_disable,
 };
 
 static const struct drm_crtc_funcs hyperv_crtc_funcs = {
@@ -125,6 +131,7 @@ static const struct drm_crtc_funcs hyperv_crtc_funcs = {
 	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+	DRM_CRTC_VBLANK_TIMER_FUNCS,
 };
 
 static int hyperv_plane_atomic_check(struct drm_plane *plane,
@@ -321,6 +328,10 @@ int hyperv_mode_config_init(struct hyperv_drm_device *hv)
 		return ret;
 	}
 
+	ret = drm_vblank_init(dev, 1);
+	if (ret)
+		return ret;
+
 	drm_mode_config_reset(dev);
 
 	return 0;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-04 14:56 [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2025-09-04 14:56 ` [PATCH v3 4/4] drm/hypervdrm: Use " Thomas Zimmermann
@ 2025-09-05  5:35 ` Michael Kelley
  2025-09-09  3:29   ` Michael Kelley
  4 siblings, 1 reply; 12+ messages in thread
From: Michael Kelley @ 2025-09-05  5:35 UTC (permalink / raw)
  To: Thomas Zimmermann, louis.chauvet@bootlin.com,
	drawat.floss@gmail.com, hamohammed.sa@gmail.com,
	melissa.srw@gmail.com, simona@ffwll.ch, airlied@gmail.com,
	maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com,
	lyude@redhat.com, javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
> 
> Compositors often depend on vblanks to limit their display-update
> rate. Without, they see vblank events ASAP, which breaks the rate-
> limit feature. This creates high CPU overhead. It is especially a
> problem with virtual devices with fast framebuffer access.
> 
> The series moves vkms' vblank timer to DRM and converts the hyperv
> DRM driver. An earlier version of this series contains examples of
> other updated drivers. In principle, any DRM driver without vblank
> hardware can use the timer.

I've tested this patch set in a Hyper-V guest against the linux-next20250829
kernel. All looks good. Results and perf are the same as reported here [4].
So far I haven't seen the "vblank timer overrun" error, which is consistent
with the changes you made since my earlier testing. I'll keep running this
test kernel for a while to see if anything anomalous occurs.

For Patches 1, 2, and 4 of the series on a Hyper-V guest,

Tested-by: Michael Kelley <mhklinux@outlook.com>

[4] https://lore.kernel.org/dri-devel/20250523161522.409504-1-mhklinux@outlook.com/T/#m2e288dddaf7b3c025bbbf88da4b4c39e7aa950a7

> 
> The series has been motivated by a recent discussion about hypervdrm [1]
> and other long-standing bug reports. [2][3]
> 
> v3:
> - fix deadlock (Ville, Lyude)
> v2:
> - rework interfaces
> 
> [1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
> [2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
> [3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
> 
> Thomas Zimmermann (4):
>   drm/vblank: Add vblank timer
>   drm/vblank: Add CRTC helpers for simple use cases
>   drm/vkms: Convert to DRM's vblank timer
>   drm/hypervdrm: Use vblank timer
> 
>  Documentation/gpu/drm-kms-helpers.rst       |  12 ++
>  drivers/gpu/drm/Makefile                    |   3 +-
>  drivers/gpu/drm/drm_vblank.c                | 161 +++++++++++++++++-
>  drivers/gpu/drm/drm_vblank_helper.c         | 176 ++++++++++++++++++++
>  drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  11 ++
>  drivers/gpu/drm/vkms/vkms_crtc.c            |  83 +--------
>  drivers/gpu/drm/vkms/vkms_drv.h             |   2 -
>  include/drm/drm_modeset_helper_vtables.h    |  12 ++
>  include/drm/drm_vblank.h                    |  32 ++++
>  include/drm/drm_vblank_helper.h             |  56 +++++++
>  10 files changed, 467 insertions(+), 81 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
>  create mode 100644 include/drm/drm_vblank_helper.h
> 
> --
> 2.50.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-05  5:35 ` [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Michael Kelley
@ 2025-09-09  3:29   ` Michael Kelley
  2025-09-16  8:30     ` Thomas Zimmermann
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Kelley @ 2025-09-09  3:29 UTC (permalink / raw)
  To: Thomas Zimmermann, louis.chauvet@bootlin.com,
	drawat.floss@gmail.com, hamohammed.sa@gmail.com,
	melissa.srw@gmail.com, simona@ffwll.ch, airlied@gmail.com,
	maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com,
	lyude@redhat.com, javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
> 
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
> >
> > Compositors often depend on vblanks to limit their display-update
> > rate. Without, they see vblank events ASAP, which breaks the rate-
> > limit feature. This creates high CPU overhead. It is especially a
> > problem with virtual devices with fast framebuffer access.
> >
> > The series moves vkms' vblank timer to DRM and converts the hyperv
> > DRM driver. An earlier version of this series contains examples of
> > other updated drivers. In principle, any DRM driver without vblank
> > hardware can use the timer.
> 
> I've tested this patch set in a Hyper-V guest against the linux-next20250829
> kernel. All looks good. Results and perf are the same as reported here [4].
> So far I haven't seen the "vblank timer overrun" error, which is consistent
> with the changes you made since my earlier testing. I'll keep running this
> test kernel for a while to see if anything anomalous occurs.

As I continued to run with this patch set, I got a single occurrence of this
WARN_ON. I can't associate it with any particular action as I didn't notice
it until well after it occurred.

[213730.719364] ------------[ cut here ]------------
[213730.719423] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] drm_WARN_ON(!ktime_compare(*vblank_time, vblank->time))
[213730.719522] WARNING: drivers/gpu/drm/drm_vblank.c:2309 at drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm], CPU#4: kworker/4:0/7172
[213730.719871] Modules linked in: nls_iso8859_1(E) dm_multipath(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) binfmt_misc(E) intel_rapl_msr(E) intel_rapl_common(E) rapl(E) hyperv_fb(E) cfbfillrect(E) cfbimgblt(E) fb_io_fops(E) serio_raw(E) cfbcopyarea(E) hv_balloon(E) joydev(E) mac_hid(E) sch_fq_codel(E) msr(E) ramoops(E) reed_solomon(E) efi_pstore(E) autofs4(E) btrfs(E) blake2b_generic(E) raid10(E) raid456(E) async_raid6_recov(E) async_memcpy(E) async_pq(E) async_xor(E) async_tx(E) xor(E) raid6_pq(E) raid1(E) raid0(E) hyperv_drm(E) drm_client_lib(E) drm_shmem_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) drm_kms_helper(E) drm(E) drm_panel_orientation_quirks(E) fb(E) hid_generic(E) hid_hyperv(E) lcd(E) hid(E) hv_storvsc(E) ledtrig_backlight(E) hyperv_keyboard(E) hv_netvsc(E) hv_utils(E) scsi_transport_fc(E) ghash_clmulni_intel(E) hv_vmbus(E) aesni_intel(E)
[213730.720514] CPU: 4 UID: 0 PID: 7172 Comm: kworker/4:0 Kdump: loaded Tainted: G            E       6.17.0-rc3-next-20250829+ #3 PREEMPT(voluntary)
[213730.723220] Tainted: [E]=UNSIGNED_MODULE
[213730.724452] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 11/21/2024
[213730.724993] Workqueue: events drm_fb_helper_damage_work [drm_kms_helper]
[213730.725491] RIP: 0010:drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm]
[213730.726082] Code: 8b 7f 08 4c 8b 67 50 4d 85 e4 74 33 e8 99 b6 7f c7 48 c7 c1 60 e8 93 c0 4c 89 e2 48 c7 c7 b5 25 94 c0 48 89 c6 e8 00 06 e3 c6 <0f> 0b eb c0 e8 07 f6 f1 c6 48 89 03 5b 41 5c 5d c3 cc cc cc cc 4c
[213730.726506] RSP: 0018:ffffbba54e0efc00 EFLAGS: 00010282
[213730.726692] RAX: 0000000000000000 RBX: ffffbba54e0efc78 RCX: 0000000000000027
[213730.726899] RDX: ffff954f07d1cec8 RSI: 0000000000000001 RDI: ffff954f07d1cec0
[213730.727094] RBP: ffffbba54e0efc10 R08: 0000000000000000 R09: ffffbba54e0efa70
[213730.727280] R10: ffffbba54e0efa68 R11: ffffffff88d4c748 R12: ffff954e010a4cc0
[213730.727456] R13: 0000000000000000 R14: ffff954e20070d80 R15: ffff954e251002c8
[213730.727636] FS:  0000000000000000(0000) GS:ffff954f7e938000(0000) knlGS:0000000000000000
[213730.727834] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[213730.728009] CR2: 00007fe11629d010 CR3: 000000011f843004 CR4: 0000000000b70ef0
[213730.728186] Call Trace:
[213730.728359]  <TASK>
[213730.728511]  drm_crtc_vblank_helper_get_vblank_timestamp_from_timer+0x15/0x20 [drm_kms_helper]
[213730.728674]  drm_crtc_get_last_vbltimestamp+0x55/0x90 [drm]
[213730.728864]  drm_crtc_next_vblank_start+0x4e/0x90 [drm]
[213730.729043]  drm_atomic_helper_wait_for_fences+0x7c/0x1e0 [drm_kms_helper]
[213730.729196]  drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper]
[213730.729335]  drm_atomic_commit+0xb0/0xe0 [drm]
[213730.729481]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
[213730.729643]  drm_atomic_helper_dirtyfb+0x1aa/0x280 [drm_kms_helper]
[213730.729800]  drm_fbdev_shmem_helper_fb_dirty+0x4c/0xb0 [drm_shmem_helper]
[213730.729939]  drm_fb_helper_damage_work+0x8d/0x170 [drm_kms_helper]
[213730.730071]  process_one_work+0x19c/0x3f0
[213730.730204]  worker_thread+0x2c3/0x3d0
[213730.730332]  kthread+0x116/0x230
[213730.730459]  ? __pfx_worker_thread+0x10/0x10
[213730.730580]  ? _raw_spin_unlock_irq+0x12/0x40
[213730.730744]  ? __pfx_kthread+0x10/0x10
[213730.730898]  ret_from_fork+0xec/0x130
[213730.731027]  ? __pfx_kthread+0x10/0x10
[213730.731152]  ret_from_fork_asm+0x1a/0x30
[213730.731277]  </TASK>
[213730.731396] ---[ end trace 0000000000000000 ]---

> 
> For Patches 1, 2, and 4 of the series on a Hyper-V guest,
> 
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> 
> [4] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
> mhklinux@outlook.com/T/#m2e288dddaf7b3c025bbbf88da4b4c39e7aa950a7
> 
> >
> > The series has been motivated by a recent discussion about hypervdrm [1]
> > and other long-standing bug reports. [2][3]
> >
> > v3:
> > - fix deadlock (Ville, Lyude)
> > v2:
> > - rework interfaces
> >
> > [1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
> mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
> > [2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
> > [3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
> >
> > Thomas Zimmermann (4):
> >   drm/vblank: Add vblank timer
> >   drm/vblank: Add CRTC helpers for simple use cases
> >   drm/vkms: Convert to DRM's vblank timer
> >   drm/hypervdrm: Use vblank timer
> >
> >  Documentation/gpu/drm-kms-helpers.rst       |  12 ++
> >  drivers/gpu/drm/Makefile                    |   3 +-
> >  drivers/gpu/drm/drm_vblank.c                | 161 +++++++++++++++++-
> >  drivers/gpu/drm/drm_vblank_helper.c         | 176 ++++++++++++++++++++
> >  drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  11 ++
> >  drivers/gpu/drm/vkms/vkms_crtc.c            |  83 +--------
> >  drivers/gpu/drm/vkms/vkms_drv.h             |   2 -
> >  include/drm/drm_modeset_helper_vtables.h    |  12 ++
> >  include/drm/drm_vblank.h                    |  32 ++++
> >  include/drm/drm_vblank_helper.h             |  56 +++++++
> >  10 files changed, 467 insertions(+), 81 deletions(-)
> >  create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
> >  create mode 100644 include/drm/drm_vblank_helper.h
> >
> > --
> > 2.50.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-09  3:29   ` Michael Kelley
@ 2025-09-16  8:30     ` Thomas Zimmermann
  2025-09-16 15:00       ` Michael Kelley
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-16  8:30 UTC (permalink / raw)
  To: Michael Kelley, louis.chauvet@bootlin.com, drawat.floss@gmail.com,
	hamohammed.sa@gmail.com, melissa.srw@gmail.com, simona@ffwll.ch,
	airlied@gmail.com, maarten.lankhorst@linux.intel.com,
	ville.syrjala@linux.intel.com, lyude@redhat.com,
	javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

Hi

Am 09.09.25 um 05:29 schrieb Michael Kelley:
> From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
>>> Compositors often depend on vblanks to limit their display-update
>>> rate. Without, they see vblank events ASAP, which breaks the rate-
>>> limit feature. This creates high CPU overhead. It is especially a
>>> problem with virtual devices with fast framebuffer access.
>>>
>>> The series moves vkms' vblank timer to DRM and converts the hyperv
>>> DRM driver. An earlier version of this series contains examples of
>>> other updated drivers. In principle, any DRM driver without vblank
>>> hardware can use the timer.
>> I've tested this patch set in a Hyper-V guest against the linux-next20250829
>> kernel. All looks good. Results and perf are the same as reported here [4].
>> So far I haven't seen the "vblank timer overrun" error, which is consistent
>> with the changes you made since my earlier testing. I'll keep running this
>> test kernel for a while to see if anything anomalous occurs.
> As I continued to run with this patch set, I got a single occurrence of this
> WARN_ON. I can't associate it with any particular action as I didn't notice
> it until well after it occurred.

I've investigated. The stack trace comes from the kernel console's 
display update. It runs concurrently to the vblank timeout. What likely 
happens here is that the update code reads two values and in between, 
the vblank timeout updates them. So the update then compares an old and 
a new value; leading to an incorrect result with triggers the warning.

I'll include a fix in the series' next iteration. But I also think that 
it's not critical. DRM's vblank helpers can usually deal with such problems.

Best regards
Thomas


>
> [213730.719364] ------------[ cut here ]------------
> [213730.719423] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] drm_WARN_ON(!ktime_compare(*vblank_time, vblank->time))
> [213730.719522] WARNING: drivers/gpu/drm/drm_vblank.c:2309 at drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm], CPU#4: kworker/4:0/7172
> [213730.719871] Modules linked in: nls_iso8859_1(E) dm_multipath(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) binfmt_misc(E) intel_rapl_msr(E) intel_rapl_common(E) rapl(E) hyperv_fb(E) cfbfillrect(E) cfbimgblt(E) fb_io_fops(E) serio_raw(E) cfbcopyarea(E) hv_balloon(E) joydev(E) mac_hid(E) sch_fq_codel(E) msr(E) ramoops(E) reed_solomon(E) efi_pstore(E) autofs4(E) btrfs(E) blake2b_generic(E) raid10(E) raid456(E) async_raid6_recov(E) async_memcpy(E) async_pq(E) async_xor(E) async_tx(E) xor(E) raid6_pq(E) raid1(E) raid0(E) hyperv_drm(E) drm_client_lib(E) drm_shmem_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) drm_kms_helper(E) drm(E) drm_panel_orientation_quirks(E) fb(E) hid_generic(E) hid_hyperv(E) lcd(E) hid(E) hv_storvsc(E) ledtrig_backlight(E) hyperv_keyboard(E) hv_netvsc(E) hv_utils(E) scsi_transport_fc(E) ghash_clmulni_intel(E) hv_vmbus(E) aesni_intel(E)
> [213730.720514] CPU: 4 UID: 0 PID: 7172 Comm: kworker/4:0 Kdump: loaded Tainted: G            E       6.17.0-rc3-next-20250829+ #3 PREEMPT(voluntary)
> [213730.723220] Tainted: [E]=UNSIGNED_MODULE
> [213730.724452] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 11/21/2024
> [213730.724993] Workqueue: events drm_fb_helper_damage_work [drm_kms_helper]
> [213730.725491] RIP: 0010:drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm]
> [213730.726082] Code: 8b 7f 08 4c 8b 67 50 4d 85 e4 74 33 e8 99 b6 7f c7 48 c7 c1 60 e8 93 c0 4c 89 e2 48 c7 c7 b5 25 94 c0 48 89 c6 e8 00 06 e3 c6 <0f> 0b eb c0 e8 07 f6 f1 c6 48 89 03 5b 41 5c 5d c3 cc cc cc cc 4c
> [213730.726506] RSP: 0018:ffffbba54e0efc00 EFLAGS: 00010282
> [213730.726692] RAX: 0000000000000000 RBX: ffffbba54e0efc78 RCX: 0000000000000027
> [213730.726899] RDX: ffff954f07d1cec8 RSI: 0000000000000001 RDI: ffff954f07d1cec0
> [213730.727094] RBP: ffffbba54e0efc10 R08: 0000000000000000 R09: ffffbba54e0efa70
> [213730.727280] R10: ffffbba54e0efa68 R11: ffffffff88d4c748 R12: ffff954e010a4cc0
> [213730.727456] R13: 0000000000000000 R14: ffff954e20070d80 R15: ffff954e251002c8
> [213730.727636] FS:  0000000000000000(0000) GS:ffff954f7e938000(0000) knlGS:0000000000000000
> [213730.727834] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [213730.728009] CR2: 00007fe11629d010 CR3: 000000011f843004 CR4: 0000000000b70ef0
> [213730.728186] Call Trace:
> [213730.728359]  <TASK>
> [213730.728511]  drm_crtc_vblank_helper_get_vblank_timestamp_from_timer+0x15/0x20 [drm_kms_helper]
> [213730.728674]  drm_crtc_get_last_vbltimestamp+0x55/0x90 [drm]
> [213730.728864]  drm_crtc_next_vblank_start+0x4e/0x90 [drm]
> [213730.729043]  drm_atomic_helper_wait_for_fences+0x7c/0x1e0 [drm_kms_helper]
> [213730.729196]  drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper]
> [213730.729335]  drm_atomic_commit+0xb0/0xe0 [drm]
> [213730.729481]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
> [213730.729643]  drm_atomic_helper_dirtyfb+0x1aa/0x280 [drm_kms_helper]
> [213730.729800]  drm_fbdev_shmem_helper_fb_dirty+0x4c/0xb0 [drm_shmem_helper]
> [213730.729939]  drm_fb_helper_damage_work+0x8d/0x170 [drm_kms_helper]
> [213730.730071]  process_one_work+0x19c/0x3f0
> [213730.730204]  worker_thread+0x2c3/0x3d0
> [213730.730332]  kthread+0x116/0x230
> [213730.730459]  ? __pfx_worker_thread+0x10/0x10
> [213730.730580]  ? _raw_spin_unlock_irq+0x12/0x40
> [213730.730744]  ? __pfx_kthread+0x10/0x10
> [213730.730898]  ret_from_fork+0xec/0x130
> [213730.731027]  ? __pfx_kthread+0x10/0x10
> [213730.731152]  ret_from_fork_asm+0x1a/0x30
> [213730.731277]  </TASK>
> [213730.731396] ---[ end trace 0000000000000000 ]---
>
>> For Patches 1, 2, and 4 of the series on a Hyper-V guest,
>>
>> Tested-by: Michael Kelley <mhklinux@outlook.com>
>>
>> [4] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
>> mhklinux@outlook.com/T/#m2e288dddaf7b3c025bbbf88da4b4c39e7aa950a7
>>
>>> The series has been motivated by a recent discussion about hypervdrm [1]
>>> and other long-standing bug reports. [2][3]
>>>
>>> v3:
>>> - fix deadlock (Ville, Lyude)
>>> v2:
>>> - rework interfaces
>>>
>>> [1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
>> mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
>>> [2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
>>> [3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
>>>
>>> Thomas Zimmermann (4):
>>>    drm/vblank: Add vblank timer
>>>    drm/vblank: Add CRTC helpers for simple use cases
>>>    drm/vkms: Convert to DRM's vblank timer
>>>    drm/hypervdrm: Use vblank timer
>>>
>>>   Documentation/gpu/drm-kms-helpers.rst       |  12 ++
>>>   drivers/gpu/drm/Makefile                    |   3 +-
>>>   drivers/gpu/drm/drm_vblank.c                | 161 +++++++++++++++++-
>>>   drivers/gpu/drm/drm_vblank_helper.c         | 176 ++++++++++++++++++++
>>>   drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  11 ++
>>>   drivers/gpu/drm/vkms/vkms_crtc.c            |  83 +--------
>>>   drivers/gpu/drm/vkms/vkms_drv.h             |   2 -
>>>   include/drm/drm_modeset_helper_vtables.h    |  12 ++
>>>   include/drm/drm_vblank.h                    |  32 ++++
>>>   include/drm/drm_vblank_helper.h             |  56 +++++++
>>>   10 files changed, 467 insertions(+), 81 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
>>>   create mode 100644 include/drm/drm_vblank_helper.h
>>>
>>> --
>>> 2.50.1

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-16  8:30     ` Thomas Zimmermann
@ 2025-09-16 15:00       ` Michael Kelley
  2025-09-30 15:03         ` Thomas Zimmermann
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Kelley @ 2025-09-16 15:00 UTC (permalink / raw)
  To: Thomas Zimmermann, louis.chauvet@bootlin.com,
	drawat.floss@gmail.com, hamohammed.sa@gmail.com,
	melissa.srw@gmail.com, simona@ffwll.ch, airlied@gmail.com,
	maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com,
	lyude@redhat.com, javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, September 16, 2025 1:31 AM
> 
> Hi
> 
> Am 09.09.25 um 05:29 schrieb Michael Kelley:
> > From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
> >> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
> >>> Compositors often depend on vblanks to limit their display-update
> >>> rate. Without, they see vblank events ASAP, which breaks the rate-
> >>> limit feature. This creates high CPU overhead. It is especially a
> >>> problem with virtual devices with fast framebuffer access.
> >>>
> >>> The series moves vkms' vblank timer to DRM and converts the hyperv
> >>> DRM driver. An earlier version of this series contains examples of
> >>> other updated drivers. In principle, any DRM driver without vblank
> >>> hardware can use the timer.
> >> I've tested this patch set in a Hyper-V guest against the linux-next20250829
> >> kernel. All looks good. Results and perf are the same as reported here [4].
> >> So far I haven't seen the "vblank timer overrun" error, which is consistent
> >> with the changes you made since my earlier testing. I'll keep running this
> >> test kernel for a while to see if anything anomalous occurs.
> > As I continued to run with this patch set, I got a single occurrence of this
> > WARN_ON. I can't associate it with any particular action as I didn't notice
> > it until well after it occurred.
> 
> I've investigated. The stack trace comes from the kernel console's
> display update. It runs concurrently to the vblank timeout. What likely
> happens here is that the update code reads two values and in between,
> the vblank timeout updates them. So the update then compares an old and
> a new value; leading to an incorrect result with triggers the warning.
> 
> I'll include a fix in the series' next iteration. But I also think that
> it's not critical. DRM's vblank helpers can usually deal with such problems.

Thanks! I'm giving your v4 series a try now. Good that the underlying
problem is not critical. But I was seeing the WARN_ON() output in
dmesg every few days (a total of 4 times now), and that's not really
acceptable even if everything continues to work correctly.

Michael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-16 15:00       ` Michael Kelley
@ 2025-09-30 15:03         ` Thomas Zimmermann
  2025-09-30 15:18           ` Michael Kelley
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2025-09-30 15:03 UTC (permalink / raw)
  To: Michael Kelley, louis.chauvet@bootlin.com, drawat.floss@gmail.com,
	hamohammed.sa@gmail.com, melissa.srw@gmail.com, simona@ffwll.ch,
	airlied@gmail.com, maarten.lankhorst@linux.intel.com,
	ville.syrjala@linux.intel.com, lyude@redhat.com,
	javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

Hi

Am 16.09.25 um 17:00 schrieb Michael Kelley:
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, September 16, 2025 1:31 AM
>> Hi
>>
>> Am 09.09.25 um 05:29 schrieb Michael Kelley:
>>> From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
>>>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
>>>>> Compositors often depend on vblanks to limit their display-update
>>>>> rate. Without, they see vblank events ASAP, which breaks the rate-
>>>>> limit feature. This creates high CPU overhead. It is especially a
>>>>> problem with virtual devices with fast framebuffer access.
>>>>>
>>>>> The series moves vkms' vblank timer to DRM and converts the hyperv
>>>>> DRM driver. An earlier version of this series contains examples of
>>>>> other updated drivers. In principle, any DRM driver without vblank
>>>>> hardware can use the timer.
>>>> I've tested this patch set in a Hyper-V guest against the linux-next20250829
>>>> kernel. All looks good. Results and perf are the same as reported here [4].
>>>> So far I haven't seen the "vblank timer overrun" error, which is consistent
>>>> with the changes you made since my earlier testing. I'll keep running this
>>>> test kernel for a while to see if anything anomalous occurs.
>>> As I continued to run with this patch set, I got a single occurrence of this
>>> WARN_ON. I can't associate it with any particular action as I didn't notice
>>> it until well after it occurred.
>> I've investigated. The stack trace comes from the kernel console's
>> display update. It runs concurrently to the vblank timeout. What likely
>> happens here is that the update code reads two values and in between,
>> the vblank timeout updates them. So the update then compares an old and
>> a new value; leading to an incorrect result with triggers the warning.
>>
>> I'll include a fix in the series' next iteration. But I also think that
>> it's not critical. DRM's vblank helpers can usually deal with such problems.
> Thanks! I'm giving your v4 series a try now. Good that the underlying
> problem is not critical. But I was seeing the WARN_ON() output in
> dmesg every few days (a total of 4 times now), and that's not really
> acceptable even if everything continues to work correctly.

So it's probably a good sign that I haven't heard from you recently. :) 
If you haven't seen any warnings with v4, I'd like to merge the series soon.

Best regards
Thomas

>
> Michael

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-30 15:03         ` Thomas Zimmermann
@ 2025-09-30 15:18           ` Michael Kelley
  2025-10-01  7:06             ` Thomas Zimmermann
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Kelley @ 2025-09-30 15:18 UTC (permalink / raw)
  To: Thomas Zimmermann, louis.chauvet@bootlin.com,
	drawat.floss@gmail.com, hamohammed.sa@gmail.com,
	melissa.srw@gmail.com, simona@ffwll.ch, airlied@gmail.com,
	maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com,
	lyude@redhat.com, javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, September 30, 2025 8:04 AM
> 
> Hi
> 
> Am 16.09.25 um 17:00 schrieb Michael Kelley:
> > From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, September 16, 2025 1:31 AM
> >> Hi
> >>
> >> Am 09.09.25 um 05:29 schrieb Michael Kelley:
> >>> From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
> >>>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
> >>>>> Compositors often depend on vblanks to limit their display-update
> >>>>> rate. Without, they see vblank events ASAP, which breaks the rate-
> >>>>> limit feature. This creates high CPU overhead. It is especially a
> >>>>> problem with virtual devices with fast framebuffer access.
> >>>>>
> >>>>> The series moves vkms' vblank timer to DRM and converts the hyperv
> >>>>> DRM driver. An earlier version of this series contains examples of
> >>>>> other updated drivers. In principle, any DRM driver without vblank
> >>>>> hardware can use the timer.
> >>>> I've tested this patch set in a Hyper-V guest against the linux-next20250829
> >>>> kernel. All looks good. Results and perf are the same as reported here [4].
> >>>> So far I haven't seen the "vblank timer overrun" error, which is consistent
> >>>> with the changes you made since my earlier testing. I'll keep running this
> >>>> test kernel for a while to see if anything anomalous occurs.
> >>> As I continued to run with this patch set, I got a single occurrence of this
> >>> WARN_ON. I can't associate it with any particular action as I didn't notice
> >>> it until well after it occurred.
> >> I've investigated. The stack trace comes from the kernel console's
> >> display update. It runs concurrently to the vblank timeout. What likely
> >> happens here is that the update code reads two values and in between,
> >> the vblank timeout updates them. So the update then compares an old and
> >> a new value; leading to an incorrect result with triggers the warning.
> >>
> >> I'll include a fix in the series' next iteration. But I also think that
> >> it's not critical. DRM's vblank helpers can usually deal with such problems.
> > Thanks! I'm giving your v4 series a try now. Good that the underlying
> > problem is not critical. But I was seeing the WARN_ON() output in
> > dmesg every few days (a total of 4 times now), and that's not really
> > acceptable even if everything continues to work correctly.
> 
> So it's probably a good sign that I haven't heard from you recently. :)
> If you haven't seen any warnings with v4, I'd like to merge the series soon.
> 
> Best regards
> Thomas
> 

Yes, all is good. I was delayed a bit due to some travel, but the test
system has been running for a week now with no warnings or other
issues. From my standpoint, the patch series is good to merge.

Michael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
  2025-09-30 15:18           ` Michael Kelley
@ 2025-10-01  7:06             ` Thomas Zimmermann
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2025-10-01  7:06 UTC (permalink / raw)
  To: Michael Kelley, louis.chauvet@bootlin.com, drawat.floss@gmail.com,
	hamohammed.sa@gmail.com, melissa.srw@gmail.com, simona@ffwll.ch,
	airlied@gmail.com, maarten.lankhorst@linux.intel.com,
	ville.syrjala@linux.intel.com, lyude@redhat.com,
	javierm@redhat.com
  Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org

Hi

Am 30.09.25 um 17:18 schrieb Michael Kelley:
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, September 30, 2025 8:04 AM
>> Hi
>>
>> Am 16.09.25 um 17:00 schrieb Michael Kelley:
>>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, September 16, 2025 1:31 AM
>>>> Hi
>>>>
>>>> Am 09.09.25 um 05:29 schrieb Michael Kelley:
>>>>> From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
>>>>>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
>>>>>>> Compositors often depend on vblanks to limit their display-update
>>>>>>> rate. Without, they see vblank events ASAP, which breaks the rate-
>>>>>>> limit feature. This creates high CPU overhead. It is especially a
>>>>>>> problem with virtual devices with fast framebuffer access.
>>>>>>>
>>>>>>> The series moves vkms' vblank timer to DRM and converts the hyperv
>>>>>>> DRM driver. An earlier version of this series contains examples of
>>>>>>> other updated drivers. In principle, any DRM driver without vblank
>>>>>>> hardware can use the timer.
>>>>>> I've tested this patch set in a Hyper-V guest against the linux-next20250829
>>>>>> kernel. All looks good. Results and perf are the same as reported here [4].
>>>>>> So far I haven't seen the "vblank timer overrun" error, which is consistent
>>>>>> with the changes you made since my earlier testing. I'll keep running this
>>>>>> test kernel for a while to see if anything anomalous occurs.
>>>>> As I continued to run with this patch set, I got a single occurrence of this
>>>>> WARN_ON. I can't associate it with any particular action as I didn't notice
>>>>> it until well after it occurred.
>>>> I've investigated. The stack trace comes from the kernel console's
>>>> display update. It runs concurrently to the vblank timeout. What likely
>>>> happens here is that the update code reads two values and in between,
>>>> the vblank timeout updates them. So the update then compares an old and
>>>> a new value; leading to an incorrect result with triggers the warning.
>>>>
>>>> I'll include a fix in the series' next iteration. But I also think that
>>>> it's not critical. DRM's vblank helpers can usually deal with such problems.
>>> Thanks! I'm giving your v4 series a try now. Good that the underlying
>>> problem is not critical. But I was seeing the WARN_ON() output in
>>> dmesg every few days (a total of 4 times now), and that's not really
>>> acceptable even if everything continues to work correctly.
>> So it's probably a good sign that I haven't heard from you recently. :)
>> If you haven't seen any warnings with v4, I'd like to merge the series soon.
>>
>> Best regards
>> Thomas
>>
> Yes, all is good. I was delayed a bit due to some travel, but the test
> system has been running for a week now with no warnings or other
> issues. From my standpoint, the patch series is good to merge.

That's great news. I've now added the series to our drm-misc-next branch 
and it should reach upstream in v6.19.

Best regards
Thomas

>
> Michael

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-10-01  7:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-04 14:56 [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Thomas Zimmermann
2025-09-04 14:56 ` [PATCH v3 1/4] drm/vblank: Add vblank timer Thomas Zimmermann
2025-09-04 14:56 ` [PATCH v3 2/4] drm/vblank: Add CRTC helpers for simple use cases Thomas Zimmermann
2025-09-04 14:56 ` [PATCH v3 3/4] drm/vkms: Convert to DRM's vblank timer Thomas Zimmermann
2025-09-04 14:56 ` [PATCH v3 4/4] drm/hypervdrm: Use " Thomas Zimmermann
2025-09-05  5:35 ` [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts Michael Kelley
2025-09-09  3:29   ` Michael Kelley
2025-09-16  8:30     ` Thomas Zimmermann
2025-09-16 15:00       ` Michael Kelley
2025-09-30 15:03         ` Thomas Zimmermann
2025-09-30 15:18           ` Michael Kelley
2025-10-01  7:06             ` Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).