[PATCH libdrm 1/2] amdgpu: add the interface of waiting multiple fences
From: Nicolai Hähnle
Signed-off-by: Junwei Zhang
[v2: allow returning the first signaled fence index]
Signed-off-by: monk.liu
[v3:
- cleanup *status setting
- fix amdgpu symbols check]
Signed-off-by: Nicolai Hähnle
Reviewed-by: Christian König (v1)
Reviewed-by: Jammy Zhou (v1)
---
amdgpu/amdgpu-symbol-check | 1 +
amdgpu/amdgpu.h| 23 ++
amdgpu/amdgpu_cs.c | 74 ++
3 files changed, 98 insertions(+)
diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
index 4d1ae65..81ef9b4 100755
--- a/amdgpu/amdgpu-symbol-check
+++ b/amdgpu/amdgpu-symbol-check
@@ -26,20 +26,21 @@ amdgpu_bo_va_op_raw
amdgpu_bo_wait_for_idle
amdgpu_create_bo_from_user_mem
amdgpu_cs_create_semaphore
amdgpu_cs_ctx_create
amdgpu_cs_ctx_free
amdgpu_cs_destroy_semaphore
amdgpu_cs_query_fence_status
amdgpu_cs_query_reset_state
amdgpu_cs_signal_semaphore
amdgpu_cs_submit
+amdgpu_cs_wait_fences
amdgpu_cs_wait_semaphore
amdgpu_device_deinitialize
amdgpu_device_initialize
amdgpu_get_marketing_name
amdgpu_query_buffer_size_alignment
amdgpu_query_crtc_from_id
amdgpu_query_firmware_version
amdgpu_query_gds_info
amdgpu_query_gpu_info
amdgpu_query_heap_info
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index 55884b2..fdea905 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -900,20 +900,43 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
* returned in the case if submission was completed or timeout error
* code.
*
* \sa amdgpu_cs_submit()
*/
int amdgpu_cs_query_fence_status(struct amdgpu_cs_fence *fence,
uint64_t timeout_ns,
uint64_t flags,
uint32_t *expired);
+/**
+ * Wait for multiple fences
+ *
+ * \param fences - \c [in] The fence array to wait
+ * \param fence_count - \c [in] The fence count
+ * \param wait_all- \c [in] If true, wait all fences to be signaled,
+ *otherwise, wait at least one fence
+ * \param timeout_ns - \c [in] The timeout to wait, in nanoseconds
+ * \param status - \c [out] '1' for signaled, '0' for timeout
+ * \param first - \c [out] the index of the first signaled fence from
@fences
+ *
+ * \return 0 on success
+ * <0 - Negative POSIX Error code
+ *
+ * \noteCurrently it supports only one amdgpu_device. All fences come from
+ * the same amdgpu_device with the same fd.
+*/
+int amdgpu_cs_wait_fences(struct amdgpu_cs_fence *fences,
+ uint32_t fence_count,
+ bool wait_all,
+ uint64_t timeout_ns,
+ uint32_t *status, uint32_t *first);
+
/*
* Query / Info API
*
*/
/**
* Query allocation size alignments
*
* UMD should query information about GPU VM MC size alignments requirements
* to be able correctly choose required allocation size and implement
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index fb5b3a8..707e6d1 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -436,20 +436,94 @@ int amdgpu_cs_query_fence_status(struct amdgpu_cs_fence
*fence,
r = amdgpu_ioctl_wait_cs(fence->context, fence->ip_type,
fence->ip_instance, fence->ring,
fence->fence, timeout_ns, flags, &busy);
if (!r && !busy)
*expired = true;
return r;
}
+static int amdgpu_ioctl_wait_fences(struct amdgpu_cs_fence *fences,
+ uint32_t fence_count,
+ bool wait_all,
+ uint64_t timeout_ns,
+ uint32_t *status,
+ uint32_t *first)
+{
+ struct drm_amdgpu_fence *drm_fences;
+ amdgpu_device_handle dev = fences[0].context->dev;
+ union drm_amdgpu_wait_fences args;
+ int r;
+ uint32_t i;
+
+ drm_fences = alloca(sizeof(struct drm_amdgpu_fence) * fence_count);
+ for (i = 0; i < fence_count; i++) {
+ drm_fences[i].ctx_id = fences[i].context->id;
+ drm_fences[i].ip_type = fences[i].ip_type;
+ drm_fences[i].ip_instance = fences[i].ip_instance;
+ drm_fences[i].ring = fences[i].ring;
+ drm_fences[i].seq_no = fences[i].fence;
+ }
+
+ memset(&args, 0, sizeof(args));
+ args.in.fences = (uint64_t)(uintptr_t)drm_fences;
+ args.in.fence_count = fence_count;
+ args.in.wait_all = wait_all;
+ args.in.timeout_ns = amdgpu_cs_calculate_timeout(timeout_ns);
+
+ r = drmIoctl(dev->fd, DRM_IOCTL_AMDGPU_WAIT_FENCES, &args);
+ if (r)
+ return -errno;
+
+ *status = args.out.status;
+
+ if (first)
+ *first = args.out.first_signaled;
+
+ return 0;
+}
+
+int amdgpu_cs
Re: [PATCH libdrm 1/2] amdgpu: add the interface of waiting multiple fences
On 04/14/2017 12:47 AM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle
>
> Signed-off-by: Junwei Zhang
> [v2: allow returning the first signaled fence index]
> Signed-off-by: monk.liu
> [v3:
> - cleanup *status setting
> - fix amdgpu symbols check]
> Signed-off-by: Nicolai Hähnle
> Reviewed-by: Christian König (v1)
> Reviewed-by: Jammy Zhou (v1)
> ---
> amdgpu/amdgpu-symbol-check | 1 +
> amdgpu/amdgpu.h| 23 ++
> amdgpu/amdgpu_cs.c | 74
> ++
> 3 files changed, 98 insertions(+)
>
> diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
> index 4d1ae65..81ef9b4 100755
> --- a/amdgpu/amdgpu-symbol-check
> +++ b/amdgpu/amdgpu-symbol-check
> @@ -26,20 +26,21 @@ amdgpu_bo_va_op_raw
> amdgpu_bo_wait_for_idle
> amdgpu_create_bo_from_user_mem
> amdgpu_cs_create_semaphore
> amdgpu_cs_ctx_create
> amdgpu_cs_ctx_free
> amdgpu_cs_destroy_semaphore
> amdgpu_cs_query_fence_status
> amdgpu_cs_query_reset_state
> amdgpu_cs_signal_semaphore
> amdgpu_cs_submit
> +amdgpu_cs_wait_fences
> amdgpu_cs_wait_semaphore
> amdgpu_device_deinitialize
> amdgpu_device_initialize
> amdgpu_get_marketing_name
> amdgpu_query_buffer_size_alignment
> amdgpu_query_crtc_from_id
> amdgpu_query_firmware_version
> amdgpu_query_gds_info
> amdgpu_query_gpu_info
> amdgpu_query_heap_info
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index 55884b2..fdea905 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -900,20 +900,43 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
> *returned in the case if submission was completed or timeout error
> *code.
> *
> * \sa amdgpu_cs_submit()
> */
> int amdgpu_cs_query_fence_status(struct amdgpu_cs_fence *fence,
>uint64_t timeout_ns,
>uint64_t flags,
>uint32_t *expired);
>
> +/**
> + * Wait for multiple fences
> + *
> + * \param fences - \c [in] The fence array to wait
> + * \param fence_count - \c [in] The fence count
> + * \param wait_all- \c [in] If true, wait all fences to be signaled,
> + *otherwise, wait at least one fence
> + * \param timeout_ns - \c [in] The timeout to wait, in nanoseconds
> + * \param status - \c [out] '1' for signaled, '0' for timeout
> + * \param first - \c [out] the index of the first signaled fence
> from @fences
> + *
> + * \return 0 on success
> + * <0 - Negative POSIX Error code
> + *
> + * \noteCurrently it supports only one amdgpu_device. All fences come
> from
> + * the same amdgpu_device with the same fd.
> +*/
> +int amdgpu_cs_wait_fences(struct amdgpu_cs_fence *fences,
> + uint32_t fence_count,
> + bool wait_all,
> + uint64_t timeout_ns,
> + uint32_t *status, uint32_t *first);
> +
> /*
> * Query / Info API
> *
> */
>
> /**
> * Query allocation size alignments
> *
> * UMD should query information about GPU VM MC size alignments requirements
> * to be able correctly choose required allocation size and implement
> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> index fb5b3a8..707e6d1 100644
> --- a/amdgpu/amdgpu_cs.c
> +++ b/amdgpu/amdgpu_cs.c
> @@ -436,20 +436,94 @@ int amdgpu_cs_query_fence_status(struct amdgpu_cs_fence
> *fence,
> r = amdgpu_ioctl_wait_cs(fence->context, fence->ip_type,
> fence->ip_instance, fence->ring,
> fence->fence, timeout_ns, flags, &busy);
>
> if (!r && !busy)
> *expired = true;
>
> return r;
> }
>
> +static int amdgpu_ioctl_wait_fences(struct amdgpu_cs_fence *fences,
> + uint32_t fence_count,
> + bool wait_all,
> + uint64_t timeout_ns,
> + uint32_t *status,
> + uint32_t *first)
> +{
> + struct drm_amdgpu_fence *drm_fences;
> + amdgpu_device_handle dev = fences[0].context->dev;
> + union drm_amdgpu_wait_fences args;
> + int r;
> + uint32_t i;
> +
> + drm_fences = alloca(sizeof(struct drm_amdgpu_fence) * fence_count);
> + for (i = 0; i < fence_count; i++) {
> + drm_fences[i].ctx_id = fences[i].context->id;
> + drm_fences[i].ip_type = fences[i].ip_type;
> + drm_fences[i].ip_instance = fences[i].ip_instance;
> + drm_fences[i].ring = fences[i].ring;
> + drm_fences[i].seq_no = fences[i].fence;
> + }
> +
> + memset(&args, 0, sizeof(args));
> + args.in.fences = (uint64_t)(uintptr_t)drm_fences;
> + args.in.fence_count = fence_count;
> + args.in.wait_all = wait_all;
> + args.in.timeout_ns = amdgpu_cs_calculate_timeout(timeout_ns);
> +
> + r =
Re: [PATCH libdrm 1/2] amdgpu: add the interface of waiting multiple fences
On 18.04.2017 17:47, Edward O'Callaghan wrote:
On 04/14/2017 12:47 AM, Nicolai Hähnle wrote:
From: Nicolai Hähnle
Signed-off-by: Junwei Zhang
[v2: allow returning the first signaled fence index]
Signed-off-by: monk.liu
[v3:
- cleanup *status setting
- fix amdgpu symbols check]
Signed-off-by: Nicolai Hähnle
Reviewed-by: Christian König (v1)
Reviewed-by: Jammy Zhou (v1)
---
amdgpu/amdgpu-symbol-check | 1 +
amdgpu/amdgpu.h| 23 ++
amdgpu/amdgpu_cs.c | 74 ++
3 files changed, 98 insertions(+)
[snip]
+static int amdgpu_ioctl_wait_fences(struct amdgpu_cs_fence *fences,
+ uint32_t fence_count,
+ bool wait_all,
+ uint64_t timeout_ns,
+ uint32_t *status,
+ uint32_t *first)
+{
+ struct drm_amdgpu_fence *drm_fences;
+ amdgpu_device_handle dev = fences[0].context->dev;
+ union drm_amdgpu_wait_fences args;
+ int r;
+ uint32_t i;
+
+ drm_fences = alloca(sizeof(struct drm_amdgpu_fence) * fence_count);
+ for (i = 0; i < fence_count; i++) {
+ drm_fences[i].ctx_id = fences[i].context->id;
+ drm_fences[i].ip_type = fences[i].ip_type;
+ drm_fences[i].ip_instance = fences[i].ip_instance;
+ drm_fences[i].ring = fences[i].ring;
+ drm_fences[i].seq_no = fences[i].fence;
+ }
+
+ memset(&args, 0, sizeof(args));
+ args.in.fences = (uint64_t)(uintptr_t)drm_fences;
+ args.in.fence_count = fence_count;
+ args.in.wait_all = wait_all;
+ args.in.timeout_ns = amdgpu_cs_calculate_timeout(timeout_ns);
+
+ r = drmIoctl(dev->fd, DRM_IOCTL_AMDGPU_WAIT_FENCES, &args);
+ if (r)
+ return -errno;
Hi Nicolai,
you will leak drm_fences here on the error branch.
It's an alloca, so it's automatically freed when the function returns.
+
+ *status = args.out.status;
+
+ if (first)
+ *first = args.out.first_signaled;
+
+ return 0;
+}
+
+int amdgpu_cs_wait_fences(struct amdgpu_cs_fence *fences,
+ uint32_t fence_count,
+ bool wait_all,
+ uint64_t timeout_ns,
+ uint32_t *status,
+ uint32_t *first)
+{
+ uint32_t i;
+ int r;
no need for a intermediate ret, just return amdgpu_ioctl_wait_fences()
directly?
Good point, I'll change that before I push.
+
+ /* Sanity check */
+ if (NULL == fences)
+ return -EINVAL;
+ if (NULL == status)
+ return -EINVAL;
+ if (fence_count <= 0)
+ return -EINVAL;
may as well combine these branches?
if (!fences || !status || !fence_count)
return -EINVAL;
as fence_count is unsigned.
Yeah, that makes some sense, but I decided to keep the separate
if-statements because other functions are written like this as well.
Thanks,
Nicolai
Kind Regards,
Edward.
+ for (i = 0; i < fence_count; i++) {
+ if (NULL == fences[i].context)
+ return -EINVAL;
+ if (fences[i].ip_type >= AMDGPU_HW_IP_NUM)
+ return -EINVAL;
+ if (fences[i].ring >= AMDGPU_CS_MAX_RINGS)
+ return -EINVAL;
+ }
+
+ *status = 0;
+
+ r = amdgpu_ioctl_wait_fences(fences, fence_count, wait_all, timeout_ns,
+status, first);
+
+ return r;
+}
+
int amdgpu_cs_create_semaphore(amdgpu_semaphore_handle *sem)
{
struct amdgpu_semaphore *gpu_semaphore;
if (NULL == sem)
return -EINVAL;
gpu_semaphore = calloc(1, sizeof(struct amdgpu_semaphore));
if (NULL == gpu_semaphore)
return -ENOMEM;
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm 1/2] amdgpu: add the interface of waiting multiple fences
On 04/19/2017 04:28 AM, Nicolai Hähnle wrote:
> On 18.04.2017 17:47, Edward O'Callaghan wrote:
>> On 04/14/2017 12:47 AM, Nicolai Hähnle wrote:
>>> From: Nicolai Hähnle
>>>
>>> Signed-off-by: Junwei Zhang
>>> [v2: allow returning the first signaled fence index]
>>> Signed-off-by: monk.liu
>>> [v3:
>>> - cleanup *status setting
>>> - fix amdgpu symbols check]
>>> Signed-off-by: Nicolai Hähnle
>>> Reviewed-by: Christian König (v1)
>>> Reviewed-by: Jammy Zhou (v1)
>>> ---
>>> amdgpu/amdgpu-symbol-check | 1 +
>>> amdgpu/amdgpu.h| 23 ++
>>> amdgpu/amdgpu_cs.c | 74
>>> ++
>>> 3 files changed, 98 insertions(+)
>>>
> [snip]
>>> +static int amdgpu_ioctl_wait_fences(struct amdgpu_cs_fence *fences,
>>> +uint32_t fence_count,
>>> +bool wait_all,
>>> +uint64_t timeout_ns,
>>> +uint32_t *status,
>>> +uint32_t *first)
>>> +{
>>> +struct drm_amdgpu_fence *drm_fences;
>>> +amdgpu_device_handle dev = fences[0].context->dev;
>>> +union drm_amdgpu_wait_fences args;
>>> +int r;
>>> +uint32_t i;
>>> +
>>> +drm_fences = alloca(sizeof(struct drm_amdgpu_fence) * fence_count);
>>> +for (i = 0; i < fence_count; i++) {
>>> +drm_fences[i].ctx_id = fences[i].context->id;
>>> +drm_fences[i].ip_type = fences[i].ip_type;
>>> +drm_fences[i].ip_instance = fences[i].ip_instance;
>>> +drm_fences[i].ring = fences[i].ring;
>>> +drm_fences[i].seq_no = fences[i].fence;
>>> +}
>>> +
>>> +memset(&args, 0, sizeof(args));
>>> +args.in.fences = (uint64_t)(uintptr_t)drm_fences;
>>> +args.in.fence_count = fence_count;
>>> +args.in.wait_all = wait_all;
>>> +args.in.timeout_ns = amdgpu_cs_calculate_timeout(timeout_ns);
>>> +
>>> +r = drmIoctl(dev->fd, DRM_IOCTL_AMDGPU_WAIT_FENCES, &args);
>>> +if (r)
>>> +return -errno;
>>
>> Hi Nicolai,
>>
>> you will leak drm_fences here on the error branch.
>
> It's an alloca, so it's automatically freed when the function returns.
>
>
>>> +
>>> +*status = args.out.status;
>>> +
>>> +if (first)
>>> +*first = args.out.first_signaled;
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +int amdgpu_cs_wait_fences(struct amdgpu_cs_fence *fences,
>>> + uint32_t fence_count,
>>> + bool wait_all,
>>> + uint64_t timeout_ns,
>>> + uint32_t *status,
>>> + uint32_t *first)
>>> +{
>>> +uint32_t i;
>>> +int r;
>>
>> no need for a intermediate ret, just return amdgpu_ioctl_wait_fences()
>> directly?
>
> Good point, I'll change that before I push.
>
>
>>> +
>>> +/* Sanity check */
>>> +if (NULL == fences)
>>> +return -EINVAL;
>>> +if (NULL == status)
>>> +return -EINVAL;
>>> +if (fence_count <= 0)
>>> +return -EINVAL;
>>
>> may as well combine these branches?
>>
>> if (!fences || !status || !fence_count)
>> return -EINVAL;
>>
>> as fence_count is unsigned.
>
> Yeah, that makes some sense, but I decided to keep the separate
> if-statements because other functions are written like this as well.
Its not completely consistent actually, I sent in a patch last night to
fix the rest.
Cheers,
Edward.
>
> Thanks,
> Nicolai
>
>
>
>>
>> Kind Regards,
>> Edward.
>>
>>> +for (i = 0; i < fence_count; i++) {
>>> +if (NULL == fences[i].context)
>>> +return -EINVAL;
>>> +if (fences[i].ip_type >= AMDGPU_HW_IP_NUM)
>>> +return -EINVAL;
>>> +if (fences[i].ring >= AMDGPU_CS_MAX_RINGS)
>>> +return -EINVAL;
>>> +}
>>> +
>>> +*status = 0;
>>> +
>>> +r = amdgpu_ioctl_wait_fences(fences, fence_count, wait_all,
>>> timeout_ns,
>>> + status, first);
>>> +
>>> +return r;
>>> +}
>>> +
>>> int amdgpu_cs_create_semaphore(amdgpu_semaphore_handle *sem)
>>> {
>>> struct amdgpu_semaphore *gpu_semaphore;
>>>
>>> if (NULL == sem)
>>> return -EINVAL;
>>>
>>> gpu_semaphore = calloc(1, sizeof(struct amdgpu_semaphore));
>>> if (NULL == gpu_semaphore)
>>> return -ENOMEM;
>>>
>>
>
>
signature.asc
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
