[PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

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.

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c | 19 +++++++++++--------
 hw/display/virtio-gpu.c         |  5 +++--
 include/hw/virtio/virtio-gpu.h  |  5 +++--
 include/ui/console.h            |  4 ++++
 stubs/virtio-gpu-udmabuf.c      |  3 ++-
 5 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 3c01a415e7..a64194c6de 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;
+    dmabuf->buf.scanout_height;
     dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
     dmabuf->buf.fd = res->dmabuf_fd;
 
@@ -195,24 +200,22 @@ 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;
-    qemu_console_resize(scanout->con,
-                        new_primary->buf.width,
-                        new_primary->buf.height);
+    g->dmabuf.primary[scanout_id] = new_primary;
     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..11a87dad79 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 &&
@@ -598,6 +598,7 @@ static void virtio_gpu_update_scanout(VirtIOGPU *g,
     scanout->y = r->y;
     scanout->width = r->width;
     scanout->height = r->height;
+    qemu_console_resize(scanout->con, scanout->width, scanout->height);
 }
 
 static void virtio_gpu_do_set_scanout(VirtIOGPU *g,
@@ -633,7 +634,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;
-- 
2.17.1

[PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Dongwon Kim]

eb_fb_blit needs more parameters which describe x and y offsets and width
and height of the actual scanout to specify the size and cordination of
partial image to blit in the guest fb in case the guest fb contains multiple
display outputs.

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c |  4 ++--
 include/ui/egl-helpers.h        |  2 +-
 ui/egl-headless.c               |  2 +-
 ui/egl-helpers.c                | 10 ++++++----
 ui/gtk-egl.c                    |  7 ++++---
 ui/sdl2-gl.c                    |  2 +-
 6 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index a64194c6de..3ea6e76371 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -186,8 +186,8 @@ static VGPUDMABuf
     dmabuf->buf.stride = fb->stride;
     dmabuf->buf.x = r->x;
     dmabuf->buf.y = r->y;
-    dmabuf->buf.scanout_width;
-    dmabuf->buf.scanout_height;
+    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;
 
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index f1bf8f97fc..e21118501e 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int height);
 void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
                           GLuint texture, bool delete);
 void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
-void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip);
+void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip);
 void egl_fb_read(DisplaySurface *dst, egl_fb *src);
 
 void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip);
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index da377a74af..bdf10fec84 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl,
                           1.0, 1.0);
     } else {
         /* no cursor -> use simple framebuffer blit */
-        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top);
+        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, edpy->y_0_top);
     }
 
     egl_fb_read(edpy->ds, &edpy->blit_fb);
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 6d0cb2b5cb..2af3dcc0a6 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
     egl_fb_setup_for_tex(fb, width, height, texture, true);
 }
 
-void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
+void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip)
 {
     GLuint y1, y2;
 
     glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
     glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
     glViewport(0, 0, dst->width, dst->height);
-    y1 = flip ? src->height : 0;
-    y2 = flip ? 0 : src->height;
-    glBlitFramebuffer(0, y1, src->width, y2,
+    w = (x + w) > src->width ? src->width - x : w;
+    h = (y + h) > src->height ? src->height - y : h;
+    y1 = flip ? h + y : y;
+    y2 = flip ? y : h + y;
+    glBlitFramebuffer(x, y1, x + w, y2,
                       0, 0, dst->width, dst->height,
                       GL_COLOR_BUFFER_BIT, GL_LINEAR);
 }
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 2a2e6d3a17..ceb52b1045 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc)
     wh = gdk_window_get_height(window);
 
     if (vc->gfx.scanout_mode) {
-        gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
+       gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, vc->gfx.h);
 
         vc->gfx.scale_x = (double)ww / vc->gfx.w;
         vc->gfx.scale_y = (double)wh / vc->gfx.h;
@@ -216,7 +216,8 @@ 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);
+                           dmabuf->x, dmabuf->y, dmabuf->scanout_width,
+                           dmabuf->scanout_height);
 #endif
 }
 
