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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 08/22/2016 10:41 PM, Edward O'Callaghan wrote:
> 
> 
> 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?

woops, ignore me. 11pm should be in bed :p .... sorry for the noise.

> 
>> 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
>>
> 
> 
> 
> _______________________________________________
> 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/30680e19/attachment.sig>


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux