Re: [PATCH] drm/syncobj: extend syncobj query ability

From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: Chunming Zhou <david1.zhou@amd.com>, dri-devel@lists.freedesktop.org
Cc: "Christian König" <Christian.Koenig@amd.com>
Subject: Re: [PATCH] drm/syncobj: extend syncobj query ability
Date: Tue, 23 Jul 2019 17:06:03 +0300	[thread overview]
Message-ID: <a6a22f62-3f4b-90cc-9ace-7f10c0bfc025@intel.com> (raw)
In-Reply-To: <20190723052257.14436-1-david1.zhou@amd.com>

On 23/07/2019 08:22, Chunming Zhou wrote:
> 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@amd.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Christian König <Christian.Koenig@amd.com>
> ---
>   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;

I think args->flags should still be 0 for this ioctl.

> -
>   	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;

You probably want to verify that (args->flags & 
~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) == 0.

> -
>   	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;
>   };
>   
>   

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