@@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
                           vc->gfx.cursor_x, vc->gfx.cursor_y,
                           vc->gfx.scale_x, vc->gfx.scale_y);
     } else {
-        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top);
+        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, !vc->gfx.y0_top);
     }
 
     eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index a21d2deed9..67bc8b5f4e 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
 
     SDL_GetWindowSize(scon->real_window, &ww, &wh);
     egl_fb_setup_default(&scon->win_fb, ww, wh);
-    egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
+    egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top);
 
     SDL_GL_SwapWindow(scon->real_window);
     graphic_hw_gl_flushed(dcl->con);
-- 
2.17.1

RE: [PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Kasireddy, Vivek]

Hi DW,

> eb_fb_blit needs more parameters which describe x and y offsets and width
> and height of the actual scanout to specify the size and cordination of
> partial image to blit in the guest fb in case the guest fb contains multiple
> display outputs.
> 
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  hw/display/virtio-gpu-udmabuf.c |  4 ++--
>  include/ui/egl-helpers.h        |  2 +-
>  ui/egl-headless.c               |  2 +-
>  ui/egl-helpers.c                | 10 ++++++----
>  ui/gtk-egl.c                    |  7 ++++---
>  ui/sdl2-gl.c                    |  2 +-
>  6 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
[Kasireddy, Vivek] You might not want to mix virtio-gpu and UI changes in the same patch.

> index a64194c6de..3ea6e76371 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -186,8 +186,8 @@ static VGPUDMABuf
>      dmabuf->buf.stride = fb->stride;
>      dmabuf->buf.x = r->x;
>      dmabuf->buf.y = r->y;
> -    dmabuf->buf.scanout_width;
> -    dmabuf->buf.scanout_height;
> +    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;
> 
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index f1bf8f97fc..e21118501e 100644
> --- a/include/ui/egl-helpers.h
> +++ b/include/ui/egl-helpers.h
> @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int height);
>  void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
>                            GLuint texture, bool delete);
>  void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
> -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip);
> +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip);
>  void egl_fb_read(DisplaySurface *dst, egl_fb *src);
> 
>  void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip);
> diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> index da377a74af..bdf10fec84 100644
> --- a/ui/egl-headless.c
> +++ b/ui/egl-headless.c
> @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl,
>                            1.0, 1.0);
>      } else {
>          /* no cursor -> use simple framebuffer blit */
> -        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top);
> +        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, edpy->y_0_top);
>      }
> 
>      egl_fb_read(edpy->ds, &edpy->blit_fb);
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 6d0cb2b5cb..2af3dcc0a6 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
>      egl_fb_setup_for_tex(fb, width, height, texture, true);
>  }
> 
> -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
> +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip)
[Kasireddy, Vivek] Instead of explicitly passing x, y, w, h to egl_fb_blit, would you be not
be able to use the dmabuf member that would be added to egl_fb that would contain x, y, w and h:
https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06746.html


>  {
>      GLuint y1, y2;
> 
>      glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
>      glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
>      glViewport(0, 0, dst->width, dst->height);
> -    y1 = flip ? src->height : 0;
> -    y2 = flip ? 0 : src->height;
> -    glBlitFramebuffer(0, y1, src->width, y2,
> +    w = (x + w) > src->width ? src->width - x : w;
> +    h = (y + h) > src->height ? src->height - y : h;
> +    y1 = flip ? h + y : y;
> +    y2 = flip ? y : h + y;
> +    glBlitFramebuffer(x, y1, x + w, y2,
[Kasireddy, Vivek] While you are at it, could you please create new local variables x1, y1, x2, y2
to store the above values and pass them to glBlitFramebuffer to improve the readability of this code? 

Thanks,
Vivek
>                        0, 0, dst->width, dst->height,
>                        GL_COLOR_BUFFER_BIT, GL_LINEAR);
>  }
> diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> index 2a2e6d3a17..ceb52b1045 100644
> --- a/ui/gtk-egl.c
> +++ b/ui/gtk-egl.c
> @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc)
>      wh = gdk_window_get_height(window);
> 
>      if (vc->gfx.scanout_mode) {
> -        gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
> +       gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, vc->gfx.h);
> 
>          vc->gfx.scale_x = (double)ww / vc->gfx.w;
>          vc->gfx.scale_y = (double)wh / vc->gfx.h;
> @@ -216,7 +216,8 @@ 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);
> +                           dmabuf->x, dmabuf->y, dmabuf->scanout_width,
> +                           dmabuf->scanout_height);
>  #endif
>  }
> 
> @@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
>                            vc->gfx.cursor_x, vc->gfx.cursor_y,
>                            vc->gfx.scale_x, vc->gfx.scale_y);
>      } else {
> -        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top);
> +        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, !vc->gfx.y0_top);
>      }
> 
>      eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> index a21d2deed9..67bc8b5f4e 100644
> --- a/ui/sdl2-gl.c
> +++ b/ui/sdl2-gl.c
> @@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
> 
>      SDL_GetWindowSize(scon->real_window, &ww, &wh);
>      egl_fb_setup_default(&scon->win_fb, ww, wh);
> -    egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
> +    egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top);
> 
>      SDL_GL_SwapWindow(scon->real_window);
>      graphic_hw_gl_flushed(dcl->con);
> --
> 2.17.1
> 

