[RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

For each plane, store the buffer object's host backing in the state
of the plane's respective CRTC. The host synchronizes output of buffer
objects with a host resource to its own refresh cycle; thus avoiding
tearing. During the CRTC's atomic flush, ignore the vblank timer if
any of the CRTC's plane's buffer object has a host resource. Instead
send the vblank event immdiatelly. Avoids corner cases where a vblank
event happens too late for the next frame to be page flipped in time.

The host synchronizes a plane's output to the host repaint cycle if
the plane's buffer object has an associated host resource. An atomic
commit blocks until the host cycle completes and then arms the vblank
event. The guest compositor is thereby synchronized to the host's
output rate.

To avoid delays, send out the vblank event immediately instead of
just arming it. Otherwise the event might be too late to page flip
the compositor's next frame.

The vblank timer is still active and fires in regular intervals
according to the guest display refresh. This rate limits clients
that only wait for the next vblank to occur, such as fbcon. These
clients would otherwise produce a very high number of frames per
second.

For commits without host resource, the vblank timer rate-limits the
guest output by generating vblank events at the guest display refresh
rate as before.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
There was a discussion about interference between vblank timers and
the host repaint cycle at [1]. This patch address a possible corner
case were the timing gets bad.

[1] https://lore.kernel.org/dri-devel/IA0PR11MB7185D91EB0CD01FD63D21AA7F8EEA@IA0PR11MB7185.namprd11.prod.outlook.com/
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 72 ++++++++++++++++++++++--
 drivers/gpu/drm/virtio/virtgpu_drv.h     | 15 +++++
 drivers/gpu/drm/virtio/virtgpu_plane.c   | 16 +++++-
 3 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index e972d9b015a9..43df1fa7d629 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -49,14 +49,76 @@
 #define drm_connector_to_virtio_gpu_output(x) \
 	container_of(x, struct virtio_gpu_output, conn)
 
+static void virtio_gpu_crtc_state_destroy(struct virtio_gpu_crtc_state *vgcrtc_state)
+{
+	__drm_atomic_helper_crtc_destroy_state(&vgcrtc_state->base);
+
+	kfree(vgcrtc_state);
+}
+
+static bool virtio_gpu_crtc_state_send_event_on_flush(struct virtio_gpu_crtc_state *vgcrtc_state)
+{
+	struct drm_crtc_state *crtc_state = &vgcrtc_state->base;
+
+	/*
+	 * The CRTC's output is vsync'ed if at least one plane's output is
+	 * sync'ed to the host refresh.
+	 */
+	return vgcrtc_state->send_event_on_flush & crtc_state->plane_mask;
+}
+
+static void virtio_gpu_crtc_reset(struct drm_crtc *crtc)
+{
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+
+	if (crtc->state)
+		virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc->state));
+
+	vgcrtc_state = kzalloc(sizeof(*vgcrtc_state), GFP_KERNEL);
+	if (vgcrtc_state) {
+		vgcrtc_state->send_event_on_flush = 0ul;
+		__drm_atomic_helper_crtc_reset(crtc, &vgcrtc_state->base);
+	} else {
+		__drm_atomic_helper_crtc_reset(crtc, NULL);
+	}
+}
+
+static struct drm_crtc_state *virtio_gpu_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_crtc_state *crtc_state = crtc->state;
+	struct virtio_gpu_crtc_state *new_vgcrtc_state;
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+
+	if (drm_WARN_ON(dev, !crtc_state))
+		return NULL;
+
+	new_vgcrtc_state = kzalloc(sizeof(*new_vgcrtc_state), GFP_KERNEL);
+	if (!new_vgcrtc_state)
+		return NULL;
+
+	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
+
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &new_vgcrtc_state->base);
+	vgcrtc_state->send_event_on_flush = vgcrtc_state->send_event_on_flush;
+
+	return &new_vgcrtc_state->base;
+}
+
+static void virtio_gpu_crtc_atomic_destroy_state(struct drm_crtc *crtc,
+						 struct drm_crtc_state *crtc_state)
+{
+	virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc_state));
+}
+
 static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
 	.set_config             = drm_atomic_helper_set_config,
 	.destroy                = drm_crtc_cleanup,
 
 	.page_flip              = drm_atomic_helper_page_flip,
-	.reset                  = drm_atomic_helper_crtc_reset,
-	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
-	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+	.reset                  = virtio_gpu_crtc_reset,
+	.atomic_duplicate_state = virtio_gpu_crtc_atomic_duplicate_state,
+	.atomic_destroy_state   = virtio_gpu_crtc_atomic_destroy_state,
 	DRM_CRTC_VBLANK_TIMER_FUNCS,
 };
 
@@ -129,7 +191,9 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
 	struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
+	bool send_event_on_flush = virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state);
 	struct drm_pending_vblank_event *event;
 
 	/*
@@ -147,7 +211,7 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 	crtc_state->event = NULL;
 
 	if (event) {
-		if (drm_crtc_vblank_get(crtc) == 0)
+		if (!send_event_on_flush && drm_crtc_vblank_get(crtc) == 0)
 			drm_crtc_arm_vblank_event(crtc, event);
 		else
 			drm_crtc_send_vblank_event(crtc, event);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..671fc3b61bc6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -195,6 +195,21 @@ struct virtio_gpu_framebuffer {
 #define to_virtio_gpu_framebuffer(x) \
 	container_of(x, struct virtio_gpu_framebuffer, base)
 
+struct virtio_gpu_crtc_state {
+	struct drm_crtc_state base;
+
+	/*
+	 * Send vblank event immediately from atomic_flush. Set from each
+	 * plane's atomic check and depends on the buffer object. Buffers
+	 * with host backing are vsync'd already and should send immediately;
+	 * others should wait for the VBLANK timer.
+	 */
+	u32 send_event_on_flush;
+};
+
+#define to_virtio_gpu_crtc_state(x) \
+	container_of(x, struct virtio_gpu_crtc_state, base)
+
 struct virtio_gpu_plane_state {
 	struct drm_plane_state base;
 	struct virtio_gpu_fence *fence;
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 29e4b458ae57..d04721c5d505 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -104,7 +104,8 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
 										 plane);
 	bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR;
 	struct drm_crtc_state *crtc_state;
-	int ret;
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+	int ret, i;
 
 	if (!new_plane_state->fb || WARN_ON(!new_plane_state->crtc))
 		return 0;
@@ -126,6 +127,19 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
 						  DRM_PLANE_NO_SCALING,
 						  DRM_PLANE_NO_SCALING,
 						  is_cursor, true);
+
+	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
+	vgcrtc_state->send_event_on_flush &= ~drm_plane_mask(plane);
+
+	for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
+		struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
+
+		if (bo->host3d_blob || bo->guest_blob) {
+			vgcrtc_state->send_event_on_flush |= drm_plane_mask(plane);
+			break; /* only need to find one */
+		}
+	}
+
 	return ret;
 }
 
-- 
2.51.0

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

On 10/16/25 17:48, Thomas Zimmermann wrote:
> For each plane, store the buffer object's host backing in the state
> of the plane's respective CRTC. The host synchronizes output of buffer
> objects with a host resource to its own refresh cycle; thus avoiding
> tearing. During the CRTC's atomic flush, ignore the vblank timer if
> any of the CRTC's plane's buffer object has a host resource. Instead
> send the vblank event immdiatelly. Avoids corner cases where a vblank
> event happens too late for the next frame to be page flipped in time.
> 
> The host synchronizes a plane's output to the host repaint cycle if
> the plane's buffer object has an associated host resource. An atomic
> commit blocks until the host cycle completes and then arms the vblank
> event. The guest compositor is thereby synchronized to the host's
> output rate.
> 
> To avoid delays, send out the vblank event immediately instead of
> just arming it. Otherwise the event might be too late to page flip
> the compositor's next frame.
> 
> The vblank timer is still active and fires in regular intervals
> according to the guest display refresh. This rate limits clients
> that only wait for the next vblank to occur, such as fbcon. These
> clients would otherwise produce a very high number of frames per
> second.
> 
> For commits without host resource, the vblank timer rate-limits the
> guest output by generating vblank events at the guest display refresh
> rate as before.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> There was a discussion about interference between vblank timers and
> the host repaint cycle at [1]. This patch address a possible corner
> case were the timing gets bad.
> 
> [1] https://lore.kernel.org/dri-devel/IA0PR11MB7185D91EB0CD01FD63D21AA7F8EEA@IA0PR11MB7185.namprd11.prod.outlook.com/
> ---
>  drivers/gpu/drm/virtio/virtgpu_display.c | 72 ++++++++++++++++++++++--
>  drivers/gpu/drm/virtio/virtgpu_drv.h     | 15 +++++
>  drivers/gpu/drm/virtio/virtgpu_plane.c   | 16 +++++-
>  3 files changed, 98 insertions(+), 5 deletions(-)

No problems spotted

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry

RE: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Kasireddy, Vivek]

Hi Thomas,

