On 9/13/23 21:18, Alex Williamson wrote:
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?
This does fix a regression reproducible on systems with an Intel Gen 8,
my T480 laptop for instance.
Tested-by: Cédric Le Goater <c...@redhat.com>
Also, queuing it in vfio-next.
Thanks,
C.
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