Re: [PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Dongwon Kim]

On Sun, Jul 18, 2021 at 11:35:35PM -0700, Kasireddy, Vivek wrote:
> Hi DW,
> 
> > eb_fb_blit needs more parameters which describe x and y offsets and width
> > and height of the actual scanout to specify the size and cordination of
> > partial image to blit in the guest fb in case the guest fb contains multiple
> > display outputs.
> > 
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  hw/display/virtio-gpu-udmabuf.c |  4 ++--
> >  include/ui/egl-helpers.h        |  2 +-
> >  ui/egl-headless.c               |  2 +-
> >  ui/egl-helpers.c                | 10 ++++++----
> >  ui/gtk-egl.c                    |  7 ++++---
> >  ui/sdl2-gl.c                    |  2 +-
> >  6 files changed, 15 insertions(+), 12 deletions(-)
> > 
> > diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> [Kasireddy, Vivek] You might not want to mix virtio-gpu and UI changes in the same patch.
[DW] Yeah, I will split it.

> 
> > index a64194c6de..3ea6e76371 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -186,8 +186,8 @@ static VGPUDMABuf
> >      dmabuf->buf.stride = fb->stride;
> >      dmabuf->buf.x = r->x;
> >      dmabuf->buf.y = r->y;
> > -    dmabuf->buf.scanout_width;
> > -    dmabuf->buf.scanout_height;
> > +    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;
> > 
> > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> > index f1bf8f97fc..e21118501e 100644
> > --- a/include/ui/egl-helpers.h
> > +++ b/include/ui/egl-helpers.h
> > @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int height);
> >  void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
> >                            GLuint texture, bool delete);
> >  void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
> > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip);
> > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip);
> >  void egl_fb_read(DisplaySurface *dst, egl_fb *src);
> > 
> >  void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip);
> > diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> > index da377a74af..bdf10fec84 100644
> > --- a/ui/egl-headless.c
> > +++ b/ui/egl-headless.c
> > @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl,
> >                            1.0, 1.0);
> >      } else {
> >          /* no cursor -> use simple framebuffer blit */
> > -        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top);
> > +        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, edpy->y_0_top);
> >      }
> > 
> >      egl_fb_read(edpy->ds, &edpy->blit_fb);
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > index 6d0cb2b5cb..2af3dcc0a6 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
> >      egl_fb_setup_for_tex(fb, width, height, texture, true);
> >  }
> > 
> > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
> > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip)
> [Kasireddy, Vivek] Instead of explicitly passing x, y, w, h to egl_fb_blit, would you be not
> be able to use the dmabuf member that would be added to egl_fb that would contain x, y, w and h:
> https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06746.html

[DW] sounds like a valid idea but wouldn't it be making this function
too specific? I think it is reasonable to specify the offset and the
size of blitted area.. although I agree that having too many parameters
don't look good.

