On Mon, Mar 24, 2025 at 6:04 PM Marc-André Lureau
<[email protected]> wrote:
>
> Hi
>
> On Mon, Mar 24, 2025 at 12:19 PM <[email protected]> wrote:
> >
> > From: Qiang Yu <[email protected]>
> >
> > mesa/radeonsi is going to support explicit midifier which
> > may export a multi-plane texture. For example, texture with
> > DCC enabled (a compressed format) has two planes, one with
> > compressed data, the other with meta data for compression.
> >
> > Signed-off-by: Qiang Yu <[email protected]>
> > ---
> > hw/display/vhost-user-gpu.c | 6 ++-
> > hw/display/virtio-gpu-udmabuf.c | 6 ++-
> > hw/vfio/display.c | 7 ++--
> > include/ui/dmabuf.h | 20 ++++++----
> > ui/dbus-listener.c | 10 ++---
> > ui/dmabuf.c | 67 +++++++++++++++++++++------------
> > ui/egl-helpers.c | 4 +-
> > ui/spice-display.c | 4 +-
> > 8 files changed, 76 insertions(+), 48 deletions(-)
> >
> > diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> > index 2aed6243f6..a7949c7078 100644
> > --- a/hw/display/vhost-user-gpu.c
> > +++ b/hw/display/vhost-user-gpu.c
> > @@ -249,6 +249,8 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> > VhostUserGpuMsg *msg)
> > case VHOST_USER_GPU_DMABUF_SCANOUT: {
> > VhostUserGpuDMABUFScanout *m = &msg->payload.dmabuf_scanout;
> > int fd = qemu_chr_fe_get_msgfd(&g->vhost_chr);
> > + uint32_t offset = 0;
> > + uint32_t stride = m->fd_stride;
> > uint64_t modifier = 0;
> > QemuDmaBuf *dmabuf;
> >
> > @@ -282,10 +284,10 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> > VhostUserGpuMsg *msg)
> > }
> >
> > dmabuf = qemu_dmabuf_new(m->width, m->height,
> > - m->fd_stride, 0, 0,
> > + &offset, &stride, 0, 0,
> > m->fd_width, m->fd_height,
> > m->fd_drm_fourcc, modifier,
> > - fd, false, m->fd_flags &
> > + &fd, 1, false, m->fd_flags &
> > VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP);
> >
> > dpy_gl_scanout_dmabuf(con, dmabuf);
> > diff --git a/hw/display/virtio-gpu-udmabuf.c
> > b/hw/display/virtio-gpu-udmabuf.c
> > index 85ca23cb32..34fbe05b7a 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -176,16 +176,18 @@ static VGPUDMABuf
> > struct virtio_gpu_rect *r)
> > {
> > VGPUDMABuf *dmabuf;
> > + uint32_t offset = 0;
> >
> > if (res->dmabuf_fd < 0) {
> > return NULL;
> > }
> >
> > dmabuf = g_new0(VGPUDMABuf, 1);
> > - dmabuf->buf = qemu_dmabuf_new(r->width, r->height, fb->stride,
> > + dmabuf->buf = qemu_dmabuf_new(r->width, r->height,
> > + &offset, &fb->stride,
> > r->x, r->y, fb->width, fb->height,
> > qemu_pixman_to_drm_format(fb->format),
> > - 0, res->dmabuf_fd, true, false);
> > + 0, &res->dmabuf_fd, 1, true, false);
> > dmabuf->scanout_id = scanout_id;
> > QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
> >
> > diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> > index ea87830fe0..9d882235fb 100644
> > --- a/hw/vfio/display.c
> > +++ b/hw/vfio/display.c
> > @@ -214,6 +214,7 @@ static VFIODMABuf
> > *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
> > struct vfio_device_gfx_plane_info plane;
> > VFIODMABuf *dmabuf;
> > int fd, ret;
> > + uint32_t offset = 0;
> >
> > memset(&plane, 0, sizeof(plane));
> > plane.argsz = sizeof(plane);
> > @@ -246,10 +247,10 @@ static VFIODMABuf
> > *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
> >
> > dmabuf = g_new0(VFIODMABuf, 1);
> > dmabuf->dmabuf_id = plane.dmabuf_id;
> > - dmabuf->buf = qemu_dmabuf_new(plane.width, plane.height,
> > - plane.stride, 0, 0, plane.width,
> > + dmabuf->buf = qemu_dmabuf_new(plane.width, plane.height, &offset,
> > + &plane.stride, 0, 0, plane.width,
> > plane.height, plane.drm_format,
> > - plane.drm_format_mod, fd, false, false);
> > + plane.drm_format_mod, &fd, 1, false,
> > false);
> >
> > if (plane_type == DRM_PLANE_TYPE_CURSOR) {
> > vfio_display_update_cursor(dmabuf, &plane);
> > diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h
> > index dc74ba895a..407fb2274e 100644
> > --- a/include/ui/dmabuf.h
> > +++ b/include/ui/dmabuf.h
> > @@ -10,24 +10,29 @@
> > #ifndef DMABUF_H
> > #define DMABUF_H
> >
> > +#define DMABUF_MAX_PLANES 4
> > +
> > typedef struct QemuDmaBuf QemuDmaBuf;
> >
> > QemuDmaBuf *qemu_dmabuf_new(uint32_t width, uint32_t height,
> > - uint32_t stride, uint32_t x,
> > - uint32_t y, uint32_t backing_width,
> > - uint32_t backing_height, uint32_t fourcc,
> > - uint64_t modifier, int dmabuf_fd,
> > + const uint32_t *offset, const uint32_t *stride,
> > + uint32_t x, uint32_t y,
> > + uint32_t backing_width, uint32_t
> > backing_height,
> > + uint32_t fourcc, uint64_t modifier,
> > + const int32_t *dmabuf_fd, uint32_t num_planes,
> > bool allow_fences, bool y0_top);
> > void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
> >
> > G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf, qemu_dmabuf_free);
> >
> > -int qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> > -int qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf);
> > +const int *qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
>
> For clarity, I think the functions should be renamed to a plural form:
> get_fds, dup_fds, get_strides, get_offsets etc.. I would also have a
> size_t *n_fds to return the length of the returned array.
>
The returned array length is always 4. Valid fds are within num_planes
elements. There's no user for n_fds argument.
> > +void qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf, int *fd);
>
> That function should take an n_fds arguments as well (fd being renamed
> to fds as well)
>
We just dup all fds in dmabuf, so no need for n_fds argument. And there's
no use for this arg either.
> thanks
>
> > void qemu_dmabuf_close(QemuDmaBuf *dmabuf);
> > uint32_t qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
> > uint32_t qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
> > -uint32_t qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf);
> > +const uint32_t *qemu_dmabuf_get_offset(QemuDmaBuf *dmabuf);
> > +const uint32_t *qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf);
> > +uint32_t qemu_dmabuf_get_num_planes(QemuDmaBuf *dmabuf);
> > uint32_t qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf);
> > uint64_t qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf);
> > uint32_t qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf);
> > @@ -44,6 +49,5 @@ void qemu_dmabuf_set_texture(QemuDmaBuf *dmabuf, uint32_t
> > texture);
> > void qemu_dmabuf_set_fence_fd(QemuDmaBuf *dmabuf, int32_t fence_fd);
> > void qemu_dmabuf_set_sync(QemuDmaBuf *dmabuf, void *sync);
> > void qemu_dmabuf_set_draw_submitted(QemuDmaBuf *dmabuf, bool
> > draw_submitted);
> > -void qemu_dmabuf_set_fd(QemuDmaBuf *dmabuf, int32_t fd);
> >
> > #endif
> > diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> > index 51244c9240..3b39a23846 100644
> > --- a/ui/dbus-listener.c
> > +++ b/ui/dbus-listener.c
> > @@ -299,7 +299,7 @@ static void dbus_scanout_dmabuf(DisplayChangeListener
> > *dcl,
> > uint64_t modifier;
> > bool y0_top;
> >
> > - fd = qemu_dmabuf_get_fd(dmabuf);
> > + fd = qemu_dmabuf_get_fd(dmabuf)[0];
> > fd_list = g_unix_fd_list_new();
> > if (g_unix_fd_list_append(fd_list, fd, &err) != 0) {
> > error_report("Failed to setup dmabuf fdlist: %s", err->message);
> > @@ -310,7 +310,7 @@ static void dbus_scanout_dmabuf(DisplayChangeListener
> > *dcl,
> >
> > width = qemu_dmabuf_get_width(dmabuf);
> > height = qemu_dmabuf_get_height(dmabuf);
> > - stride = qemu_dmabuf_get_stride(dmabuf);
> > + stride = qemu_dmabuf_get_stride(dmabuf)[0];
> > fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> > modifier = qemu_dmabuf_get_modifier(dmabuf);
> > y0_top = qemu_dmabuf_get_y0_top(dmabuf);
> > @@ -505,7 +505,7 @@ static void dbus_scanout_texture(DisplayChangeListener
> > *dcl,
> > #ifdef CONFIG_GBM
> > g_autoptr(QemuDmaBuf) dmabuf = NULL;
> > int fd;
> > - uint32_t stride, fourcc;
> > + uint32_t offset = 0, stride, fourcc;
> > uint64_t modifier;
> >
> > assert(tex_id);
> > @@ -515,8 +515,8 @@ static void dbus_scanout_texture(DisplayChangeListener
> > *dcl,
> > error_report("%s: failed to get fd for texture", __func__);
> > return;
> > }
> > - dmabuf = qemu_dmabuf_new(w, h, stride, x, y, backing_width,
> > - backing_height, fourcc, modifier, fd,
> > + dmabuf = qemu_dmabuf_new(w, h, &offset, &stride, x, y, backing_width,
> > + backing_height, fourcc, modifier, &fd, 1,
> > false, backing_y_0_top);
> >
> > dbus_scanout_dmabuf(dcl, dmabuf);
> > diff --git a/ui/dmabuf.c b/ui/dmabuf.c
> > index df7a09703f..c4eb307042 100644
> > --- a/ui/dmabuf.c
> > +++ b/ui/dmabuf.c
> > @@ -11,10 +11,12 @@
> > #include "ui/dmabuf.h"
> >
> > struct QemuDmaBuf {
> > - int fd;
> > + int fd[DMABUF_MAX_PLANES];
> > uint32_t width;
> > uint32_t height;
> > - uint32_t stride;
> > + uint32_t offset[DMABUF_MAX_PLANES];
> > + uint32_t stride[DMABUF_MAX_PLANES];
> > + uint32_t num_planes;
> > uint32_t fourcc;
> > uint64_t modifier;
> > uint32_t texture;
> > @@ -30,28 +32,33 @@ struct QemuDmaBuf {
> > };
> >
> > QemuDmaBuf *qemu_dmabuf_new(uint32_t width, uint32_t height,
> > - uint32_t stride, uint32_t x,
> > - uint32_t y, uint32_t backing_width,
> > - uint32_t backing_height, uint32_t fourcc,
> > - uint64_t modifier, int32_t dmabuf_fd,
> > + const uint32_t *offset, const uint32_t *stride,
> > + uint32_t x, uint32_t y,
> > + uint32_t backing_width, uint32_t
> > backing_height,
> > + uint32_t fourcc, uint64_t modifier,
> > + const int32_t *dmabuf_fd, uint32_t num_planes,
> > bool allow_fences, bool y0_top) {
> > QemuDmaBuf *dmabuf;
> >
> > + assert(num_planes > 0 && num_planes <= DMABUF_MAX_PLANES);
> > +
> > dmabuf = g_new0(QemuDmaBuf, 1);
> >
> > dmabuf->width = width;
> > dmabuf->height = height;
> > - dmabuf->stride = stride;
> > + memcpy(dmabuf->offset, offset, num_planes * sizeof(*offset));
> > + memcpy(dmabuf->stride, stride, num_planes * sizeof(*stride));
> > dmabuf->x = x;
> > dmabuf->y = y;
> > dmabuf->backing_width = backing_width;
> > dmabuf->backing_height = backing_height;
> > dmabuf->fourcc = fourcc;
> > dmabuf->modifier = modifier;
> > - dmabuf->fd = dmabuf_fd;
> > + memcpy(dmabuf->fd, dmabuf_fd, num_planes * sizeof(*dmabuf_fd));
> > dmabuf->allow_fences = allow_fences;
> > dmabuf->y0_top = y0_top;
> > dmabuf->fence_fd = -1;
> > + dmabuf->num_planes = num_planes;
> >
> > return dmabuf;
> > }
> > @@ -65,31 +72,35 @@ void qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> > g_free(dmabuf);
> > }
> >
> > -int qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf)
> > +const int *qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf)
> > {
> > assert(dmabuf != NULL);
> >
> > return dmabuf->fd;
> > }
> >
> > -int qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf)
> > +void qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf, int *fd)
> > {
> > + int i;
> > +
> > assert(dmabuf != NULL);
> >
> > - if (dmabuf->fd >= 0) {
> > - return dup(dmabuf->fd);
> > - } else {
> > - return -1;
> > + for (i = 0; i < dmabuf->num_planes; i++) {
> > + fd[i] = dmabuf->fd[i] >= 0 ? dup(dmabuf->fd[i]) : -1;
> > }
> > }
> >
> > void qemu_dmabuf_close(QemuDmaBuf *dmabuf)
> > {
> > + int i;
> > +
> > assert(dmabuf != NULL);
> >
> > - if (dmabuf->fd >= 0) {
> > - close(dmabuf->fd);
> > - dmabuf->fd = -1;
> > + for (i = 0; i < dmabuf->num_planes; i++) {
> > + if (dmabuf->fd[i] >= 0) {
> > + close(dmabuf->fd[i]);
> > + dmabuf->fd[i] = -1;
> > + }
> > }
> > }
> >
> > @@ -107,13 +118,27 @@ uint32_t qemu_dmabuf_get_height(QemuDmaBuf *dmabuf)
> > return dmabuf->height;
> > }
> >
> > -uint32_t qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf)
> > +const uint32_t *qemu_dmabuf_get_offset(QemuDmaBuf *dmabuf)
> > +{
> > + assert(dmabuf != NULL);
> > +
> > + return dmabuf->offset;
> > +}
> > +
> > +const uint32_t *qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf)
> > {
> > assert(dmabuf != NULL);
> >
> > return dmabuf->stride;
> > }
> >
> > +uint32_t qemu_dmabuf_get_num_planes(QemuDmaBuf *dmabuf)
> > +{
> > + assert(dmabuf != NULL);
> > +
> > + return dmabuf->num_planes;
> > +}
> > +
> > uint32_t qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf)
> > {
> > assert(dmabuf != NULL);
> > @@ -221,9 +246,3 @@ void qemu_dmabuf_set_draw_submitted(QemuDmaBuf *dmabuf,
> > bool draw_submitted)
> > assert(dmabuf != NULL);
> > dmabuf->draw_submitted = draw_submitted;
> > }
> > -
> > -void qemu_dmabuf_set_fd(QemuDmaBuf *dmabuf, int32_t fd)
> > -{
> > - assert(dmabuf != NULL);
> > - dmabuf->fd = fd;
> > -}
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > index d591159480..72a1405782 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -323,9 +323,9 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
> > attrs[i++] = qemu_dmabuf_get_fourcc(dmabuf);
> >
> > attrs[i++] = EGL_DMA_BUF_PLANE0_FD_EXT;
> > - attrs[i++] = qemu_dmabuf_get_fd(dmabuf);
> > + attrs[i++] = qemu_dmabuf_get_fd(dmabuf)[0];
> > attrs[i++] = EGL_DMA_BUF_PLANE0_PITCH_EXT;
> > - attrs[i++] = qemu_dmabuf_get_stride(dmabuf);
> > + attrs[i++] = qemu_dmabuf_get_stride(dmabuf)[0];
> > attrs[i++] = EGL_DMA_BUF_PLANE0_OFFSET_EXT;
> > attrs[i++] = 0;
> > #ifdef EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT
> > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > index c794ae0649..82598fe9e8 100644
> > --- a/ui/spice-display.c
> > +++ b/ui/spice-display.c
> > @@ -1075,10 +1075,10 @@ static void
> > qemu_spice_gl_update(DisplayChangeListener *dcl,
> > stride, fourcc, false);
> > }
> > } else {
> > - stride = qemu_dmabuf_get_stride(dmabuf);
> > + stride = qemu_dmabuf_get_stride(dmabuf)[0];
> > fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> > y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> > - fd = qemu_dmabuf_dup_fd(dmabuf);
> > + qemu_dmabuf_dup_fd(dmabuf, &fd);
> >
> > trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> > /* note: spice server will close the fd, so hand over a dup */
> > --
> > 2.43.0
> >
>
