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)