> 
> 
> >  {
> >      GLuint y1, y2;
> > 
> >      glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
> >      glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
> >      glViewport(0, 0, dst->width, dst->height);
> > -    y1 = flip ? src->height : 0;
> > -    y2 = flip ? 0 : src->height;
> > -    glBlitFramebuffer(0, y1, src->width, y2,
> > +    w = (x + w) > src->width ? src->width - x : w;
> > +    h = (y + h) > src->height ? src->height - y : h;
> > +    y1 = flip ? h + y : y;
> > +    y2 = flip ? y : h + y;
> > +    glBlitFramebuffer(x, y1, x + w, y2,
> [Kasireddy, Vivek] While you are at it, could you please create new local variables x1, y1, x2, y2
> to store the above values and pass them to glBlitFramebuffer to improve the readability of this code? 
[DW] I will think about making this look more undertandable.

> 
> Thanks,
> Vivek
> >                        0, 0, dst->width, dst->height,
> >                        GL_COLOR_BUFFER_BIT, GL_LINEAR);
> >  }
> > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > index 2a2e6d3a17..ceb52b1045 100644
> > --- a/ui/gtk-egl.c
> > +++ b/ui/gtk-egl.c
> > @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc)
> >      wh = gdk_window_get_height(window);
> > 
> >      if (vc->gfx.scanout_mode) {
> > -        gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
> > +       gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, vc->gfx.h);
> > 
> >          vc->gfx.scale_x = (double)ww / vc->gfx.w;
> >          vc->gfx.scale_y = (double)wh / vc->gfx.h;
> > @@ -216,7 +216,8 @@ 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);
> > +                           dmabuf->x, dmabuf->y, dmabuf->scanout_width,
> > +                           dmabuf->scanout_height);
> >  #endif
> >  }
> > 
> > @@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
> >                            vc->gfx.cursor_x, vc->gfx.cursor_y,
> >                            vc->gfx.scale_x, vc->gfx.scale_y);
> >      } else {
> > -        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top);
> > +        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, !vc->gfx.y0_top);
> >      }
> > 
> >      eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
> > diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> > index a21d2deed9..67bc8b5f4e 100644
> > --- a/ui/sdl2-gl.c
> > +++ b/ui/sdl2-gl.c
> > @@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
> > 
> >      SDL_GetWindowSize(scon->real_window, &ww, &wh);
> >      egl_fb_setup_default(&scon->win_fb, ww, wh);
> > -    egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
> > +    egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top);
> > 
> >      SDL_GL_SwapWindow(scon->real_window);
> >      graphic_hw_gl_flushed(dcl->con);
> > --
> > 2.17.1
> > 
> 

RE: [PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Kasireddy, Vivek]

Hi DW,

> 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.
> 
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  hw/display/virtio-gpu-udmabuf.c | 19 +++++++++++--------
>  hw/display/virtio-gpu.c         |  5 +++--
>  include/hw/virtio/virtio-gpu.h  |  5 +++--
>  include/ui/console.h            |  4 ++++
>  stubs/virtio-gpu-udmabuf.c      |  3 ++-
>  5 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index 3c01a415e7..a64194c6de 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;
> +    dmabuf->buf.scanout_height;
[Kasireddy, Vivek] Would you not be able to use buf.width and buf.height?
What is the difference between these and scanout_width/height?

>      dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
>      dmabuf->buf.fd = res->dmabuf_fd;
> 
> @@ -195,24 +200,22 @@ 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;
> -    qemu_console_resize(scanout->con,
> -                        new_primary->buf.width,
> -                        new_primary->buf.height);
> +    g->dmabuf.primary[scanout_id] = new_primary;
>      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..11a87dad79 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,
[Kasireddy, Vivek] Unrelated to your change but shouldn't 0,0 be replaced with 
scanout->x and scanout->y?