> Subject: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host
> resource
> 
> For each plane, store the buffer object's host backing in the state
> of the plane's respective CRTC. The host synchronizes output of buffer
> objects with a host resource to its own refresh cycle; thus avoiding
> tearing. During the CRTC's atomic flush, ignore the vblank timer if
> any of the CRTC's plane's buffer object has a host resource. Instead
> send the vblank event immdiatelly. Avoids corner cases where a vblank
> event happens too late for the next frame to be page flipped in time.
> 
> The host synchronizes a plane's output to the host repaint cycle if
> the plane's buffer object has an associated host resource. An atomic
> commit blocks until the host cycle completes and then arms the vblank
> event. The guest compositor is thereby synchronized to the host's
> output rate.
> 
> To avoid delays, send out the vblank event immediately instead of
> just arming it. Otherwise the event might be too late to page flip
> the compositor's next frame.
> 
> The vblank timer is still active and fires in regular intervals
> according to the guest display refresh. This rate limits clients
> that only wait for the next vblank to occur, such as fbcon. These
> clients would otherwise produce a very high number of frames per
> second.
> 
> For commits without host resource, the vblank timer rate-limits the
> guest output by generating vblank events at the guest display refresh
> rate as before.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> There was a discussion about interference between vblank timers and
> the host repaint cycle at [1]. This patch address a possible corner
> case were the timing gets bad.
> 
> [1] https://lore.kernel.org/dri-
> devel/IA0PR11MB7185D91EB0CD01FD63D21AA7F8EEA@IA0PR11MB7185.na
> mprd11.prod.outlook.com/
> ---
>  drivers/gpu/drm/virtio/virtgpu_display.c | 72 ++++++++++++++++++++++--
>  drivers/gpu/drm/virtio/virtgpu_drv.h     | 15 +++++
>  drivers/gpu/drm/virtio/virtgpu_plane.c   | 16 +++++-
>  3 files changed, 98 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index e972d9b015a9..43df1fa7d629 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -49,14 +49,76 @@
>  #define drm_connector_to_virtio_gpu_output(x) \
>  	container_of(x, struct virtio_gpu_output, conn)
> 
> +static void virtio_gpu_crtc_state_destroy(struct virtio_gpu_crtc_state
> *vgcrtc_state)
> +{
> +	__drm_atomic_helper_crtc_destroy_state(&vgcrtc_state->base);
> +
> +	kfree(vgcrtc_state);
> +}
> +
> +static bool virtio_gpu_crtc_state_send_event_on_flush(struct
> virtio_gpu_crtc_state *vgcrtc_state)
> +{
> +	struct drm_crtc_state *crtc_state = &vgcrtc_state->base;
> +
> +	/*
> +	 * The CRTC's output is vsync'ed if at least one plane's output is
> +	 * sync'ed to the host refresh.
> +	 */
> +	return vgcrtc_state->send_event_on_flush & crtc_state->plane_mask;
> +}
> +
> +static void virtio_gpu_crtc_reset(struct drm_crtc *crtc)
> +{
> +	struct virtio_gpu_crtc_state *vgcrtc_state;
> +
> +	if (crtc->state)
> +		virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc-
> >state));
> +
> +	vgcrtc_state = kzalloc(sizeof(*vgcrtc_state), GFP_KERNEL);
> +	if (vgcrtc_state) {
> +		vgcrtc_state->send_event_on_flush = 0ul;
> +		__drm_atomic_helper_crtc_reset(crtc, &vgcrtc_state->base);
> +	} else {
> +		__drm_atomic_helper_crtc_reset(crtc, NULL);
> +	}
> +}
> +
> +static struct drm_crtc_state *virtio_gpu_crtc_atomic_duplicate_state(struct
> drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_crtc_state *crtc_state = crtc->state;
> +	struct virtio_gpu_crtc_state *new_vgcrtc_state;
> +	struct virtio_gpu_crtc_state *vgcrtc_state;
> +
> +	if (drm_WARN_ON(dev, !crtc_state))
> +		return NULL;
> +
> +	new_vgcrtc_state = kzalloc(sizeof(*new_vgcrtc_state), GFP_KERNEL);
> +	if (!new_vgcrtc_state)
> +		return NULL;
> +
> +	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
> +
> +	__drm_atomic_helper_crtc_duplicate_state(crtc, &new_vgcrtc_state-
> >base);
> +	vgcrtc_state->send_event_on_flush = vgcrtc_state-
> >send_event_on_flush;
> +
> +	return &new_vgcrtc_state->base;
> +}
> +
> +static void virtio_gpu_crtc_atomic_destroy_state(struct drm_crtc *crtc,
> +						 struct drm_crtc_state *crtc_state)
> +{
> +	virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc_state));
> +}
> +
>  static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
>  	.set_config             = drm_atomic_helper_set_config,
>  	.destroy                = drm_crtc_cleanup,
> 
>  	.page_flip              = drm_atomic_helper_page_flip,
> -	.reset                  = drm_atomic_helper_crtc_reset,
> -	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> -	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> +	.reset                  = virtio_gpu_crtc_reset,
> +	.atomic_duplicate_state = virtio_gpu_crtc_atomic_duplicate_state,
> +	.atomic_destroy_state   = virtio_gpu_crtc_atomic_destroy_state,
>  	DRM_CRTC_VBLANK_TIMER_FUNCS,
>  };
> 
> @@ -129,7 +191,9 @@ static void virtio_gpu_crtc_atomic_flush(struct
> drm_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_crtc_state *crtc_state =
> drm_atomic_get_new_crtc_state(state, crtc);
> +	struct virtio_gpu_crtc_state *vgcrtc_state =
> to_virtio_gpu_crtc_state(crtc_state);
>  	struct virtio_gpu_output *output =
> drm_crtc_to_virtio_gpu_output(crtc);
> +	bool send_event_on_flush =
> virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state);
>  	struct drm_pending_vblank_event *event;
> 
>  	/*
> @@ -147,7 +211,7 @@ static void virtio_gpu_crtc_atomic_flush(struct
> drm_crtc *crtc,
>  	crtc_state->event = NULL;
> 
>  	if (event) {
> -		if (drm_crtc_vblank_get(crtc) == 0)
> +		if (!send_event_on_flush && drm_crtc_vblank_get(crtc) == 0)
>  			drm_crtc_arm_vblank_event(crtc, event);
>  		else
>  			drm_crtc_send_vblank_event(crtc, event);
As suspected, before applying this patch, the frame rate was halved:
root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-egl -o &
Using config: r8g8b8a0
has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
has EGL_EXT_surface_compression
150 frames in 5 seconds: 30.000000 fps
149 frames in 5 seconds: 29.799999 fps
152 frames in 5 seconds: 30.400000 fps

And, after applying this patch:
root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-egl -o &
Using config: r8g8b8a0
has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
has EGL_EXT_surface_compression
277 frames in 5 seconds: 55.400002 fps
273 frames in 5 seconds: 54.599998 fps
298 frames in 5 seconds: 59.599998 fps
284 frames in 5 seconds: 56.799999 fps
287 frames in 5 seconds: 57.400002 fps
272 frames in 5 seconds: 54.400002 fps
289 frames in 5 seconds: 57.799999 fps
287 frames in 5 seconds: 57.400002 fps
289 frames in 5 seconds: 57.799999 fps
272 frames in 5 seconds: 54.400002 fps
266 frames in 5 seconds: 53.200001 fps
261 frames in 5 seconds: 52.200001 fps
277 frames in 5 seconds: 55.400002 fps
256 frames in 5 seconds: 51.200001 fps
179 frames in 5 seconds: 35.799999 fps
169 frames in 5 seconds: 33.799999 fps
178 frames in 5 seconds: 35.599998 fps
211 frames in 5 seconds: 42.200001 fps
255 frames in 5 seconds: 51.000000 fps

As you can see, the frame rate/performance improves but it occasionally
drops into the 30s and 40s, which is a bit concerning because if I revert
this patch and a036f5fceedb ("drm/virtgpu: Use vblank timer") and test
again, I do not see this drop.

This suggests that the vblank event is still delayed in some other corner
cases, which might be challenging to figure out.

Tested by running Gnome Wayland after launching Qemu with following
(relevant) parameters:
qemu-system-x86_64 -m 4096m -enable-kvm .........
-device vfio-pci,host=0000:03:00.1
-device virtio-gpu,max_outputs=1,xres=1920,yres=1080,blob=true
-display gtk,gl=on
-object memory-backend-memfd,id=mem1,size=4096M
-machine q35,accel=kvm,memory-backend=mem1

Thanks,
Vivek

> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index f17660a71a3e..671fc3b61bc6 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -195,6 +195,21 @@ struct virtio_gpu_framebuffer {
>  #define to_virtio_gpu_framebuffer(x) \
>  	container_of(x, struct virtio_gpu_framebuffer, base)
> 
> +struct virtio_gpu_crtc_state {
> +	struct drm_crtc_state base;
> +
> +	/*
> +	 * Send vblank event immediately from atomic_flush. Set from each
> +	 * plane's atomic check and depends on the buffer object. Buffers
> +	 * with host backing are vsync'd already and should send immediately;
> +	 * others should wait for the VBLANK timer.
> +	 */
> +	u32 send_event_on_flush;
> +};
> +
> +#define to_virtio_gpu_crtc_state(x) \
> +	container_of(x, struct virtio_gpu_crtc_state, base)
> +
>  struct virtio_gpu_plane_state {
>  	struct drm_plane_state base;
>  	struct virtio_gpu_fence *fence;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
> b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 29e4b458ae57..d04721c5d505 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -104,7 +104,8 @@ static int virtio_gpu_plane_atomic_check(struct
> drm_plane *plane,
> 
> plane);
>  	bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR;
>  	struct drm_crtc_state *crtc_state;
> -	int ret;
> +	struct virtio_gpu_crtc_state *vgcrtc_state;
> +	int ret, i;
> 
>  	if (!new_plane_state->fb || WARN_ON(!new_plane_state->crtc))
>  		return 0;
> @@ -126,6 +127,19 @@ static int virtio_gpu_plane_atomic_check(struct
> drm_plane *plane,
>  						  DRM_PLANE_NO_SCALING,
>  						  DRM_PLANE_NO_SCALING,
>  						  is_cursor, true);
> +
> +	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
> +	vgcrtc_state->send_event_on_flush &= ~drm_plane_mask(plane);
> +
> +	for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
> +		struct virtio_gpu_object *bo =
> gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
> +
> +		if (bo->host3d_blob || bo->guest_blob) {
> +			vgcrtc_state->send_event_on_flush |=
> drm_plane_mask(plane);
> +			break; /* only need to find one */
> +		}
> +	}
> +
>  	return ret;
>  }
> 
> --
> 2.51.0

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

