[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-
From: |
Kasireddy, Vivek |
Subject: |
RE: [PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts |
Date: |
Mon, 19 Jul 2021 06:17:00 +0000 |
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
>