>                                scanout->height);
> -                return;
>              }
>          }
> +        return;
>      }
> 
>      if (!res->blob &&
> @@ -598,6 +598,7 @@ static void virtio_gpu_update_scanout(VirtIOGPU *g,
>      scanout->y = r->y;
>      scanout->width = r->width;
>      scanout->height = r->height;
> +    qemu_console_resize(scanout->con, scanout->width, scanout->height);
[Kasireddy, Vivek] IIRC, there was no qemu_console_resize for the non-blob resources case.
Moving this call to update_scanout means that it will also be called for non-blob resources
Path; not sure if this is OK but you might want restrict this call to only blob.

>  }
> 
>  static void virtio_gpu_do_set_scanout(VirtIOGPU *g,
> @@ -633,7 +634,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)) {
[Kasireddy, Vivek] Instead of passing the rectangle "r", you might want to consider using
the scanout object which is specific to each scanout and can be easily retried by:
scanout = &g->parent_obj.scanout[scanout_id];

Thanks,
Vivek
>                  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;
> --
> 2.17.1
> 

Re: [PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

On Sun, Jul 18, 2021 at 11:17:00PM -0700, Kasireddy, Vivek wrote:
> Hi DW,
> 
> > 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.
> > 
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  hw/display/virtio-gpu-udmabuf.c | 19 +++++++++++--------
> >  hw/display/virtio-gpu.c         |  5 +++--
> >  include/hw/virtio/virtio-gpu.h  |  5 +++--
> >  include/ui/console.h            |  4 ++++
> >  stubs/virtio-gpu-udmabuf.c      |  3 ++-
> >  5 files changed, 23 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> > index 3c01a415e7..a64194c6de 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;
> > +    dmabuf->buf.scanout_height;
> [Kasireddy, Vivek] Would you not be able to use buf.width and buf.height?
> What is the difference between these and scanout_width/height?
[DW] buf.width and buf.height is the combined width/height. For example,
if guest has two 1920x1080 w/ extended display setup, buf.width = 3840
and buf.height = 1080. buf.scanout_width and buf.scanout_height are
individual display's, so they are 1920 and 1080 respectively.

> 
> >      dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
> >      dmabuf->buf.fd = res->dmabuf_fd;
> > 
> > @@ -195,24 +200,22 @@ 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;
> > -    qemu_console_resize(scanout->con,
> > -                        new_primary->buf.width,
> > -                        new_primary->buf.height);
> > +    g->dmabuf.primary[scanout_id] = new_primary;
> >      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..11a87dad79 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,
> [Kasireddy, Vivek] Unrelated to your change but shouldn't 0,0 be replaced with 
> scanout->x and scanout->y?
[DW] Each scanout's x and y are all 0. Actual offsets are r->s and r->y.

> 
> >                                scanout->height);
> > -                return;
> >              }
> >          }
> > +        return;
> >      }
> > 
> >      if (!res->blob &&
> > @@ -598,6 +598,7 @@ static void virtio_gpu_update_scanout(VirtIOGPU *g,
> >      scanout->y = r->y;
> >      scanout->width = r->width;
> >      scanout->height = r->height;
> > +    qemu_console_resize(scanout->con, scanout->width, scanout->height);
> [Kasireddy, Vivek] IIRC, there was no qemu_console_resize for the non-blob resources case.
> Moving this call to update_scanout means that it will also be called for non-blob resources
> Path; not sure if this is OK but you might want restrict this call to only blob.
> 

[DW] This may not be needed for default case but I don't think it will
break anything as I see it won't do anything if the size matches. We can
limit this to blob case anyway. I will make the corresponding change in
v2.

> >  }
> > 
> >  static void virtio_gpu_do_set_scanout(VirtIOGPU *g,
> > @@ -633,7 +634,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)) {
> [Kasireddy, Vivek] Instead of passing the rectangle "r", you might want to consider using
> the scanout object which is specific to each scanout and can be easily retried by:
> scanout = &g->parent_obj.scanout[scanout_id];

[DW] Again, scanout's x and y are all 0.

> 
> Thanks,
> Vivek
> >                  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;
> > --
> > 2.17.1
> > 
> 

[PATCH v2 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

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(-)

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;
-- 
2.17.1

[PATCH v2 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Dongwon Kim]

eb_fb_blit should be able to blit partial image that represent a specific
guest display in case there are multiple displays connected to the guest
(One guest scanout FB contains all display outputs in a single dmabuf (blob-
resource).).

v2: egl_fb includes dmabuf info then make egl_fb_blit position and size
    parameters programmed in dmabuf structure (previously position/size
    parameters were given to egl_fb_blit separately)
    (Vivek Kasireddy)

    changed the commit message as there is no interface change to egl_fb_blit

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/ui/egl-helpers.h |  1 +
 ui/egl-helpers.c         | 25 +++++++++++++++++++++----
 ui/gtk-egl.c             |  4 ++++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index f1bf8f97fc..b6fced7062 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/ui/egl-helpers.c b/ui/egl-helpers.c
index 6d0cb2b5cb..80bc61fb70 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -90,14 +90,31 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
 
 void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
 {
-    GLuint y1, y2;
+    GLuint x1 = 0;
+    GLuint y1 = 0;
+    GLuint x2, y2;
+    GLuint w = src->width;
+    GLuint h = src->height;
 
     glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
     glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
     glViewport(0, 0, dst->width, dst->height);
-    y1 = flip ? src->height : 0;
-    y2 = flip ? 0 : src->height;
-    glBlitFramebuffer(0, y1, src->width, y2,
+
+    if (src->dmabuf) {
+        x1 = src->dmabuf->x;
+        y1 = src->dmabuf->y;
+        w = src->dmabuf->scanout_width;
+        h = src->dmabuf->scanout_height;
+    }
+
+    w = (x1 + w) > src->width ? src->width - x1 : w;
+    h = (y1 + h) > src->height ? src->height - y1 : h;
+
+    y2 = flip ? y1 : h + y1;
+    y1 = flip ? h + y1 : y1;
+    x2 = x1 + w;
+
+    glBlitFramebuffer(x1, y1, x2, y2,
                       0, 0, dst->width, dst->height,
                       GL_COLOR_BUFFER_BIT, GL_LINEAR);
 }
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 2a2e6d3a17..636b2aa02c 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -209,6 +209,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 +219,8 @@ 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);
+
+    vc->gfx.guest_fb.dmabuf = dmabuf;
 #endif
 }
 
-- 
2.17.1

Re: [PATCH v2 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Gerd Hoffmann]

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

Re: [PATCH v2 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

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
> 

Re: [PATCH v2 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Gerd Hoffmann]

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

[PATCH v2 1/6] ui/gtk-egl: un-tab and re-tab should destroy egl surface and context

[Dongwon Kim]

An old esurface should be destroyed and set to be NULL when doing
un-tab and re-tab so that a new esurface an context can be created
for the window widget that those will be bound to.

v2: enabling opengl specific routines only when CONFIG_OPENGL is set

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@intel.com>
---
 ui/gtk.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index b0564d80c1..8da673c18c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1242,6 +1242,16 @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event,
                                     vc->tab_item, vc->label);
     gtk_widget_destroy(vc->window);
     vc->window = NULL;
+#if defined(CONFIG_OPENGL)
+    if (vc->gfx.esurface) {
+        eglDestroySurface(qemu_egl_display, vc->gfx.esurface);
+        vc->gfx.esurface = NULL;
+    }
+    if (vc->gfx.ectx) {
+        eglDestroyContext(qemu_egl_display, vc->gfx.ectx);
+        vc->gfx.ectx = NULL;
+    }
+#endif
     return TRUE;
 }
 