[-- Attachment #1: Type: text/plain, Size: 11491 bytes --]

Hi

Am 17.10.25 um 08:03 schrieb Kasireddy, Vivek:
> Hi Thomas,
>
>> Subject: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host
>> resource
>>
>> For each plane, store the buffer object's host backing in the state
>> of the plane's respective CRTC. The host synchronizes output of buffer
>> objects with a host resource to its own refresh cycle; thus avoiding
>> tearing. During the CRTC's atomic flush, ignore the vblank timer if
>> any of the CRTC's plane's buffer object has a host resource. Instead
>> send the vblank event immdiatelly. Avoids corner cases where a vblank
>> event happens too late for the next frame to be page flipped in time.
>>
>> The host synchronizes a plane's output to the host repaint cycle if
>> the plane's buffer object has an associated host resource. An atomic
>> commit blocks until the host cycle completes and then arms the vblank
>> event. The guest compositor is thereby synchronized to the host's
>> output rate.
>>
>> To avoid delays, send out the vblank event immediately instead of
>> just arming it. Otherwise the event might be too late to page flip
>> the compositor's next frame.
>>
>> The vblank timer is still active and fires in regular intervals
>> according to the guest display refresh. This rate limits clients
>> that only wait for the next vblank to occur, such as fbcon. These
>> clients would otherwise produce a very high number of frames per
>> second.
>>
>> For commits without host resource, the vblank timer rate-limits the
>> guest output by generating vblank events at the guest display refresh
>> rate as before.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>> There was a discussion about interference between vblank timers and
>> the host repaint cycle at [1]. This patch address a possible corner
>> case were the timing gets bad.
>>
>> [1] https://lore.kernel.org/dri-
>> devel/IA0PR11MB7185D91EB0CD01FD63D21AA7F8EEA@IA0PR11MB7185.na
>> mprd11.prod.outlook.com/
>> ---
>>   drivers/gpu/drm/virtio/virtgpu_display.c | 72 ++++++++++++++++++++++--
>>   drivers/gpu/drm/virtio/virtgpu_drv.h     | 15 +++++
>>   drivers/gpu/drm/virtio/virtgpu_plane.c   | 16 +++++-
>>   3 files changed, 98 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
>> b/drivers/gpu/drm/virtio/virtgpu_display.c
>> index e972d9b015a9..43df1fa7d629 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
>> @@ -49,14 +49,76 @@
>>   #define drm_connector_to_virtio_gpu_output(x) \
>>   	container_of(x, struct virtio_gpu_output, conn)
>>
>> +static void virtio_gpu_crtc_state_destroy(struct virtio_gpu_crtc_state
>> *vgcrtc_state)
>> +{
>> +	__drm_atomic_helper_crtc_destroy_state(&vgcrtc_state->base);
>> +
>> +	kfree(vgcrtc_state);
>> +}
>> +
>> +static bool virtio_gpu_crtc_state_send_event_on_flush(struct
>> virtio_gpu_crtc_state *vgcrtc_state)
>> +{
>> +	struct drm_crtc_state *crtc_state = &vgcrtc_state->base;
>> +
>> +	/*
>> +	 * The CRTC's output is vsync'ed if at least one plane's output is
>> +	 * sync'ed to the host refresh.
>> +	 */
>> +	return vgcrtc_state->send_event_on_flush & crtc_state->plane_mask;
>> +}
>> +
>> +static void virtio_gpu_crtc_reset(struct drm_crtc *crtc)
>> +{
>> +	struct virtio_gpu_crtc_state *vgcrtc_state;
>> +
>> +	if (crtc->state)
>> +		virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc-
>>> state));
>> +
>> +	vgcrtc_state = kzalloc(sizeof(*vgcrtc_state), GFP_KERNEL);
>> +	if (vgcrtc_state) {
>> +		vgcrtc_state->send_event_on_flush = 0ul;
>> +		__drm_atomic_helper_crtc_reset(crtc, &vgcrtc_state->base);
>> +	} else {
>> +		__drm_atomic_helper_crtc_reset(crtc, NULL);
>> +	}
>> +}
>> +
>> +static struct drm_crtc_state *virtio_gpu_crtc_atomic_duplicate_state(struct
>> drm_crtc *crtc)
>> +{
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_crtc_state *crtc_state = crtc->state;
>> +	struct virtio_gpu_crtc_state *new_vgcrtc_state;
>> +	struct virtio_gpu_crtc_state *vgcrtc_state;
>> +
>> +	if (drm_WARN_ON(dev, !crtc_state))
>> +		return NULL;
>> +
>> +	new_vgcrtc_state = kzalloc(sizeof(*new_vgcrtc_state), GFP_KERNEL);
>> +	if (!new_vgcrtc_state)
>> +		return NULL;
>> +
>> +	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
>> +
>> +	__drm_atomic_helper_crtc_duplicate_state(crtc, &new_vgcrtc_state-
>>> base);
>> +	vgcrtc_state->send_event_on_flush = vgcrtc_state-
>>> send_event_on_flush;
>> +
>> +	return &new_vgcrtc_state->base;
>> +}
>> +
>> +static void virtio_gpu_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>> +						 struct drm_crtc_state *crtc_state)
>> +{
>> +	virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc_state));
>> +}
>> +
>>   static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
>>   	.set_config             = drm_atomic_helper_set_config,
>>   	.destroy                = drm_crtc_cleanup,
>>
>>   	.page_flip              = drm_atomic_helper_page_flip,
>> -	.reset                  = drm_atomic_helper_crtc_reset,
>> -	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>> -	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
>> +	.reset                  = virtio_gpu_crtc_reset,
>> +	.atomic_duplicate_state = virtio_gpu_crtc_atomic_duplicate_state,
>> +	.atomic_destroy_state   = virtio_gpu_crtc_atomic_destroy_state,
>>   	DRM_CRTC_VBLANK_TIMER_FUNCS,
>>   };
>>
>> @@ -129,7 +191,9 @@ static void virtio_gpu_crtc_atomic_flush(struct
>> drm_crtc *crtc,
>>   {
>>   	struct drm_device *dev = crtc->dev;
>>   	struct drm_crtc_state *crtc_state =
>> drm_atomic_get_new_crtc_state(state, crtc);
>> +	struct virtio_gpu_crtc_state *vgcrtc_state =
>> to_virtio_gpu_crtc_state(crtc_state);
>>   	struct virtio_gpu_output *output =
>> drm_crtc_to_virtio_gpu_output(crtc);
>> +	bool send_event_on_flush =
>> virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state);
>>   	struct drm_pending_vblank_event *event;
>>
>>   	/*
>> @@ -147,7 +211,7 @@ static void virtio_gpu_crtc_atomic_flush(struct
>> drm_crtc *crtc,
>>   	crtc_state->event = NULL;
>>
>>   	if (event) {
>> -		if (drm_crtc_vblank_get(crtc) == 0)
>> +		if (!send_event_on_flush && drm_crtc_vblank_get(crtc) == 0)
>>   			drm_crtc_arm_vblank_event(crtc, event);
>>   		else
>>   			drm_crtc_send_vblank_event(crtc, event);
> As suspected, before applying this patch, the frame rate was halved:
> root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-egl -o &
> Using config: r8g8b8a0
> has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
> has EGL_EXT_surface_compression
> 150 frames in 5 seconds: 30.000000 fps
> 149 frames in 5 seconds: 29.799999 fps
> 152 frames in 5 seconds: 30.400000 fps
>
> And, after applying this patch:
> root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-egl -o &
> Using config: r8g8b8a0
> has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
> has EGL_EXT_surface_compression
> 277 frames in 5 seconds: 55.400002 fps
> 273 frames in 5 seconds: 54.599998 fps
> 298 frames in 5 seconds: 59.599998 fps
> 284 frames in 5 seconds: 56.799999 fps
> 287 frames in 5 seconds: 57.400002 fps
> 272 frames in 5 seconds: 54.400002 fps
> 289 frames in 5 seconds: 57.799999 fps
> 287 frames in 5 seconds: 57.400002 fps
> 289 frames in 5 seconds: 57.799999 fps
> 272 frames in 5 seconds: 54.400002 fps
> 266 frames in 5 seconds: 53.200001 fps
> 261 frames in 5 seconds: 52.200001 fps
> 277 frames in 5 seconds: 55.400002 fps
> 256 frames in 5 seconds: 51.200001 fps
> 179 frames in 5 seconds: 35.799999 fps
> 169 frames in 5 seconds: 33.799999 fps
> 178 frames in 5 seconds: 35.599998 fps
> 211 frames in 5 seconds: 42.200001 fps
> 255 frames in 5 seconds: 51.000000 fps
>
> As you can see, the frame rate/performance improves but it occasionally
> drops into the 30s and 40s, which is a bit concerning because if I revert
> this patch and a036f5fceedb ("drm/virtgpu: Use vblank timer") and test
> again, I do not see this drop.
>
> This suggests that the vblank event is still delayed in some other corner
> cases, which might be challenging to figure out.

There's little difference between the current event handling and the one 
where no vblanks have been set up. I suspect that the vblank timer 
callback interferes with the locking in atomic_flush. That would also 
explain why the fps drop at no clear pattern.

Could you please test the attached patch? It enables/disables the vblank 
timer depending on the buffer resources; as you suggested before.  Does 
this make a difference?

Best regards
Thomas

>
> Tested by running Gnome Wayland after launching Qemu with following
> (relevant) parameters:
> qemu-system-x86_64 -m 4096m -enable-kvm .........
> -device vfio-pci,host=0000:03:00.1
> -device virtio-gpu,max_outputs=1,xres=1920,yres=1080,blob=true
> -display gtk,gl=on
> -object memory-backend-memfd,id=mem1,size=4096M
> -machine q35,accel=kvm,memory-backend=mem1
>
> Thanks,
> Vivek
>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
>> b/drivers/gpu/drm/virtio/virtgpu_drv.h
>> index f17660a71a3e..671fc3b61bc6 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
>> @@ -195,6 +195,21 @@ struct virtio_gpu_framebuffer {
>>   #define to_virtio_gpu_framebuffer(x) \
>>   	container_of(x, struct virtio_gpu_framebuffer, base)
>>
>> +struct virtio_gpu_crtc_state {
>> +	struct drm_crtc_state base;
>> +
>> +	/*
>> +	 * Send vblank event immediately from atomic_flush. Set from each
>> +	 * plane's atomic check and depends on the buffer object. Buffers
>> +	 * with host backing are vsync'd already and should send immediately;
>> +	 * others should wait for the VBLANK timer.
>> +	 */
>> +	u32 send_event_on_flush;
>> +};
>> +
>> +#define to_virtio_gpu_crtc_state(x) \
>> +	container_of(x, struct virtio_gpu_crtc_state, base)
>> +
>>   struct virtio_gpu_plane_state {
>>   	struct drm_plane_state base;
>>   	struct virtio_gpu_fence *fence;
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> index 29e4b458ae57..d04721c5d505 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> @@ -104,7 +104,8 @@ static int virtio_gpu_plane_atomic_check(struct
>> drm_plane *plane,
>>
>> plane);
>>   	bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR;
>>   	struct drm_crtc_state *crtc_state;
>> -	int ret;
>> +	struct virtio_gpu_crtc_state *vgcrtc_state;
>> +	int ret, i;
>>
>>   	if (!new_plane_state->fb || WARN_ON(!new_plane_state->crtc))
>>   		return 0;
>> @@ -126,6 +127,19 @@ static int virtio_gpu_plane_atomic_check(struct
>> drm_plane *plane,
>>   						  DRM_PLANE_NO_SCALING,
>>   						  DRM_PLANE_NO_SCALING,
>>   						  is_cursor, true);
>> +
>> +	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
>> +	vgcrtc_state->send_event_on_flush &= ~drm_plane_mask(plane);
>> +
>> +	for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
>> +		struct virtio_gpu_object *bo =
>> gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
>> +
>> +		if (bo->host3d_blob || bo->guest_blob) {
>> +			vgcrtc_state->send_event_on_flush |=
>> drm_plane_mask(plane);
>> +			break; /* only need to find one */
>> +		}
>> +	}
>> +
>>   	return ret;
>>   }
>>
>> --
>> 2.51.0

-- 
--
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)


[-- Attachment #2: 0001-drm-virtgpu-Make-vblank-event-dependent-on-the-host-.patch --]
[-- Type: text/x-patch, Size: 9623 bytes --]

From f0768d9d7455f9c6eec1f97a2595f6a28eb1e638 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu, 16 Oct 2025 15:01:23 +0200
Subject: [PATCH] drm/virtgpu: Make vblank event dependent on the host resource

For each plane, store the buffer object's host backing in the state
of the plane's respective CRTC. The host synchronizes output of buffer
objects with a host resource to its own refresh cycle; thus avoiding
tearing. During the CRTC's atomic flush, ignore the vblank timer if
any of the CRTC's plane's buffer object has a host resource. Instead
send the vblank event immdiatelly. Avoids corner cases where a vblank
event happens too late for the next frame to be page flipped in time.

The host synchronizes a plane's output to the host repaint cycle if
the plane's buffer object has an associated host resource. An atomic
commit blocks until the host cycle completes and then arms the vblank
event. The guest compositor is thereby synchronized to the host's
output rate.

To avoid delays, send out the vblank event immediately instead of
just arming it. Otherwise the event might be too late to page flip
the compositor's next frame.

The vblank timer is still active and fires in regular intervals
according to the guest display refresh. This rate limits clients
that only wait for the next vblank to occur, such as fbcon. These
clients would otherwise produce a very high number of frames per
second.

For commits without host resource, the vblank timer rate-limits the
guest output by generating vblank events at the guest display refresh
rate as before.

v2:
- enable/disable vblank timer according to buffer setup

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 94 ++++++++++++++++++++++--
 drivers/gpu/drm/virtio/virtgpu_drv.h     | 14 ++++
 drivers/gpu/drm/virtio/virtgpu_plane.c   | 16 +++-
 3 files changed, 117 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index e972d9b015a9..71dc87e21e7c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -49,14 +49,76 @@
 #define drm_connector_to_virtio_gpu_output(x) \
 	container_of(x, struct virtio_gpu_output, conn)
 
+static void virtio_gpu_crtc_state_destroy(struct virtio_gpu_crtc_state *vgcrtc_state)
+{
+	__drm_atomic_helper_crtc_destroy_state(&vgcrtc_state->base);
+
+	kfree(vgcrtc_state);
+}
+
+static bool virtio_gpu_crtc_state_send_event_on_flush(struct virtio_gpu_crtc_state *vgcrtc_state)
+{
+	struct drm_crtc_state *crtc_state = &vgcrtc_state->base;
+
+	/*
+	 * The CRTC's output is vsync'ed if at least one plane's output is
+	 * sync'ed to the host refresh.
+	 */
+	return vgcrtc_state->plane_synced_to_host & crtc_state->plane_mask;
+}
+
+static void virtio_gpu_crtc_reset(struct drm_crtc *crtc)
+{
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+
+	if (crtc->state)
+		virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc->state));
+
+	vgcrtc_state = kzalloc(sizeof(*vgcrtc_state), GFP_KERNEL);
+	if (vgcrtc_state) {
+		vgcrtc_state->plane_synced_to_host = 0ul;
+		__drm_atomic_helper_crtc_reset(crtc, &vgcrtc_state->base);
+	} else {
+		__drm_atomic_helper_crtc_reset(crtc, NULL);
+	}
+}
+
+static struct drm_crtc_state *virtio_gpu_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_crtc_state *crtc_state = crtc->state;
+	struct virtio_gpu_crtc_state *new_vgcrtc_state;
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+
+	if (drm_WARN_ON(dev, !crtc_state))
+		return NULL;
+
+	new_vgcrtc_state = kzalloc(sizeof(*new_vgcrtc_state), GFP_KERNEL);
+	if (!new_vgcrtc_state)
+		return NULL;
+
+	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
+
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &new_vgcrtc_state->base);
+	vgcrtc_state->plane_synced_to_host = vgcrtc_state->plane_synced_to_host;
+
+	return &new_vgcrtc_state->base;
+}
+
+static void virtio_gpu_crtc_atomic_destroy_state(struct drm_crtc *crtc,
+						 struct drm_crtc_state *crtc_state)
+{
+	virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc_state));
+}
+
 static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
 	.set_config             = drm_atomic_helper_set_config,
 	.destroy                = drm_crtc_cleanup,
 
 	.page_flip              = drm_atomic_helper_page_flip,
-	.reset                  = drm_atomic_helper_crtc_reset,
-	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
-	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+	.reset                  = virtio_gpu_crtc_reset,
+	.atomic_duplicate_state = virtio_gpu_crtc_atomic_duplicate_state,
+	.atomic_destroy_state   = virtio_gpu_crtc_atomic_destroy_state,
 	DRM_CRTC_VBLANK_TIMER_FUNCS,
 };
 
@@ -102,7 +164,10 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
 					  struct drm_atomic_state *state)
 {
-	drm_crtc_vblank_on(crtc);
+	struct virtio_gpu_crtc_state *vgcrtc_state = to_virtio_gpu_crtc_state(crtc->state);
+
+	if (!virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state))
+		drm_crtc_vblank_on(crtc);
 }
 
 static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -111,8 +176,10 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
+	struct virtio_gpu_crtc_state *vgcrtc_state = to_virtio_gpu_crtc_state(crtc->state);
 
