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

[Marek Olšák]

On Wed, Dec 6, 2017 at 5:08 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 6 December 2017 at 15:52, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> 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.
>>
> Humble request: please include the highlights of the discussion in the
> commit message ;-)

Sorry, I just pushed the patches.

Marek