@@ -1271,6 +1281,16 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
     if (!vc->window) {
         gtk_widget_set_sensitive(vc->menu_item, false);
         vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+#if defined(CONFIG_OPENGL)
+        if (vc->gfx.esurface) {
+            eglDestroySurface(qemu_egl_display, vc->gfx.esurface);
+            vc->gfx.esurface = NULL;
+        }
+        if (vc->gfx.esurface) {
+            eglDestroyContext(qemu_egl_display, vc->gfx.ectx);
+            vc->gfx.ectx = NULL;
+        }
+#endif
         gd_widget_reparent(s->notebook, vc->window, vc->tab_item);
 
         g_signal_connect(vc->window, "delete-event",
-- 
2.30.2

[PATCH 2/6] ui/gtk-egl: make sure the right context is set as the current

[Dongwon Kim]

Making the vc->gfx.ectx current before handling texture
associated with it

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk-egl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index e912b20075..2164995098 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -152,6 +152,7 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
         }
         vc->gfx.gls = qemu_gl_init_shader();
         if (vc->gfx.ds) {
+            surface_gl_destroy_texture(vc->gfx.gls, vc->gfx.ds);
             surface_gl_create_texture(vc->gfx.gls, vc->gfx.ds);
         }
     }
@@ -178,6 +179,8 @@ void gd_egl_switch(DisplayChangeListener *dcl,
         surface_height(vc->gfx.ds) == surface_height(surface)) {
         resized = false;
     }