-	drm_crtc_vblank_off(crtc);
+	if (!virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state))
+		drm_crtc_vblank_off(crtc);
 
 	virtio_gpu_cmd_set_scanout(vgdev, output->index, 0, 0, 0, 0, 0);
 	virtio_gpu_notify(vgdev);
@@ -121,6 +188,19 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
 static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc,
 					struct drm_atomic_state *state)
 {
+	struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *new_vgcrtc_state = to_virtio_gpu_crtc_state(new_crtc_state);
+	struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *old_vgcrtc_state = to_virtio_gpu_crtc_state(old_crtc_state);
+
+	/*
+	 * Handling of vblank events changes across updates. Do a full modeset
+	 * to enable/disable the vblank timer.
+	 */
+	if (virtio_gpu_crtc_state_send_event_on_flush(new_vgcrtc_state) !=
+	    virtio_gpu_crtc_state_send_event_on_flush(old_vgcrtc_state))
+		new_crtc_state->mode_changed = true;
+
 	return 0;
 }
 
@@ -129,7 +209,9 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
 	struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
+	bool send_event_on_flush = virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state);
 	struct drm_pending_vblank_event *event;
 
 	/*
@@ -147,7 +229,7 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 	crtc_state->event = NULL;
 
 	if (event) {
-		if (drm_crtc_vblank_get(crtc) == 0)
+		if (!send_event_on_flush && drm_crtc_vblank_get(crtc) == 0)
 			drm_crtc_arm_vblank_event(crtc, event);
 		else
 			drm_crtc_send_vblank_event(crtc, event);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..ff1aac6e3c3a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -195,6 +195,20 @@ struct virtio_gpu_framebuffer {
 #define to_virtio_gpu_framebuffer(x) \
 	container_of(x, struct virtio_gpu_framebuffer, base)
 
+struct virtio_gpu_crtc_state {
+	struct drm_crtc_state base;
+
+	/*
+	 * Set from each plane's atomic check and depends on the plane's buffer
+	 * objects. Buffers with host resources are vsync'd already. Used by the
+	 * CRTC for vblank handling.
+	 */
+	u32 plane_synced_to_host;
+};
+
+#define to_virtio_gpu_crtc_state(x) \
+	container_of(x, struct virtio_gpu_crtc_state, base)
+
 struct virtio_gpu_plane_state {
 	struct drm_plane_state base;
 	struct virtio_gpu_fence *fence;
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 29e4b458ae57..d60539334a8e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -104,7 +104,8 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
 										 plane);
 	bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR;
 	struct drm_crtc_state *crtc_state;
-	int ret;
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+	int ret, i;
 
 	if (!new_plane_state->fb || WARN_ON(!new_plane_state->crtc))
 		return 0;
@@ -126,6 +127,19 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
 						  DRM_PLANE_NO_SCALING,
 						  DRM_PLANE_NO_SCALING,
 						  is_cursor, true);
+
+	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
+	vgcrtc_state->plane_synced_to_host &= ~drm_plane_mask(plane);
+
+	for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
+		struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
+
+		if (bo->host3d_blob || bo->guest_blob) {
+			vgcrtc_state->plane_synced_to_host |= drm_plane_mask(plane);
+			break; /* only need to find one */
+		}
+	}
+
 	return ret;
 }
 
-- 
2.51.0

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

Hi,

On 10/17/25 10:38, Thomas Zimmermann wrote:
...
> There's little difference between the current event handling and the one
> where no vblanks have been set up. I suspect that the vblank timer
> callback interferes with the locking in atomic_flush. That would also
> explain why the fps drop at no clear pattern.
> 
> Could you please test the attached patch? It enables/disables the vblank
> timer depending on the buffer resources; as you suggested before.  Does
> this make a difference?

The attached patch doesn't work, please see the trace below.

@Vivek Please clarify whether you only see frames drop with your
multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
problem with frames pacing for virgl and nctx modes yesterday, will
check again.

[   11.456513] ------------[ cut here ]------------
[   11.460050] driver forgot to call drm_crtc_vblank_off()
[   11.461253] WARNING: CPU: 12 PID: 445 at
drivers/gpu/drm/drm_atomic_helper.c:1279
drm_atomic_helper_commit_modeset_disables+0x6c6/0x6d0
[   11.461824] Modules linked in:
[   11.461989] CPU: 12 UID: 0 PID: 445 Comm: Xorg Not tainted
6.17.0-rc6-01184-ga95b3b198869 #174 PREEMPT(voluntary)
[   11.462638] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
[   11.463214] RIP:
0010:drm_atomic_helper_commit_modeset_disables+0x6c6/0x6d0
[   11.463574] Code: 32 53 02 01 e8 4b d1 33 ff 0f 0b 48 8b 43 08 e9 ae
fb ff ff 48 c7 c7 30 9f c7 83 89 45 c8 c6 05 e8 31 53 02 01 e8 2a d1 33
ff <0f> 0b 8b 45 c8 eb 9c 0f 1f 00 90 90 90 90 90 90 90 90 90 90 90 90
[   11.464613] RSP: 0018:ffffc90001007960 EFLAGS: 00010286
[   11.464906] RAX: 0000000000000000 RBX: ffff88810b24ff80 RCX:
0000000000000000
[   11.465301] RDX: 0000000000000002 RSI: 0000000000000001 RDI:
00000000ffffffff
[   11.465706] RBP: ffffc900010079a8 R08: 0000000000000000 R09:
ffffc90001007790
[   11.466104] R10: 0000000000000001 R11: 6620726576697264 R12:
0000000000000000
[   11.466495] R13: ffff888103920040 R14: 0000000000000000 R15:
0000000000000000
[   11.467043] FS:  00007f8d2868dec0(0000) GS:ffff8888d66f7000(0000)
knlGS:0000000000000000
[   11.467415] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   11.467688] CR2: 00007f8d285b1000 CR3: 000000010109a000 CR4:
0000000000750ef0
[   11.468023] PKRU: 55555554
[   11.468165] Call Trace:
[   11.468286]  <TASK>
[   11.468405]  drm_atomic_helper_commit_tail+0x1e/0x70
[   11.468635]  commit_tail+0x112/0x180
[   11.468798]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.469027]  drm_atomic_helper_commit+0x126/0x150
[   11.469244]  drm_atomic_commit+0xaa/0xe0
[   11.469432]  ? __pfx___drm_printfn_info+0x10/0x10
[   11.469651]  drm_atomic_helper_dirtyfb+0x1eb/0x2f0
[   11.469873]  drm_mode_dirtyfb_ioctl+0xfd/0x1c0
[   11.470078]  ? __pfx_drm_mode_dirtyfb_ioctl+0x10/0x10
[   11.470318]  drm_ioctl_kernel+0xa3/0x100
[   11.470497]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.470719]  drm_ioctl+0x2be/0x570
[   11.470877]  ? __pfx_drm_mode_dirtyfb_ioctl+0x10/0x10
[   11.471105]  ? do_syscall_64+0x1e7/0x1f0
[   11.471302]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.471529]  ? __mark_inode_dirty+0xc5/0x340
[   11.471735]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.471968]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.472199]  __x64_sys_ioctl+0x9a/0xf0
[   11.472377]  x64_sys_call+0x1009/0x1d80
[   11.472565]  do_syscall_64+0x6e/0x1f0
[   11.472731]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.472946]  ? __handle_mm_fault+0x92d/0xfa0
[   11.473157]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.473381]  ? debug_smp_processor_id+0x17/0x20
[   11.473596]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.473813]  ? count_memcg_events+0x93/0x230
[   11.474014]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.474242]  ? debug_smp_processor_id+0x17/0x20
[   11.474446]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.474669]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.474885]  ? irqentry_exit_to_user_mode+0x18c/0x190
[   11.475111]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.475338]  ? irqentry_exit+0x3b/0x50
[   11.475515]  ? srso_alias_return_thunk+0x5/0xfbef5
[   11.475730]  ? exc_page_fault+0x86/0x180
[   11.475918]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   11.476130] RIP: 0033:0x7f8d2891674d


-- 
Best regards,
Dmitry

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

On 10/17/25 16:58, Dmitry Osipenko wrote:
> Hi,
> 
> On 10/17/25 10:38, Thomas Zimmermann wrote:
> ...
>> There's little difference between the current event handling and the one
>> where no vblanks have been set up. I suspect that the vblank timer
>> callback interferes with the locking in atomic_flush. That would also
>> explain why the fps drop at no clear pattern.
>>
>> Could you please test the attached patch? It enables/disables the vblank
>> timer depending on the buffer resources; as you suggested before.  Does
>> this make a difference?
> 
> The attached patch doesn't work, please see the trace below.
> 
> @Vivek Please clarify whether you only see frames drop with your
> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
> problem with frames pacing for virgl and nctx modes yesterday, will
> check again.

On a second look, I now see that this RFC (not the attached) patch
doesn't work properly with host blobs.

I'm getting 100-150fps with this patch applied instead of expected
60fps. Without this RFC patch I'm getting constant 60fps with native
context displaying host blobs.

Not sure why guest blob would behave differently from the host blob.
Suspect something if off with the prime sharing that Vivek uses in the
vfio testing setup. I'd suggest to disable vblank timer only for guest
blobs if no quick solution will be found.

-- 
Best regards,
Dmitry

RE: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Kasireddy, Vivek]

Hi Dmitry,

> ...
> > There's little difference between the current event handling and the one
> > where no vblanks have been set up. I suspect that the vblank timer
> > callback interferes with the locking in atomic_flush. That would also
> > explain why the fps drop at no clear pattern.
> >
> > Could you please test the attached patch? It enables/disables the vblank
> > timer depending on the buffer resources; as you suggested before.  Does
> > this make a difference?
> 
> The attached patch doesn't work, please see the trace below.
> 
> @Vivek Please clarify whether you only see frames drop with your
> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
Sorry, I should have mentioned this detail about setup. I only (tested and)
see this frame drop in multi-gpu guest-blob scenarios.

Thanks,
Vivek

> problem with frames pacing for virgl and nctx modes yesterday, will
> check again.
> 
> [   11.456513] ------------[ cut here ]------------
> [   11.460050] driver forgot to call drm_crtc_vblank_off()
> [   11.461253] WARNING: CPU: 12 PID: 445 at
> drivers/gpu/drm/drm_atomic_helper.c:1279
> drm_atomic_helper_commit_modeset_disables+0x6c6/0x6d0
> [   11.461824] Modules linked in:
> [   11.461989] CPU: 12 UID: 0 PID: 445 Comm: Xorg Not tainted
> 6.17.0-rc6-01184-ga95b3b198869 #174 PREEMPT(voluntary)
> [   11.462638] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
> [   11.463214] RIP:
> 0010:drm_atomic_helper_commit_modeset_disables+0x6c6/0x6d0
> [   11.463574] Code: 32 53 02 01 e8 4b d1 33 ff 0f 0b 48 8b 43 08 e9 ae
> fb ff ff 48 c7 c7 30 9f c7 83 89 45 c8 c6 05 e8 31 53 02 01 e8 2a d1 33
> ff <0f> 0b 8b 45 c8 eb 9c 0f 1f 00 90 90 90 90 90 90 90 90 90 90 90 90
> [   11.464613] RSP: 0018:ffffc90001007960 EFLAGS: 00010286
> [   11.464906] RAX: 0000000000000000 RBX: ffff88810b24ff80 RCX:
> 0000000000000000
> [   11.465301] RDX: 0000000000000002 RSI: 0000000000000001 RDI:
> 00000000ffffffff
> [   11.465706] RBP: ffffc900010079a8 R08: 0000000000000000 R09:
> ffffc90001007790
> [   11.466104] R10: 0000000000000001 R11: 6620726576697264 R12:
> 0000000000000000
> [   11.466495] R13: ffff888103920040 R14: 0000000000000000 R15:
> 0000000000000000
> [   11.467043] FS:  00007f8d2868dec0(0000) GS:ffff8888d66f7000(0000)
> knlGS:0000000000000000
> [   11.467415] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   11.467688] CR2: 00007f8d285b1000 CR3: 000000010109a000 CR4:
> 0000000000750ef0
> [   11.468023] PKRU: 55555554
> [   11.468165] Call Trace:
> [   11.468286]  <TASK>
> [   11.468405]  drm_atomic_helper_commit_tail+0x1e/0x70
> [   11.468635]  commit_tail+0x112/0x180
> [   11.468798]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.469027]  drm_atomic_helper_commit+0x126/0x150
> [   11.469244]  drm_atomic_commit+0xaa/0xe0
> [   11.469432]  ? __pfx___drm_printfn_info+0x10/0x10
> [   11.469651]  drm_atomic_helper_dirtyfb+0x1eb/0x2f0
> [   11.469873]  drm_mode_dirtyfb_ioctl+0xfd/0x1c0
> [   11.470078]  ? __pfx_drm_mode_dirtyfb_ioctl+0x10/0x10
> [   11.470318]  drm_ioctl_kernel+0xa3/0x100
> [   11.470497]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.470719]  drm_ioctl+0x2be/0x570
> [   11.470877]  ? __pfx_drm_mode_dirtyfb_ioctl+0x10/0x10
> [   11.471105]  ? do_syscall_64+0x1e7/0x1f0
> [   11.471302]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.471529]  ? __mark_inode_dirty+0xc5/0x340
> [   11.471735]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.471968]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.472199]  __x64_sys_ioctl+0x9a/0xf0
> [   11.472377]  x64_sys_call+0x1009/0x1d80
> [   11.472565]  do_syscall_64+0x6e/0x1f0
> [   11.472731]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.472946]  ? __handle_mm_fault+0x92d/0xfa0
> [   11.473157]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.473381]  ? debug_smp_processor_id+0x17/0x20
> [   11.473596]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.473813]  ? count_memcg_events+0x93/0x230
> [   11.474014]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.474242]  ? debug_smp_processor_id+0x17/0x20
> [   11.474446]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.474669]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.474885]  ? irqentry_exit_to_user_mode+0x18c/0x190
> [   11.475111]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.475338]  ? irqentry_exit+0x3b/0x50
> [   11.475515]  ? srso_alias_return_thunk+0x5/0xfbef5
> [   11.475730]  ? exc_page_fault+0x86/0x180
> [   11.475918]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [   11.476130] RIP: 0033:0x7f8d2891674d
> 
> 
> --
> Best regards,
> Dmitry

