Re: [PATCH 2/3] drm: add support of syncobj timeline point wait

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 06.09.2018 um 08:25 schrieb Chunming Zhou:
> points array is one-to-one match with syncobjs array.
>
> Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 23 ++++++++++++++++++++---
>   include/uapi/drm/drm.h        |  2 ++
>   2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 94b31de23858..4e43c62ab432 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1000,6 +1000,7 @@ static void syncobj_wait_fence_func(struct dma_fence *fence,
>   }
>   
>   static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> +						  void __user *user_points,
>   						  uint32_t count,
>   						  uint32_t flags,
>   						  signed long timeout,
> @@ -1007,13 +1008,25 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   {
>   	struct syncobj_wait_entry *entries;
>   	struct dma_fence *fence;
> +	uint64_t *points;
>   	signed long ret;
>   	uint32_t signaled_count, i;
>   
> -	entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
> -	if (!entries)
> +	points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
> +	if (points == NULL)
>   		return -ENOMEM;
>   
> +	if (copy_from_user(points, user_points, sizeof(uint64_t) * count)) {
> +		ret = -EFAULT;
> +		goto err_free_points;
> +	}
> +
> +
> +	entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
> +	if (!entries) {
> +		ret = -ENOMEM;
> +		goto err_free_points;
> +	}
>   	/* Walk the list of sync objects and initialize entries.  We do
>   	 * this up-front so that we can properly return -EINVAL if there is
>   	 * a syncobj with a missing fence and then never have the chance of
> @@ -1022,7 +1035,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   	signaled_count = 0;
>   	for (i = 0; i < count; ++i) {
>   		entries[i].task = current;
> -		ret = drm_syncobj_search_fence(syncobjs[i], 0, 0,
> +		ret = drm_syncobj_search_fence(syncobjs[i], points[i], 0,
>   					       &entries[i].fence);
>   		if (!entries[i].fence) {
>   			ret = -EINVAL;
> @@ -1108,6 +1121,9 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   	}
>   	kfree(entries);
>   
> +err_free_points:
> +	kfree(points);
> +
>   	return ret;
>   }
>   
> @@ -1153,6 +1169,7 @@ static int drm_syncobj_array_wait(struct drm_device *dev,
>   	uint32_t first = ~0;
>   
>   	ret = drm_syncobj_array_wait_timeout(syncobjs,
> +					     u64_to_user_ptr(wait->points),
>   					     wait->count_handles,
>   					     wait->flags,
>   					     timeout, &first);
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index cebdb2541eb7..4420c7604690 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -740,6 +740,8 @@ struct drm_syncobj_handle {
>   #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
>   struct drm_syncobj_wait {
>   	__u64 handles;
> +	/* wait on specific timeline point for every handles*/
> +	__u64 points;

That would break UAPI and isn't allowed like this.

You need to add a new operation to wait for multiple sync objects and 
their sequence number or add a flag or something like this here.

Regards,
Christian.

>   	/* absolute timeout */
>   	__s64 timeout_nsec;
>   	__u32 count_handles;

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel