Hi Vivek
Am 29.10.25 um 06:47 schrieb Kasireddy, Vivek:
Hi Thomas, Dmitry,
Subject: Re: [PATCH v4] drm/virtgpu: Make vblank event dependent on the
external sync
On 10/27/25 12:52, Thomas Zimmermann wrote:
For each plane, store the buffer object's sync status in the state
of the plane's respective CRTC. During the CRTC's atomic flush,
ignore the vblank timer if any of the CRTC's plane's buffer object
is synchronized to an external source. Instead send the vblank event
immediately. Avoids corner cases where a vblank event happens too
late for the next frame to be page flipped in time.
Exporters of GEM objects sometimes interfere with the vblank timer;
resulting in framerate drops. Hence detect this case and handle it
as outlined above.
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.
v4:
- only handle imported GEM buffer objects (Dmitri, Vivek)
- fix test for plane visibility (Vivek)
- always enable vblank timer to make waiting clients happy
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]>
---
This patch was previously called "drm/virtgpu: Make vblank event
dependent on the host resource". Earlier versions where meant for
testing, rather than being merged. See [1] for a discussion of when
the fixed problem happens.
[1] https://lore.kernel.org/dri-devel/20251016145230.79270-1-
[email protected]/
---
drivers/gpu/drm/virtio/virtgpu_display.c | 67 ++++++++++++++++++++++--
drivers/gpu/drm/virtio/virtgpu_drv.h | 15 ++++++
drivers/gpu/drm/virtio/virtgpu_plane.c | 28 +++++++++-
3 files changed, 104 insertions(+), 6 deletions(-)
No problems spotted. I'd only extended comment in the code explicitly
stating that drm_gem_is_imported() is the workaround, might edit it on
applying. Will wait for Vivek's feedback. Thanks.
I applied this patch and tested 3 different scenarios:
1) blob=true + imported buffers
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
245 frames in 5 seconds: 49.000000 fps
236 frames in 5 seconds: 47.200001 fps
279 frames in 5 seconds: 55.799999 fps
240 frames in 5 seconds: 48.000000 fps
244 frames in 5 seconds: 48.799999 fps
283 frames in 5 seconds: 56.599998 fps
274 frames in 5 seconds: 54.799999 fps
2) blob=true
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
167 frames in 5 seconds: 33.400002 fps
208 frames in 5 seconds: 41.599998 fps
203 frames in 5 seconds: 40.599998 fps
252 frames in 5 seconds: 50.400002 fps
180 frames in 5 seconds: 36.000000 fps
219 frames in 5 seconds: 43.799999 fps
176 frames in 5 seconds: 35.200001 fps
220 frames in 5 seconds: 44.000000 fps
3) blob=false
root@localhost:/weston_upstream/weston# ./build/clients/weston-simple-egl -o &
[1] 1137
Using config: r8g8b8a0
has EGL_EXT_buffer_age and EGL_EXT_swap_buffers_with_damage
has EGL_EXT_surface_compression
285 frames in 5 seconds: 57.000000 fps
291 frames in 5 seconds: 58.200001 fps
290 frames in 5 seconds: 58.000000 fps
297 frames in 5 seconds: 59.400002 fps
291 frames in 5 seconds: 58.200001 fps
292 frames in 5 seconds: 58.400002 fps
The problem in the first 2 cases above is that as long as blob=true, the vblank
timer
is not helpful as we would be over-synchronizing. Note that even in the second
case, if blob=true, then even dumb BOs would satisfy bo->guest_blob condition.
So, I added the following changes to Thomas' patch and can now achieve FPS >= 58
in all 3 cases above:
Thanks for testing. These changes mostly revert the patch back to an
older rev. Let's merge this then to make progress with this bug. I'll
send you an update soon.
Best regards
Thomas
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
b/drivers/gpu/drm/virtio/virtgpu_display.c
index a4fa948023da..10c4a16a3908 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -159,7 +159,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,
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 634e1a930783..8381892e6d4e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -99,6 +99,8 @@ static const struct drm_plane_funcs virtio_gpu_plane_funcs = {
static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
struct drm_atomic_state *state)
{
+ struct drm_device *dev = plane->dev;
+ struct virtio_gpu_device *vgdev = dev->dev_private;
struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
plane);
struct drm_plane_state *old_plane_state =
drm_atomic_get_old_plane_state(state,
@@ -138,13 +140,14 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane
*plane,
for (i = 0; i < new_plane_state->fb->format->num_planes; ++i) {
struct drm_gem_object *obj =
drm_gem_fb_get_obj(new_plane_state->fb, i);
+ struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
/*
* Exporters of GEM buffer objects sometimes interfere with the
* vblank timer. Mark the plane as externally synchronized if
we
* find an imported GEM buffer object.
*/
- if (drm_gem_is_imported(obj)) {
+ if (bo->guest_blob && !vgdev->has_virgl_3d) {
vgcrtc_state->plane_synced_to_ext |=
drm_plane_mask(plane);
break; /* only need to find one */
For testing the 3 cases, I ran Gnome Wayland like this:
XDG_SESSION_TYPE=wayland dbus-run-session -- ./bin/gnome-shell --wayland --no-x11
&
and here are the relevant options for Qemu cmd line I tested with:
./qemu-system-x86_64 -m 4096m
-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 -usb -device usb-tablet -serial stdio
Thanks,
Vivek
Reviewed-by: Dmitry Osipenko <[email protected]>
Tested-by: Dmitry Osipenko <[email protected]>
--
Best regards,
Dmitry
--
--
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)