RE: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Kasireddy, Vivek]

Hi Thomas,

> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the
> host resource
> 
> Hi
> 
> Am 17.10.25 um 08:03 schrieb Kasireddy, Vivek:
> > Hi Thomas,
> >
> >> Subject: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the
> host
> >> resource
> >>
> >> For each plane, store the buffer object's host backing in the state
> >> of the plane's respective CRTC. The host synchronizes output of buffer
> >> objects with a host resource to its own refresh cycle; thus avoiding
> >> tearing. During the CRTC's atomic flush, ignore the vblank timer if
> >> any of the CRTC's plane's buffer object has a host resource. Instead
> >> send the vblank event immdiatelly. Avoids corner cases where a vblank
> >> event happens too late for the next frame to be page flipped in time.
> >>
> >> The host synchronizes a plane's output to the host repaint cycle if
> >> the plane's buffer object has an associated host resource. An atomic
> >> commit blocks until the host cycle completes and then arms the vblank
> >> event. The guest compositor is thereby synchronized to the host's
> >> output rate.
> >>
> >> To avoid delays, send out the vblank event immediately instead of
> >> just arming it. Otherwise the event might be too late to page flip
> >> the compositor's next frame.
> >>
> >> The vblank timer is still active and fires in regular intervals
> >> according to the guest display refresh. This rate limits clients
> >> that only wait for the next vblank to occur, such as fbcon. These
> >> clients would otherwise produce a very high number of frames per
> >> second.
> >>
> >> For commits without host resource, the vblank timer rate-limits the
> >> guest output by generating vblank events at the guest display refresh
> >> rate as before.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >> ---
> >> There was a discussion about interference between vblank timers and
> >> the host repaint cycle at [1]. This patch address a possible corner
> >> case were the timing gets bad.
> >>
> >> [1] https://lore.kernel.org/dri-
> >>
> devel/IA0PR11MB7185D91EB0CD01FD63D21AA7F8EEA@IA0PR11MB7185.na
> >> mprd11.prod.outlook.com/
> >> ---
> >>   drivers/gpu/drm/virtio/virtgpu_display.c | 72 ++++++++++++++++++++++--
> >>   drivers/gpu/drm/virtio/virtgpu_drv.h     | 15 +++++
> >>   drivers/gpu/drm/virtio/virtgpu_plane.c   | 16 +++++-
> >>   3 files changed, 98 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> >> b/drivers/gpu/drm/virtio/virtgpu_display.c
> >> index e972d9b015a9..43df1fa7d629 100644
> >> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> >> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> >> @@ -49,14 +49,76 @@
> >>   #define drm_connector_to_virtio_gpu_output(x) \
> >>   	container_of(x, struct virtio_gpu_output, conn)
> >>
> >> +static void virtio_gpu_crtc_state_destroy(struct virtio_gpu_crtc_state
> >> *vgcrtc_state)
> >> +{
> >> +	__drm_atomic_helper_crtc_destroy_state(&vgcrtc_state->base);
> >> +
> >> +	kfree(vgcrtc_state);
> >> +}
> >> +
> >> +static bool virtio_gpu_crtc_state_send_event_on_flush(struct
> >> virtio_gpu_crtc_state *vgcrtc_state)
> >> +{
> >> +	struct drm_crtc_state *crtc_state = &vgcrtc_state->base;
> >> +
> >> +	/*
> >> +	 * The CRTC's output is vsync'ed if at least one plane's output is
> >> +	 * sync'ed to the host refresh.
> >> +	 */
> >> +	return vgcrtc_state->send_event_on_flush & crtc_state->plane_mask;
> >> +}
> >> +
> >> +static void virtio_gpu_crtc_reset(struct drm_crtc *crtc)
> >> +{
> >> +	struct virtio_gpu_crtc_state *vgcrtc_state;
> >> +
> >> +	if (crtc->state)
> >> +		virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc-
> >>> state));
> >> +
> >> +	vgcrtc_state = kzalloc(sizeof(*vgcrtc_state), GFP_KERNEL);
> >> +	if (vgcrtc_state) {
> >> +		vgcrtc_state->send_event_on_flush = 0ul;
> >> +		__drm_atomic_helper_crtc_reset(crtc, &vgcrtc_state->base);
> >> +	} else {
> >> +		__drm_atomic_helper_crtc_reset(crtc, NULL);
> >> +	}
> >> +}
> >> +
> >> +static struct drm_crtc_state
> *virtio_gpu_crtc_atomic_duplicate_state(struct
> >> drm_crtc *crtc)
> >> +{
> >> +	struct drm_device *dev = crtc->dev;
> >> +	struct drm_crtc_state *crtc_state = crtc->state;
> >> +	struct virtio_gpu_crtc_state *new_vgcrtc_state;
> >> +	struct virtio_gpu_crtc_state *vgcrtc_state;
> >> +
> >> +	if (drm_WARN_ON(dev, !crtc_state))
> >> +		return NULL;
> >> +
> >> +	new_vgcrtc_state = kzalloc(sizeof(*new_vgcrtc_state), GFP_KERNEL);
> >> +	if (!new_vgcrtc_state)
> >> +		return NULL;
> >> +
> >> +	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
> >> +
> >> +	__drm_atomic_helper_crtc_duplicate_state(crtc, &new_vgcrtc_state-
> >>> base);
> >> +	vgcrtc_state->send_event_on_flush = vgcrtc_state-
> >>> send_event_on_flush;
> >> +
> >> +	return &new_vgcrtc_state->base;
> >> +}
> >> +
> >> +static void virtio_gpu_crtc_atomic_destroy_state(struct drm_crtc *crtc,
> >> +						 struct drm_crtc_state *crtc_state)
> >> +{
> >> +	virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc_state));
> >> +}
> >> +
> >>   static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> >>   	.set_config             = drm_atomic_helper_set_config,
> >>   	.destroy                = drm_crtc_cleanup,
> >>
> >>   	.page_flip              = drm_atomic_helper_page_flip,
> >> -	.reset                  = drm_atomic_helper_crtc_reset,
> >> -	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> >> -	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> >> +	.reset                  = virtio_gpu_crtc_reset,
> >> +	.atomic_duplicate_state = virtio_gpu_crtc_atomic_duplicate_state,
> >> +	.atomic_destroy_state   = virtio_gpu_crtc_atomic_destroy_state,
> >>   	DRM_CRTC_VBLANK_TIMER_FUNCS,
> >>   };
> >>
> >> @@ -129,7 +191,9 @@ static void virtio_gpu_crtc_atomic_flush(struct
> >> drm_crtc *crtc,
> >>   {
> >>   	struct drm_device *dev = crtc->dev;
> >>   	struct drm_crtc_state *crtc_state =
> >> drm_atomic_get_new_crtc_state(state, crtc);
> >> +	struct virtio_gpu_crtc_state *vgcrtc_state =
> >> to_virtio_gpu_crtc_state(crtc_state);
> >>   	struct virtio_gpu_output *output =
> >> drm_crtc_to_virtio_gpu_output(crtc);
> >> +	bool send_event_on_flush =
> >> virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state);
> >>   	struct drm_pending_vblank_event *event;
> >>
> >>   	/*
> >> @@ -147,7 +211,7 @@ static void virtio_gpu_crtc_atomic_flush(struct
> >> drm_crtc *crtc,
> >>   	crtc_state->event = NULL;
> >>
> >>   	if (event) {
> >> -		if (drm_crtc_vblank_get(crtc) == 0)
> >> +		if (!send_event_on_flush && drm_crtc_vblank_get(crtc) == 0)
> >>   			drm_crtc_arm_vblank_event(crtc, event);
> >>   		else
> >>   			drm_crtc_send_vblank_event(crtc, event);
> > As suspected, before applying this patch, the frame rate was halved:
> > root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-
> egl -o &
> > Using config: r8g8b8a0
> > has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
> > has EGL_EXT_surface_compression
> > 150 frames in 5 seconds: 30.000000 fps
> > 149 frames in 5 seconds: 29.799999 fps
> > 152 frames in 5 seconds: 30.400000 fps
> >
> > And, after applying this patch:
> > root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-
> egl -o &
> > Using config: r8g8b8a0
> > has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
> > has EGL_EXT_surface_compression
> > 277 frames in 5 seconds: 55.400002 fps
> > 273 frames in 5 seconds: 54.599998 fps
> > 298 frames in 5 seconds: 59.599998 fps
> > 284 frames in 5 seconds: 56.799999 fps
> > 287 frames in 5 seconds: 57.400002 fps
> > 272 frames in 5 seconds: 54.400002 fps
> > 289 frames in 5 seconds: 57.799999 fps
> > 287 frames in 5 seconds: 57.400002 fps
> > 289 frames in 5 seconds: 57.799999 fps
> > 272 frames in 5 seconds: 54.400002 fps
> > 266 frames in 5 seconds: 53.200001 fps
> > 261 frames in 5 seconds: 52.200001 fps
> > 277 frames in 5 seconds: 55.400002 fps
> > 256 frames in 5 seconds: 51.200001 fps
> > 179 frames in 5 seconds: 35.799999 fps
> > 169 frames in 5 seconds: 33.799999 fps
> > 178 frames in 5 seconds: 35.599998 fps
> > 211 frames in 5 seconds: 42.200001 fps
> > 255 frames in 5 seconds: 51.000000 fps
> >
> > As you can see, the frame rate/performance improves but it occasionally
> > drops into the 30s and 40s, which is a bit concerning because if I revert
> > this patch and a036f5fceedb ("drm/virtgpu: Use vblank timer") and test
> > again, I do not see this drop.
> >
> > This suggests that the vblank event is still delayed in some other corner
> > cases, which might be challenging to figure out.
> 
> There's little difference between the current event handling and the one
> where no vblanks have been set up. I suspect that the vblank timer
> callback interferes with the locking in atomic_flush. That would also
> explain why the fps drop at no clear pattern.
> 
> Could you please test the attached patch? It enables/disables the vblank
> timer depending on the buffer resources; as you suggested before.  Does
> this make a difference?
Tested the attached patch and the frame rate/FPS is now consistent:
root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-egl -o &
Using config: r8g8b8a0
has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
has EGL_EXT_surface_compression
283 frames in 5 seconds: 56.599998 fps
300 frames in 5 seconds: 60.000000 fps
301 frames in 5 seconds: 60.200001 fps
300 frames in 5 seconds: 60.000000 fps
299 frames in 5 seconds: 59.799999 fps
300 frames in 5 seconds: 60.000000 fps
300 frames in 5 seconds: 60.000000 fps
301 frames in 5 seconds: 60.200001 fps
300 frames in 5 seconds: 60.000000 fps
297 frames in 5 seconds: 59.400002 fps
301 frames in 5 seconds: 60.200001 fps
300 frames in 5 seconds: 60.000000 fps
301 frames in 5 seconds: 60.200001 fps
301 frames in 5 seconds: 60.200001 fps
300 frames in 5 seconds: 60.000000 fps
301 frames in 5 seconds: 60.200001 fps

Thanks,
Vivek

> 
> Best regards
> Thomas
> 
> >
> > Tested by running Gnome Wayland after launching Qemu with following
> > (relevant) parameters:
> > qemu-system-x86_64 -m 4096m -enable-kvm .........
> > -device vfio-pci,host=0000:03:00.1
> > -device virtio-gpu,max_outputs=1,xres=1920,yres=1080,blob=true
> > -display gtk,gl=on
> > -object memory-backend-memfd,id=mem1,size=4096M
> > -machine q35,accel=kvm,memory-backend=mem1
> >
> > Thanks,
> > Vivek
> >
> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> >> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> >> index f17660a71a3e..671fc3b61bc6 100644
> >> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> >> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> >> @@ -195,6 +195,21 @@ struct virtio_gpu_framebuffer {
> >>   #define to_virtio_gpu_framebuffer(x) \
> >>   	container_of(x, struct virtio_gpu_framebuffer, base)
> >>
> >> +struct virtio_gpu_crtc_state {
> >> +	struct drm_crtc_state base;
> >> +
> >> +	/*
> >> +	 * Send vblank event immediately from atomic_flush. Set from each
> >> +	 * plane's atomic check and depends on the buffer object. Buffers
> >> +	 * with host backing are vsync'd already and should send immediately;
> >> +	 * others should wait for the VBLANK timer.
> >> +	 */
> >> +	u32 send_event_on_flush;
> >> +};
> >> +
> >> +#define to_virtio_gpu_crtc_state(x) \
> >> +	container_of(x, struct virtio_gpu_crtc_state, base)
> >> +
> >>   struct virtio_gpu_plane_state {
> >>   	struct drm_plane_state base;
> >>   	struct virtio_gpu_fence *fence;
> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
> >> b/drivers/gpu/drm/virtio/virtgpu_plane.c
> >> index 29e4b458ae57..d04721c5d505 100644
> >> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> >> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> >> @@ -104,7 +104,8 @@ static int virtio_gpu_plane_atomic_check(struct
> >> drm_plane *plane,
> >>
> >> plane);
> >>   	bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR;
> >>   	struct drm_crtc_state *crtc_state;
> >> -	int ret;
> >> +	struct virtio_gpu_crtc_state *vgcrtc_state;
> >> +	int ret, i;
> >>
> >>   	if (!new_plane_state->fb || WARN_ON(!new_plane_state->crtc))
> >>   		return 0;
> >> @@ -126,6 +127,19 @@ static int virtio_gpu_plane_atomic_check(struct
> >> drm_plane *plane,
> >>   						  DRM_PLANE_NO_SCALING,
> >>   						  DRM_PLANE_NO_SCALING,
> >>   						  is_cursor, true);
> >> +
> >> +	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
> >> +	vgcrtc_state->send_event_on_flush &= ~drm_plane_mask(plane);
> >> +
> >> +	for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
> >> +		struct virtio_gpu_object *bo =
> >> gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
> >> +
> >> +		if (bo->host3d_blob || bo->guest_blob) {
> >> +			vgcrtc_state->send_event_on_flush |=
> >> drm_plane_mask(plane);
> >> +			break; /* only need to find one */
> >> +		}
> >> +	}
> >> +
> >>   	return ret;
> >>   }
> >>
> >> --
> >> 2.51.0
> 
> --
> --
> 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)

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

