diff mbox series

virtio-gpu-udmabuf: replacing scanout_width/height with backing_width/height

Message ID 20230705224635.25343-1-dongwon.kim@intel.com (mailing list archive)
State New, archived
Headers show
Series virtio-gpu-udmabuf: replacing scanout_width/height with backing_width/height | expand

Commit Message

Kim, Dongwon July 5, 2023, 10:46 p.m. UTC
'backing_width' and 'backing_height' are commonly used to indicate the size
of the whole backing region so it makes sense to use those terms for
VGAUDMABuf as well in place of 'scanout_width' and 'scanout_height'.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c | 8 ++++----
 include/ui/console.h            | 4 ++--
 ui/dbus-listener.c              | 4 ++--
 ui/egl-helpers.c                | 4 ++--
 ui/gtk-egl.c                    | 4 ++--
 ui/gtk-gl-area.c                | 4 ++--
 6 files changed, 14 insertions(+), 14 deletions(-)

Comments

Marc-André Lureau July 10, 2023, 11:57 a.m. UTC | #1
Hi

On Thu, Jul 6, 2023 at 3:10 AM Dongwon Kim <dongwon.kim@intel.com> wrote:

> 'backing_width' and 'backing_height' are commonly used to indicate the size
> of the whole backing region so it makes sense to use those terms for
> VGAUDMABuf as well in place of 'scanout_width' and 'scanout_height'.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  hw/display/virtio-gpu-udmabuf.c | 8 ++++----
>  include/ui/console.h            | 4 ++--
>  ui/dbus-listener.c              | 4 ++--
>  ui/egl-helpers.c                | 4 ++--
>  ui/gtk-egl.c                    | 4 ++--
>  ui/gtk-gl-area.c                | 4 ++--
>  6 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-udmabuf.c
> b/hw/display/virtio-gpu-udmabuf.c
> index ef1a740de5..920d457d4a 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 = r->width;
> -    dmabuf->buf.scanout_height = r->height;
> +    dmabuf->buf.backing_width = r->width;
> +    dmabuf->buf.backing_height = r->height;
>      dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
>      dmabuf->buf.fd = res->dmabuf_fd;
>      dmabuf->buf.allow_fences = true;
> @@ -218,8 +218,8 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>
>      g->dmabuf.primary[scanout_id] = new_primary;
>      qemu_console_resize(scanout->con,
> -                        new_primary->buf.scanout_width,
> -                        new_primary->buf.scanout_height);
> +                        new_primary->buf.backing_width,
> +                        new_primary->buf.backing_height);
>      dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
>
>      if (old_primary) {
> diff --git a/include/ui/console.h b/include/ui/console.h
> index f27b2aad4f..3e8b22d6c6 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -201,8 +201,8 @@ typedef struct QemuDmaBuf {
>      uint32_t  texture;
>      uint32_t  x;
>      uint32_t  y;
> -    uint32_t  scanout_width;
> -    uint32_t  scanout_height;
> +    uint32_t  backing_width;
> +    uint32_t  backing_height;
>      bool      y0_top;
>      void      *sync;
>      int       fence_fd;
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 0240c39510..7d73681cbc 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -420,8 +420,8 @@ static void dbus_scanout_texture(DisplayChangeListener
> *dcl,
>          .y0_top = backing_y_0_top,
>          .x = x,
>          .y = y,
> -        .scanout_width = w,
> -        .scanout_height = h,
> +        .backing_width = w,
> +        .backing_height = h,
>

This is not consistent with the function arguments. I think it should be
after:

.width = w, .height = h, .backing_width = backing_wdth, .backing_height =
backing_height

Hopefully this inconsistency is not repeated elsewhere.

thanks


>      };
>
>      assert(tex_id);
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 8f9fbf583e..6b7be5753d 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -148,8 +148,8 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
>      if (src->dmabuf) {
>          x1 = src->dmabuf->x;
>          y1 = src->dmabuf->y;
> -        w = src->dmabuf->scanout_width;
> -        h = src->dmabuf->scanout_height;
> +        w = src->dmabuf->backing_width;
> +        h = src->dmabuf->backing_height;
>      }
>
>      w = (x1 + w) > src->width ? src->width - x1 : w;
> diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> index d59b8cd7d7..7604696d4a 100644
> --- a/ui/gtk-egl.c
> +++ b/ui/gtk-egl.c
> @@ -259,8 +259,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
>
>      gd_egl_scanout_texture(dcl, dmabuf->texture,
>                             dmabuf->y0_top, dmabuf->width, dmabuf->height,
> -                           dmabuf->x, dmabuf->y, dmabuf->scanout_width,
> -                           dmabuf->scanout_height, NULL);
> +                           dmabuf->x, dmabuf->y, dmabuf->backing_width,
> +                           dmabuf->backing_height, NULL);
>
>      if (dmabuf->allow_fences) {
>          vc->gfx.guest_fb.dmabuf = dmabuf;
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 7367dfd793..3337a4baa3 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -300,8 +300,8 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener
> *dcl,
>
>      gd_gl_area_scanout_texture(dcl, dmabuf->texture,
>                                 dmabuf->y0_top, dmabuf->width,
> dmabuf->height,
> -                               dmabuf->x, dmabuf->y,
> dmabuf->scanout_width,
> -                               dmabuf->scanout_height, NULL);
> +                               dmabuf->x, dmabuf->y,
> dmabuf->backing_width,
> +                               dmabuf->backing_height, NULL);
>
>      if (dmabuf->allow_fences) {
>          vc->gfx.guest_fb.dmabuf = dmabuf;
> --
> 2.34.1
>
>
>
Kim, Dongwon July 13, 2023, 2:08 a.m. UTC | #2
On 7/10/2023 4:57 AM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Jul 6, 2023 at 3:10 AM Dongwon Kim <dongwon.kim@intel.com> wrote:
>
>     'backing_width' and 'backing_height' are commonly used to indicate
>     the size
>     of the whole backing region so it makes sense to use those terms for
>     VGAUDMABuf as well in place of 'scanout_width' and 'scanout_height'.
>
>     Cc: Gerd Hoffmann <kraxel@redhat.com>
>     Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>     Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
>     Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
>     ---
>      hw/display/virtio-gpu-udmabuf.c | 8 ++++----
>      include/ui/console.h            | 4 ++--
>      ui/dbus-listener.c              | 4 ++--
>      ui/egl-helpers.c                | 4 ++--
>      ui/gtk-egl.c                    | 4 ++--
>      ui/gtk-gl-area.c                | 4 ++--
>      6 files changed, 14 insertions(+), 14 deletions(-)
>
>     diff --git a/hw/display/virtio-gpu-udmabuf.c
>     b/hw/display/virtio-gpu-udmabuf.c
>     index ef1a740de5..920d457d4a 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 = r->width;
>     -    dmabuf->buf.scanout_height = r->height;
>     +    dmabuf->buf.backing_width = r->width;
>     +    dmabuf->buf.backing_height = r->height;
>          dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
>          dmabuf->buf.fd = res->dmabuf_fd;
>          dmabuf->buf.allow_fences = true;
>     @@ -218,8 +218,8 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>
>          g->dmabuf.primary[scanout_id] = new_primary;
>          qemu_console_resize(scanout->con,
>     -                        new_primary->buf.scanout_width,
>     - new_primary->buf.scanout_height);
>     +                        new_primary->buf.backing_width,
>     + new_primary->buf.backing_height);
>          dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
>
>          if (old_primary) {
>     diff --git a/include/ui/console.h b/include/ui/console.h
>     index f27b2aad4f..3e8b22d6c6 100644
>     --- a/include/ui/console.h
>     +++ b/include/ui/console.h
>     @@ -201,8 +201,8 @@ typedef struct QemuDmaBuf {
>          uint32_t  texture;
>          uint32_t  x;
>          uint32_t  y;
>     -    uint32_t  scanout_width;
>     -    uint32_t  scanout_height;
>     +    uint32_t  backing_width;
>     +    uint32_t  backing_height;
>          bool      y0_top;
>          void      *sync;
>          int       fence_fd;
>     diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
>     index 0240c39510..7d73681cbc 100644
>     --- a/ui/dbus-listener.c
>     +++ b/ui/dbus-listener.c
>     @@ -420,8 +420,8 @@ static void
>     dbus_scanout_texture(DisplayChangeListener *dcl,
>              .y0_top = backing_y_0_top,
>              .x = x,
>              .y = y,
>     -        .scanout_width = w,
>     -        .scanout_height = h,
>     +        .backing_width = w,
>     +        .backing_height = h,
>
>
> This is not consistent with the function arguments. I think it should 
> be after:
>
> .width = w, .height = h, .backing_width = backing_wdth, 
> .backing_height = backing_height
>
> Hopefully this inconsistency is not repeated elsewhere.
>
Yes, you are right. Backing_* is for the whole surface. And normal 
width/height or w/h specifies the sub region as you mentioned earlier in 
all other places. Inconsistency was caused in QemuDmabuf where 
width/height was used as backing_width/height. We should have corrected 
it first. I will send another version of patch to correct this.

> thanks
>
>          };
>
>          assert(tex_id);
>     diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
>     index 8f9fbf583e..6b7be5753d 100644
>     --- a/ui/egl-helpers.c
>     +++ b/ui/egl-helpers.c
>     @@ -148,8 +148,8 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src,
>     bool flip)
>          if (src->dmabuf) {
>              x1 = src->dmabuf->x;
>              y1 = src->dmabuf->y;
>     -        w = src->dmabuf->scanout_width;
>     -        h = src->dmabuf->scanout_height;
>     +        w = src->dmabuf->backing_width;
>     +        h = src->dmabuf->backing_height;
>          }
>
>          w = (x1 + w) > src->width ? src->width - x1 : w;
>     diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
>     index d59b8cd7d7..7604696d4a 100644
>     --- a/ui/gtk-egl.c
>     +++ b/ui/gtk-egl.c
>     @@ -259,8 +259,8 @@ void
>     gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
>
>          gd_egl_scanout_texture(dcl, dmabuf->texture,
>                                 dmabuf->y0_top, dmabuf->width,
>     dmabuf->height,
>     -                           dmabuf->x, dmabuf->y,
>     dmabuf->scanout_width,
>     -                           dmabuf->scanout_height, NULL);
>     +                           dmabuf->x, dmabuf->y,
>     dmabuf->backing_width,
>     +                           dmabuf->backing_height, NULL);
>
>          if (dmabuf->allow_fences) {
>              vc->gfx.guest_fb.dmabuf = dmabuf;
>     diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
>     index 7367dfd793..3337a4baa3 100644
>     --- a/ui/gtk-gl-area.c
>     +++ b/ui/gtk-gl-area.c
>     @@ -300,8 +300,8 @@ void
>     gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
>
>          gd_gl_area_scanout_texture(dcl, dmabuf->texture,
>                                     dmabuf->y0_top, dmabuf->width,
>     dmabuf->height,
>     -                               dmabuf->x, dmabuf->y,
>     dmabuf->scanout_width,
>     -                               dmabuf->scanout_height, NULL);
>     +                               dmabuf->x, dmabuf->y,
>     dmabuf->backing_width,
>     +                               dmabuf->backing_height, NULL);
>
>          if (dmabuf->allow_fences) {
>              vc->gfx.guest_fb.dmabuf = dmabuf;
>     -- 
>     2.34.1
>
>
>
>
> -- 
> Marc-André Lureau
diff mbox series

Patch

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index ef1a740de5..920d457d4a 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 = r->width;
-    dmabuf->buf.scanout_height = r->height;
+    dmabuf->buf.backing_width = r->width;
+    dmabuf->buf.backing_height = r->height;
     dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
     dmabuf->buf.fd = res->dmabuf_fd;
     dmabuf->buf.allow_fences = true;
@@ -218,8 +218,8 @@  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
 
     g->dmabuf.primary[scanout_id] = new_primary;
     qemu_console_resize(scanout->con,
-                        new_primary->buf.scanout_width,
-                        new_primary->buf.scanout_height);
+                        new_primary->buf.backing_width,
+                        new_primary->buf.backing_height);
     dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
 
     if (old_primary) {
diff --git a/include/ui/console.h b/include/ui/console.h
index f27b2aad4f..3e8b22d6c6 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -201,8 +201,8 @@  typedef struct QemuDmaBuf {
     uint32_t  texture;
     uint32_t  x;
     uint32_t  y;
-    uint32_t  scanout_width;
-    uint32_t  scanout_height;
+    uint32_t  backing_width;
+    uint32_t  backing_height;
     bool      y0_top;
     void      *sync;
     int       fence_fd;
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 0240c39510..7d73681cbc 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -420,8 +420,8 @@  static void dbus_scanout_texture(DisplayChangeListener *dcl,
         .y0_top = backing_y_0_top,
         .x = x,
         .y = y,
-        .scanout_width = w,
-        .scanout_height = h,
+        .backing_width = w,
+        .backing_height = h,
     };
 
     assert(tex_id);
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 8f9fbf583e..6b7be5753d 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -148,8 +148,8 @@  void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
     if (src->dmabuf) {
         x1 = src->dmabuf->x;
         y1 = src->dmabuf->y;
-        w = src->dmabuf->scanout_width;
-        h = src->dmabuf->scanout_height;
+        w = src->dmabuf->backing_width;
+        h = src->dmabuf->backing_height;
     }
 
     w = (x1 + w) > src->width ? src->width - x1 : w;
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index d59b8cd7d7..7604696d4a 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -259,8 +259,8 @@  void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
 
     gd_egl_scanout_texture(dcl, dmabuf->texture,
                            dmabuf->y0_top, dmabuf->width, dmabuf->height,
-                           dmabuf->x, dmabuf->y, dmabuf->scanout_width,
-                           dmabuf->scanout_height, NULL);
+                           dmabuf->x, dmabuf->y, dmabuf->backing_width,
+                           dmabuf->backing_height, NULL);
 
     if (dmabuf->allow_fences) {
         vc->gfx.guest_fb.dmabuf = dmabuf;
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 7367dfd793..3337a4baa3 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -300,8 +300,8 @@  void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
 
     gd_gl_area_scanout_texture(dcl, dmabuf->texture,
                                dmabuf->y0_top, dmabuf->width, dmabuf->height,
-                               dmabuf->x, dmabuf->y, dmabuf->scanout_width,
-                               dmabuf->scanout_height, NULL);
+                               dmabuf->x, dmabuf->y, dmabuf->backing_width,
+                               dmabuf->backing_height, NULL);
 
     if (dmabuf->allow_fences) {
         vc->gfx.guest_fb.dmabuf = dmabuf;