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.z...@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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
