[PATCH 2/4] amdgpu: add export/import semaphore apis

[Edward O'Callaghan]

On 08/18/2016 05:55 PM, Chunming Zhou wrote:
> They are used for sharing semaphore across process.
> 
> Change-Id: I262adf10913d365bb93368b492e69140af522c64
> Signed-off-by: Chunming Zhou <David1.Zhou at amd.com>
> ---
>  amdgpu/amdgpu.h          | 40 ++++++++++++++++++++++++++++++
>  amdgpu/amdgpu_cs.c       | 63 ++++++++++++++++++++++++++++++++++++++++++++++--
>  amdgpu/amdgpu_internal.h |  2 ++
>  3 files changed, 103 insertions(+), 2 deletions(-)
> 
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index 693d841..e716855 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -1379,6 +1379,19 @@ int amdgpu_svm_commit(amdgpu_va_handle va_range_handle,
>  int amdgpu_svm_uncommit(amdgpu_va_handle va_range_handle);
>  
>  /**
> + *  create shared semaphore
> + *
> + * \param amdgpu_device_handle
> + * \param   sem	   - \c [out] semaphore handle
> + *
> + * \return   0 on success\n
> + *          <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_cs_create_semaphore_object(amdgpu_device_handle device_handle,
> +				      amdgpu_semaphore_handle *sem);
> +
> +/**
>   *  create semaphore
>   *
>   * \param   sem	   - \c [out] semaphore handle
> @@ -1439,6 +1452,33 @@ int amdgpu_cs_wait_semaphore(amdgpu_context_handle ctx,
>  int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
>  
>  /**
> + *  export semaphore
> + *
> + * \param   sem	    - \c [in] semaphore handle
> + * \param   shared_handle    - \c [out] handle across process
> + *
> + * \return   0 on success\n
> + *          <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_cs_export_semaphore(amdgpu_semaphore_handle sem,
> +			       uint32_t *shared_handle);
> +/**
> + *  import semaphore
> + *
> + * \param   sem	    - \c [out] semaphore handle
> + * \param   dev	    - \c [in] device handle
> + * \param   shared_handle    - \c [in] handle across process
> + *
> + * \return   0 on success\n
> + *          <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_cs_import_semaphore(amdgpu_semaphore_handle *sem,
> +			       amdgpu_device_handle dev,
> +			       uint32_t shared_handle);
> +
> +/**
>   *  Get the ASIC marketing name
>   *
>   * \param   dev         - \c [in] Device handle. See #amdgpu_device_initialize()
> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> index c76a675..a3ff34e 100644
> --- a/amdgpu/amdgpu_cs.c
> +++ b/amdgpu/amdgpu_cs.c
> @@ -518,6 +518,34 @@ int amdgpu_cs_wait_fences(struct amdgpu_cs_fence *fences,
>  	return r;
>  }
>  
> +int amdgpu_cs_create_semaphore_object(amdgpu_device_handle device_handle,
> +				      amdgpu_semaphore_handle *sem)
> +{
> +	struct amdgpu_bo_alloc_request req = {0};
> +	amdgpu_bo_handle buf_handle;
> +	int r;
> +
> +	if (NULL == sem)

Since sem is ** then should we not check that *both* sem && *sem are
non-NULL too? Further you can use the canonical form of (!sem)

> +		return -EINVAL;
> +
> +	req.alloc_size = sizeof(struct amdgpu_semaphore);
> +	req.preferred_heap = AMDGPU_GEM_DOMAIN_GTT;
> +
> +	r = amdgpu_bo_alloc(device_handle, &req, &buf_handle);
> +	if (r)
> +		return r;
> +	r = amdgpu_bo_cpu_map(buf_handle, sem);
> +	if (r) {
> +		amdgpu_bo_free(buf_handle);
> +		return r;
> +	}
> +	(*sem)->buf_handle = buf_handle;
> +	atomic_set(&(*sem)->refcount, 1);

Hi Chunming,

When/where was 'amdgpu_semaphore_handle' introduced? I am not sure I
like pointers being hidden behind typedef's as opaque types this can
lead to really really bad things.. I only noticed sem was a ** because
of the weird looking deference then address operator application, then
deference again here, &(*sem)->..

Cheers,
Edward.

> +	(*sem)->version = 2;
> +
> +	return 0;
> +}
> +
>  int amdgpu_cs_create_semaphore(amdgpu_semaphore_handle *sem)
>  {
>  	struct amdgpu_semaphore *gpu_semaphore;
> @@ -529,6 +557,7 @@ int amdgpu_cs_create_semaphore(amdgpu_semaphore_handle *sem)
>  	if (NULL == gpu_semaphore)
>  		return -ENOMEM;
>  
> +	gpu_semaphore->version = 1;
>  	atomic_set(&gpu_semaphore->refcount, 1);
>  	*sem = gpu_semaphore;
>  
> @@ -608,8 +637,15 @@ static int amdgpu_cs_unreference_sem(amdgpu_semaphore_handle sem)
>  	if (NULL == sem)
>  		return -EINVAL;
>  
> -	if (update_references(&sem->refcount, NULL))
> -		free(sem);
> +	if (update_references(&sem->refcount, NULL)) {
> +		if (sem->version == 1)
> +			free(sem);
> +		else if (sem->version == 2) {
> +			amdgpu_bo_handle buf_handle = sem->buf_handle;
> +			amdgpu_bo_cpu_unmap(buf_handle);
> +			amdgpu_bo_free(buf_handle);
> +		}
> +	}
>  	return 0;
>  }
>  
> @@ -618,4 +654,27 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem)
>  	return amdgpu_cs_unreference_sem(sem);
>  }
>  
> +int amdgpu_cs_export_semaphore(amdgpu_semaphore_handle sem,
> +			       uint32_t *shared_handle)
> +{
> +	return amdgpu_bo_export(sem->buf_handle,
> +				amdgpu_bo_handle_type_dma_buf_fd,
> +				shared_handle);
> +
> +}
> +
> +int amdgpu_cs_import_semaphore(amdgpu_semaphore_handle *sem,
> +			       amdgpu_device_handle dev, uint32_t shared_handle)
> +{
> +	struct amdgpu_bo_import_result output;
> +	int r;
> +
> +	r = amdgpu_bo_import(dev,
> +			     amdgpu_bo_handle_type_dma_buf_fd,
> +			     shared_handle,
> +			     &output);
> +	if (r)
> +		return r;
>  
> +	return amdgpu_bo_cpu_map(output.buf_handle, sem);
> +}
> diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
> index ccc85d7..7c422da 100644
> --- a/amdgpu/amdgpu_internal.h
> +++ b/amdgpu/amdgpu_internal.h
> @@ -134,6 +134,8 @@ struct amdgpu_semaphore {
>  	atomic_t refcount;
>  	struct list_head list;
>  	struct drm_amdgpu_fence signal_fence;
> +	amdgpu_bo_handle buf_handle;
> +	uint32_t version;
>  };
>  
>  /**
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160821/d88ae8dd/attachment.sig>