Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table
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
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
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
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
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
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
[PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table
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