| Message ID | 20210621192425.1188442-4-vivek.kasireddy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-gpu: Add a default synchronization mechanism for blobs | expand |
Hi, > dmabuf->buf.fd = res->dmabuf_fd; > + dmabuf->buf.blob = true; Do you actually need the 'blob' field? I think checking 'fd' instead should work too. take care, Gerd
Hi Gerd, > Hi, > > > dmabuf->buf.fd = res->dmabuf_fd; > > + dmabuf->buf.blob = true; > > Do you actually need the 'blob' field? > I think checking 'fd' instead should work too. [Kasireddy, Vivek] I want these changes to be limited to blob resources only as I do not know how they might affect other use-cases or whether they are needed there or not. I don't think I can rely on fd as vfio/display.c also populates the fd field: dmabuf = g_new0(VFIODMABuf, 1); dmabuf->dmabuf_id = plane.dmabuf_id; dmabuf->buf.width = plane.width; dmabuf->buf.height = plane.height; dmabuf->buf.stride = plane.stride; dmabuf->buf.fourcc = plane.drm_format; dmabuf->buf.modifier = plane.drm_format_mod; dmabuf->buf.fd = fd; Therefore, I need a way to identify a dmabuf that is associated with blobs vs others. Thanks, Vivek
> > Hi, > > > > > dmabuf->buf.fd = res->dmabuf_fd; > > > + dmabuf->buf.blob = true; > > > > Do you actually need the 'blob' field? > > I think checking 'fd' instead should work too. > [Kasireddy, Vivek] I want these changes to be limited to blob resources only as I do not > know how they might affect other use-cases or whether they are needed there or not. I > don't think I can rely on fd as vfio/display.c also populates the fd field: > dmabuf = g_new0(VFIODMABuf, 1); > dmabuf->dmabuf_id = plane.dmabuf_id; > dmabuf->buf.width = plane.width; > dmabuf->buf.height = plane.height; > dmabuf->buf.stride = plane.stride; > dmabuf->buf.fourcc = plane.drm_format; > dmabuf->buf.modifier = plane.drm_format_mod; > dmabuf->buf.fd = fd; > > Therefore, I need a way to identify a dmabuf that is associated with blobs vs others. And it actually is a dma-buf too (the guest display provided by i915 gvt mdev driver). So fencing that should work, right? Even if we have to restrict it to some kinds of dma-bufs the field should have a more descriptive name like "allow_fences". take care, Gerd
Hi Gerd, > > > > > > > dmabuf->buf.fd = res->dmabuf_fd; > > > > + dmabuf->buf.blob = true; > > > > > > Do you actually need the 'blob' field? > > > I think checking 'fd' instead should work too. > > [Kasireddy, Vivek] I want these changes to be limited to blob resources only as I do not > > know how they might affect other use-cases or whether they are needed there or not. I > > don't think I can rely on fd as vfio/display.c also populates the fd field: > > dmabuf = g_new0(VFIODMABuf, 1); > > dmabuf->dmabuf_id = plane.dmabuf_id; > > dmabuf->buf.width = plane.width; > > dmabuf->buf.height = plane.height; > > dmabuf->buf.stride = plane.stride; > > dmabuf->buf.fourcc = plane.drm_format; > > dmabuf->buf.modifier = plane.drm_format_mod; > > dmabuf->buf.fd = fd; > > > > Therefore, I need a way to identify a dmabuf that is associated with blobs vs others. > > And it actually is a dma-buf too (the guest display provided by i915 gvt > mdev driver). So fencing that should work, right? [Kasireddy, Vivek] Well, for virtio-gpu, as you know we are adding a dma fence to resource_flush to make it wait until it gets signalled by Qemu. We might have to do to something similar on i915 GVT side but I do not have the hardware to write a patch and test it out -- as i915 GVT is not supported for > Gen 9 platforms. > > Even if we have to restrict it to some kinds of dma-bufs the field > should have a more descriptive name like "allow_fences". [Kasireddy, Vivek] I think limiting this to blobs makes sense at the moment. If need be, we can extend it to include dma-bufs generated by i915 GVT later. Let me send a v4 with your suggestion to change the name. Thanks, Vivek > > take care, > Gerd
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c index 3c01a415e7..632ba06cbc 100644 --- a/hw/display/virtio-gpu-udmabuf.c +++ b/hw/display/virtio-gpu-udmabuf.c @@ -185,6 +185,7 @@ static VGPUDMABuf dmabuf->buf.stride = fb->stride; dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format); dmabuf->buf.fd = res->dmabuf_fd; + dmabuf->buf.blob = true; dmabuf->scanout_id = scanout_id; QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next); diff --git a/include/ui/console.h b/include/ui/console.h index 49978fdae3..570d827644 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -170,6 +170,7 @@ typedef struct QemuDmaBuf { bool y0_top; void *sync; int fence_fd; + bool blob; } QemuDmaBuf; typedef struct DisplayState DisplayState; diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h index 2c3ba92b53..2fb6e0dd6b 100644 --- a/include/ui/egl-helpers.h +++ b/include/ui/egl-helpers.h @@ -19,6 +19,7 @@ typedef struct egl_fb { GLuint texture; GLuint framebuffer; bool delete_texture; + QemuDmaBuf *dmabuf; } egl_fb; void egl_fb_destroy(egl_fb *fb); diff --git a/include/ui/gtk.h b/include/ui/gtk.h index e6cbf0507c..3e6a48b978 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -152,6 +152,7 @@ extern bool gtk_use_gl_area; /* ui/gtk.c */ void gd_update_windowsize(VirtualConsole *vc); int gd_monitor_update_interval(GtkWidget *widget); +void gd_hw_gl_flushed(void *vc); /* ui/gtk-egl.c */ void gd_egl_init(VirtualConsole *vc); diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index b671181272..f530bcd940 100644 --- a/ui/gtk-egl.c +++ b/ui/gtk-egl.c @@ -12,6 +12,7 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "trace.h" @@ -63,6 +64,7 @@ void gd_egl_draw(VirtualConsole *vc) { GdkWindow *window; int ww, wh; + QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; if (!vc->gfx.gls) { return; @@ -94,6 +96,14 @@ void gd_egl_draw(VirtualConsole *vc) } glFlush(); + if (dmabuf) { + egl_dmabuf_create_fence(dmabuf); + if (dmabuf->fence_fd > 0) { + qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc); + return; + } + graphic_hw_gl_block(vc->gfx.dcl.con, false); + } graphic_hw_gl_flushed(vc->gfx.dcl.con); } @@ -209,6 +219,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl, QemuDmaBuf *dmabuf) { #ifdef CONFIG_GBM + VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); + egl_dmabuf_import_texture(dmabuf); if (!dmabuf->texture) { return; @@ -217,6 +229,10 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl, gd_egl_scanout_texture(dcl, dmabuf->texture, false, dmabuf->width, dmabuf->height, 0, 0, dmabuf->width, dmabuf->height); + + if (dmabuf->blob) { + vc->gfx.guest_fb.dmabuf = dmabuf; + } #endif } @@ -281,6 +297,10 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top); } + if (vc->gfx.guest_fb.dmabuf) { + egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf); + } + eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); } diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index dd5783fec7..091194789e 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -8,6 +8,7 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "trace.h" @@ -38,6 +39,7 @@ static void gtk_gl_area_set_scanout_mode(VirtualConsole *vc, bool scanout) void gd_gl_area_draw(VirtualConsole *vc) { int ww, wh, y1, y2; + QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; if (!vc->gfx.gls) { return; @@ -71,7 +73,18 @@ void gd_gl_area_draw(VirtualConsole *vc) surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds); } + if (dmabuf) { + egl_dmabuf_create_sync(dmabuf); + } glFlush(); + if (dmabuf) { + egl_dmabuf_create_fence(dmabuf); + if (dmabuf->fence_fd > 0) { + qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc); + return; + } + graphic_hw_gl_block(vc->gfx.dcl.con, false); + } graphic_hw_gl_flushed(vc->gfx.dcl.con); } @@ -213,6 +226,9 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl, { VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); + if (vc->gfx.guest_fb.dmabuf) { + graphic_hw_gl_block(vc->gfx.dcl.con, true); + } gtk_gl_area_queue_render(GTK_GL_AREA(vc->gfx.drawing_area)); } @@ -231,6 +247,10 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl, gd_gl_area_scanout_texture(dcl, dmabuf->texture, false, dmabuf->width, dmabuf->height, 0, 0, dmabuf->width, dmabuf->height); + + if (dmabuf->blob) { + vc->gfx.guest_fb.dmabuf = dmabuf; + } #endif } diff --git a/ui/gtk.c b/ui/gtk.c index 6132bab52f..ee3a084c21 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -36,6 +36,7 @@ #include "qapi/qapi-commands-machine.h" #include "qapi/qapi-commands-misc.h" #include "qemu/cutils.h" +#include "qemu/main-loop.h" #include "ui/console.h" #include "ui/gtk.h" @@ -583,6 +584,18 @@ static void gd_gl_release_dmabuf(DisplayChangeListener *dcl, #endif } +void gd_hw_gl_flushed(void *vcon) +{ + VirtualConsole *vc = vcon; + QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf; + + graphic_hw_gl_block(vc->gfx.dcl.con, false); + graphic_hw_gl_flushed(vc->gfx.dcl.con); + qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL); + close(dmabuf->fence_fd); + dmabuf->fence_fd = -1; +} + /** DisplayState Callbacks (opengl version) **/ static const DisplayChangeListenerOps dcl_gl_area_ops = {
Create sync objects and fences only for dmabufs that are blobs. Once a fence is created (after glFlush) and is signalled, graphic_hw_gl_flushed() will be called and virtio-gpu cmd processing will be resumed. Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> --- hw/display/virtio-gpu-udmabuf.c | 1 + include/ui/console.h | 1 + include/ui/egl-helpers.h | 1 + include/ui/gtk.h | 1 + ui/gtk-egl.c | 20 ++++++++++++++++++++ ui/gtk-gl-area.c | 20 ++++++++++++++++++++ ui/gtk.c | 13 +++++++++++++ 7 files changed, 57 insertions(+)