Re: [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  wrote:
> On 6 December 2017 at 15:52, Nicolai Hähnle  wrote:
>> On 06.12.2017 13:43, Marek Olšák wrote:
>>>
>>>
>>>
>>> On Dec 6, 2017 12:34 PM, "Nicolai Hähnle" >> > wrote:
>>>
>>> On 05.12.2017 20:05, Marek Olšák wrote:
>>>
>>> From: Marek Olšák >> >
>>>
>>> Cc: 17.3 >> >
>>>
>>> ---
>>>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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 6 December 2017 at 15:52, Nicolai Hähnle  wrote:
> On 06.12.2017 13:43, Marek Olšák wrote:
>>
>>
>>
>> On Dec 6, 2017 12:34 PM, "Nicolai Hähnle" > > wrote:
>>
>> On 05.12.2017 20:05, Marek Olšák wrote:
>>
>> From: Marek Olšák > >
>>
>> Cc: 17.3 > >
>>
>> ---
>>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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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" > wrote:


On 05.12.2017 20:05, Marek Olšák wrote:

From: Marek Olšák >

Cc: 17.3 >
---
   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,
);
                         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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

On Dec 6, 2017 12:34 PM, "Nicolai Hähnle"  wrote:

On 05.12.2017 20:05, Marek Olšák wrote:

> From: Marek Olšák 
>
> Cc: 17.3 
> ---
>   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.

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, );
> if (!newb)
> return false;
>
>

-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Nicolai Hähnle]

On 05.12.2017 20:05, Marek Olšák wrote:

From: Marek Olšák 

Cc: 17.3 
---
  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.


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, );
if (!newb)
return false;




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

From: Marek Olšák 

Cc: 17.3 
---
 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)) {
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, );
if (!newb)
return false;
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev