From: Dongwon Kim <dongwon.kim@intel.com>
Replace 'width' and 'height' in QemuDmaBuf with 'backing_widht'
and 'backing_height' as these commonly indicate the size of the
whole surface (e.g. guest's Xorg extended display). Then use
'width' and 'height' for sub region in there (e.g. guest's
scanouts).
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>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20230713040444.32267-1-dongwon.kim@intel.com>
---
include/ui/console.h | 4 ++--
hw/display/virtio-gpu-udmabuf.c | 12 ++++++------
ui/dbus-listener.c | 8 ++++----
ui/egl-helpers.c | 8 ++++----
ui/gtk-egl.c | 10 ++++++----
ui/gtk-gl-area.c | 7 ++++---
6 files changed, 26 insertions(+), 23 deletions(-)
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/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index ef1a740de5..d51184d658 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -181,13 +181,13 @@ static VGPUDMABuf
}
dmabuf = g_new0(VGPUDMABuf, 1);
- dmabuf->buf.width = fb->width;
- dmabuf->buf.height = fb->height;
+ dmabuf->buf.width = r->width;
+ dmabuf->buf.height = r->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.backing_width = fb->width;
+ dmabuf->buf.backing_height = fb->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.width,
+ new_primary->buf.height);
dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
if (old_primary) {
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 0240c39510..68ff343799 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -415,13 +415,13 @@ static void dbus_scanout_texture(DisplayChangeListener *dcl,
backing_width, backing_height, x, y, w, h);
#ifdef CONFIG_GBM
QemuDmaBuf dmabuf = {
- .width = backing_width,
- .height = backing_height,
+ .width = w,
+ .height = h,
.y0_top = backing_y_0_top,
.x = x,
.y = y,
- .scanout_width = w,
- .scanout_height = h,
+ .backing_width = backing_width,
+ .backing_height = backing_height,
};
assert(tex_id);
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 8f9fbf583e..3d19dbe382 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->width;
+ h = src->dmabuf->height;
}
w = (x1 + w) > src->width ? src->width - x1 : w;
@@ -314,9 +314,9 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
}
attrs[i++] = EGL_WIDTH;
- attrs[i++] = dmabuf->width;
+ attrs[i++] = dmabuf->backing_width;
attrs[i++] = EGL_HEIGHT;
- attrs[i++] = dmabuf->height;
+ attrs[i++] = dmabuf->backing_height;
attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
attrs[i++] = dmabuf->fourcc;
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 42db1bb6cf..eee821d73a 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -262,9 +262,10 @@ 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->y0_top,
+ dmabuf->backing_width, dmabuf->backing_height,
+ dmabuf->x, dmabuf->y, dmabuf->width,
+ dmabuf->height, NULL);
if (dmabuf->allow_fences) {
vc->gfx.guest_fb.dmabuf = dmabuf;
@@ -284,7 +285,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
if (!dmabuf->texture) {
return;
}
- egl_fb_setup_for_tex(&vc->gfx.cursor_fb, dmabuf->width, dmabuf->height,
+ egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
+ dmabuf->backing_width, dmabuf->backing_height,
dmabuf->texture, false);
} else {
egl_fb_destroy(&vc->gfx.cursor_fb);
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index a9a7fdf50c..4513d3d059 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -301,9 +301,10 @@ 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->y0_top,
+ dmabuf->backing_width, dmabuf->backing_height,
+ dmabuf->x, dmabuf->y, dmabuf->width,
+ dmabuf->height, NULL);
if (dmabuf->allow_fences) {
vc->gfx.guest_fb.dmabuf = dmabuf;
--
2.41.0
This commit introduces a regression when using GVT-g with the display
and ramfb options. At the point where it appears the guest is switching
from ramfb to the vGPU display, the display goes black and QEMU reports:
qemu: eglCreateImageKHR failed
This message occurs repeatedly.
VM command line:
/usr/local/bin/qemu-system-x86_64 \
-M pc \
-machine kernel_irqchip=on \
-accel kvm \
-cpu host \
-m 6G \
-smp 4,sockets=1,dies=1,cores=4,threads=1 \
-smbios 'type=0,vendor=HP,version=P02 Ver. 02.44,date=09/13/2022,release=2.44' \
-smbios 'type=1,manufacturer=HP,product=HP ProDesk 600 G3 MT,serial=MXL745130H,sku=Y3E02AV,family=103C_53307F HP ProDesk' \
-smbios 'type=2,manufacturer=HP,product=829D,version=KBC Version 06.29,serial=PFYUT0FCY9731K' \
-smbios type=3,manufacturer=HP,serial=MXL745130H,asset=MXL745130H \
-acpitable sig=SLIC,file=/var/lib/libvirt/images/slic.bin \
-acpitable sig=MSDM,file=/var/lib/libvirt/images/msdm.bin \
-global PIIX4_PM.disable_s3=1 \
-global PIIX4_PM.disable_s4=1 \
-rtc base=localtime,driftfix=slew \
-global kvm-pit.lost_tick_policy=delay \
-no-user-config \
-nodefaults \
-monitor stdio \
-serial none \
-parallel none \
-no-hpet \
-net nic,macaddr=5c:25:8e:a6:34:6e,model=virtio \
-net user \
-usb \
-usbdevice tablet \
-vga none \
-display gtk,gl=on \
-hda /var/lib/libvirt/images/win10-gvtg.qcow2 \
-device vfio-pci-nohotplug,sysfsdev=/sys/bus/mdev/devices/b1338b2d-a709-4c23-b766-cc436c36cdf0,display=on,ramfb=on,x-igd-opregion=on \
-snapshot
Thanks,
Alex
On Mon, 17 Jul 2023 16:45:30 +0400
marcandre.lureau@redhat.com wrote:
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> Replace 'width' and 'height' in QemuDmaBuf with 'backing_widht'
> and 'backing_height' as these commonly indicate the size of the
> whole surface (e.g. guest's Xorg extended display). Then use
> 'width' and 'height' for sub region in there (e.g. guest's
> scanouts).
>
> 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>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-ID: <20230713040444.32267-1-dongwon.kim@intel.com>
> ---
> include/ui/console.h | 4 ++--
> hw/display/virtio-gpu-udmabuf.c | 12 ++++++------
> ui/dbus-listener.c | 8 ++++----
> ui/egl-helpers.c | 8 ++++----
> ui/gtk-egl.c | 10 ++++++----
> ui/gtk-gl-area.c | 7 ++++---
> 6 files changed, 26 insertions(+), 23 deletions(-)
>
> 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/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index ef1a740de5..d51184d658 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -181,13 +181,13 @@ static VGPUDMABuf
> }
>
> dmabuf = g_new0(VGPUDMABuf, 1);
> - dmabuf->buf.width = fb->width;
> - dmabuf->buf.height = fb->height;
> + dmabuf->buf.width = r->width;
> + dmabuf->buf.height = r->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.backing_width = fb->width;
> + dmabuf->buf.backing_height = fb->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.width,
> + new_primary->buf.height);
> dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
>
> if (old_primary) {
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 0240c39510..68ff343799 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -415,13 +415,13 @@ static void dbus_scanout_texture(DisplayChangeListener *dcl,
> backing_width, backing_height, x, y, w, h);
> #ifdef CONFIG_GBM
> QemuDmaBuf dmabuf = {
> - .width = backing_width,
> - .height = backing_height,
> + .width = w,
> + .height = h,
> .y0_top = backing_y_0_top,
> .x = x,
> .y = y,
> - .scanout_width = w,
> - .scanout_height = h,
> + .backing_width = backing_width,
> + .backing_height = backing_height,
> };
>
> assert(tex_id);
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 8f9fbf583e..3d19dbe382 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->width;
> + h = src->dmabuf->height;
> }
>
> w = (x1 + w) > src->width ? src->width - x1 : w;
> @@ -314,9 +314,9 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
> }
>
> attrs[i++] = EGL_WIDTH;
> - attrs[i++] = dmabuf->width;
> + attrs[i++] = dmabuf->backing_width;
> attrs[i++] = EGL_HEIGHT;
> - attrs[i++] = dmabuf->height;
> + attrs[i++] = dmabuf->backing_height;
> attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
> attrs[i++] = dmabuf->fourcc;
>
> diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> index 42db1bb6cf..eee821d73a 100644
> --- a/ui/gtk-egl.c
> +++ b/ui/gtk-egl.c
> @@ -262,9 +262,10 @@ 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->y0_top,
> + dmabuf->backing_width, dmabuf->backing_height,
> + dmabuf->x, dmabuf->y, dmabuf->width,
> + dmabuf->height, NULL);
>
> if (dmabuf->allow_fences) {
> vc->gfx.guest_fb.dmabuf = dmabuf;
> @@ -284,7 +285,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
> if (!dmabuf->texture) {
> return;
> }
> - egl_fb_setup_for_tex(&vc->gfx.cursor_fb, dmabuf->width, dmabuf->height,
> + egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
> + dmabuf->backing_width, dmabuf->backing_height,
> dmabuf->texture, false);
> } else {
> egl_fb_destroy(&vc->gfx.cursor_fb);
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index a9a7fdf50c..4513d3d059 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -301,9 +301,10 @@ 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->y0_top,
> + dmabuf->backing_width, dmabuf->backing_height,
> + dmabuf->x, dmabuf->y, dmabuf->width,
> + dmabuf->height, NULL);
>
> if (dmabuf->allow_fences) {
> vc->gfx.guest_fb.dmabuf = dmabuf;
On Wed, 16 Aug 2023 15:08:10 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > index 8f9fbf583e..3d19dbe382 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -314,9 +314,9 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
> > }
> >
> > attrs[i++] = EGL_WIDTH;
> > - attrs[i++] = dmabuf->width;
> > + attrs[i++] = dmabuf->backing_width;
> > attrs[i++] = EGL_HEIGHT;
> > - attrs[i++] = dmabuf->height;
> > + attrs[i++] = dmabuf->backing_height;
> > attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
> > attrs[i++] = dmabuf->fourcc;
> >
> > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > index 42db1bb6cf..eee821d73a 100644
> > --- a/ui/gtk-egl.c
> > +++ b/ui/gtk-egl.c
> > @@ -262,9 +262,10 @@ 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->y0_top,
> > + dmabuf->backing_width, dmabuf->backing_height,
> > + dmabuf->x, dmabuf->y, dmabuf->width,
> > + dmabuf->height, NULL);
> >
> > if (dmabuf->allow_fences) {
> > vc->gfx.guest_fb.dmabuf = dmabuf;
> > @@ -284,7 +285,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
> > if (!dmabuf->texture) {
> > return;
> > }
> > - egl_fb_setup_for_tex(&vc->gfx.cursor_fb, dmabuf->width, dmabuf->height,
> > + egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
> > + dmabuf->backing_width, dmabuf->backing_height,
> > dmabuf->texture, false);
> > } else {
> > egl_fb_destroy(&vc->gfx.cursor_fb);
> > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> > index a9a7fdf50c..4513d3d059 100644
> > --- a/ui/gtk-gl-area.c
> > +++ b/ui/gtk-gl-area.c
> > @@ -301,9 +301,10 @@ 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->y0_top,
> > + dmabuf->backing_width, dmabuf->backing_height,
> > + dmabuf->x, dmabuf->y, dmabuf->width,
> > + dmabuf->height, NULL);
> >
> > if (dmabuf->allow_fences) {
> > vc->gfx.guest_fb.dmabuf = dmabuf;
>
I suspect the issues is in these last few chunks where width and height
are replaced with backing_width and backing height, but
hw/vfio/display.c never sets backing_*. It appears that the following
resolves the issue:
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index bec864f482f4..837d9e6a309e 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -243,6 +243,8 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
dmabuf->dmabuf_id = plane.dmabuf_id;
dmabuf->buf.width = plane.width;
dmabuf->buf.height = plane.height;
+ dmabuf->buf.backing_width = plane.width;
+ dmabuf->buf.backing_height = plane.height;
dmabuf->buf.stride = plane.stride;
dmabuf->buf.fourcc = plane.drm_format;
dmabuf->buf.modifier = plane.drm_format_mod;
I'll post that formally, but I really have no idea how dmabuf display
works, so confirmation would be appreciated. Thanks,
Alex
Hi Alex
On Thu, Aug 17, 2023 at 1:25 AM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
> On Wed, 16 Aug 2023 15:08:10 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
> > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > > index 8f9fbf583e..3d19dbe382 100644
> > > --- a/ui/egl-helpers.c
> > > +++ b/ui/egl-helpers.c
> > > @@ -314,9 +314,9 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
> > > }
> > >
> > > attrs[i++] = EGL_WIDTH;
> > > - attrs[i++] = dmabuf->width;
> > > + attrs[i++] = dmabuf->backing_width;
> > > attrs[i++] = EGL_HEIGHT;
> > > - attrs[i++] = dmabuf->height;
> > > + attrs[i++] = dmabuf->backing_height;
> > > attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
> > > attrs[i++] = dmabuf->fourcc;
> > >
> > > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > > index 42db1bb6cf..eee821d73a 100644
> > > --- a/ui/gtk-egl.c
> > > +++ b/ui/gtk-egl.c
> > > @@ -262,9 +262,10 @@ 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->y0_top,
> > > + dmabuf->backing_width, dmabuf->backing_height,
> > > + dmabuf->x, dmabuf->y, dmabuf->width,
> > > + dmabuf->height, NULL);
> > >
> > > if (dmabuf->allow_fences) {
> > > vc->gfx.guest_fb.dmabuf = dmabuf;
> > > @@ -284,7 +285,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
> > > if (!dmabuf->texture) {
> > > return;
> > > }
> > > - egl_fb_setup_for_tex(&vc->gfx.cursor_fb, dmabuf->width, dmabuf->height,
> > > + egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
> > > + dmabuf->backing_width, dmabuf->backing_height,
> > > dmabuf->texture, false);
> > > } else {
> > > egl_fb_destroy(&vc->gfx.cursor_fb);
> > > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> > > index a9a7fdf50c..4513d3d059 100644
> > > --- a/ui/gtk-gl-area.c
> > > +++ b/ui/gtk-gl-area.c
> > > @@ -301,9 +301,10 @@ 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->y0_top,
> > > + dmabuf->backing_width, dmabuf->backing_height,
> > > + dmabuf->x, dmabuf->y, dmabuf->width,
> > > + dmabuf->height, NULL);
> > >
> > > if (dmabuf->allow_fences) {
> > > vc->gfx.guest_fb.dmabuf = dmabuf;
> >
>
> I suspect the issues is in these last few chunks where width and height
> are replaced with backing_width and backing height, but
> hw/vfio/display.c never sets backing_*. It appears that the following
> resolves the issue:
>
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index bec864f482f4..837d9e6a309e 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -243,6 +243,8 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
> dmabuf->dmabuf_id = plane.dmabuf_id;
> dmabuf->buf.width = plane.width;
> dmabuf->buf.height = plane.height;
> + dmabuf->buf.backing_width = plane.width;
> + dmabuf->buf.backing_height = plane.height;
> dmabuf->buf.stride = plane.stride;
> dmabuf->buf.fourcc = plane.drm_format;
> dmabuf->buf.modifier = plane.drm_format_mod;
>
> I'll post that formally, but I really have no idea how dmabuf display
> works, so confirmation would be appreciated. Thanks,
Looks correct to me. I wish Kim would chime in.
I am not familiar with vfio/display. Looking at the kernel side, it
seems it doesn't have a concept for scanout geometry that is different
from the backing dmabuf/texture dimension.
Should we make this a blocker for release? Are you sending the patch?
On Mon, 21 Aug 2023 14:20:38 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> Hi Alex
>
> On Thu, Aug 17, 2023 at 1:25 AM Alex Williamson
> <alex.williamson@redhat.com> wrote:
> >
> > On Wed, 16 Aug 2023 15:08:10 -0600
> > Alex Williamson <alex.williamson@redhat.com> wrote:
> > > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > > > index 8f9fbf583e..3d19dbe382 100644
> > > > --- a/ui/egl-helpers.c
> > > > +++ b/ui/egl-helpers.c
> > > > @@ -314,9 +314,9 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
> > > > }
> > > >
> > > > attrs[i++] = EGL_WIDTH;
> > > > - attrs[i++] = dmabuf->width;
> > > > + attrs[i++] = dmabuf->backing_width;
> > > > attrs[i++] = EGL_HEIGHT;
> > > > - attrs[i++] = dmabuf->height;
> > > > + attrs[i++] = dmabuf->backing_height;
> > > > attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
> > > > attrs[i++] = dmabuf->fourcc;
> > > >
> > > > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > > > index 42db1bb6cf..eee821d73a 100644
> > > > --- a/ui/gtk-egl.c
> > > > +++ b/ui/gtk-egl.c
> > > > @@ -262,9 +262,10 @@ 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->y0_top,
> > > > + dmabuf->backing_width, dmabuf->backing_height,
> > > > + dmabuf->x, dmabuf->y, dmabuf->width,
> > > > + dmabuf->height, NULL);
> > > >
> > > > if (dmabuf->allow_fences) {
> > > > vc->gfx.guest_fb.dmabuf = dmabuf;
> > > > @@ -284,7 +285,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
> > > > if (!dmabuf->texture) {
> > > > return;
> > > > }
> > > > - egl_fb_setup_for_tex(&vc->gfx.cursor_fb, dmabuf->width, dmabuf->height,
> > > > + egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
> > > > + dmabuf->backing_width, dmabuf->backing_height,
> > > > dmabuf->texture, false);
> > > > } else {
> > > > egl_fb_destroy(&vc->gfx.cursor_fb);
> > > > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> > > > index a9a7fdf50c..4513d3d059 100644
> > > > --- a/ui/gtk-gl-area.c
> > > > +++ b/ui/gtk-gl-area.c
> > > > @@ -301,9 +301,10 @@ 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->y0_top,
> > > > + dmabuf->backing_width, dmabuf->backing_height,
> > > > + dmabuf->x, dmabuf->y, dmabuf->width,
> > > > + dmabuf->height, NULL);
> > > >
> > > > if (dmabuf->allow_fences) {
> > > > vc->gfx.guest_fb.dmabuf = dmabuf;
> > >
> >
> > I suspect the issues is in these last few chunks where width and height
> > are replaced with backing_width and backing height, but
> > hw/vfio/display.c never sets backing_*. It appears that the following
> > resolves the issue:
> >
> > diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> > index bec864f482f4..837d9e6a309e 100644
> > --- a/hw/vfio/display.c
> > +++ b/hw/vfio/display.c
> > @@ -243,6 +243,8 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
> > dmabuf->dmabuf_id = plane.dmabuf_id;
> > dmabuf->buf.width = plane.width;
> > dmabuf->buf.height = plane.height;
> > + dmabuf->buf.backing_width = plane.width;
> > + dmabuf->buf.backing_height = plane.height;
> > dmabuf->buf.stride = plane.stride;
> > dmabuf->buf.fourcc = plane.drm_format;
> > dmabuf->buf.modifier = plane.drm_format_mod;
> >
> > I'll post that formally, but I really have no idea how dmabuf display
> > works, so confirmation would be appreciated. Thanks,
>
> Looks correct to me. I wish Kim would chime in.
>
> I am not familiar with vfio/display. Looking at the kernel side, it
> seems it doesn't have a concept for scanout geometry that is different
> from the backing dmabuf/texture dimension.
>
> Should we make this a blocker for release? Are you sending the patch?
I did send a patch, Kim commented there:
https://lore.kernel.org/all/20230816215550.1723696-1-alex.williamson@redhat.com/
Follow-up suggest vhost-user-gpu is also affected. Empirically the
patch I sent works, I think it's correct, but Gerd is probably most
qualified to respond to the comments. I don't know how a "scanout"
relates to what we provide in vfio_device_gfx_plane_info. Thanks,
Alex
© 2016 - 2026 Red Hat, Inc.