Hi Gerd,
Some consultation would be appreciated on this thread to get this patch out of limbo. Is there a better solution that what I've proposed? AFAICT, we don't have position fields to indicate the dmabuf plane is relative to some scanout, so I think it represents the entire display. Thanks, Alex On Tue, 5 Sep 2023 09:09:07 -0600 Alex Williamson <alex.william...@redhat.com> wrote: > On Mon, 4 Sep 2023 21:00:53 +0400 > Marc-André Lureau <marcandre.lur...@gmail.com> wrote: > > > Hi > > > > On Mon, Sep 4, 2023 at 6:11 PM Alex Williamson > > <alex.william...@redhat.com> wrote: > > > > > > On Mon, 4 Sep 2023 15:06:21 +0400 > > > Marc-André Lureau <marcandre.lur...@gmail.com> wrote: > > > > > > > Hi > > > > > > > > On Thu, Aug 17, 2023 at 8:29 PM Kim, Dongwon <dongwon....@intel.com> > > > > wrote: > > > > > > > > > > Ok, this regression happened not just because of renaming. Originally > > > > > width and height were representing the size of whole surface that > > > > > guest > > > > > shares while scanout width and height are for the each scanout. We > > > > > realized backing_width/height are more commonly used to specify the > > > > > size > > > > > of the whole guest surface so put them in the place of width/height > > > > > then > > > > > replaced scanout_width/height as well with normal width/height. > > > > > > > > > > On 8/16/2023 3:31 PM, Philippe Mathieu-Daudé wrote: > > > > > > On 16/8/23 23:55, Alex Williamson wrote: > > > > > >> The below referenced commit renames scanout_width/height to > > > > > >> backing_width/height, but also promotes these fields in various > > > > > >> portions > > > > > >> of the egl interface. Meanwhile vfio dmabuf support has never > > > > > >> used the > > > > > >> previous scanout fields and is therefore missed in the update. This > > > > > >> results in a black screen when transitioning from ramfb to dmabuf > > > > > >> display > > > > > >> when using Intel vGPU with these features. > > > > > > > > > > > > Referenced commit isn't trivial. Maybe because it is too late here. > > > > > > I'd have tried to split it. Anyhow, too late (again). > > > > > > > > > > > > Is vhost-user-gpu also affected? (see VHOST_USER_GPU_DMABUF_SCANOUT > > > > > > in vhost_user_gpu_handle_display()). > > > > > > > > > > Yeah, backing_width/height should be programmed with > > > > > plane.width/height > > > > > as well in vhost_user_gpu_handle_display(). > > > > > > > > > > Link: > > > > > https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg02726.html > > > > > > > > > > >> Fixes: 9ac06df8b684 ("virtio-gpu-udmabuf: correct naming of > > > > > >> QemuDmaBuf size properties") > > > > > >> Signed-off-by: Alex Williamson <alex.william...@redhat.com> > > > > > >> --- > > > > > >> > > > > > >> This fixes a regression in dmabuf/EGL support for Intel GVT-g and > > > > > >> potentially the mbochs mdev driver as well. Once validated by > > > > > >> those > > > > > >> that understand dmabuf/EGL integration, I'd welcome QEMU > > > > > >> maintainers to > > > > > >> take this directly for v8.1 or queue it as soon as possible for > > > > > >> v8.1.1. > > > > > >> > > > > > >> hw/vfio/display.c | 2 ++ > > > > > >> 1 file changed, 2 insertions(+) > > > > > >> > > > > > >> 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; > > > > > > > > > > One thing to note here is the normal width and height in the > > > > > QemuDmaBuf > > > > > are of a scanout, which could be just a partial area of the guest > > > > > plane > > > > > here. So we should not use those as normal width and height of the > > > > > QemuDmaBuf unless it is guaranteed the given guest surface (plane in > > > > > this case) is always of single display's. > > > > > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04737.html > > > > > > > > > > >> + 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 agree with what Kim said. Alex, are you sending a new patch? > > > > > > What would be different? > > > > > > struct vfio_device_gfx_plane_info { > > > __u32 argsz; > > > __u32 flags; > > > #define VFIO_GFX_PLANE_TYPE_PROBE (1 << 0) > > > #define VFIO_GFX_PLANE_TYPE_DMABUF (1 << 1) > > > #define VFIO_GFX_PLANE_TYPE_REGION (1 << 2) > > > /* in */ > > > __u32 drm_plane_type; /* type of plane: DRM_PLANE_TYPE_* */ > > > /* out */ > > > __u32 drm_format; /* drm format of plane */ > > > __u64 drm_format_mod; /* tiled mode */ > > > __u32 width; /* width of plane */ > > > __u32 height; /* height of plane */ > > > __u32 stride; /* stride of plane */ > > > __u32 size; /* size of plane in bytes, align on page*/ > > > __u32 x_pos; /* horizontal position of cursor plane */ > > > __u32 y_pos; /* vertical position of cursor plane*/ > > > __u32 x_hot; /* horizontal position of cursor hotspot */ > > > __u32 y_hot; /* vertical position of cursor hotspot */ > > > union { > > > __u32 region_index; /* region index */ > > > __u32 dmabuf_id; /* dma-buf id */ > > > }; > > > }; > > > > > > > Perhaps VFIO is missing extra infos to set the actual x/y/w/h > > region(s) of the visible monitor(s). This could be an extra message. I > > am not familiar with the kernel/driver side for this, perhaps it is > > always guaranteed to be the whole plane (+0+0+w*h). In which case, we > > simply to set the QemuDmabuf fields accordingly. > > Isn't that what the proposed patch does? Gerd is likely going to need > to chime in for any sort of authoritative answer. Gerd? Thanks, > > Alex