Hi

Am 19.10.25 um 19:10 schrieb Dmitry Osipenko:
> On 10/17/25 16:58, Dmitry Osipenko wrote:
>> Hi,
>>
>> On 10/17/25 10:38, Thomas Zimmermann wrote:
>> ...
>>> There's little difference between the current event handling and the one
>>> where no vblanks have been set up. I suspect that the vblank timer
>>> callback interferes with the locking in atomic_flush. That would also
>>> explain why the fps drop at no clear pattern.
>>>
>>> Could you please test the attached patch? It enables/disables the vblank
>>> timer depending on the buffer resources; as you suggested before.  Does
>>> this make a difference?
>> The attached patch doesn't work, please see the trace below.
>>
>> @Vivek Please clarify whether you only see frames drop with your
>> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
>> problem with frames pacing for virgl and nctx modes yesterday, will
>> check again.
> On a second look, I now see that this RFC (not the attached) patch
> doesn't work properly with host blobs.
>
> I'm getting 100-150fps with this patch applied instead of expected
> 60fps. Without this RFC patch I'm getting constant 60fps with native
> context displaying host blobs.
>
> Not sure why guest blob would behave differently from the host blob.
> Suspect something if off with the prime sharing that Vivek uses in the
> vfio testing setup. I'd suggest to disable vblank timer only for guest
> blobs if no quick solution will be found.

After reading your reply and Vivek's new results, I'm confused now. Does 
it work or is there another patch needed?

Best regards
Thomas

>

-- 
--
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)

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

On 10/21/25 09:39, Thomas Zimmermann wrote:
> Hi
> 
> Am 19.10.25 um 19:10 schrieb Dmitry Osipenko:
>> On 10/17/25 16:58, Dmitry Osipenko wrote:
>>> Hi,
>>>
>>> On 10/17/25 10:38, Thomas Zimmermann wrote:
>>> ...
>>>> There's little difference between the current event handling and the
>>>> one
>>>> where no vblanks have been set up. I suspect that the vblank timer
>>>> callback interferes with the locking in atomic_flush. That would also
>>>> explain why the fps drop at no clear pattern.
>>>>
>>>> Could you please test the attached patch? It enables/disables the
>>>> vblank
>>>> timer depending on the buffer resources; as you suggested before.  Does
>>>> this make a difference?
>>> The attached patch doesn't work, please see the trace below.
>>>
>>> @Vivek Please clarify whether you only see frames drop with your
>>> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
>>> problem with frames pacing for virgl and nctx modes yesterday, will
>>> check again.
>> On a second look, I now see that this RFC (not the attached) patch
>> doesn't work properly with host blobs.
>>
>> I'm getting 100-150fps with this patch applied instead of expected
>> 60fps. Without this RFC patch I'm getting constant 60fps with native
>> context displaying host blobs.
>>
>> Not sure why guest blob would behave differently from the host blob.
>> Suspect something if off with the prime sharing that Vivek uses in the
>> vfio testing setup. I'd suggest to disable vblank timer only for guest
>> blobs if no quick solution will be found.
> 
> After reading your reply and Vivek's new results, I'm confused now. Does
> it work or is there another patch needed?

Didn't work for me, apparently worked for Vivek. Got a black screen,
flashing sometimes, and that error splat.

Now realized that I only tested with enabled virgl 3d context, while in
Vivek's case 3d is disabled. Will test further.

-- 
Best regards,
Dmitry

RE: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Kasireddy, Vivek]

Hi Thomas,

> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on
> the host resource
> 
> >>
> >> On 10/17/25 10:38, Thomas Zimmermann wrote:
> >> ...
> >>> There's little difference between the current event handling and the
> one
> >>> where no vblanks have been set up. I suspect that the vblank timer
> >>> callback interferes with the locking in atomic_flush. That would also
> >>> explain why the fps drop at no clear pattern.
> >>>
> >>> Could you please test the attached patch? It enables/disables the
> vblank
> >>> timer depending on the buffer resources; as you suggested
> before.  Does
> >>> this make a difference?
> >> The attached patch doesn't work, please see the trace below.
> >>
> >> @Vivek Please clarify whether you only see frames drop with your
> >> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
> >> problem with frames pacing for virgl and nctx modes yesterday, will
> >> check again.
> > On a second look, I now see that this RFC (not the attached) patch
> > doesn't work properly with host blobs.
> >
> > I'm getting 100-150fps with this patch applied instead of expected
> > 60fps. Without this RFC patch I'm getting constant 60fps with native
> > context displaying host blobs.
> >
> > Not sure why guest blob would behave differently from the host blob.
> > Suspect something if off with the prime sharing that Vivek uses in the
> > vfio testing setup. I'd suggest to disable vblank timer only for guest
> > blobs if no quick solution will be found.
> 
> After reading your reply and Vivek's new results, I'm confused now. Does
> it work or is there another patch needed?
Considering my use-case and Dmitry's virgl/venus/native context use-cases
and the benefits offered by vblank timer in these different scenarios, I think
the following patch should be sufficient for now:

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index e972d9b015a9..c1a8f88f0a20 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -102,7 +102,11 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
                                          struct drm_atomic_state *state)
 {
-       drm_crtc_vblank_on(crtc);
+       struct drm_device *dev = crtc->dev;
+       struct virtio_gpu_device *vgdev = dev->dev_private;
+
+       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
+               drm_crtc_vblank_on(crtc);
 }
 
 static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -112,7 +116,8 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
        struct virtio_gpu_device *vgdev = dev->dev_private;
        struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
 
-       drm_crtc_vblank_off(crtc);
+       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
+               drm_crtc_vblank_off(crtc);

Thanks,
Vivek

> 
> Best regards
> Thomas
> 
> >
> 
> --
> --
> 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)
> 

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

On 10/22/25 08:02, Kasireddy, Vivek wrote:
> Hi Thomas,
> 
>> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on
>> the host resource
>>
>>>>
>>>> On 10/17/25 10:38, Thomas Zimmermann wrote:
>>>> ...
>>>>> There's little difference between the current event handling and the
>> one
>>>>> where no vblanks have been set up. I suspect that the vblank timer
>>>>> callback interferes with the locking in atomic_flush. That would also
>>>>> explain why the fps drop at no clear pattern.
>>>>>
>>>>> Could you please test the attached patch? It enables/disables the
>> vblank
>>>>> timer depending on the buffer resources; as you suggested
>> before.  Does
>>>>> this make a difference?
>>>> The attached patch doesn't work, please see the trace below.
>>>>
>>>> @Vivek Please clarify whether you only see frames drop with your
>>>> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
>>>> problem with frames pacing for virgl and nctx modes yesterday, will
>>>> check again.
>>> On a second look, I now see that this RFC (not the attached) patch
>>> doesn't work properly with host blobs.
>>>
>>> I'm getting 100-150fps with this patch applied instead of expected
>>> 60fps. Without this RFC patch I'm getting constant 60fps with native
>>> context displaying host blobs.
>>>
>>> Not sure why guest blob would behave differently from the host blob.
>>> Suspect something if off with the prime sharing that Vivek uses in the
>>> vfio testing setup. I'd suggest to disable vblank timer only for guest
>>> blobs if no quick solution will be found.
>>
>> After reading your reply and Vivek's new results, I'm confused now. Does
>> it work or is there another patch needed?
> Considering my use-case and Dmitry's virgl/venus/native context use-cases
> and the benefits offered by vblank timer in these different scenarios, I think
> the following patch should be sufficient for now:
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
> index e972d9b015a9..c1a8f88f0a20 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -102,7 +102,11 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
>                                           struct drm_atomic_state *state)
>  {
> -       drm_crtc_vblank_on(crtc);
> +       struct drm_device *dev = crtc->dev;
> +       struct virtio_gpu_device *vgdev = dev->dev_private;
> +
> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
> +               drm_crtc_vblank_on(crtc);
>  }
>  
>  static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
> @@ -112,7 +116,8 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
>         struct virtio_gpu_device *vgdev = dev->dev_private;
>         struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
>  
> -       drm_crtc_vblank_off(crtc);
> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
> +               drm_crtc_vblank_off(crtc);

I'm fine with this change. Will need a clarifying comment in the code.

On the other hand, I tried with "-device virtio-vga,blob=true" and still
don't see problem with the incorrect frame rate.

Vivek, could you please clarify whether you only seeing problem when
using prime sharing? I.e. whether you can reproduce the wrong fps by
running qemu casually without using vfio.