+    eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+                   vc->gfx.esurface, vc->gfx.ectx);
 
     surface_gl_destroy_texture(vc->gfx.gls, vc->gfx.ds);
     vc->gfx.ds = surface;
@@ -237,6 +240,9 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
 #ifdef CONFIG_GBM
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+                   vc->gfx.esurface, vc->gfx.ectx);
+
     egl_dmabuf_import_texture(dmabuf);
     if (!dmabuf->texture) {
         return;
-- 
2.30.2

[PATCH v2 3/6] ui/gtk-egl: guest fb texture needs to be regenerated when reinitializing egl

[Dongwon Kim]

If guest fb is backed by dmabuf (blob-resource), the texture bound to the
old context needs to be recreated in case the egl is re-initialized (e.g.
new window for vc is created in case of detaching/reattaching of the tab)

v2: call egl_dmabuf_release_texutre instead of putting 0 to dmabuf->texture
    (Vivek Kasireddy)

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk-egl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 2164995098..f2026e4b5c 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -155,6 +155,10 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
             surface_gl_destroy_texture(vc->gfx.gls, vc->gfx.ds);
             surface_gl_create_texture(vc->gfx.gls, vc->gfx.ds);
         }
+        if (vc->gfx.guest_fb.dmabuf) {
+            egl_dmabuf_release_texture(vc->gfx.guest_fb.dmabuf);
+            gd_egl_scanout_dmabuf(dcl, vc->gfx.guest_fb.dmabuf);
+        }
     }
 
     graphic_hw_update(dcl->con);
-- 
2.30.2

[PATCH 4/6] ui/gtk: gd_draw_event returns FALSE when no cairo surface is bound

[Dongwon Kim]

gd_draw_event shouldn't try to repaint if surface does not exist
for the VC.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 8da673c18c..d2892ea6b4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -778,6 +778,9 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     if (!vc->gfx.ds) {
         return FALSE;
     }
+    if (!vc->gfx.surface) {
+        return FALSE;
+    }
 
     vc->gfx.dcl.update_interval =
         gd_monitor_update_interval(vc->window ? vc->window : s->window);
-- 
2.30.2

[PATCH v3 5/6] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

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

v3: updating stub function of virtio_gpu_update_dmabuf to match the type

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu-udmabuf-stubs.c |  3 ++-
 hw/display/virtio-gpu-udmabuf.c       | 22 ++++++++++++++--------
 hw/display/virtio-gpu.c               |  4 ++--
 include/hw/virtio/virtio-gpu.h        |  5 +++--
 include/ui/console.h                  |  4 ++++
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/hw/display/virtio-gpu-udmabuf-stubs.c b/hw/display/virtio-gpu-udmabuf-stubs.c
index 81f661441a..f692e13510 100644
--- a/hw/display/virtio-gpu-udmabuf-stubs.c
+++ b/hw/display/virtio-gpu-udmabuf-stubs.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;
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 60ea7f8f49..1597921c51 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;
     dmabuf->buf.allow_fences = true;
@@ -196,24 +201,25 @@ 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;
+    if (g->dmabuf.primary[scanout_id]) {
+        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 182e0868b0..d78b9700c7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -517,9 +517,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 &&
@@ -627,7 +627,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 24c6628944..acfba7c76c 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 b6bedc5f41..6d678924f6 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;
     void      *sync;
     int       fence_fd;
-- 
2.30.2

[PATCH v2 6/6] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Dongwon Kim]

eb_fb_blit should be able to blit partial image of guest display (blob res)
in case multiple displays are configured for the guest and they are set as extended-
desktop mode.

