[Qemu-devel] [PATCH] virtio-gpu: fix scanout rectangles

[Qemu-devel] [PATCH] virtio-gpu: fix scanout rectangles

[Gerd Hoffmann]

Commit "ca58b45 ui/virtio-gpu: add and use qemu_create_displaysurface_pixman"
breaks scanouts which use a region of the underlying resource only.

So, we need another way to handle the underlying issue.  Lets create a
new pixman image, grab a reference on the pixman providing the
underlying storage, hook up a destroy callback which releases the
reference.  That way regions work again and releasing the backing
storage should still be impossible thanks to the extra reference we are
holding.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 2116106..4746edc 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -497,6 +497,11 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
     pixman_region_fini(&flush_region);
 }
 
+static void virtio_unref_resource(pixman_image_t *image, void *data)
+{
+    pixman_image_unref(data);
+}
+
 static void virtio_gpu_set_scanout(VirtIOGPU *g,
                                    struct virtio_gpu_ctrl_command *cmd)
 {
@@ -576,8 +581,15 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
         != ((uint8_t *)pixman_image_get_data(res->image) + offset) ||
         scanout->width != ss.r.width ||
         scanout->height != ss.r.height) {
+        pixman_image_t *rect;
+        void *ptr = (uint8_t *)pixman_image_get_data(res->image) + offset;
+        rect = pixman_image_create_bits(format, ss.r.width, ss.r.height, ptr,
+                                        pixman_image_get_stride(res->image));
+        pixman_image_ref(res->image);
+        pixman_image_set_destroy_function(rect, virtio_unref_resource,
+                                          res->image);
         /* realloc the surface ptr */
-        scanout->ds = qemu_create_displaysurface_pixman(res->image);
+        scanout->ds = qemu_create_displaysurface_pixman(rect);
         if (!scanout->ds) {
             cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
             return;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] virtio-gpu: fix scanout rectangles

[Marc-André Lureau]

Hi

On Mon, May 30, 2016 at 10:40 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Commit "ca58b45 ui/virtio-gpu: add and use qemu_create_displaysurface_pixman"
> breaks scanouts which use a region of the underlying resource only.
>
> So, we need another way to handle the underlying issue.  Lets create a
> new pixman image, grab a reference on the pixman providing the
> underlying storage, hook up a destroy callback which releases the
> reference.  That way regions work again and releasing the backing
> storage should still be impossible thanks to the extra reference we are
> holding.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I proposed some similar changes in virgl mailing list, so:

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  hw/display/virtio-gpu.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 2116106..4746edc 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -497,6 +497,11 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
>      pixman_region_fini(&flush_region);
>  }
>
> +static void virtio_unref_resource(pixman_image_t *image, void *data)
> +{
> +    pixman_image_unref(data);
> +}
> +
>  static void virtio_gpu_set_scanout(VirtIOGPU *g,
>                                     struct virtio_gpu_ctrl_command *cmd)
>  {
> @@ -576,8 +581,15 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
>          != ((uint8_t *)pixman_image_get_data(res->image) + offset) ||
>          scanout->width != ss.r.width ||
>          scanout->height != ss.r.height) {
> +        pixman_image_t *rect;
> +        void *ptr = (uint8_t *)pixman_image_get_data(res->image) + offset;
> +        rect = pixman_image_create_bits(format, ss.r.width, ss.r.height, ptr,
> +                                        pixman_image_get_stride(res->image));
> +        pixman_image_ref(res->image);
> +        pixman_image_set_destroy_function(rect, virtio_unref_resource,
> +                                          res->image);
>          /* realloc the surface ptr */
> -        scanout->ds = qemu_create_displaysurface_pixman(res->image);
> +        scanout->ds = qemu_create_displaysurface_pixman(rect);
>          if (!scanout->ds) {
>              cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>              return;
> --
> 1.8.3.1
>
>



-- 
Marc-André Lureau