Might test the vfio setup myself sometime later. It's a bit cumbersome
to set it up, will need to re-plug GPUs and etc, currently busy with
other stuff.

-- 
Best regards,
Dmitry

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

Hi

Am 22.10.25 um 07:37 schrieb Dmitry Osipenko:
> On 10/22/25 08:02, Kasireddy, Vivek wrote:
>> Hi Thomas,
>>
>>> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on
>>> the host resource
>>>
>>>>> On 10/17/25 10:38, Thomas Zimmermann wrote:
>>>>> ...
>>>>>> There's little difference between the current event handling and the
>>> one
>>>>>> where no vblanks have been set up. I suspect that the vblank timer
>>>>>> callback interferes with the locking in atomic_flush. That would also
>>>>>> explain why the fps drop at no clear pattern.
>>>>>>
>>>>>> Could you please test the attached patch? It enables/disables the
>>> vblank
>>>>>> timer depending on the buffer resources; as you suggested
>>> before.  Does
>>>>>> this make a difference?
>>>>> The attached patch doesn't work, please see the trace below.
>>>>>
>>>>> @Vivek Please clarify whether you only see frames drop with your
>>>>> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
>>>>> problem with frames pacing for virgl and nctx modes yesterday, will
>>>>> check again.
>>>> On a second look, I now see that this RFC (not the attached) patch
>>>> doesn't work properly with host blobs.
>>>>
>>>> I'm getting 100-150fps with this patch applied instead of expected
>>>> 60fps. Without this RFC patch I'm getting constant 60fps with native
>>>> context displaying host blobs.
>>>>
>>>> Not sure why guest blob would behave differently from the host blob.
>>>> Suspect something if off with the prime sharing that Vivek uses in the
>>>> vfio testing setup. I'd suggest to disable vblank timer only for guest
>>>> blobs if no quick solution will be found.
>>> After reading your reply and Vivek's new results, I'm confused now. Does
>>> it work or is there another patch needed?
>> Considering my use-case and Dmitry's virgl/venus/native context use-cases
>> and the benefits offered by vblank timer in these different scenarios, I think
>> the following patch should be sufficient for now:
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
>> index e972d9b015a9..c1a8f88f0a20 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
>> @@ -102,7 +102,11 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>   static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
>>                                            struct drm_atomic_state *state)
>>   {
>> -       drm_crtc_vblank_on(crtc);
>> +       struct drm_device *dev = crtc->dev;
>> +       struct virtio_gpu_device *vgdev = dev->dev_private;
>> +
>> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
>> +               drm_crtc_vblank_on(crtc);
>>   }
>>   
>>   static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
>> @@ -112,7 +116,8 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
>>          struct virtio_gpu_device *vgdev = dev->dev_private;
>>          struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
>>   
>> -       drm_crtc_vblank_off(crtc);
>> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
>> +               drm_crtc_vblank_off(crtc);

That's what my latest patch did, but it tracks the GEM object directly. 
Sending also requires the code in atomic_flush. Or no event will be send 
if vblanks are off.

Let me send another patch for testing.

Best regards
Thomas

> I'm fine with this change. Will need a clarifying comment in the code.
>
> On the other hand, I tried with "-device virtio-vga,blob=true" and still
> don't see problem with the incorrect frame rate.
>
> Vivek, could you please clarify whether you only seeing problem when
> using prime sharing? I.e. whether you can reproduce the wrong fps by
> running qemu casually without using vfio.
>
> Might test the vfio setup myself sometime later. It's a bit cumbersome
> to set it up, will need to re-plug GPUs and etc, currently busy with
> other stuff.
>

-- 
--
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)

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

[-- Attachment #1: Type: text/plain, Size: 4110 bytes --]

Hi

Am 22.10.25 um 07:37 schrieb Dmitry Osipenko:
> On 10/22/25 08:02, Kasireddy, Vivek wrote:
>> Hi Thomas,
>>
>>> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on
>>> the host resource
>>>
>>>>> On 10/17/25 10:38, Thomas Zimmermann wrote:
>>>>> ...
>>>>>> There's little difference between the current event handling and the
>>> one
>>>>>> where no vblanks have been set up. I suspect that the vblank timer
>>>>>> callback interferes with the locking in atomic_flush. That would also
>>>>>> explain why the fps drop at no clear pattern.
>>>>>>
>>>>>> Could you please test the attached patch? It enables/disables the
>>> vblank
>>>>>> timer depending on the buffer resources; as you suggested
>>> before.  Does
>>>>>> this make a difference?
>>>>> The attached patch doesn't work, please see the trace below.
>>>>>
>>>>> @Vivek Please clarify whether you only see frames drop with your
>>>>> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
>>>>> problem with frames pacing for virgl and nctx modes yesterday, will
>>>>> check again.
>>>> On a second look, I now see that this RFC (not the attached) patch
>>>> doesn't work properly with host blobs.
>>>>
>>>> I'm getting 100-150fps with this patch applied instead of expected
>>>> 60fps. Without this RFC patch I'm getting constant 60fps with native
>>>> context displaying host blobs.
>>>>
>>>> Not sure why guest blob would behave differently from the host blob.
>>>> Suspect something if off with the prime sharing that Vivek uses in the
>>>> vfio testing setup. I'd suggest to disable vblank timer only for guest
>>>> blobs if no quick solution will be found.
>>> After reading your reply and Vivek's new results, I'm confused now. Does
>>> it work or is there another patch needed?
>> Considering my use-case and Dmitry's virgl/venus/native context use-cases
>> and the benefits offered by vblank timer in these different scenarios, I think
>> the following patch should be sufficient for now:
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
>> index e972d9b015a9..c1a8f88f0a20 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
>> @@ -102,7 +102,11 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>   static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
>>                                            struct drm_atomic_state *state)
>>   {
>> -       drm_crtc_vblank_on(crtc);
>> +       struct drm_device *dev = crtc->dev;
>> +       struct virtio_gpu_device *vgdev = dev->dev_private;
>> +
>> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
>> +               drm_crtc_vblank_on(crtc);
>>   }
>>   
>>   static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
>> @@ -112,7 +116,8 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
>>          struct virtio_gpu_device *vgdev = dev->dev_private;
>>          struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
>>   
>> -       drm_crtc_vblank_off(crtc);
>> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
>> +               drm_crtc_vblank_off(crtc);
> I'm fine with this change. Will need a clarifying comment in the code.
>
> On the other hand, I tried with "-device virtio-vga,blob=true" and still
> don't see problem with the incorrect frame rate.
>
> Vivek, could you please clarify whether you only seeing problem when
> using prime sharing? I.e. whether you can reproduce the wrong fps by
> running qemu casually without using vfio.
>
> Might test the vfio setup myself sometime later. It's a bit cumbersome
> to set it up, will need to re-plug GPUs and etc, currently busy with
> other stuff.

Here's another variant of the patch for you to test. This should resolve 
the warning.

Best regards
Thomas


>

-- 
--
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)


[-- Attachment #2: 0001-drm-virtgpu-Make-vblank-event-dependent-on-the-host-.patch --]
[-- Type: text/x-patch, Size: 9154 bytes --]

From 9ca0ecd3f0815474a3ecb798ada3be3af37bb8cd Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu, 16 Oct 2025 15:01:23 +0200
Subject: [PATCH] drm/virtgpu: Make vblank event dependent on the host resource

For each plane, store the buffer object's host backing in the state
of the plane's respective CRTC. The host synchronizes output of buffer
objects with a host resource to its own refresh cycle; thus avoiding
tearing. During the CRTC's atomic flush, ignore the vblank timer if
any of the CRTC's plane's buffer object has a host resource. Instead
send the vblank event immdiatelly. Avoids corner cases where a vblank
event happens too late for the next frame to be page flipped in time.

The host synchronizes a plane's output to the host repaint cycle if
the plane's buffer object has an associated host resource. An atomic
commit blocks until the host cycle completes and then arms the vblank
event. The guest compositor is thereby synchronized to the host's
output rate.

To avoid delays, send out the vblank event immediately instead of
just arming it. Otherwise the event might be too late to page flip
the compositor's next frame.

The vblank timer is still active and fires in regular intervals
according to the guest display refresh. This rate limits clients
that only wait for the next vblank to occur, such as fbcon. These
clients would otherwise produce a very high number of frames per
second.

For commits without host resource, the vblank timer rate-limits the
guest output by generating vblank events at the guest display refresh
rate as before.

v3:
- disable vblank unconditionally
- compute status on each commit
v2:
- enable/disable vblank timer according to buffer setup

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 88 ++++++++++++++++++++++--
 drivers/gpu/drm/virtio/virtgpu_drv.h     | 14 ++++
 drivers/gpu/drm/virtio/virtgpu_plane.c   | 22 +++++-
 3 files changed, 117 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index e972d9b015a9..c381a20ca3c8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -49,14 +49,74 @@
 #define drm_connector_to_virtio_gpu_output(x) \
 	container_of(x, struct virtio_gpu_output, conn)
 
+static void virtio_gpu_crtc_state_destroy(struct virtio_gpu_crtc_state *vgcrtc_state)
+{
+	__drm_atomic_helper_crtc_destroy_state(&vgcrtc_state->base);
+
+	kfree(vgcrtc_state);
+}
+
+static bool virtio_gpu_crtc_state_send_event_on_flush(struct virtio_gpu_crtc_state *vgcrtc_state)
+{
+	struct drm_crtc_state *crtc_state = &vgcrtc_state->base;
+
+	/*
+	 * The CRTC's output is vsync'ed if at least one plane's output is
+	 * sync'ed to the host refresh.
+	 */
+	return vgcrtc_state->plane_synced_to_host & crtc_state->plane_mask;
+}
+
+static void virtio_gpu_crtc_reset(struct drm_crtc *crtc)
+{
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+
+	if (crtc->state)
+		virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc->state));
+
+	vgcrtc_state = kzalloc(sizeof(*vgcrtc_state), GFP_KERNEL);
+	if (vgcrtc_state) {
+		__drm_atomic_helper_crtc_reset(crtc, &vgcrtc_state->base);
+	} else {
+		__drm_atomic_helper_crtc_reset(crtc, NULL);
+	}
+}
+
+static struct drm_crtc_state *virtio_gpu_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_crtc_state *crtc_state = crtc->state;
+	struct virtio_gpu_crtc_state *new_vgcrtc_state;
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+
+	if (drm_WARN_ON(dev, !crtc_state))
+		return NULL;
+
+	new_vgcrtc_state = kzalloc(sizeof(*new_vgcrtc_state), GFP_KERNEL);
+	if (!new_vgcrtc_state)
+		return NULL;
+
+	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
+
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &new_vgcrtc_state->base);
+
+	return &new_vgcrtc_state->base;
+}
+
+static void virtio_gpu_crtc_atomic_destroy_state(struct drm_crtc *crtc,
+						 struct drm_crtc_state *crtc_state)
+{
+	virtio_gpu_crtc_state_destroy(to_virtio_gpu_crtc_state(crtc_state));
+}
+
 static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
 	.set_config             = drm_atomic_helper_set_config,
 	.destroy                = drm_crtc_cleanup,
 
 	.page_flip              = drm_atomic_helper_page_flip,
-	.reset                  = drm_atomic_helper_crtc_reset,
-	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
-	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+	.reset                  = virtio_gpu_crtc_reset,
+	.atomic_duplicate_state = virtio_gpu_crtc_atomic_duplicate_state,
+	.atomic_destroy_state   = virtio_gpu_crtc_atomic_destroy_state,
 	DRM_CRTC_VBLANK_TIMER_FUNCS,
 };
 
@@ -102,7 +162,10 @@ static void virtio_gpu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
 					  struct drm_atomic_state *state)
 {
-	drm_crtc_vblank_on(crtc);
+	struct virtio_gpu_crtc_state *vgcrtc_state = to_virtio_gpu_crtc_state(crtc->state);
+
+	if (!virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state))
+		drm_crtc_vblank_on(crtc);
 }
 
 static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -121,6 +184,19 @@ static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
 static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc,
 					struct drm_atomic_state *state)
 {
+	struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *new_vgcrtc_state = to_virtio_gpu_crtc_state(new_crtc_state);
+	struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *old_vgcrtc_state = to_virtio_gpu_crtc_state(old_crtc_state);
+
+	/*
+	 * Handling of vblank events changes across updates. Do a full modeset
+	 * to enable/disable the vblank timer.
+	 */
+	if (virtio_gpu_crtc_state_send_event_on_flush(new_vgcrtc_state) !=
+	    virtio_gpu_crtc_state_send_event_on_flush(old_vgcrtc_state))
+		new_crtc_state->mode_changed = true;
+
 	return 0;
 }
 
