| Message ID | 20210726215950.26705-1-dongwon.kim@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts | expand |
On Mon, Jul 26, 2021 at 02:59:49PM -0700, Dongwon Kim wrote: > When guest is running Linux/X11 with extended multiple displays mode enabled, > the guest shares one scanout resource each time containing whole surface > rather than sharing individual display output separately. This extended frame > is properly splited and rendered on the corresponding scanout surfaces but > not in case of blob-resource (zero copy). > > This code change lets the qemu split this one large surface data into multiple > in case of blob-resource as well so that each sub frame then can be blitted > properly to each scanout. Fails windows test build too. take care, Gerd
I double-checked the patch and also tried to build with --disable-opengl but couldn't find any issue. Can you please give me some hint? Like build errors you saw. What are changed by the patch are pretty much limited to virtio-gpu blob case and just one change in common area is egl_fb_blit function but the function interface stays same and there are no variables enabled only with certain define. Is there any chance that the build system was building it with the previous patch, "ui/gtk-egl: un-tab and re-tab should destroy egl surface and context" still applied? Thanks! DW On Tue, Nov 02, 2021 at 02:51:54PM +0100, Gerd Hoffmann wrote: > On Mon, Jul 26, 2021 at 02:59:49PM -0700, Dongwon Kim wrote: > > When guest is running Linux/X11 with extended multiple displays mode enabled, > > the guest shares one scanout resource each time containing whole surface > > rather than sharing individual display output separately. This extended frame > > is properly splited and rendered on the corresponding scanout surfaces but > > not in case of blob-resource (zero copy). > > > > This code change lets the qemu split this one large surface data into multiple > > in case of blob-resource as well so that each sub frame then can be blitted > > properly to each scanout. > > Fails windows test build too. > > take care, > Gerd >
On Tue, Nov 02, 2021 at 05:41:10PM -0700, Dongwon Kim wrote: > I double-checked the patch and also tried to build with --disable-opengl > but couldn't find any issue. Can you please give me some hint? Like > build errors you saw. What are changed by the patch are pretty much > limited to virtio-gpu blob case and just one change in common area is > egl_fb_blit function but the function interface stays same and there are > no variables enabled only with certain define. > > Is there any chance that the build system was building it with the > previous patch, "ui/gtk-egl: un-tab and re-tab should destroy egl > surface and context" still applied? Hmm, I had applied both, then dropped the failing series, but possibly I missed some broken patch. Trying to apply the series on top of the v2 just sent failed. Any chance you can send a single patch series with all pending patches (including the "virtio-gpu: Add a default synchronization mechanism for blobs" series by Vivek which also doesn't apply cleanly any more)? Ideally tested in gitlab ci? thanks, Gerd
I found the virtio-gpu-splitting-one-extended-mode-guest-fb-into.patch didn't include the func interface change in one of stubs. I believe that was the reason for win32/64 build failure (verified it with gitlab CI). I updated that patch then I combined all 6 (4 for untabifying issue and 2 for multiple guest scanouts with blob) into one patch series and just sent it to as a reply. Regarding Vivek's series for default sync mechanism that you referred below, that series has already been merged. Thanks, DW On Wed, Nov 03, 2021 at 06:39:37AM +0100, Gerd Hoffmann wrote: > On Tue, Nov 02, 2021 at 05:41:10PM -0700, Dongwon Kim wrote: > > I double-checked the patch and also tried to build with --disable-opengl > > but couldn't find any issue. Can you please give me some hint? Like > > build errors you saw. What are changed by the patch are pretty much > > limited to virtio-gpu blob case and just one change in common area is > > egl_fb_blit function but the function interface stays same and there are > > no variables enabled only with certain define. > > > > Is there any chance that the build system was building it with the > > previous patch, "ui/gtk-egl: un-tab and re-tab should destroy egl > > surface and context" still applied? > > Hmm, I had applied both, then dropped the failing series, but possibly I > missed some broken patch. > > Trying to apply the series on top of the v2 just sent failed. > > Any chance you can send a single patch series with all pending patches > (including the "virtio-gpu: Add a default synchronization mechanism for > blobs" series by Vivek which also doesn't apply cleanly any more)? > > Ideally tested in gitlab ci? > > thanks, > Gerd >
On Thu, Nov 04, 2021 at 12:19:37AM -0700, Dongwon Kim wrote: > I found the virtio-gpu-splitting-one-extended-mode-guest-fb-into.patch > didn't include the func interface change in one of stubs. I believe > that was the reason for win32/64 build failure (verified it with gitlab > CI). I updated that patch then I combined all 6 (4 for untabifying issue > and 2 for multiple guest scanouts with blob) into one patch series and just > sent it to as a reply. Versioning the individual patches doesn't work very well, it confuses tools like b4 (https://pypi.org/project/b4/). Just bumb the version for all patches. One is applied already, probably slipped through when dropping the failing patches. Current patch queue: https://gitlab.com/kraxel/qemu/-/commits/queue/egl/ CI running right now. > Regarding Vivek's series for default sync mechanism that you referred > below, that series has already been merged. That explains nicely why it doesn't apply cleanly ;) take care, Gerd
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c index 3c01a415e7..aadf1221ef 100644 --- a/hw/display/virtio-gpu-udmabuf.c +++ b/hw/display/virtio-gpu-udmabuf.c @@ -171,7 +171,8 @@ static VGPUDMABuf *virtio_gpu_create_dmabuf(VirtIOGPU *g, uint32_t scanout_id, struct virtio_gpu_simple_resource *res, - struct virtio_gpu_framebuffer *fb) + struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_rect *r) { VGPUDMABuf *dmabuf; @@ -183,6 +184,10 @@ static VGPUDMABuf dmabuf->buf.width = fb->width; dmabuf->buf.height = fb->height; dmabuf->buf.stride = fb->stride; + dmabuf->buf.x = r->x; + dmabuf->buf.y = r->y; + dmabuf->buf.scanout_width = r->width; + dmabuf->buf.scanout_height = r->height; dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format); dmabuf->buf.fd = res->dmabuf_fd; @@ -195,24 +200,26 @@ static VGPUDMABuf int virtio_gpu_update_dmabuf(VirtIOGPU *g, uint32_t scanout_id, struct virtio_gpu_simple_resource *res, - struct virtio_gpu_framebuffer *fb) + struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_rect *r) { struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id]; VGPUDMABuf *new_primary, *old_primary = NULL; - new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb); + new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb, r); if (!new_primary) { return -EINVAL; } if (g->dmabuf.primary) { - old_primary = g->dmabuf.primary; + old_primary = g->dmabuf.primary[scanout_id]; } - g->dmabuf.primary = new_primary; + g->dmabuf.primary[scanout_id] = new_primary; + qemu_console_resize(scanout->con, - new_primary->buf.width, - new_primary->buf.height); + new_primary->buf.scanout_width, + new_primary->buf.scanout_height); dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf); if (old_primary) { diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index e183f4ecda..60db480719 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -523,9 +523,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, console_has_gl(scanout->con)) { dpy_gl_update(scanout->con, 0, 0, scanout->width, scanout->height); - return; } } + return; } if (!res->blob && @@ -633,7 +633,7 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g, if (res->blob) { if (console_has_gl(scanout->con)) { - if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb)) { + if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb, r)) { virtio_gpu_update_scanout(g, scanout_id, res, r); return; } diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index bcf54d970f..6372f4bbb5 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -187,7 +187,7 @@ struct VirtIOGPU { struct { QTAILQ_HEAD(, VGPUDMABuf) bufs; - VGPUDMABuf *primary; + VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS]; } dmabuf; }; @@ -273,7 +273,8 @@ void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res); int virtio_gpu_update_dmabuf(VirtIOGPU *g, uint32_t scanout_id, struct virtio_gpu_simple_resource *res, - struct virtio_gpu_framebuffer *fb); + struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_rect *r); /* virtio-gpu-3d.c */ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g, diff --git a/include/ui/console.h b/include/ui/console.h index b30b63976a..87316aef83 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -167,6 +167,10 @@ typedef struct QemuDmaBuf { uint32_t fourcc; uint64_t modifier; uint32_t texture; + uint32_t x; + uint32_t y; + uint32_t scanout_width; + uint32_t scanout_height; bool y0_top; } QemuDmaBuf; diff --git a/stubs/virtio-gpu-udmabuf.c b/stubs/virtio-gpu-udmabuf.c index 81f661441a..f692e13510 100644 --- a/stubs/virtio-gpu-udmabuf.c +++ b/stubs/virtio-gpu-udmabuf.c @@ -20,7 +20,8 @@ void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res) int virtio_gpu_update_dmabuf(VirtIOGPU *g, uint32_t scanout_id, struct virtio_gpu_simple_resource *res, - struct virtio_gpu_framebuffer *fb) + struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_rect *r) { /* nothing (stub) */ return 0;
When guest is running Linux/X11 with extended multiple displays mode enabled, the guest shares one scanout resource each time containing whole surface rather than sharing individual display output separately. This extended frame is properly splited and rendered on the corresponding scanout surfaces but not in case of blob-resource (zero copy). This code change lets the qemu split this one large surface data into multiple in case of blob-resource as well so that each sub frame then can be blitted properly to each scanout. v2: resizing qemu console in virtio_gpu_update_dmabuf to scanout's width and height Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- hw/display/virtio-gpu-udmabuf.c | 21 ++++++++++++++------- hw/display/virtio-gpu.c | 4 ++-- include/hw/virtio/virtio-gpu.h | 5 +++-- include/ui/console.h | 4 ++++ stubs/virtio-gpu-udmabuf.c | 3 ++- 5 files changed, 25 insertions(+), 12 deletions(-)