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

Edward O'Callaghan funfunctor at folklore1984.net
Mon Aug 22 12:41:29 UTC 2016



On 08/22/2016 12:42 PM, zhoucm1 wrote:
> 
> 
> On 2016年08月21日 14:23, Edward O'Callaghan wrote:
>>
>> 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)->..
> Hi Edward,
> semaphore was introduced last year, which is the wrapper of existing
> dependency.

Well no, I mean 'amdgpu_semaphore_handle' in particular as I didn't see
that in mainline. Where was it introduced?

> Yeah, this whole sharing semaphore approach has been NAKed by Christian.
> So we can skip this series now, we are going to use sync_file instead.

No worries.

Kind Regards,
Edward.

> 
> Thanks,
> David Zhou
> 
>>
>> 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;
>>>  };
>>>  
>>>  /**
>>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 

-------------- 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/20160822/8b51bb4b/attachment.sig>


More information about the amd-gfx mailing list