* [PATCH libdrm] amdgpu: add a function to create amdgpu bo internally (v4)
@ 2018-08-13 10:06 Junwei Zhang
[not found] ` <1534154763-2287-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Junwei Zhang @ 2018-08-13 10:06 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Junwei Zhang, christian.koenig-5C7GfCeVMHo
a helper function to create and initialize amdgpu bo
v2: update error handling: add label and free bo
v3: update error handling: separate each error label
v4: update error handling and free flink bo in bo import
Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
amdgpu/amdgpu_bo.c | 208 +++++++++++++++++++++++++++--------------------------
1 file changed, 107 insertions(+), 101 deletions(-)
diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index b790e9b..ad72f09 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -48,11 +48,31 @@ static void amdgpu_close_kms_handle(amdgpu_device_handle dev,
drmIoctl(dev->fd, DRM_IOCTL_GEM_CLOSE, &args);
}
+static int amdgpu_bo_create(amdgpu_device_handle dev,
+ uint64_t size,
+ uint32_t handle,
+ amdgpu_bo_handle *buf_handle)
+{
+ struct amdgpu_bo *bo;
+
+ bo = calloc(1, sizeof(struct amdgpu_bo));
+ if (!bo)
+ return -ENOMEM;
+
+ atomic_set(&bo->refcount, 1);
+ bo->dev = dev;
+ bo->alloc_size = size;
+ bo->handle = handle;
+ pthread_mutex_init(&bo->cpu_access_mutex, NULL);
+
+ *buf_handle = bo;
+ return 0;
+}
+
int amdgpu_bo_alloc(amdgpu_device_handle dev,
struct amdgpu_bo_alloc_request *alloc_buffer,
amdgpu_bo_handle *buf_handle)
{
- struct amdgpu_bo *bo;
union drm_amdgpu_gem_create args;
unsigned heap = alloc_buffer->preferred_heap;
int r = 0;
@@ -61,14 +81,6 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
if (!(heap & (AMDGPU_GEM_DOMAIN_GTT | AMDGPU_GEM_DOMAIN_VRAM)))
return -EINVAL;
- bo = calloc(1, sizeof(struct amdgpu_bo));
- if (!bo)
- return -ENOMEM;
-
- atomic_set(&bo->refcount, 1);
- bo->dev = dev;
- bo->alloc_size = alloc_buffer->alloc_size;
-
memset(&args, 0, sizeof(args));
args.in.bo_size = alloc_buffer->alloc_size;
args.in.alignment = alloc_buffer->phys_alignment;
@@ -80,24 +92,23 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
/* Allocate the buffer with the preferred heap. */
r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_CREATE,
&args, sizeof(args));
+ if (r)
+ goto out;
+
+ r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle,
+ buf_handle);
if (r) {
- free(bo);
- return r;
+ amdgpu_close_kms_handle(dev, args.out.handle);
+ goto out;
}
- 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);
-
+ 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(bo);
- else
- *buf_handle = bo;
-
+ amdgpu_bo_free(*buf_handle);
+out:
return r;
}
@@ -255,8 +266,11 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
struct amdgpu_bo_import_result *output)
{
struct drm_gem_open open_arg = {};
+ struct drm_gem_close close_args = {};
struct amdgpu_bo *bo = NULL;
- int r;
+ uint32_t handle = 0, flink_name = 0;
+ uint64_t alloc_size = 0;
+ int r = 0;
int dma_fd;
uint64_t dma_buf_size = 0;
@@ -266,22 +280,18 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
/* Convert a DMA buf handle to a KMS handle now. */
if (type == amdgpu_bo_handle_type_dma_buf_fd) {
- uint32_t handle;
off_t size;
/* Get a KMS handle. */
r = drmPrimeFDToHandle(dev->fd, shared_handle, &handle);
- if (r) {
- pthread_mutex_unlock(&dev->bo_table_mutex);
- return r;
- }
+ if (r)
+ goto unlock;
/* Query the buffer size. */
size = lseek(shared_handle, 0, SEEK_END);
if (size == (off_t)-1) {
- pthread_mutex_unlock(&dev->bo_table_mutex);
- amdgpu_close_kms_handle(dev, handle);
- return -errno;
+ r = -errno;
+ goto free_bo_handle;
}
lseek(shared_handle, 0, SEEK_SET);
@@ -302,12 +312,12 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
case amdgpu_bo_handle_type_kms:
case amdgpu_bo_handle_type_kms_noimport:
/* Importing a KMS handle in not allowed. */
- pthread_mutex_unlock(&dev->bo_table_mutex);
- return -EPERM;
+ r = -EPERM;
+ goto unlock;
default:
- pthread_mutex_unlock(&dev->bo_table_mutex);
- return -EINVAL;
+ r = -EINVAL;
+ goto unlock;
}
if (bo) {
@@ -320,58 +330,37 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
return 0;
}
- bo = calloc(1, sizeof(struct amdgpu_bo));
- if (!bo) {
- pthread_mutex_unlock(&dev->bo_table_mutex);
- if (type == amdgpu_bo_handle_type_dma_buf_fd) {
- amdgpu_close_kms_handle(dev, shared_handle);
- }
- return -ENOMEM;
- }
-
/* Open the handle. */
switch (type) {
case amdgpu_bo_handle_type_gem_flink_name:
open_arg.name = shared_handle;
r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_OPEN, &open_arg);
- if (r) {
- free(bo);
- pthread_mutex_unlock(&dev->bo_table_mutex);
- return r;
- }
+ if (r)
+ goto unlock;
- bo->handle = open_arg.handle;
+ flink_name = shared_handle;
+ handle = open_arg.handle;
+ alloc_size = open_arg.size;
if (dev->flink_fd != dev->fd) {
- r = drmPrimeHandleToFD(dev->flink_fd, bo->handle, DRM_CLOEXEC, &dma_fd);
- if (r) {
- free(bo);
- pthread_mutex_unlock(&dev->bo_table_mutex);
- return r;
- }
- r = drmPrimeFDToHandle(dev->fd, dma_fd, &bo->handle );
-
+ r = drmPrimeHandleToFD(dev->flink_fd, handle,
+ DRM_CLOEXEC, &dma_fd);
+ if (r)
+ goto free_bo_handle;
+ r = drmPrimeFDToHandle(dev->fd, dma_fd, &handle);
close(dma_fd);
-
- if (r) {
- free(bo);
- pthread_mutex_unlock(&dev->bo_table_mutex);
- return r;
- }
- }
- bo->flink_name = shared_handle;
- bo->alloc_size = open_arg.size;
- r = handle_table_insert(&dev->bo_flink_names, shared_handle,
- bo);
- if (r) {
- pthread_mutex_unlock(&dev->bo_table_mutex);
- amdgpu_bo_free(bo);
- return r;
+ if (r)
+ goto free_bo_handle;
+ close_args.handle = open_arg.handle;
+ r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_CLOSE,
+ &close_args);
+ if (r)
+ goto free_bo_handle;
}
break;
case amdgpu_bo_handle_type_dma_buf_fd:
- bo->handle = shared_handle;
- bo->alloc_size = dma_buf_size;
+ handle = shared_handle;
+ alloc_size = dma_buf_size;
break;
case amdgpu_bo_handle_type_kms:
@@ -380,16 +369,41 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
}
/* Initialize it. */
- atomic_set(&bo->refcount, 1);
- bo->dev = dev;
- pthread_mutex_init(&bo->cpu_access_mutex, NULL);
+ r = amdgpu_bo_create(dev, alloc_size, handle, &bo);
+ if (r)
+ goto free_bo_handle;
- handle_table_insert(&dev->bo_handles, bo->handle, bo);
- pthread_mutex_unlock(&dev->bo_table_mutex);
+ r = handle_table_insert(&dev->bo_handles, bo->handle, bo);
+ if (r)
+ goto free_bo_handle;
+ if (flink_name) {
+ bo->flink_name = flink_name;
+ r = handle_table_insert(&dev->bo_flink_names, flink_name,
+ bo);
+ if (r)
+ goto remove_handle;
+
+ }
output->buf_handle = bo;
output->alloc_size = bo->alloc_size;
+ pthread_mutex_unlock(&dev->bo_table_mutex);
return 0;
+
+remove_handle:
+ handle_table_remove(&dev->bo_handles, bo->handle);
+free_bo_handle:
+ if (flink_name && !close_args.handle && open_arg.handle) {
+ close_args.handle = open_arg.handle;
+ drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_CLOSE, &close_args);
+ }
+ if (bo)
+ amdgpu_bo_free(bo);
+ else
+ amdgpu_close_kms_handle(dev, handle);
+unlock:
+ pthread_mutex_unlock(&dev->bo_table_mutex);
+ return r;
}
int amdgpu_bo_free(amdgpu_bo_handle buf_handle)
@@ -574,7 +588,6 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
amdgpu_bo_handle *buf_handle)
{
int r;
- struct amdgpu_bo *bo;
struct drm_amdgpu_gem_userptr args;
args.addr = (uintptr_t)cpu;
@@ -584,28 +597,21 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_USERPTR,
&args, sizeof(args));
if (r)
- return r;
-
- bo = calloc(1, sizeof(struct amdgpu_bo));
- if (!bo)
- return -ENOMEM;
-
- atomic_set(&bo->refcount, 1);
- bo->dev = dev;
- bo->alloc_size = size;
- bo->handle = args.handle;
+ goto out;
- 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);
+ r = amdgpu_bo_create(dev, size, args.handle, buf_handle);
+ if (r) {
+ amdgpu_close_kms_handle(dev, args.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(bo);
- else
- *buf_handle = bo;
-
+ amdgpu_bo_free(*buf_handle);
+out:
return r;
}
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread[parent not found: <1534154763-2287-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH libdrm] amdgpu: add a function to create amdgpu bo internally (v4) [not found] ` <1534154763-2287-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org> @ 2018-08-13 10:14 ` Christian König [not found] ` <e5ed0c82-74ad-f7ea-58af-84f3dee3e090-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 3+ messages in thread From: Christian König @ 2018-08-13 10:14 UTC (permalink / raw) To: Junwei Zhang, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 13.08.2018 um 12:06 schrieb Junwei Zhang: > a helper function to create and initialize amdgpu bo > > v2: update error handling: add label and free bo > v3: update error handling: separate each error label > v4: update error handling and free flink bo in bo import > > Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> A separate patch for the fix to flink handle handling would be nice, but patch is Reviewed-by: Christian König <christian.koenig@amd.com> anyway. Regards, Christian. > --- > amdgpu/amdgpu_bo.c | 208 +++++++++++++++++++++++++++-------------------------- > 1 file changed, 107 insertions(+), 101 deletions(-) > > diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c > index b790e9b..ad72f09 100644 > --- a/amdgpu/amdgpu_bo.c > +++ b/amdgpu/amdgpu_bo.c > @@ -48,11 +48,31 @@ static void amdgpu_close_kms_handle(amdgpu_device_handle dev, > drmIoctl(dev->fd, DRM_IOCTL_GEM_CLOSE, &args); > } > > +static int amdgpu_bo_create(amdgpu_device_handle dev, > + uint64_t size, > + uint32_t handle, > + amdgpu_bo_handle *buf_handle) > +{ > + struct amdgpu_bo *bo; > + > + bo = calloc(1, sizeof(struct amdgpu_bo)); > + if (!bo) > + return -ENOMEM; > + > + atomic_set(&bo->refcount, 1); > + bo->dev = dev; > + bo->alloc_size = size; > + bo->handle = handle; > + pthread_mutex_init(&bo->cpu_access_mutex, NULL); > + > + *buf_handle = bo; > + return 0; > +} > + > int amdgpu_bo_alloc(amdgpu_device_handle dev, > struct amdgpu_bo_alloc_request *alloc_buffer, > amdgpu_bo_handle *buf_handle) > { > - struct amdgpu_bo *bo; > union drm_amdgpu_gem_create args; > unsigned heap = alloc_buffer->preferred_heap; > int r = 0; > @@ -61,14 +81,6 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev, > if (!(heap & (AMDGPU_GEM_DOMAIN_GTT | AMDGPU_GEM_DOMAIN_VRAM))) > return -EINVAL; > > - bo = calloc(1, sizeof(struct amdgpu_bo)); > - if (!bo) > - return -ENOMEM; > - > - atomic_set(&bo->refcount, 1); > - bo->dev = dev; > - bo->alloc_size = alloc_buffer->alloc_size; > - > memset(&args, 0, sizeof(args)); > args.in.bo_size = alloc_buffer->alloc_size; > args.in.alignment = alloc_buffer->phys_alignment; > @@ -80,24 +92,23 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev, > /* Allocate the buffer with the preferred heap. */ > r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_CREATE, > &args, sizeof(args)); > + if (r) > + goto out; > + > + r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle, > + buf_handle); > if (r) { > - free(bo); > - return r; > + amdgpu_close_kms_handle(dev, args.out.handle); > + goto out; > } > > - 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); > - > + 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(bo); > - else > - *buf_handle = bo; > - > + amdgpu_bo_free(*buf_handle); > +out: > return r; > } > > @@ -255,8 +266,11 @@ int amdgpu_bo_import(amdgpu_device_handle dev, > struct amdgpu_bo_import_result *output) > { > struct drm_gem_open open_arg = {}; > + struct drm_gem_close close_args = {}; > struct amdgpu_bo *bo = NULL; > - int r; > + uint32_t handle = 0, flink_name = 0; > + uint64_t alloc_size = 0; > + int r = 0; > int dma_fd; > uint64_t dma_buf_size = 0; > > @@ -266,22 +280,18 @@ int amdgpu_bo_import(amdgpu_device_handle dev, > > /* Convert a DMA buf handle to a KMS handle now. */ > if (type == amdgpu_bo_handle_type_dma_buf_fd) { > - uint32_t handle; > off_t size; > > /* Get a KMS handle. */ > r = drmPrimeFDToHandle(dev->fd, shared_handle, &handle); > - if (r) { > - pthread_mutex_unlock(&dev->bo_table_mutex); > - return r; > - } > + if (r) > + goto unlock; > > /* Query the buffer size. */ > size = lseek(shared_handle, 0, SEEK_END); > if (size == (off_t)-1) { > - pthread_mutex_unlock(&dev->bo_table_mutex); > - amdgpu_close_kms_handle(dev, handle); > - return -errno; > + r = -errno; > + goto free_bo_handle; > } > lseek(shared_handle, 0, SEEK_SET); > > @@ -302,12 +312,12 @@ int amdgpu_bo_import(amdgpu_device_handle dev, > case amdgpu_bo_handle_type_kms: > case amdgpu_bo_handle_type_kms_noimport: > /* Importing a KMS handle in not allowed. */ > - pthread_mutex_unlock(&dev->bo_table_mutex); > - return -EPERM; > + r = -EPERM; > + goto unlock; > > default: > - pthread_mutex_unlock(&dev->bo_table_mutex); > - return -EINVAL; > + r = -EINVAL; > + goto unlock; > } > > if (bo) { > @@ -320,58 +330,37 @@ int amdgpu_bo_import(amdgpu_device_handle dev, > return 0; > } > > - bo = calloc(1, sizeof(struct amdgpu_bo)); > - if (!bo) { > - pthread_mutex_unlock(&dev->bo_table_mutex); > - if (type == amdgpu_bo_handle_type_dma_buf_fd) { > - amdgpu_close_kms_handle(dev, shared_handle); > - } > - return -ENOMEM; > - } > - > /* Open the handle. */ > switch (type) { > case amdgpu_bo_handle_type_gem_flink_name: > open_arg.name = shared_handle; > r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_OPEN, &open_arg); > - if (r) { > - free(bo); > - pthread_mutex_unlock(&dev->bo_table_mutex); > - return r; > - } > + if (r) > + goto unlock; > > - bo->handle = open_arg.handle; > + flink_name = shared_handle; > + handle = open_arg.handle; > + alloc_size = open_arg.size; > if (dev->flink_fd != dev->fd) { > - r = drmPrimeHandleToFD(dev->flink_fd, bo->handle, DRM_CLOEXEC, &dma_fd); > - if (r) { > - free(bo); > - pthread_mutex_unlock(&dev->bo_table_mutex); > - return r; > - } > - r = drmPrimeFDToHandle(dev->fd, dma_fd, &bo->handle ); > - > + r = drmPrimeHandleToFD(dev->flink_fd, handle, > + DRM_CLOEXEC, &dma_fd); > + if (r) > + goto free_bo_handle; > + r = drmPrimeFDToHandle(dev->fd, dma_fd, &handle); > close(dma_fd); > - > - if (r) { > - free(bo); > - pthread_mutex_unlock(&dev->bo_table_mutex); > - return r; > - } > - } > - bo->flink_name = shared_handle; > - bo->alloc_size = open_arg.size; > - r = handle_table_insert(&dev->bo_flink_names, shared_handle, > - bo); > - if (r) { > - pthread_mutex_unlock(&dev->bo_table_mutex); > - amdgpu_bo_free(bo); > - return r; > + if (r) > + goto free_bo_handle; > + close_args.handle = open_arg.handle; > + r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_CLOSE, > + &close_args); > + if (r) > + goto free_bo_handle; > } > break; > > case amdgpu_bo_handle_type_dma_buf_fd: > - bo->handle = shared_handle; > - bo->alloc_size = dma_buf_size; > + handle = shared_handle; > + alloc_size = dma_buf_size; > break; > > case amdgpu_bo_handle_type_kms: > @@ -380,16 +369,41 @@ int amdgpu_bo_import(amdgpu_device_handle dev, > } > > /* Initialize it. */ > - atomic_set(&bo->refcount, 1); > - bo->dev = dev; > - pthread_mutex_init(&bo->cpu_access_mutex, NULL); > + r = amdgpu_bo_create(dev, alloc_size, handle, &bo); > + if (r) > + goto free_bo_handle; > > - handle_table_insert(&dev->bo_handles, bo->handle, bo); > - pthread_mutex_unlock(&dev->bo_table_mutex); > + r = handle_table_insert(&dev->bo_handles, bo->handle, bo); > + if (r) > + goto free_bo_handle; > + if (flink_name) { > + bo->flink_name = flink_name; > + r = handle_table_insert(&dev->bo_flink_names, flink_name, > + bo); > + if (r) > + goto remove_handle; > + > + } > > output->buf_handle = bo; > output->alloc_size = bo->alloc_size; > + pthread_mutex_unlock(&dev->bo_table_mutex); > return 0; > + > +remove_handle: > + handle_table_remove(&dev->bo_handles, bo->handle); > +free_bo_handle: > + if (flink_name && !close_args.handle && open_arg.handle) { > + close_args.handle = open_arg.handle; > + drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_CLOSE, &close_args); > + } > + if (bo) > + amdgpu_bo_free(bo); > + else > + amdgpu_close_kms_handle(dev, handle); > +unlock: > + pthread_mutex_unlock(&dev->bo_table_mutex); > + return r; > } > > int amdgpu_bo_free(amdgpu_bo_handle buf_handle) > @@ -574,7 +588,6 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev, > amdgpu_bo_handle *buf_handle) > { > int r; > - struct amdgpu_bo *bo; > struct drm_amdgpu_gem_userptr args; > > args.addr = (uintptr_t)cpu; > @@ -584,28 +597,21 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev, > r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_USERPTR, > &args, sizeof(args)); > if (r) > - return r; > - > - bo = calloc(1, sizeof(struct amdgpu_bo)); > - if (!bo) > - return -ENOMEM; > - > - atomic_set(&bo->refcount, 1); > - bo->dev = dev; > - bo->alloc_size = size; > - bo->handle = args.handle; > + goto out; > > - 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); > + r = amdgpu_bo_create(dev, size, args.handle, buf_handle); > + if (r) { > + amdgpu_close_kms_handle(dev, args.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(bo); > - else > - *buf_handle = bo; > - > + amdgpu_bo_free(*buf_handle); > +out: > return r; > } > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <e5ed0c82-74ad-f7ea-58af-84f3dee3e090-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH libdrm] amdgpu: add a function to create amdgpu bo internally (v4) [not found] ` <e5ed0c82-74ad-f7ea-58af-84f3dee3e090-5C7GfCeVMHo@public.gmane.org> @ 2018-08-14 2:59 ` Zhang, Jerry (Junwei) 0 siblings, 0 replies; 3+ messages in thread From: Zhang, Jerry (Junwei) @ 2018-08-14 2:59 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 08/13/2018 06:14 PM, Christian König wrote: > Am 13.08.2018 um 12:06 schrieb Junwei Zhang: >> a helper function to create and initialize amdgpu bo >> >> v2: update error handling: add label and free bo >> v3: update error handling: separate each error label >> v4: update error handling and free flink bo in bo import >> >> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> > > A separate patch for the fix to flink handle handling would be nice, but patch is Reviewed-by: Christian König <christian.koenig@amd.com> anyway. Thanks, yes. I will prepare the fix patch for original code base, then it could be ported for legacy libdrm. This patch could be rebased as final version. Jerry > > Regards, > Christian. > >> --- >> amdgpu/amdgpu_bo.c | 208 +++++++++++++++++++++++++++-------------------------- >> 1 file changed, 107 insertions(+), 101 deletions(-) >> >> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c >> index b790e9b..ad72f09 100644 >> --- a/amdgpu/amdgpu_bo.c >> +++ b/amdgpu/amdgpu_bo.c >> @@ -48,11 +48,31 @@ static void amdgpu_close_kms_handle(amdgpu_device_handle dev, >> drmIoctl(dev->fd, DRM_IOCTL_GEM_CLOSE, &args); >> } >> +static int amdgpu_bo_create(amdgpu_device_handle dev, >> + uint64_t size, >> + uint32_t handle, >> + amdgpu_bo_handle *buf_handle) >> +{ >> + struct amdgpu_bo *bo; >> + >> + bo = calloc(1, sizeof(struct amdgpu_bo)); >> + if (!bo) >> + return -ENOMEM; >> + >> + atomic_set(&bo->refcount, 1); >> + bo->dev = dev; >> + bo->alloc_size = size; >> + bo->handle = handle; >> + pthread_mutex_init(&bo->cpu_access_mutex, NULL); >> + >> + *buf_handle = bo; >> + return 0; >> +} >> + >> int amdgpu_bo_alloc(amdgpu_device_handle dev, >> struct amdgpu_bo_alloc_request *alloc_buffer, >> amdgpu_bo_handle *buf_handle) >> { >> - struct amdgpu_bo *bo; >> union drm_amdgpu_gem_create args; >> unsigned heap = alloc_buffer->preferred_heap; >> int r = 0; >> @@ -61,14 +81,6 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev, >> if (!(heap & (AMDGPU_GEM_DOMAIN_GTT | AMDGPU_GEM_DOMAIN_VRAM))) >> return -EINVAL; >> - bo = calloc(1, sizeof(struct amdgpu_bo)); >> - if (!bo) >> - return -ENOMEM; >> - >> - atomic_set(&bo->refcount, 1); >> - bo->dev = dev; >> - bo->alloc_size = alloc_buffer->alloc_size; >> - >> memset(&args, 0, sizeof(args)); >> args.in.bo_size = alloc_buffer->alloc_size; >> args.in.alignment = alloc_buffer->phys_alignment; >> @@ -80,24 +92,23 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev, >> /* Allocate the buffer with the preferred heap. */ >> r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_CREATE, >> &args, sizeof(args)); >> + if (r) >> + goto out; >> + >> + r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle, >> + buf_handle); >> if (r) { >> - free(bo); >> - return r; >> + amdgpu_close_kms_handle(dev, args.out.handle); >> + goto out; >> } >> - 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); >> - >> + 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(bo); >> - else >> - *buf_handle = bo; >> - >> + amdgpu_bo_free(*buf_handle); >> +out: >> return r; >> } >> @@ -255,8 +266,11 @@ int amdgpu_bo_import(amdgpu_device_handle dev, >> struct amdgpu_bo_import_result *output) >> { >> struct drm_gem_open open_arg = {}; >> + struct drm_gem_close close_args = {}; >> struct amdgpu_bo *bo = NULL; >> - int r; >> + uint32_t handle = 0, flink_name = 0; >> + uint64_t alloc_size = 0; >> + int r = 0; >> int dma_fd; >> uint64_t dma_buf_size = 0; >> @@ -266,22 +280,18 @@ int amdgpu_bo_import(amdgpu_device_handle dev, >> /* Convert a DMA buf handle to a KMS handle now. */ >> if (type == amdgpu_bo_handle_type_dma_buf_fd) { >> - uint32_t handle; >> off_t size; >> /* Get a KMS handle. */ >> r = drmPrimeFDToHandle(dev->fd, shared_handle, &handle); >> - if (r) { >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - return r; >> - } >> + if (r) >> + goto unlock; >> /* Query the buffer size. */ >> size = lseek(shared_handle, 0, SEEK_END); >> if (size == (off_t)-1) { >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - amdgpu_close_kms_handle(dev, handle); >> - return -errno; >> + r = -errno; >> + goto free_bo_handle; >> } >> lseek(shared_handle, 0, SEEK_SET); >> @@ -302,12 +312,12 @@ int amdgpu_bo_import(amdgpu_device_handle dev, >> case amdgpu_bo_handle_type_kms: >> case amdgpu_bo_handle_type_kms_noimport: >> /* Importing a KMS handle in not allowed. */ >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - return -EPERM; >> + r = -EPERM; >> + goto unlock; >> default: >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - return -EINVAL; >> + r = -EINVAL; >> + goto unlock; >> } >> if (bo) { >> @@ -320,58 +330,37 @@ int amdgpu_bo_import(amdgpu_device_handle dev, >> return 0; >> } >> - bo = calloc(1, sizeof(struct amdgpu_bo)); >> - if (!bo) { >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - if (type == amdgpu_bo_handle_type_dma_buf_fd) { >> - amdgpu_close_kms_handle(dev, shared_handle); >> - } >> - return -ENOMEM; >> - } >> - >> /* Open the handle. */ >> switch (type) { >> case amdgpu_bo_handle_type_gem_flink_name: >> open_arg.name = shared_handle; >> r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_OPEN, &open_arg); >> - if (r) { >> - free(bo); >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - return r; >> - } >> + if (r) >> + goto unlock; >> - bo->handle = open_arg.handle; >> + flink_name = shared_handle; >> + handle = open_arg.handle; >> + alloc_size = open_arg.size; >> if (dev->flink_fd != dev->fd) { >> - r = drmPrimeHandleToFD(dev->flink_fd, bo->handle, DRM_CLOEXEC, &dma_fd); >> - if (r) { >> - free(bo); >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - return r; >> - } >> - r = drmPrimeFDToHandle(dev->fd, dma_fd, &bo->handle ); >> - >> + r = drmPrimeHandleToFD(dev->flink_fd, handle, >> + DRM_CLOEXEC, &dma_fd); >> + if (r) >> + goto free_bo_handle; >> + r = drmPrimeFDToHandle(dev->fd, dma_fd, &handle); >> close(dma_fd); >> - >> - if (r) { >> - free(bo); >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - return r; >> - } >> - } >> - bo->flink_name = shared_handle; >> - bo->alloc_size = open_arg.size; >> - r = handle_table_insert(&dev->bo_flink_names, shared_handle, >> - bo); >> - if (r) { >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> - amdgpu_bo_free(bo); >> - return r; >> + if (r) >> + goto free_bo_handle; >> + close_args.handle = open_arg.handle; >> + r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_CLOSE, >> + &close_args); >> + if (r) >> + goto free_bo_handle; >> } >> break; >> case amdgpu_bo_handle_type_dma_buf_fd: >> - bo->handle = shared_handle; >> - bo->alloc_size = dma_buf_size; >> + handle = shared_handle; >> + alloc_size = dma_buf_size; >> break; >> case amdgpu_bo_handle_type_kms: >> @@ -380,16 +369,41 @@ int amdgpu_bo_import(amdgpu_device_handle dev, >> } >> /* Initialize it. */ >> - atomic_set(&bo->refcount, 1); >> - bo->dev = dev; >> - pthread_mutex_init(&bo->cpu_access_mutex, NULL); >> + r = amdgpu_bo_create(dev, alloc_size, handle, &bo); >> + if (r) >> + goto free_bo_handle; >> - handle_table_insert(&dev->bo_handles, bo->handle, bo); >> - pthread_mutex_unlock(&dev->bo_table_mutex); >> + r = handle_table_insert(&dev->bo_handles, bo->handle, bo); >> + if (r) >> + goto free_bo_handle; >> + if (flink_name) { >> + bo->flink_name = flink_name; >> + r = handle_table_insert(&dev->bo_flink_names, flink_name, >> + bo); >> + if (r) >> + goto remove_handle; >> + >> + } >> output->buf_handle = bo; >> output->alloc_size = bo->alloc_size; >> + pthread_mutex_unlock(&dev->bo_table_mutex); >> return 0; >> + >> +remove_handle: >> + handle_table_remove(&dev->bo_handles, bo->handle); >> +free_bo_handle: >> + if (flink_name && !close_args.handle && open_arg.handle) { >> + close_args.handle = open_arg.handle; >> + drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_CLOSE, &close_args); >> + } >> + if (bo) >> + amdgpu_bo_free(bo); >> + else >> + amdgpu_close_kms_handle(dev, handle); >> +unlock: >> + pthread_mutex_unlock(&dev->bo_table_mutex); >> + return r; >> } >> int amdgpu_bo_free(amdgpu_bo_handle buf_handle) >> @@ -574,7 +588,6 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev, >> amdgpu_bo_handle *buf_handle) >> { >> int r; >> - struct amdgpu_bo *bo; >> struct drm_amdgpu_gem_userptr args; >> args.addr = (uintptr_t)cpu; >> @@ -584,28 +597,21 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev, >> r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_USERPTR, >> &args, sizeof(args)); >> if (r) >> - return r; >> - >> - bo = calloc(1, sizeof(struct amdgpu_bo)); >> - if (!bo) >> - return -ENOMEM; >> - >> - atomic_set(&bo->refcount, 1); >> - bo->dev = dev; >> - bo->alloc_size = size; >> - bo->handle = args.handle; >> + goto out; >> - 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); >> + r = amdgpu_bo_create(dev, size, args.handle, buf_handle); >> + if (r) { >> + amdgpu_close_kms_handle(dev, args.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(bo); >> - else >> - *buf_handle = bo; >> - >> + amdgpu_bo_free(*buf_handle); >> +out: >> return r; >> } > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-14 2:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-13 10:06 [PATCH libdrm] amdgpu: add a function to create amdgpu bo internally (v4) Junwei Zhang
[not found] ` <1534154763-2287-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
2018-08-13 10:14 ` Christian König
[not found] ` <e5ed0c82-74ad-f7ea-58af-84f3dee3e090-5C7GfCeVMHo@public.gmane.org>
2018-08-14 2:59 ` Zhang, Jerry (Junwei)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.