@@ -129,7 +205,9 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct virtio_gpu_crtc_state *vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
 	struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
+	bool send_event_on_flush = virtio_gpu_crtc_state_send_event_on_flush(vgcrtc_state);
 	struct drm_pending_vblank_event *event;
 
 	/*
@@ -147,7 +225,7 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 	crtc_state->event = NULL;
 
 	if (event) {
-		if (drm_crtc_vblank_get(crtc) == 0)
+		if (!send_event_on_flush && drm_crtc_vblank_get(crtc) == 0)
 			drm_crtc_arm_vblank_event(crtc, event);
 		else
 			drm_crtc_send_vblank_event(crtc, event);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..ba1c150b6a11 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -195,6 +195,20 @@ struct virtio_gpu_framebuffer {
 #define to_virtio_gpu_framebuffer(x) \
 	container_of(x, struct virtio_gpu_framebuffer, base)
 
+struct virtio_gpu_crtc_state {
+	struct drm_crtc_state base;
+
+	/*
+	 * Set from each plane's atomic check and depends on the plane's buffer
+	 * objects. Buffers with host resources are vsync'd already. Used by the
+	 * CRTC for vblank handling. Only valid during mode setting.
+	 */
+	u32 plane_synced_to_host;
+};
+
+#define to_virtio_gpu_crtc_state(x) \
+	container_of(x, struct virtio_gpu_crtc_state, base)
+
 struct virtio_gpu_plane_state {
 	struct drm_plane_state base;
 	struct virtio_gpu_fence *fence;
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 29e4b458ae57..31f6548ef0fe 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -104,7 +104,8 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
 										 plane);
 	bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR;
 	struct drm_crtc_state *crtc_state;
-	int ret;
+	struct virtio_gpu_crtc_state *vgcrtc_state;
+	int ret, i;
 
 	if (!new_plane_state->fb || WARN_ON(!new_plane_state->crtc))
 		return 0;
@@ -126,7 +127,24 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
 						  DRM_PLANE_NO_SCALING,
 						  DRM_PLANE_NO_SCALING,
 						  is_cursor, true);
-	return ret;
+	if (ret)
+		return ret;
+	else if (new_plane_state->visible)
+		return 0;
+
+	vgcrtc_state = to_virtio_gpu_crtc_state(crtc_state);
+	vgcrtc_state->plane_synced_to_host &= ~drm_plane_mask(plane);
+
+	for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
+		struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
+
+		if (bo->host3d_blob || bo->guest_blob) {
+			vgcrtc_state->plane_synced_to_host |= drm_plane_mask(plane);
+			break; /* only need to find one */
+		}
+	}
+
+	return 0;
 }
 
 /* For drm panic */
-- 
2.51.0

RE: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Kasireddy, Vivek]

Hi Thomas, Dmitry,

> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the
> host resource
> 
> Hi
> 
> Am 22.10.25 um 07:37 schrieb Dmitry Osipenko:
> > On 10/22/25 08:02, Kasireddy, Vivek wrote:
> >> Hi Thomas,
> >>
> >>> Subject: Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on
> >>> the host resource
> >>>
> >>>>> On 10/17/25 10:38, Thomas Zimmermann wrote:
> >>>>> ...
> >>>>>> There's little difference between the current event handling and the
> >>> one
> >>>>>> where no vblanks have been set up. I suspect that the vblank timer
> >>>>>> callback interferes with the locking in atomic_flush. That would also
> >>>>>> explain why the fps drop at no clear pattern.
> >>>>>>
> >>>>>> Could you please test the attached patch? It enables/disables the
> >>> vblank
> >>>>>> timer depending on the buffer resources; as you suggested
> >>> before.  Does
> >>>>>> this make a difference?
> >>>>> The attached patch doesn't work, please see the trace below.
> >>>>>
> >>>>> @Vivek Please clarify whether you only see frames drop with your
> >>>>> multi-gpu guest-blob setup or with a usual virgl too. I haven't noticed
> >>>>> problem with frames pacing for virgl and nctx modes yesterday, will
> >>>>> check again.
> >>>> On a second look, I now see that this RFC (not the attached) patch
> >>>> doesn't work properly with host blobs.
> >>>>
> >>>> I'm getting 100-150fps with this patch applied instead of expected
> >>>> 60fps. Without this RFC patch I'm getting constant 60fps with native
> >>>> context displaying host blobs.
> >>>>
> >>>> Not sure why guest blob would behave differently from the host blob.
> >>>> Suspect something if off with the prime sharing that Vivek uses in the
> >>>> vfio testing setup. I'd suggest to disable vblank timer only for guest
> >>>> blobs if no quick solution will be found.
> >>> After reading your reply and Vivek's new results, I'm confused now. Does
> >>> it work or is there another patch needed?
> >> Considering my use-case and Dmitry's virgl/venus/native context use-cases
> >> and the benefits offered by vblank timer in these different scenarios, I think
> >> the following patch should be sufficient for now:
> >>
> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> >> index e972d9b015a9..c1a8f88f0a20 100644
> >> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> >> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> >> @@ -102,7 +102,11 @@ static void virtio_gpu_crtc_mode_set_nofb(struct
> drm_crtc *crtc)
> >>   static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc,
> >>                                            struct drm_atomic_state *state)
> >>   {
> >> -       drm_crtc_vblank_on(crtc);
> >> +       struct drm_device *dev = crtc->dev;
> >> +       struct virtio_gpu_device *vgdev = dev->dev_private;
> >> +
> >> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
> >> +               drm_crtc_vblank_on(crtc);
> >>   }
> >>
> >>   static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
> >> @@ -112,7 +116,8 @@ static void virtio_gpu_crtc_atomic_disable(struct
> drm_crtc *crtc,
> >>          struct virtio_gpu_device *vgdev = dev->dev_private;
> >>          struct virtio_gpu_output *output =
> drm_crtc_to_virtio_gpu_output(crtc);
> >>
> >> -       drm_crtc_vblank_off(crtc);
> >> +       if (!vgdev->has_resource_blob || vgdev->has_virgl_3d)
> >> +               drm_crtc_vblank_off(crtc);
> > I'm fine with this change. Will need a clarifying comment in the code.
> >
> > On the other hand, I tried with "-device virtio-vga,blob=true" and still
> > don't see problem with the incorrect frame rate.
> >
> > Vivek, could you please clarify whether you only seeing problem when
> > using prime sharing? I.e. whether you can reproduce the wrong fps by
> > running qemu casually without using vfio.
I'll check again but looks like this problem of inconsistent FPS seems to be
only affecting prime sharing use-case.

> >
> > Might test the vfio setup myself sometime later. It's a bit cumbersome
> > to set it up, will need to re-plug GPUs and etc, currently busy with
> > other stuff.
> 
> Here's another variant of the patch for you to test. This should resolve
> the warning.
This new patch does not work (meaning FPS is low/inconsistent) while testing
my use-case. However, it does work if I invert the visibility check:
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 31f6548ef0fe..1ee8924b12c8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -129,7 +129,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
                                                  is_cursor, true);
        if (ret)
                return ret;
-       else if (new_plane_state->visible)
+       if (!new_plane_state->visible)
                return 0

Also, I think you might want to limit the plane sync to host mechanism to just guest
blobs only because based on what Dmitry said the vblank timer helps in virgl/venus/
native context use-cases. That is,
@@ -138,7 +140,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
        for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
                struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);

-               if (bo->host3d_blob || bo->guest_blob) {
+               if (bo->guest_blob && !vgdev->has_virgl_3d) {
                        vgcrtc_state->plane_synced_to_host |= drm_plane_mask(plane);
                        break; /* only need to find one */
                }

Thanks,
Vivek

> 
> Best regards
> Thomas
> 
> 
> >
> 
> --
> --
> 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)

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

Hi

Am 23.10.25 um 08:22 schrieb Kasireddy, Vivek:
[...]
>> Here's another variant of the patch for you to test. This should resolve
>> the warning.
> This new patch does not work (meaning FPS is low/inconsistent) while testing
> my use-case. However, it does work if I invert the visibility check:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 31f6548ef0fe..1ee8924b12c8 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -129,7 +129,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
>                                                    is_cursor, true);
>          if (ret)
>                  return ret;
> -       else if (new_plane_state->visible)
> +       if (!new_plane_state->visible)
>                  return 0

Oops, that's a bug in the patch. Thanks for the fix.

>
> Also, I think you might want to limit the plane sync to host mechanism to just guest
> blobs only because based on what Dmitry said the vblank timer helps in virgl/venus/
> native context use-cases. That is,
> @@ -138,7 +140,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
>          for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
>                  struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
>
> -               if (bo->host3d_blob || bo->guest_blob) {
> +               if (bo->guest_blob && !vgdev->has_virgl_3d) {

I've been looking at [1] for the test. In this case, I expected the 
output to be in sync already. I've also found that test in several other 
places. But OK, no problem.

[1] 
https://elixir.bootlin.com/linux/v6.17.4/source/drivers/gpu/drm/virtio/virtgpu_plane.c#L282

Best regards
Thomas

>                          vgcrtc_state->plane_synced_to_host |= drm_plane_mask(plane);
>                          break; /* only need to find one */
>                  }
>
> Thanks,
> Vivek
>
>> Best regards
>> Thomas
>>
>>
>> --
>> --
>> 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)

-- 
--
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)

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

On 10/23/25 09:22, Kasireddy, Vivek wrote:
> Also, I think you might want to limit the plane sync to host mechanism to just guest
> blobs only because based on what Dmitry said the vblank timer helps in virgl/venus/
> native context use-cases. That is,
> @@ -138,7 +140,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
>         for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
>                 struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
> 
> -               if (bo->host3d_blob || bo->guest_blob) {
> +               if (bo->guest_blob && !vgdev->has_virgl_3d) {

Checking for obj->import_attach should be enough if it's only prime
sharing that doesn't work properly with vblank timer.

Please verify that only prime needs the workaround and send the updated
patch.

-- 
Best regards,
Dmitry

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Dmitry Osipenko]

On 10/24/25 05:55, Dmitry Osipenko wrote:
> On 10/23/25 09:22, Kasireddy, Vivek wrote:
>> Also, I think you might want to limit the plane sync to host mechanism to just guest
>> blobs only because based on what Dmitry said the vblank timer helps in virgl/venus/
>> native context use-cases. That is,
>> @@ -138,7 +140,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
>>         for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
>>                 struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
>>
>> -               if (bo->host3d_blob || bo->guest_blob) {
>> +               if (bo->guest_blob && !vgdev->has_virgl_3d) {
> 
> Checking for obj->import_attach should be enough if it's only prime
> sharing that doesn't work properly with vblank timer.
> 
> Please verify that only prime needs the workaround and send the updated
> patch.

Don't forget to add clarifying comment to the code explaining the
workaround.

-- 
Best regards,
Dmitry

Re: [RFC][PATCH] drm/virtgpu: Make vblank event dependent on the host resource

[Thomas Zimmermann]

Hi

Am 24.10.25 um 04:57 schrieb Dmitry Osipenko:
> On 10/24/25 05:55, Dmitry Osipenko wrote:
>> On 10/23/25 09:22, Kasireddy, Vivek wrote:
>>> Also, I think you might want to limit the plane sync to host mechanism to just guest
>>> blobs only because based on what Dmitry said the vblank timer helps in virgl/venus/
>>> native context use-cases. That is,
>>> @@ -138,7 +140,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
>>>          for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
>>>                  struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(new_plane_state->fb->obj[i]);
>>>
>>> -               if (bo->host3d_blob || bo->guest_blob) {
>>> +               if (bo->guest_blob && !vgdev->has_virgl_3d) {
>> Checking for obj->import_attach should be enough if it's only prime
>> sharing that doesn't work properly with vblank timer.
>>
>> Please verify that only prime needs the workaround and send the updated
>> patch.
> Don't forget to add clarifying comment to the code explaining the
> workaround.

Sent out as "drm/virtgpu: Make vblank event dependent on the external sync".

Best regards
Thomas

>

-- 
--
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)