v2: egl_fb includes dmabuf info then make egl_fb_blit position and size
    parameters programmed in dmabuf structure (previously position/size
    parameters were given to egl_fb_blit separately)
    (Vivek Kasireddy)

    changed the commit message as there is no interface change to egl_fb_blit

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/egl-helpers.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 385a3fa752..3a88245b67 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -90,14 +90,31 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
 
 void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
 {
-    GLuint y1, y2;
+    GLuint x1 = 0;
+    GLuint y1 = 0;
+    GLuint x2, y2;
+    GLuint w = src->width;
+    GLuint h = src->height;
 
     glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
     glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
     glViewport(0, 0, dst->width, dst->height);
-    y1 = flip ? src->height : 0;
-    y2 = flip ? 0 : src->height;
-    glBlitFramebuffer(0, y1, src->width, y2,
+
+    if (src->dmabuf) {
+        x1 = src->dmabuf->x;
+        y1 = src->dmabuf->y;
+        w = src->dmabuf->scanout_width;
+        h = src->dmabuf->scanout_height;
+    }
+
+    w = (x1 + w) > src->width ? src->width - x1 : w;
+    h = (y1 + h) > src->height ? src->height - y1 : h;
+
+    y2 = flip ? y1 : h + y1;
+    y1 = flip ? h + y1 : y1;
+    x2 = x1 + w;
+
+    glBlitFramebuffer(x1, y1, x2, y2,
                       0, 0, dst->width, dst->height,
                       GL_COLOR_BUFFER_BIT, GL_LINEAR);
 }
-- 
2.30.2

Re: [PATCH v2 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

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
> 

Re: [PATCH v2 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Gerd Hoffmann]

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

[PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

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

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c | 22 ++++++++++++++--------
 hw/display/virtio-gpu.c         |  4 ++--
 include/hw/virtio/virtio-gpu.h  |  5 +++--
 include/ui/console.h            |  4 ++++
 4 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index c6f7f58784..e3a75971d5 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;
     dmabuf->buf.allow_fences = true;
@@ -196,24 +201,25 @@ 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;
+    if (g->dmabuf.primary[scanout_id]) {
+        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 182e0868b0..d78b9700c7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -517,9 +517,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 &&
@@ -627,7 +627,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 24c6628944..acfba7c76c 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 244664d727..01bc6cdc82 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;
     void      *sync;
     int       fence_fd;
-- 
2.17.1

[PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Dongwon Kim]

eb_fb_blit should be able to blit partial image of guest display (blob res)
in case multiple displays are configured for the guest and they are set as extended-
desktop mode.

v2: egl_fb includes dmabuf info then make egl_fb_blit position and size
    parameters programmed in dmabuf structure (previously position/size
    parameters were given to egl_fb_blit separately)
    (Vivek Kasireddy)

    changed the commit message as there is no interface change to egl_fb_blit

Cc: Gerd Hoffmann <kraxel@redhat.com>                                                                                                                                                         Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/egl-helpers.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 385a3fa752..3a88245b67 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -90,14 +90,31 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
 
 void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
 {
-    GLuint y1, y2;
+    GLuint x1 = 0;
+    GLuint y1 = 0;
+    GLuint x2, y2;
+    GLuint w = src->width;
+    GLuint h = src->height;
 
     glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
     glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
     glViewport(0, 0, dst->width, dst->height);
-    y1 = flip ? src->height : 0;
-    y2 = flip ? 0 : src->height;
-    glBlitFramebuffer(0, y1, src->width, y2,
+
+    if (src->dmabuf) {
+        x1 = src->dmabuf->x;
+        y1 = src->dmabuf->y;
+        w = src->dmabuf->scanout_width;
+        h = src->dmabuf->scanout_height;
+    }
+
+    w = (x1 + w) > src->width ? src->width - x1 : w;
+    h = (y1 + h) > src->height ? src->height - y1 : h;
+
+    y2 = flip ? y1 : h + y1;
+    y1 = flip ? h + y1 : y1;
+    x2 = x1 + w;
+
+    glBlitFramebuffer(x1, y1, x2, y2,
                       0, 0, dst->width, dst->height,
                       GL_COLOR_BUFFER_BIT, GL_LINEAR);
 }
-- 
2.17.1