Re: [PATCH libdrm 2/2] amdgpu: don't track handles for non-memory allocations

2018-10-29 Thread Marek Olšák 
OK. I'll drop this patch.

Marek

On Wed, Oct 24, 2018 at 4:14 AM Christian König <
ckoenig.leichtzumer...@gmail.com> wrote:

> Am 24.10.18 um 10:04 schrieb Michel Dänzer:
> > On 2018-10-23 9:07 p.m., Marek Olšák wrote:
> >> From: Marek Olšák 
> >>
> >> ---
> >>   amdgpu/amdgpu_bo.c | 15 +--
> >>   1 file changed, 9 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> >> index 81f8a5f7..00b9b54a 100644
> >> --- a/amdgpu/amdgpu_bo.c
> >> +++ b/amdgpu/amdgpu_bo.c
> >> @@ -91,26 +91,29 @@ drm_public int amdgpu_bo_alloc(amdgpu_device_handle
> dev,
> >>  if (r)
> >>  goto out;
> >>
> >>  r = amdgpu_bo_create(dev, alloc_buffer->alloc_size,
> args.out.handle,
> >>   buf_handle);
> >>  if (r) {
> >>  amdgpu_close_kms_handle(dev, args.out.handle);
> >>  goto out;
> >>  }
> >>
> >> -pthread_mutex_lock(&dev->bo_table_mutex);
> >> -r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
> >> -*buf_handle);
> >> -pthread_mutex_unlock(&dev->bo_table_mutex);
> >> -if (r)
> >> -amdgpu_bo_free(*buf_handle);
> >> +if (alloc_buffer->preferred_heap &
> >> +(AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
> > What about AMDGPU_GEM_DOMAIN_CPU? I mean, that's unlikely to actually be
> > used here, but if it were, exporting and importing the resulting BO
> > should work fine?
> >
> > Instead of white-listing the domains which can be shared, it might be
> > better to black-list those which can't, i.e. GDS/GWS/OA.
>
> Well first of all GDS can be shared between applications.
>
> Then adding a BO to the tracking doesn't add much overhead (only 8 bytes
> and only if it was the last allocated).
>
> So I don't really see a reason why we should do this?
>
> Christian.
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 2/2] amdgpu: don't track handles for non-memory allocations

2018-10-24 Thread Christian König 

Am 24.10.18 um 10:04 schrieb Michel Dänzer:

On 2018-10-23 9:07 p.m., Marek Olšák wrote:

From: Marek Olšák 

---
  amdgpu/amdgpu_bo.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 81f8a5f7..00b9b54a 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -91,26 +91,29 @@ drm_public int amdgpu_bo_alloc(amdgpu_device_handle dev,
if (r)
goto out;
  
  	r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle,

 buf_handle);
if (r) {
amdgpu_close_kms_handle(dev, args.out.handle);
goto out;
}
  
-	pthread_mutex_lock(&dev->bo_table_mutex);

-   r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
-   *buf_handle);
-   pthread_mutex_unlock(&dev->bo_table_mutex);
-   if (r)
-   amdgpu_bo_free(*buf_handle);
+   if (alloc_buffer->preferred_heap &
+   (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {

What about AMDGPU_GEM_DOMAIN_CPU? I mean, that's unlikely to actually be
used here, but if it were, exporting and importing the resulting BO
should work fine?

Instead of white-listing the domains which can be shared, it might be
better to black-list those which can't, i.e. GDS/GWS/OA.


Well first of all GDS can be shared between applications.

Then adding a BO to the tracking doesn't add much overhead (only 8 bytes 
and only if it was the last allocated).


So I don't really see a reason why we should do this?

Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 2/2] amdgpu: don't track handles for non-memory allocations

2018-10-24 Thread Michel Dänzer 
On 2018-10-23 9:07 p.m., Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  amdgpu/amdgpu_bo.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index 81f8a5f7..00b9b54a 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -91,26 +91,29 @@ drm_public int amdgpu_bo_alloc(amdgpu_device_handle dev,
>   if (r)
>   goto out;
>  
>   r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle,
>buf_handle);
>   if (r) {
>   amdgpu_close_kms_handle(dev, args.out.handle);
>   goto out;
>   }
>  
> - pthread_mutex_lock(&dev->bo_table_mutex);
> - r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
> - *buf_handle);
> - pthread_mutex_unlock(&dev->bo_table_mutex);
> - if (r)
> - amdgpu_bo_free(*buf_handle);
> + if (alloc_buffer->preferred_heap &
> + (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {

What about AMDGPU_GEM_DOMAIN_CPU? I mean, that's unlikely to actually be
used here, but if it were, exporting and importing the resulting BO
should work fine?

Instead of white-listing the domains which can be shared, it might be
better to black-list those which can't, i.e. GDS/GWS/OA.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 2/2] amdgpu: don't track handles for non-memory allocations

2018-10-23 Thread Zhang, Jerry(Junwei) 

On 10/24/18 3:07 AM, Marek Olšák wrote:

From: Marek Olšák 


commit log and sign-off here as well.
And any reason for that?

Regards,
Jerry



---
  amdgpu/amdgpu_bo.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 81f8a5f7..00b9b54a 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -91,26 +91,29 @@ drm_public int amdgpu_bo_alloc(amdgpu_device_handle dev,
if (r)
goto out;
  
  	r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle,

 buf_handle);
if (r) {
amdgpu_close_kms_handle(dev, args.out.handle);
goto out;
}
  
-	pthread_mutex_lock(&dev->bo_table_mutex);

-   r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
-   *buf_handle);
-   pthread_mutex_unlock(&dev->bo_table_mutex);
-   if (r)
-   amdgpu_bo_free(*buf_handle);
+   if (alloc_buffer->preferred_heap &
+   (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
+   pthread_mutex_lock(&dev->bo_table_mutex);
+   r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
+   *buf_handle);
+   pthread_mutex_unlock(&dev->bo_table_mutex);
+   if (r)
+   amdgpu_bo_free(*buf_handle);
+   }
  out:
return r;
  }
  
  drm_public int amdgpu_bo_set_metadata(amdgpu_bo_handle bo,

  struct amdgpu_bo_metadata *info)
  {
struct drm_amdgpu_gem_metadata args = {};
  
  	args.handle = bo->handle;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 2/2] amdgpu: don't track handles for non-memory allocations

2018-10-23 Thread Marek Olšák 
From: Marek Olšák 

---
 amdgpu/amdgpu_bo.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 81f8a5f7..00b9b54a 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -91,26 +91,29 @@ drm_public int amdgpu_bo_alloc(amdgpu_device_handle dev,
if (r)
goto out;
 
r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle,
 buf_handle);
if (r) {
amdgpu_close_kms_handle(dev, args.out.handle);
goto out;
}
 
-   pthread_mutex_lock(&dev->bo_table_mutex);
-   r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
-   *buf_handle);
-   pthread_mutex_unlock(&dev->bo_table_mutex);
-   if (r)
-   amdgpu_bo_free(*buf_handle);
+   if (alloc_buffer->preferred_heap &
+   (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
+   pthread_mutex_lock(&dev->bo_table_mutex);
+   r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
+   *buf_handle);
+   pthread_mutex_unlock(&dev->bo_table_mutex);
+   if (r)
+   amdgpu_bo_free(*buf_handle);
+   }
 out:
return r;
 }
 
 drm_public int amdgpu_bo_set_metadata(amdgpu_bo_handle bo,
  struct amdgpu_bo_metadata *info)
 {
struct drm_amdgpu_gem_metadata args = {};
 
args.handle = bo->handle;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx