Re: [RFC PATCH v3 2/3] drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag

[Tvrtko Ursulin]

On 25/02/2026 12:46, Yicong Hui wrote:
> Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag for the
> ioctls DRM_IOCTL_SYNCOBJ_WAIT and DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, which
> will make them abort their wait and return the error code and its
> associated syncobj.
>
> Suggested-by: Christian König <christian.koenig@xxxxxxx>
> Suggested-by: Michel Dänzer <michel.daenzer@xxxxxxxxxxx>
> Signed-off-by: Yicong Hui <yiconghui@xxxxxxxxx>
> ---
> Changes in v3:
> * Fixed inline comments by converting to multi-line comments in
> accordance to kernel style guidelines.
> * Used dma_fence_get_status to query error instead of getting it
> directly.
>
>   drivers/gpu/drm/drm_syncobj.c | 27 +++++++++++++++++++++++----
>   include/uapi/drm/drm.h        |  6 ++++++
>   2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b74e491f9d8b..2b23f638c1cc 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1042,6 +1042,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   	struct dma_fence *fence;
>   	uint64_t *points;
>   	uint32_t signaled_count, i;
> +	int status;
>   
>   	if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>   		     DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) {
> @@ -1139,6 +1140,14 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   			if (!fence)
>   				continue;
>   
> +			status = dma_fence_get_status(fence);
> +			if ((flags & DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR) && status < 0) {
> +				if (idx)
> +					*idx = i;
> +				timeout = status;
> +				goto done_waiting;
> +			}
> +
>   			if ((flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE) ||
>   			    dma_fence_is_signaled(fence) ||
>   			    (!entries[i].fence_cb.func &&
> @@ -1242,8 +1251,12 @@ static int drm_syncobj_array_wait(struct drm_device *dev,
>   							 wait->flags,
>   							 timeout, &first,
>   							 deadline);

I am calling this helper an unhelpful helper and was suggesting to get 
rid of it:

https://lore.kernel.org/dri-devel/20250611140057.27259-2-tvrtko.ursulin@xxxxxxxxxx/

"""
Helper which fails to consolidate the code and instead just forks into two
copies of the code based on a boolean parameter is not very helpful or
readable. Lets just remove it and proof in the pudding is the net smaller
code.
"""

Can we please nuke it?

> -		if (timeout < 0)
> +		if (timeout < 0) {
> +			if (wait->flags & DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR)
> +				wait->first_signaled = first;

There can be timeout < 0 for reasons other than 
DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR. Will 'first' contain a valid 
value to return to userspace in that case? Will userspace even be able 
to meaningfully distinguish between ioctl returning negative due some 
random error, versus a fence error where otherwise nothing else failed?

> +
>   			return timeout;
> +		}
>   		wait->first_signaled = first;
>   	} else {
>   		timeout = drm_timeout_abs_to_jiffies(timeline_wait->timeout_nsec);
> @@ -1253,8 +1266,12 @@ static int drm_syncobj_array_wait(struct drm_device *dev,
>   							 timeline_wait->flags,
>   							 timeout, &first,
>   							 deadline);
> -		if (timeout < 0)
> +		if (timeout < 0) {
> +			if (timeline_wait->flags & DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR)
> +				timeline_wait->first_signaled = first;
> +
>   			return timeout;
> +		}
>   		timeline_wait->first_signaled = first;
>   	}
>   	return 0;
> @@ -1332,7 +1349,8 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
>   
>   	possible_flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
>   			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> -			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE;
> +			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE |
> +			 DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR;
>   
>   	if (args->flags & ~possible_flags)
>   		return -EINVAL;
> @@ -1376,7 +1394,8 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
>   	possible_flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
>   			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>   			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE |
> -			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE;
> +			 DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE |
> +			 DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR;
>   
>   	if (args->flags & ~possible_flags)
>   		return -EINVAL;
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 213b4dc9b612..e998d9351525 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -977,6 +977,12 @@ struct drm_syncobj_transfer {
>   #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
>   #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) /* wait for time point to become available */
>   #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE (1 << 3) /* set fence deadline to deadline_nsec */
> +/*
> + * As soon as any of the fences in the set have an error,
> + * abort waiting and return its error code. Index of this
> + * first failed fence is returned in first_signaled.

What else can be returned in first_signaled?

Regards,

Tvrtko

> + */
> +#define DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR (1 << 4)
>   struct drm_syncobj_wait {
>   	__u64 handles;
>   	/* absolute timeout */