[Mesa-dev] [RFC v5 11/19] vulkan/wsi: Add multiple planes to wsi_image_base

Jason Ekstrand jason at jlekstrand.net
Mon Nov 20 21:50:44 UTC 2017


On Mon, Nov 20, 2017 at 1:41 PM, Louis-Francis Ratté-Boulianne <
lfrb at collabora.com> wrote:

> Hi,
>
> On Mon, 2017-11-13 at 14:20 -0800, Jason Ekstrand wrote:
> > On Mon, Nov 6, 2017 at 2:02 PM, Louis-Francis Ratté-Boulianne <lfrb at c
> > ollabora.com> wrote:
> > > From: Daniel Stone <daniels at collabora.com>
> > >
> > > Not currently used.
> > >
> > > Signed-off-by: Daniel Stone <daniels at collabora.com>
> > > ---
> > >  src/amd/vulkan/radv_wsi.c           | 13 +++++++------
> > >  src/intel/vulkan/anv_wsi.c          |  9 +++++----
> > >  src/vulkan/wsi/wsi_common.h         |  9 +++++----
> > >  src/vulkan/wsi/wsi_common_wayland.c | 11 +++++++----
> > >  src/vulkan/wsi/wsi_common_x11.c     | 12 ++++++++----
> > >  5 files changed, 32 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c
> > > index b24cf28d42..b535dc22f4 100644
> > > --- a/src/amd/vulkan/radv_wsi.c
> > > +++ b/src/amd/vulkan/radv_wsi.c
> > > @@ -213,22 +213,23 @@ radv_wsi_image_create(VkDevice device_h,
> > >                 RADV_FROM_HANDLE(radv_device_memory, memory,
> > > memory_h);
> > >                 if (!radv_get_memory_fd(device, memory, &fd))
> > >                         goto fail_alloc_memory;
> > > -               wsi_image->fd = fd;
> > > +               wsi_image->fds[0] = fd;
> > >         } else {
> > > -               wsi_image->fd = -1;
> > > +               wsi_image->fds[0] = -1;
> > >         }
> > >
> > >         surface = &image->surface;
> > >
> > >         wsi_image->image = image_h;
> > >         wsi_image->memory = memory_h;
> > > -       wsi_image->size = image->size;
> > > -       wsi_image->offset = image->offset;
> > > +       wsi_image->num_planes = 1;
> > > +       wsi_image->sizes[0] = image->size;
> > > +       wsi_image->offsets[0] = image->offset;
> > >         if (device->physical_device->rad_info.chip_class >= GFX9)
> > > -               wsi_image->row_pitch =
> > > +               wsi_image->row_pitches[0] =
> > >                         surface->u.gfx9.surf_pitch * surface->bpe;
> > >         else
> > > -               wsi_image->row_pitch =
> > > +               wsi_image->row_pitches[0] =
> > >                         surface->u.legacy.level[0].nblk_x *
> > > surface->bpe;
> > >
> > >         return VK_SUCCESS;
> > > diff --git a/src/intel/vulkan/anv_wsi.c
> > > b/src/intel/vulkan/anv_wsi.c
> > > index 916c62cad9..ae40f1f2f4 100644
> > > --- a/src/intel/vulkan/anv_wsi.c
> > > +++ b/src/intel/vulkan/anv_wsi.c
> > > @@ -266,10 +266,11 @@ anv_wsi_image_create(VkDevice device_h,
> > >
> > >     wsi_image->image = image_h;
> > >     wsi_image->memory = memory_h;
> > > -   wsi_image->fd = fd;
> > > -   wsi_image->size = image->size;
> > > -   wsi_image->offset = 0;
> > > -   wsi_image->row_pitch = surface->isl.row_pitch;
> > > +   wsi_image->num_planes = 1;
> > > +   wsi_image->fds[0] = fd;
> > > +   wsi_image->sizes[0] = image->size;
> > > +   wsi_image->offsets[0] = 0;
> > > +   wsi_image->row_pitches[0] = surface->isl.row_pitch;
> > >     return VK_SUCCESS;
> > >  fail_alloc_memory:
> > >     anv_FreeMemory(device_h, memory_h, pAllocator);
> > > diff --git a/src/vulkan/wsi/wsi_common.h
> > > b/src/vulkan/wsi/wsi_common.h
> > > index 1103703b0e..b6c5a438b1 100644
> > > --- a/src/vulkan/wsi/wsi_common.h
> > > +++ b/src/vulkan/wsi/wsi_common.h
> > > @@ -33,10 +33,11 @@
> > >  struct wsi_image_base {
> > >     VkImage image;
> > >     VkDeviceMemory memory;
> >
> > If any of the FDs ever point to different BOs, this will need to be
> > per-plane as well.
>
> I've noticed that while rebasing you didn't touch that part. Were you
> only pointing out that might be needed at some point? Should we make it
> per-plane right away?
>

I figured we could make that change when we came to it.


> > > -   uint32_t size;
> > > -   uint32_t offset;
> > > -   uint32_t row_pitch;
> > > -   int fd;
> > > +   int num_planes;
> > > +   uint32_t sizes[4];
> > > +   uint32_t offsets[4];
> > > +   uint32_t row_pitches[4];
> > > +   int fds[4];
> >
> > Would it be better to have an array of structs rather than SOA?
>
> While you were rewriting that part, it remained untouched. Any strong
> opinion about that one?
>

I looked at the kernel struct in drm_mode.h and saw that it was also done
this way.  We might as well keep it consistent.  The above two comments
were questions more than suggestions.

--Jason


> >
> > >  };
> > >
> > >  struct wsi_device;
> > > diff --git a/src/vulkan/wsi/wsi_common_wayland.c
> > > b/src/vulkan/wsi/wsi_common_wayland.c
> > > index 36cc4d0821..a76e29d26e 100644
> > > --- a/src/vulkan/wsi/wsi_common_wayland.c
> > > +++ b/src/vulkan/wsi/wsi_common_wayland.c
> > > @@ -736,15 +736,18 @@ wsi_wl_image_init(struct wsi_wl_swapchain
> > > *chain,
> > >     if (result != VK_SUCCESS)
> > >        return result;
> > >
> > > +   /* Without passing modifiers, we can't have multi-plane RGB
> > > images. */
> > > +   assert(image->base.num_planes == 1);
> > > +
> > >     image->buffer = wl_drm_create_prime_buffer(chain->drm_wrapper,
> > > -                                              image->base.fd, /*
> > > name */
> > > +                                              image->base.fds[0],
> > > /* name */
> > >                                                chain->extent.width,
> > >                                                chain-
> > > >extent.height,
> > >                                                chain->drm_format,
> > > -                                              image->base.offset,
> > > -                                              image-
> > > >base.row_pitch,
> > > +                                              image-
> > > >base.offsets[0],
> > > +                                              image-
> > > >base.row_pitches[0],
> > >                                                0, 0, 0, 0 /* unused
> > > */);
> > > -   close(image->base.fd);
> > > +   close(image->base.fds[0]);
> > >
> > >     if (!image->buffer)
> > >        goto fail_image;
> > > diff --git a/src/vulkan/wsi/wsi_common_x11.c
> > > b/src/vulkan/wsi/wsi_common_x11.c
> > > index 78fd406aa1..e48d746305 100644
> > > --- a/src/vulkan/wsi/wsi_common_x11.c
> > > +++ b/src/vulkan/wsi/wsi_common_x11.c
> > > @@ -988,18 +988,22 @@ x11_image_init(VkDevice device_h, struct
> > > x11_swapchain *chain,
> > >
> > >     struct wsi_image_base *image_ws =
> > >        chain->base.needs_linear_copy ? &image->linear_base :
> > > &image->base;
> > > +
> > > +   /* Without passing modifiers, we can't have multi-plane RGB
> > > images. */
> > > +   assert(image_ws->num_planes == 1);
> > > +
> > >     cookie =
> > >        xcb_dri3_pixmap_from_buffer_checked(chain->conn,
> > >                                            image->pixmap,
> > >                                            chain->window,
> > > -                                          image_ws->size,
> > > +                                          image_ws->sizes[0],
> > >                                            pCreateInfo-
> > > >imageExtent.width,
> > >                                            pCreateInfo-
> > > >imageExtent.height,
> > > -                                          image_ws->row_pitch,
> > > +                                          image_ws-
> > > >row_pitches[0],
> > >                                            chain->depth, bpp,
> > > -                                          image_ws->fd);
> > > +                                          image_ws->fds[0]);
> > >     xcb_discard_reply(chain->conn, cookie.sequence);
> > > -   image_ws->fd = -1; /* XCB has now taken ownership of the FD */
> > > +   image_ws->fds[0] = -1; /* XCB has now taken ownership of the FD
> > > */
> > >
> > >     int fence_fd = xshmfence_alloc_shm();
> > >     if (fence_fd < 0)
> > > --
> > > 2.13.0
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171120/b1ee30ca/attachment-0001.html>


More information about the mesa-dev mailing list