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

[Emil Velikov]

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 ;-)

Thanks
Emil