Hi Thomas, > Subject: [PATCH] drm/virtgpu: Use vblank timer > > Use a vblank timer to simulate the vblank interrupt. The DRM vblank > helpers provide an implementation on top of Linux' hrtimer. Virtgpu > enables and disables the timer as part of the CRTC. The atomic_flush > callback sets up the event. Like vblank interrupts, the vblank timer > fires at the rate of the display refresh. > > Most userspace limits its page flip rate according to the DRM vblank > event. Virtgpu's virtual hardware does not provide vblank interrupts, so > DRM sends each event ASAP. With the fast access times of virtual display > memory, the event rate is much higher than the display mode's refresh > rate; creating the next page flip almost immediately. This leads to > excessive CPU overhead from even small display updates, such as moving > the mouse pointer. > > This problem affects virtgpu and all other virtual displays. See [1] for > a discussion in the context of hypervdrm. When running Virtio-gpu with some of the UIs (gtk, spice, etc) in Qemu, the Guest display updates are synchronized with a timer (or repaint callback in the case of Gtk) the UI layer provides particularly when blob=true. In other words, the Guest's atomic commit does not complete until the Host signals (via a fence) that the update (or flush) is done.
This is because when blob=true, the Guest's FB is accessed by the Host without making any copies. So, to avoid tearing, the Guest needs to be prevented from accessing its FB until the Host is done. Therefore, I think for virtio-gpu, the virtual vblank timer can only be helpful when blob=false as this is the only case where Guest's display updates are unbounded (and Host makes a copy of the Guest's FB). 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 <tzimmerm...@suse.de> > Link: https://lore.kernel.org/dri- > devel/SN6PR02MB415702B00D6D52B0EE962C98D46CA@SN6PR02MB4157.n > 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 >
