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)
From 9ca0ecd3f0815474a3ecb798ada3be3af37bb8cd Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <[email protected]>
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 <[email protected]>
---
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
