Re: [PATCH libdrm 2/2] amdgpu: don't track handles for non-memory allocations
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
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
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
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
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