Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

2018-08-10 Thread Marek Olšák
OK. Thanks.

Marek

On Fri, Aug 10, 2018 at 9:06 AM, Christian König
 wrote:
> Why should it? Adding the handle is now not more than setting an array
> entry.
>
> I've tested with allocating 250k BOs of 4k size each and there wasn't any
> measurable performance differences.
>
> Christian.
>
>
> Am 09.08.2018 um 18:56 schrieb Marek Olšák:
>>
>> I don't think this is a good idea. Can you please explain why this
>> won't cause performance regressions?
>>
>> Thanks
>> Marek
>>
>> On Fri, Aug 3, 2018 at 7:34 AM, Christian König
>>  wrote:
>>>
>>> This way we can always find a BO structure by its handle.
>>>
>>> Signed-off-by: Christian König 
>>> ---
>>>   amdgpu/amdgpu_bo.c | 14 --
>>>   1 file changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>> index 02592377..422c7c99 100644
>>> --- a/amdgpu/amdgpu_bo.c
>>> +++ b/amdgpu/amdgpu_bo.c
>>> @@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
>>>
>>>  bo->handle = args.out.handle;
>>>
>>> +   pthread_mutex_lock(&bo->dev->bo_table_mutex);
>>> +   r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>>> +   pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>>> +
>>>  pthread_mutex_init(&bo->cpu_access_mutex, NULL);
>>>
>>>  if (r)
>>> @@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
>>>  return 0;
>>>   }
>>>
>>> -static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
>>> -{
>>> -   pthread_mutex_lock(&bo->dev->bo_table_mutex);
>>> -   handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>>> -   pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>>> -}
>>> -
>>>   static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
>>>   {
>>>  struct drm_gem_flink flink;
>>> @@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
>>>  return 0;
>>>
>>>  case amdgpu_bo_handle_type_kms:
>>> -   amdgpu_add_handle_to_table(bo);
>>> -   /* fall through */
>>>  case amdgpu_bo_handle_type_kms_noimport:
>>>  *shared_handle = bo->handle;
>>>  return 0;
>>>
>>>  case amdgpu_bo_handle_type_dma_buf_fd:
>>> -   amdgpu_add_handle_to_table(bo);
>>>  return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
>>>DRM_CLOEXEC | DRM_RDWR,
>>>(int*)shared_handle);
>>> --
>>> 2.14.1
>>>
>>> ___
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

2018-08-10 Thread Christian König
Why should it? Adding the handle is now not more than setting an array 
entry.


I've tested with allocating 250k BOs of 4k size each and there wasn't 
any measurable performance differences.


Christian.

Am 09.08.2018 um 18:56 schrieb Marek Olšák:

I don't think this is a good idea. Can you please explain why this
won't cause performance regressions?

Thanks
Marek

On Fri, Aug 3, 2018 at 7:34 AM, Christian König
 wrote:

This way we can always find a BO structure by its handle.

Signed-off-by: Christian König 
---
  amdgpu/amdgpu_bo.c | 14 --
  1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 02592377..422c7c99 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,

 bo->handle = args.out.handle;

+   pthread_mutex_lock(&bo->dev->bo_table_mutex);
+   r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
+   pthread_mutex_unlock(&bo->dev->bo_table_mutex);
+
 pthread_mutex_init(&bo->cpu_access_mutex, NULL);

 if (r)
@@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
 return 0;
  }

-static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
-{
-   pthread_mutex_lock(&bo->dev->bo_table_mutex);
-   handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
-   pthread_mutex_unlock(&bo->dev->bo_table_mutex);
-}
-
  static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
  {
 struct drm_gem_flink flink;
@@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
 return 0;

 case amdgpu_bo_handle_type_kms:
-   amdgpu_add_handle_to_table(bo);
-   /* fall through */
 case amdgpu_bo_handle_type_kms_noimport:
 *shared_handle = bo->handle;
 return 0;

 case amdgpu_bo_handle_type_dma_buf_fd:
-   amdgpu_add_handle_to_table(bo);
 return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
   DRM_CLOEXEC | DRM_RDWR,
   (int*)shared_handle);
--
2.14.1

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


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


Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

2018-08-09 Thread Marek Olšák
I don't think this is a good idea. Can you please explain why this
won't cause performance regressions?

Thanks
Marek

On Fri, Aug 3, 2018 at 7:34 AM, Christian König
 wrote:
> This way we can always find a BO structure by its handle.
>
> Signed-off-by: Christian König 
> ---
>  amdgpu/amdgpu_bo.c | 14 --
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index 02592377..422c7c99 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
>
> bo->handle = args.out.handle;
>
> +   pthread_mutex_lock(&bo->dev->bo_table_mutex);
> +   r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
> +   pthread_mutex_unlock(&bo->dev->bo_table_mutex);
> +
> pthread_mutex_init(&bo->cpu_access_mutex, NULL);
>
> if (r)
> @@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
> return 0;
>  }
>
> -static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
> -{
> -   pthread_mutex_lock(&bo->dev->bo_table_mutex);
> -   handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
> -   pthread_mutex_unlock(&bo->dev->bo_table_mutex);
> -}
> -
>  static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
>  {
> struct drm_gem_flink flink;
> @@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
> return 0;
>
> case amdgpu_bo_handle_type_kms:
> -   amdgpu_add_handle_to_table(bo);
> -   /* fall through */
> case amdgpu_bo_handle_type_kms_noimport:
> *shared_handle = bo->handle;
> return 0;
>
> case amdgpu_bo_handle_type_dma_buf_fd:
> -   amdgpu_add_handle_to_table(bo);
> return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
>   DRM_CLOEXEC | DRM_RDWR,
>   (int*)shared_handle);
> --
> 2.14.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

2018-08-03 Thread Deucher, Alexander
Ping ajax or daniels or anholt on IRC.


Alex


From: amd-gfx  on behalf of Christian 
König 
Sent: Friday, August 3, 2018 9:54:42 AM
To: Michel Dänzer
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

Am 03.08.2018 um 15:52 schrieb Michel Dänzer:
> On 2018-08-03 01:34 PM, Christian König wrote:
>> This way we can always find a BO structure by its handle.
>>
>> Signed-off-by: Christian König 
> In the shortlog, should be "handle table" instead of "lockup table"?
>
> With that fixed, the series is
>
> Reviewed-by: Michel Dänzer 
>
>
> Thanks for doing this!
No, problem. Could you fix up the subject line and push it upstream?

I still don't have a working fdo account again :( Alternatively do you
know an admin I can ping about that?

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


Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

2018-08-03 Thread Christian König

Am 03.08.2018 um 15:52 schrieb Michel Dänzer:

On 2018-08-03 01:34 PM, Christian König wrote:

This way we can always find a BO structure by its handle.

Signed-off-by: Christian König 

In the shortlog, should be "handle table" instead of "lockup table"?

With that fixed, the series is

Reviewed-by: Michel Dänzer 


Thanks for doing this!

No, problem. Could you fix up the subject line and push it upstream?

I still don't have a working fdo account again :( Alternatively do you 
know an admin I can ping about that?


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


Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

2018-08-03 Thread Michel Dänzer
On 2018-08-03 01:34 PM, Christian König wrote:
> This way we can always find a BO structure by its handle.
> 
> Signed-off-by: Christian König 

In the shortlog, should be "handle table" instead of "lockup table"?

With that fixed, the series is

Reviewed-by: Michel Dänzer 


Thanks for doing this!


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