Hi
Am 10.10.25 um 06:25 schrieb Kasireddy, Vivek:
[...]
Because virtgpu did not initialize vblanking, DRM always sent out a
vblank event after the completed commit. [5] It's not synchronized to
the display output. This means that virtgpu has always been subject to
tearing and the guest always required to hold buffers until the host
completed its display update. IOW adding vblank timers into the mix
should not change the behavior of virtgpu. It just limits the output
As I mentioned, the output or display update frequency is already limited
when blob=true, so adding a vblank timer would be redundant in this case.
To summarize my understanding: virtgpu currently generates vblank events
as soon as if finished updating the display. Which can be too fast for
blob=false. For blob=true, the display update is (reasonably assumed to
be) synchronized with the host update and thereby rate limited by the
host's display output.
Adding the vblank timer: For blob=true, if a vblank timeout happens
during an ongoing display update, it does not send an event. If the
display update finishes before the vblank times out, it arms the vblank
event and the next vblank timeout will send it out. The vblank timer
thereby makes events appear at regular intervals instead of ASAP.
So why add the complexity of handling blob=true separately when it
should integrate well with the existing vblank framework?
Please also note that it's not just about compositors. DRM's fbcon
synchronizes its output to the vblank period. If no vblank is supported,
it'll happily fire out display updates ASAP even for blob=true.
Best regards
Thomas
Note that blob=true is a performance optimization where we create a
dmabuf (on the Host) backed by Guest FB's memory that is shared with the
UI layer. And, AFAIK, the only case where virtio-gpu updates are not limited
is when blob=false. Even in this case, there would be no tearing issues seen
because the Guest is made to wait until the Host makes a copy of its FB.
frequency. If GNOME's pageflip is synchronized to the vblank, it should
immediately benefit.
The GTK repaint callback is an interface for applications. How does it
affect, or is affected by, any of this?
So, virtio-gpu is almost always coupled with a UI (on the Host) in order
to display the Guest's desktop content (fbcon and compositor's FB data)
on the Host locally (Gtk, SDL, etc) or streamed to a remote system (Spice,
Vnc, etc). And, the rate at which the UI updates are submitted (to the
Host compositor for local UIs) is controlled by the UI timer.
However, in the case of Qemu Gtk UI, the UI timer is used as a backup
as we mostly rely on the repaint callback to figure out when to submit
updates. This is because the repaint callback is a more reliable mechanism
to determine when it is appropriate to submit an update to the Host
compositor.
And, until the UI's update is not submitted (and executed by the Host
GPU and signaled via an EGL fence), the Guest's update fence is not
signaled. We can reliably achieve 60 FPS this way (assuming the Host's
refresh rate is 60) for Guest's display updates.
Thanks,
Vivek
[5]
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/drm_atomi
c_helper.c#L2606
Best regards
Thomas
I am not sure how this would affect virgl use-cases but Dmitry can help
explain if
the vblank timer would be useful there or not.
Thanks,
Vivek
Signed-off-by: Thomas Zimmermann <[email protected]>
Link: https://lore.kernel.org/dri-
devel/[email protected]
amprd02.prod.outlook.com/ # [1]
---
drivers/gpu/drm/virtio/virtgpu_display.c | 29
++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
b/drivers/gpu/drm/virtio/virtgpu_display.c
index c3315935d8bc..ee134e46eeba 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -32,6 +32,8 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
#include "virtgpu_drv.h"
@@ -55,6 +57,7 @@ static const struct drm_crtc_funcs
virtio_gpu_crtc_funcs
= {
.reset = drm_atomic_helper_crtc_reset,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+ DRM_CRTC_VBLANK_TIMER_FUNCS,
};
static const struct drm_framebuffer_funcs virtio_gpu_fb_funcs = {
@@ -99,6 +102,7 @@ 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);
}
static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -108,6 +112,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);
+
virtio_gpu_cmd_set_scanout(vgdev, output->index, 0, 0, 0, 0, 0);
virtio_gpu_notify(vgdev);
}
@@ -121,9 +127,10 @@ static int virtio_gpu_crtc_atomic_check(struct
drm_crtc *crtc,
static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
- struct drm_crtc_state *crtc_state =
drm_atomic_get_new_crtc_state(state,
- crtc);
+ struct drm_device *dev = crtc->dev;
+ struct drm_crtc_state *crtc_state =
drm_atomic_get_new_crtc_state(state, crtc);
struct virtio_gpu_output *output =
drm_crtc_to_virtio_gpu_output(crtc);
+ struct drm_pending_vblank_event *event;
/*
* virtio-gpu can't do modeset and plane update operations
@@ -133,6 +140,20 @@ static void virtio_gpu_crtc_atomic_flush(struct
drm_crtc *crtc,
*/
if (drm_atomic_crtc_needs_modeset(crtc_state))
output->needs_modeset = true;
+
+ spin_lock_irq(&dev->event_lock);
+
+ event = crtc_state->event;
+ crtc_state->event = NULL;
+
+ if (event) {
+ if (drm_crtc_vblank_get(crtc) == 0)
+ drm_crtc_arm_vblank_event(crtc, event);
+ else
+ drm_crtc_send_vblank_event(crtc, event);
+ }
+
+ spin_unlock_irq(&dev->event_lock);
}
static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs
= {
@@ -356,6 +377,10 @@ int virtio_gpu_modeset_init(struct
virtio_gpu_device
*vgdev)
for (i = 0 ; i < vgdev->num_scanouts; ++i)
vgdev_output_init(vgdev, i);
+ ret = drm_vblank_init(vgdev->ddev, vgdev->num_scanouts);
+ if (ret)
+ return ret;
+
drm_mode_config_reset(vgdev->ddev);
return 0;
}
--
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)
--
--
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)
