[PATCH] drm/syncobj: extend syncobj query ability

[Koenig, Christian]

Am 23.07.19 um 07:22 schrieb Chunming Zhou:
> user space needs a flexiable query ability.
> So that umd can get last signaled or submitted point.
>
> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Cc: Christian König <Christian.Koenig at amd.com>

I've recently found a bug in drm_syncobj_query_ioctl() which I'm going 
to commit tomorrow.

Apart from that it looks good to me, but I think we should cleanup a bit 
and move the dma_fence_chain_for_each()... into a helper function in 
dma-fence-chain.c

Christian.

> ---
>   drivers/gpu/drm/drm_syncobj.c | 36 +++++++++++++++++------------------
>   include/uapi/drm/drm.h        |  3 ++-
>   2 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 3d400905100b..f70dedf3ef4f 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
>   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   		return -EOPNOTSUPP;
>   
> -	if (args->pad != 0)
> -		return -EINVAL;
> -
>   	if (args->count_handles == 0)
>   		return -EINVAL;
>   
> @@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   		return -EOPNOTSUPP;
>   
> -	if (args->pad != 0)
> -		return -EINVAL;
> -
>   	if (args->count_handles == 0)
>   		return -EINVAL;
>   
> @@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   		if (chain) {
>   			struct dma_fence *iter, *last_signaled = NULL;
>   
> -			dma_fence_chain_for_each(iter, fence) {
> -				if (!iter)
> -					break;
> -				dma_fence_put(last_signaled);
> -				last_signaled = dma_fence_get(iter);
> -				if (!to_dma_fence_chain(last_signaled)->prev_seqno)
> -					/* It is most likely that timeline has
> -					 * unorder points. */
> -					break;
> +			if (args->flags &
> +			    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> +				point = fence->seqno;
> +			} else {
> +				dma_fence_chain_for_each(iter, fence) {
> +					if (!iter)
> +						break;
> +					dma_fence_put(last_signaled);
> +					last_signaled = dma_fence_get(iter);
> +					if (!to_dma_fence_chain(last_signaled)->prev_seqno)
> +						/* It is most likely that timeline has
> +						* unorder points. */
> +						break;
> +				}
> +				point = dma_fence_is_signaled(last_signaled) ?
> +					last_signaled->seqno :
> +					to_dma_fence_chain(last_signaled)->prev_seqno;
>   			}
> -			point = dma_fence_is_signaled(last_signaled) ?
> -				last_signaled->seqno :
> -				to_dma_fence_chain(last_signaled)->prev_seqno;
>   			dma_fence_put(last_signaled);
>   		} else {
>   			point = 0;
>   		}
> +		dma_fence_put(fence);
>   		ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
>   		ret = ret ? -EFAULT : 0;
>   		if (ret)
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 661d73f9a919..fd987ce24d9f 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>   	__u32 pad;
>   };
>   
> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
>   struct drm_syncobj_timeline_array {
>   	__u64 handles;
>   	__u64 points;
>   	__u32 count_handles;
> -	__u32 pad;
> +	__u32 flags;
>   };
>   
>