[Mesa-dev] [PATCH 1/6] radeonsi: allow DMABUF exports for local buffers

[Nicolai Hähnle]

On 06.12.2017 13:43, Marek Olšák wrote:
> 
> 
> On Dec 6, 2017 12:34 PM, "Nicolai Hähnle" <nhaehnle at gmail.com 
> <mailto:nhaehnle at gmail.com>> wrote:
> 
>     On 05.12.2017 20:05, Marek Olšák wrote:
> 
>         From: Marek Olšák <marek.olsak at amd.com <mailto:marek.olsak at amd.com>>
> 
>         Cc: 17.3 <mesa-stable at lists.freedesktop.org
>         <mailto:mesa-stable at lists.freedesktop.org>>
>         ---
>            src/gallium/drivers/radeon/r600_texture.c | 6 +++++-
>            1 file changed, 5 insertions(+), 1 deletion(-)
> 
>         diff --git a/src/gallium/drivers/radeon/r600_texture.c
>         b/src/gallium/drivers/radeon/r600_texture.c
>         index 2aa47b5..07f7c33 100644
>         --- a/src/gallium/drivers/radeon/r600_texture.c
>         +++ b/src/gallium/drivers/radeon/r600_texture.c
>         @@ -739,22 +739,26 @@ static boolean
>         r600_texture_get_handle(struct pipe_screen* screen,
>                                  stride = rtex->surface.u.gfx9.surf_pitch *
>                                           rtex->surface.bpe;
>                                  slice_size =
>         rtex->surface.u.gfx9.surf_slice_size;
>                          } else {
>                                  offset =
>         rtex->surface.u.legacy.level[0].offset;
>                                  stride =
>         rtex->surface.u.legacy.level[0].nblk_x *
>                                           rtex->surface.bpe;
>                                  slice_size =
>         (uint64_t)rtex->surface.u.legacy.level[0].slice_size_dw * 4;
>                          }
>                  } else {
>         +               /* Buffer exports are for the OpenCL interop. */
>                          /* Move a suballocated buffer into a
>         non-suballocated allocation. */
>         -               if (sscreen->ws->buffer_is_suballocated(res->buf)) {
>         +               if (sscreen->ws->buffer_is_suballocated(res->buf) ||
>         +                   /* A DMABUF export always fails if the BO is
>         local. */
>         +                   (rtex->resource.flags &
>         RADEON_FLAG_NO_INTERPROCESS_SHARING &&
>         +                    whandle->type != DRM_API_HANDLE_TYPE_KMS)) {
> 
> 
>     I still don't think this is right. Or at least, it's bound to blow
>     up in our faces at some point. Though I think we may have talked
>     past each other, apologies that I haven't made myself clear.
> 
>     The issues I have in mind are scenarios like this:
> 
>     1. Buffer allocated in OpenGL.
>     2. Buffer exported as KMS handle for importing to OpenCL in the same
>     process.
>     3. Buffer exported as an FD <-- at this point, the OpenGL and OpenCL
>     buffers go out of sync because OpenGL re-allocates the buffer but
>     OpenCL isn't informed.
> 
>     Or:
> 
>     1. Buffer allocated in OpenGL.
>     2. Buffer exported as KMS handle for importing to OpenCL in the same
>     process.
>     3. Buffer attempted to be exported as an FD from OpenCL <-- fails
>     because the buffer is local (has NO_INTERPROCESS_SHARING), and
>     people will be utterly clueless as to what's going on.
> 
>     FWIW, I think the patch is good if you drop the whandle->type check
>     so that we re-allocate unconditionally.
> 
> 
> I can remove the check, but buffers are only exported as DMABUF. This 
> patch isn't just random - it does fix OpenCL interop.

Well, I doubt it's critical for performance, OpenCL doesn't know about 
the NO_INTERPROCESS_SHARING status anyway. So I'd prefer to remove the 
check, in case we change the exporting at some point.

Cheers,
Nicolai


> 
> Marek
> 
> 
> 
>     Cheers,
>     Nicolai
> 
> 
> 
>                                  assert(!res->b.is_shared);
>                                  /* Allocate a new buffer with
>         PIPE_BIND_SHARED. */
>                                  struct pipe_resource templ = res->b.b;
>                                  templ.bind |= PIPE_BIND_SHARED;
>                                  struct pipe_resource *newb =
>                                          screen->resource_create(screen,
>         &templ);
>                                  if (!newb)
>                                          return false;
> 
> 
> 
>     -- 
>     Lerne, wie die Welt wirklich ist,
>     Aber vergiss niemals, wie sie sein sollte.
> 
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.