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

Yicong Hui posted 3 patches 1 month ago
[RFC PATCH v3 2/3] drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag
Posted by Yicong Hui 1 month ago
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@amd.com>
Suggested-by: Michel Dänzer <michel.daenzer@mailbox.org>
Signed-off-by: Yicong Hui <yiconghui@gmail.com>
---
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);
-		if (timeout < 0)
+		if (timeout < 0) {
+			if (wait->flags & DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR)
+				wait->first_signaled = first;
+
 			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.
+ */
+#define DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR (1 << 4)
 struct drm_syncobj_wait {
 	__u64 handles;
 	/* absolute timeout */
-- 
2.53.0

Re: [RFC PATCH v3 2/3] drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag
Posted by Tvrtko Ursulin 1 month ago
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@amd.com>
> Suggested-by: Michel Dänzer <michel.daenzer@mailbox.org>
> Signed-off-by: Yicong Hui <yiconghui@gmail.com>
> ---
> 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@igalia.com/

"""
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 */