[RFC PATCH v3 0/3] Querying errors from drm_syncobj

[RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Yicong Hui]

This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
codes from multiple syncobjs and abort early upon error of any of them.

Based on discussions from Michel Dänzer and Christian König, and a
starter task from the DRM todo documentation.

See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
on userspace implementation.

I have looked into adding sub test cases into syncobj_wait.c and
syncobj_timeline.c, igt-tests for this and I think I understand the 
process for writing tests and submitting them, however, these ioctls 
only trigger in the case that there is an error, but I am not sure what
is the best way to artifically trigger an error from userspace in order
to test that these ioctl flags work. What's the recommended way to 
approach this?

---
Changes:
v3:
* Fixed inline comments by converting to multi-line comments in
accordance to kernel style guidelines.
* No longer using a separate superfluous function to walk the fence
chain, and instead queries the last signaled fence in in the chain for
its error code
* Fixed types for error and handles array.
* Used dma_fence_get_status to query error instead of getting it
directly.

v2:
https://lore.kernel.org/dri-devel/20260220022631.2205037-1-yiconghui@gmail.com/T/#m6ab4f94a19c769193895d7728383f84e452cbbfa
* Went from adding a new ioctl to implementing flags for existing
ones.

v1:
* https://lore.kernel.org/all/20260213120836.81283-1-yiconghui@gmail.com/T/#mfdbc7f97e91ca5731b51b69c8cf8173cb0b2fb3e

Yicong Hui (3):
  drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors
  drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag
  drm/syncobj/doc: Remove starter task from todo list

 Documentation/gpu/todo.rst    | 16 ------------
 drivers/gpu/drm/drm_syncobj.c | 49 ++++++++++++++++++++++++++++++-----
 include/uapi/drm/drm.h        | 11 ++++++++
 3 files changed, 54 insertions(+), 22 deletions(-)

-- 
2.53.0

[RFC PATCH v3 1/3] drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors

[Yicong Hui]

Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to make the
DRM_IOCTL_SYNCOBJ_QUERY ioctl fill out the handles array with the
error code of the first fence found per syncobj and 0 if one is not
found and maintain the normal return value in points.

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.
* No longer using a separate superfluous function to walk the fence
chain, and instead queries the last signaled fence in in the chain for
its error code
* Fixed types for error and handles array.


 drivers/gpu/drm/drm_syncobj.c | 22 ++++++++++++++++++++--
 include/uapi/drm/drm.h        |  5 +++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 2d4ab745fdad..b74e491f9d8b 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1654,14 +1654,17 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_syncobj_timeline_array *args = data;
 	struct drm_syncobj **syncobjs;
+	unsigned int valid_flags = DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED |
+				   DRM_SYNCOBJ_QUERY_FLAGS_ERROR;
 	uint64_t __user *points = u64_to_user_ptr(args->points);
+	uint32_t __user *handles = u64_to_user_ptr(args->handles);
 	uint32_t i;
-	int ret;
+	int ret, error;
 
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
 		return -EOPNOTSUPP;
 
-	if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
+	if (args->flags & ~valid_flags)
 		return -EINVAL;
 
 	if (args->count_handles == 0)
@@ -1681,6 +1684,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 
 		fence = drm_syncobj_fence_get(syncobjs[i]);
 		chain = to_dma_fence_chain(fence);
+
 		if (chain) {
 			struct dma_fence *iter, *last_signaled =
 				dma_fence_get(fence);
@@ -1688,6 +1692,8 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 			if (args->flags &
 			    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
 				point = fence->seqno;
+				error = dma_fence_get_status(fence);
+
 			} else {
 				dma_fence_chain_for_each(iter, fence) {
 					if (iter->context != fence->context) {
@@ -1702,16 +1708,28 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 				point = dma_fence_is_signaled(last_signaled) ?
 					last_signaled->seqno :
 					to_dma_fence_chain(last_signaled)->prev_seqno;
+
+				error = dma_fence_get_status(last_signaled);
 			}
 			dma_fence_put(last_signaled);
 		} else {
 			point = 0;
+			error = fence ? dma_fence_get_status(fence) : 0;
 		}
 		dma_fence_put(fence);
+
 		ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
 		ret = ret ? -EFAULT : 0;
 		if (ret)
 			break;
+
+		if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) {
+			ret = copy_to_user(&handles[i], &error, sizeof(*handles));
+
+			ret = ret ? -EFAULT : 0;
+			if (ret)
+				break;
+		}
 	}
 	drm_syncobj_array_free(syncobjs, args->count_handles);
 
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 27cc159c1d27..213b4dc9b612 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -1044,6 +1044,11 @@ struct drm_syncobj_array {
 };
 
 #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
+/*
+ * Copy the status of the fence as output into the handles array.
+ * The handles array is overwritten by that.
+ */
+#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1)
 struct drm_syncobj_timeline_array {
 	__u64 handles;
 	__u64 points;
-- 
2.53.0

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

[Yicong Hui]

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

[RFC PATCH v3 3/3] drm/syncobj/doc: Remove starter task from todo list

[Yicong Hui]

Remove the starter task for adding a way to query syncobjs error codes
through an ioctl.

Signed-off-by: Yicong Hui <yiconghui@gmail.com>
---
 Documentation/gpu/todo.rst | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 520da44a04a6..8dcb1901142e 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -878,22 +878,6 @@ Contact: Javier Martinez Canillas <javierm@redhat.com>
 
 Level: Advanced
 
-Querying errors from drm_syncobj
-================================
-
-The drm_syncobj container can be used by driver independent code to signal
-complection of submission.
-
-One minor feature still missing is a generic DRM IOCTL to query the error
-status of binary and timeline drm_syncobj.
-
-This should probably be improved by implementing the necessary kernel interface
-and adding support for that in the userspace stack.
-
-Contact: Christian König
-
-Level: Starter
-
 DRM GPU Scheduler
 =================
 
-- 
2.53.0

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Christian König]

On 2/25/26 13:46, Yicong Hui wrote:
> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
> codes from multiple syncobjs and abort early upon error of any of them.

Patch #1 looks good enough to add my rb.

Patch #2 looks good as well, but I'm not familiar enough with the code and have no time to wrap my head around it to give a review.

Adding a few people on CC, maybe somebody has time to take another look.

> 
> Based on discussions from Michel Dänzer and Christian König, and a
> starter task from the DRM todo documentation.
> 
> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
> on userspace implementation.
> 
> I have looked into adding sub test cases into syncobj_wait.c and
> syncobj_timeline.c, igt-tests for this and I think I understand the 
> process for writing tests and submitting them, however, these ioctls 
> only trigger in the case that there is an error, but I am not sure what
> is the best way to artifically trigger an error from userspace in order
> to test that these ioctl flags work. What's the recommended way to 
> approach this?

When Michel agrees that this is the way to go then we either need an in-kernel selftest (see directory drivers/gpu/drm/tests/) or an userspace IGT test.

Not sure what is more appropriate, maybe somebody on CC has more experience with that.

Thanks,
Christian.

> 
> ---
> Changes:
> v3:
> * Fixed inline comments by converting to multi-line comments in
> accordance to kernel style guidelines.
> * No longer using a separate superfluous function to walk the fence
> chain, and instead queries the last signaled fence in in the chain for
> its error code
> * Fixed types for error and handles array.
> * Used dma_fence_get_status to query error instead of getting it
> directly.
> 
> v2:
> https://lore.kernel.org/dri-devel/20260220022631.2205037-1-yiconghui@gmail.com/T/#m6ab4f94a19c769193895d7728383f84e452cbbfa
> * Went from adding a new ioctl to implementing flags for existing
> ones.
> 
> v1:
> * https://lore.kernel.org/all/20260213120836.81283-1-yiconghui@gmail.com/T/#mfdbc7f97e91ca5731b51b69c8cf8173cb0b2fb3e
> 
> Yicong Hui (3):
>   drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors
>   drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag
>   drm/syncobj/doc: Remove starter task from todo list
> 
>  Documentation/gpu/todo.rst    | 16 ------------
>  drivers/gpu/drm/drm_syncobj.c | 49 ++++++++++++++++++++++++++++++-----
>  include/uapi/drm/drm.h        | 11 ++++++++
>  3 files changed, 54 insertions(+), 22 deletions(-)
> 

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Michel Dänzer]

On 2/25/26 14:25, Christian König wrote:
> On 2/25/26 13:46, Yicong Hui wrote:
>> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
>> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
>> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
>> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
>> codes from multiple syncobjs and abort early upon error of any of them.
> 
> Patch #1 looks good enough to add my rb.
> 
> Patch #2 looks good as well, but I'm not familiar enough with the code and have no time to wrap my head around it to give a review.
> 
> Adding a few people on CC, maybe somebody has time to take another look.
> 
>>
>> Based on discussions from Michel Dänzer and Christian König, and a
>> starter task from the DRM todo documentation.
>>
>> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
>> on userspace implementation.
>>
>> I have looked into adding sub test cases into syncobj_wait.c and
>> syncobj_timeline.c, igt-tests for this and I think I understand the 
>> process for writing tests and submitting them, however, these ioctls 
>> only trigger in the case that there is an error, but I am not sure what
>> is the best way to artifically trigger an error from userspace in order
>> to test that these ioctl flags work. What's the recommended way to 
>> approach this?
> 
> When Michel agrees that this is the way to go then we either need an in-kernel selftest (see directory drivers/gpu/drm/tests/) or an userspace IGT test.
> 
> Not sure what is more appropriate, maybe somebody on CC has more experience with that.

I'd advise against landing this in the kernel before there's a corresponding display server implementation making use of it, in a mergeable state.

Otherwise you might end up with the kernel having to support UAPI which no real-world user space actually uses. Been there, done that myself.


I don't have the capacity to contribute anything more than advice at this point.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Christian König]

On 2/25/26 14:37, Michel Dänzer wrote:
> On 2/25/26 14:25, Christian König wrote:
>> On 2/25/26 13:46, Yicong Hui wrote:
>>> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
>>> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
>>> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
>>> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
>>> codes from multiple syncobjs and abort early upon error of any of them.
>>
>> Patch #1 looks good enough to add my rb.
>>
>> Patch #2 looks good as well, but I'm not familiar enough with the code and have no time to wrap my head around it to give a review.
>>
>> Adding a few people on CC, maybe somebody has time to take another look.
>>
>>>
>>> Based on discussions from Michel Dänzer and Christian König, and a
>>> starter task from the DRM todo documentation.
>>>
>>> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
>>> on userspace implementation.
>>>
>>> I have looked into adding sub test cases into syncobj_wait.c and
>>> syncobj_timeline.c, igt-tests for this and I think I understand the 
>>> process for writing tests and submitting them, however, these ioctls 
>>> only trigger in the case that there is an error, but I am not sure what
>>> is the best way to artifically trigger an error from userspace in order
>>> to test that these ioctl flags work. What's the recommended way to 
>>> approach this?
>>
>> When Michel agrees that this is the way to go then we either need an in-kernel selftest (see directory drivers/gpu/drm/tests/) or an userspace IGT test.
>>
>> Not sure what is more appropriate, maybe somebody on CC has more experience with that.
> 
> I'd advise against landing this in the kernel before there's a corresponding display server implementation making use of it, in a mergeable state.

Yeah we clearly have the rule that this can't be pushed into the kernel without userspace code as well. 

> Otherwise you might end up with the kernel having to support UAPI which no real-world user space actually uses. Been there, done that myself.
> 
> 
> I don't have the capacity to contribute anything more than advice at this point.

Oh that is sad. Do you know anybody who could work on that?

It is a clear improvement to error handling and I don't like to keep Yicong's work only on the mailing list.

Thanks,
Christian.

Re: [RFC PATCH v3 1/3] drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors

[Tvrtko Ursulin]

On 25/02/2026 12:46, Yicong Hui wrote:
> Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to make the
> DRM_IOCTL_SYNCOBJ_QUERY ioctl fill out the handles array with the
> error code of the first fence found per syncobj and 0 if one is not
> found and maintain the normal return value in points.
> 
> 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.
> * No longer using a separate superfluous function to walk the fence
> chain, and instead queries the last signaled fence in in the chain for
> its error code
> * Fixed types for error and handles array.
> 
> 
>   drivers/gpu/drm/drm_syncobj.c | 22 ++++++++++++++++++++--
>   include/uapi/drm/drm.h        |  5 +++++
>   2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 2d4ab745fdad..b74e491f9d8b 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1654,14 +1654,17 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   {
>   	struct drm_syncobj_timeline_array *args = data;
>   	struct drm_syncobj **syncobjs;
> +	unsigned int valid_flags = DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED |
> +				   DRM_SYNCOBJ_QUERY_FLAGS_ERROR;
>   	uint64_t __user *points = u64_to_user_ptr(args->points);
> +	uint32_t __user *handles = u64_to_user_ptr(args->handles);
>   	uint32_t i;
> -	int ret;
> +	int ret, error;
>   
>   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   		return -EOPNOTSUPP;
>   
> -	if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> +	if (args->flags & ~valid_flags)
>   		return -EINVAL;
>   
>   	if (args->count_handles == 0)
> @@ -1681,6 +1684,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   
>   		fence = drm_syncobj_fence_get(syncobjs[i]);
>   		chain = to_dma_fence_chain(fence);
> +

Random whitespace changes should be avoided.

>   		if (chain) {
>   			struct dma_fence *iter, *last_signaled =
>   				dma_fence_get(fence);
> @@ -1688,6 +1692,8 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   			if (args->flags &
>   			    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
>   				point = fence->seqno;
> +				error = dma_fence_get_status(fence);
> +

Ditto.

>   			} else {
>   				dma_fence_chain_for_each(iter, fence) {
>   					if (iter->context != fence->context) {
> @@ -1702,16 +1708,28 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   				point = dma_fence_is_signaled(last_signaled) ?
>   					last_signaled->seqno :
>   					to_dma_fence_chain(last_signaled)->prev_seqno;
> +
> +				error = dma_fence_get_status(last_signaled);
>   			}
>   			dma_fence_put(last_signaled);
>   		} else {
>   			point = 0;
> +			error = fence ? dma_fence_get_status(fence) : 0;
>   		}
>   		dma_fence_put(fence);
> +

More of the same. Although in this case I think it is an improvement so 
you may keep it.

>   		ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
>   		ret = ret ? -EFAULT : 0;
>   		if (ret)
>   			break;
> +
> +		if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) {
> +			ret = copy_to_user(&handles[i], &error, sizeof(*handles));
> +

This blank line is not inserted between the existing code but still 
please remove it - it is not separating any logical blocks so it is not 
improving readability.

Apart from nitpicks, the implementation looks correct to me. But 
userspace folks need to bless it and use it, as other people have 
already commented.

And uapi is fine since fence status is already UABI courtesy of 
sync_file. So it is not promoting anything kernel internal to UABI.

> +			ret = ret ? -EFAULT : 0;
> +			if (ret)
> +				break;
> +		}
>   	}
>   	drm_syncobj_array_free(syncobjs, args->count_handles);
>   
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 27cc159c1d27..213b4dc9b612 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -1044,6 +1044,11 @@ struct drm_syncobj_array {
>   };
>   
>   #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
> +/*
> + * Copy the status of the fence as output into the handles array.
> + * The handles array is overwritten by that.

The documentation could be improved though. Make it clear that one 
status per handle is returned (use more plural) and we need an 
explanation of what is the status, or a link to something existing.

For example sync_file uapi header documents it like this:

  * @status:		status of the fence 0:active 1:signaled <0:error

See if you can come up with something clear and to the point for this 
comment block?

Regards,

Tvrtko

> + */
> +#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1)
>   struct drm_syncobj_timeline_array {
>   	__u64 handles;
>   	__u64 points;

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

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Matthew Brost]

On Wed, Feb 25, 2026 at 12:46:06PM +0000, Yicong Hui wrote:

I thought it was a very intentional choice that fences are a completion
mechanism only—they are not a mechanism to report or propagate errors.

This series seems to change that way of thinking—why?

Also consider these cases:

- An input dependency to a job has an error in its fence, and the output
of the job is installed in a syncobj. The job successfully runs but
produces garbage because of the bad input. The job’s fence will not
indicate an error because we don’t propagate input dependency errors to
the job. This makes DRM_SYNCOBJ_QUERY_FLAGS_ERROR seem a bit pointless
now.

- A driver, for whatever reason, sets fence->error, and this fence is
installed in a syncobj. Now user space starts using this new uAPI on
syncobjs and everything breaks. This is odd behavior from the driver,
but it was completely valid because fence->error never propagated to
user space.

I could probably come up with more examples of potential issues, but
let’s start with the above.

Matt

> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
> codes from multiple syncobjs and abort early upon error of any of them.
> 
> Based on discussions from Michel Dänzer and Christian König, and a
> starter task from the DRM todo documentation.
> 
> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
> on userspace implementation.
> 
> I have looked into adding sub test cases into syncobj_wait.c and
> syncobj_timeline.c, igt-tests for this and I think I understand the 
> process for writing tests and submitting them, however, these ioctls 
> only trigger in the case that there is an error, but I am not sure what
> is the best way to artifically trigger an error from userspace in order
> to test that these ioctl flags work. What's the recommended way to 
> approach this?
> 
> ---
> Changes:
> v3:
> * Fixed inline comments by converting to multi-line comments in
> accordance to kernel style guidelines.
> * No longer using a separate superfluous function to walk the fence
> chain, and instead queries the last signaled fence in in the chain for
> its error code
> * Fixed types for error and handles array.
> * Used dma_fence_get_status to query error instead of getting it
> directly.
> 
> v2:
> https://lore.kernel.org/dri-devel/20260220022631.2205037-1-yiconghui@gmail.com/T/#m6ab4f94a19c769193895d7728383f84e452cbbfa
> * Went from adding a new ioctl to implementing flags for existing
> ones.
> 
> v1:
> * https://lore.kernel.org/all/20260213120836.81283-1-yiconghui@gmail.com/T/#mfdbc7f97e91ca5731b51b69c8cf8173cb0b2fb3e
> 
> Yicong Hui (3):
>   drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors
>   drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag
>   drm/syncobj/doc: Remove starter task from todo list
> 
>  Documentation/gpu/todo.rst    | 16 ------------
>  drivers/gpu/drm/drm_syncobj.c | 49 ++++++++++++++++++++++++++++++-----
>  include/uapi/drm/drm.h        | 11 ++++++++
>  3 files changed, 54 insertions(+), 22 deletions(-)
> 
> -- 
> 2.53.0
> 

Re: [RFC PATCH v3 1/3] drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors

[Matthew Brost]

On Wed, Feb 25, 2026 at 12:46:07PM +0000, Yicong Hui wrote:
> Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to make the
> DRM_IOCTL_SYNCOBJ_QUERY ioctl fill out the handles array with the
> error code of the first fence found per syncobj and 0 if one is not
> found and maintain the normal return value in points.
> 
> 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.
> * No longer using a separate superfluous function to walk the fence
> chain, and instead queries the last signaled fence in in the chain for
> its error code
> * Fixed types for error and handles array.
> 
> 
>  drivers/gpu/drm/drm_syncobj.c | 22 ++++++++++++++++++++--
>  include/uapi/drm/drm.h        |  5 +++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 2d4ab745fdad..b74e491f9d8b 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1654,14 +1654,17 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  {
>  	struct drm_syncobj_timeline_array *args = data;
>  	struct drm_syncobj **syncobjs;
> +	unsigned int valid_flags = DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED |
> +				   DRM_SYNCOBJ_QUERY_FLAGS_ERROR;
>  	uint64_t __user *points = u64_to_user_ptr(args->points);
> +	uint32_t __user *handles = u64_to_user_ptr(args->handles);
>  	uint32_t i;
> -	int ret;
> +	int ret, error;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>  		return -EOPNOTSUPP;
>  
> -	if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> +	if (args->flags & ~valid_flags)
>  		return -EINVAL;
>  
>  	if (args->count_handles == 0)
> @@ -1681,6 +1684,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  
>  		fence = drm_syncobj_fence_get(syncobjs[i]);
>  		chain = to_dma_fence_chain(fence);
> +
>  		if (chain) {
>  			struct dma_fence *iter, *last_signaled =
>  				dma_fence_get(fence);
> @@ -1688,6 +1692,8 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  			if (args->flags &
>  			    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
>  				point = fence->seqno;
> +				error = dma_fence_get_status(fence);
> +
>  			} else {
>  				dma_fence_chain_for_each(iter, fence) {
>  					if (iter->context != fence->context) {
> @@ -1702,16 +1708,28 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  				point = dma_fence_is_signaled(last_signaled) ?
>  					last_signaled->seqno :
>  					to_dma_fence_chain(last_signaled)->prev_seqno;
> +
> +				error = dma_fence_get_status(last_signaled);
>  			}
>  			dma_fence_put(last_signaled);
>  		} else {
>  			point = 0;
> +			error = fence ? dma_fence_get_status(fence) : 0;

dma_fence_get_status returns 0 (unsignaled), 1 (signaled with no error),
or fence->error (signaled with error != 0).

Is it intentional to return 1 to user space for a signaled fence? What
if a driver sets fence->error to 1?

Side note: the fence error kernel doc says fence->error is only valid if
< 0, but dma_fence_get_status doesn’t enforce that.

Also, returning fence->error directly to user space seems like a massive
problem. Right now, drivers can set fence->error to whatever they want,
but now this gets reported to user space and suddenly has meaning. Does
user space take certain actions based on the specific error code (e.g.,
-ECANCELED, -ETIME, etc.)? It certainly can’t, because we have no
internal kernel standards for what fence->error actually means. Two
different drivers could assign the same error code but mean entirely
different things—or the opposite could be true.

Thus, without some standardization plus fixing every single driver, I
really think the best we can report in a generic mechanism like a
syncobj is simply “error” or “no error."

Matt

>  		}
>  		dma_fence_put(fence);
> +
>  		ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
>  		ret = ret ? -EFAULT : 0;
>  		if (ret)
>  			break;
> +
> +		if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) {
> +			ret = copy_to_user(&handles[i], &error, sizeof(*handles));
> +
> +			ret = ret ? -EFAULT : 0;
> +			if (ret)
> +				break;
> +		}
>  	}
>  	drm_syncobj_array_free(syncobjs, args->count_handles);
>  
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 27cc159c1d27..213b4dc9b612 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -1044,6 +1044,11 @@ struct drm_syncobj_array {
>  };
>  
>  #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
> +/*
> + * Copy the status of the fence as output into the handles array.
> + * The handles array is overwritten by that.
> + */
> +#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1)
>  struct drm_syncobj_timeline_array {
>  	__u64 handles;
>  	__u64 points;
> -- 
> 2.53.0
> 

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Christian König]

On 2/27/26 00:56, Matthew Brost wrote:
> On Wed, Feb 25, 2026 at 12:46:06PM +0000, Yicong Hui wrote:
> 
> I thought it was a very intentional choice that fences are a completion
> mechanism only—they are not a mechanism to report or propagate errors.
> 
> This series seems to change that way of thinking—why?

We have already changed that a long long time ago. See the whole error reporting for syncfiles.

It was just missing for drm_syncobj which this patch set here fixes.

Regards,
Christian.

> 
> Also consider these cases:
> 
> - An input dependency to a job has an error in its fence, and the output
> of the job is installed in a syncobj. The job successfully runs but
> produces garbage because of the bad input. The job’s fence will not
> indicate an error because we don’t propagate input dependency errors to
> the job. This makes DRM_SYNCOBJ_QUERY_FLAGS_ERROR seem a bit pointless
> now.
> 
> - A driver, for whatever reason, sets fence->error, and this fence is
> installed in a syncobj. Now user space starts using this new uAPI on
> syncobjs and everything breaks. This is odd behavior from the driver,
> but it was completely valid because fence->error never propagated to
> user space.
> 
> I could probably come up with more examples of potential issues, but
> let’s start with the above.
> 
> Matt
> 
>> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
>> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
>> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
>> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
>> codes from multiple syncobjs and abort early upon error of any of them.
>>
>> Based on discussions from Michel Dänzer and Christian König, and a
>> starter task from the DRM todo documentation.
>>
>> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
>> on userspace implementation.
>>
>> I have looked into adding sub test cases into syncobj_wait.c and
>> syncobj_timeline.c, igt-tests for this and I think I understand the 
>> process for writing tests and submitting them, however, these ioctls 
>> only trigger in the case that there is an error, but I am not sure what
>> is the best way to artifically trigger an error from userspace in order
>> to test that these ioctl flags work. What's the recommended way to 
>> approach this?
>>
>> ---
>> Changes:
>> v3:
>> * Fixed inline comments by converting to multi-line comments in
>> accordance to kernel style guidelines.
>> * No longer using a separate superfluous function to walk the fence
>> chain, and instead queries the last signaled fence in in the chain for
>> its error code
>> * Fixed types for error and handles array.
>> * Used dma_fence_get_status to query error instead of getting it
>> directly.
>>
>> v2:
>> https://lore.kernel.org/dri-devel/20260220022631.2205037-1-yiconghui@gmail.com/T/#m6ab4f94a19c769193895d7728383f84e452cbbfa
>> * Went from adding a new ioctl to implementing flags for existing
>> ones.
>>
>> v1:
>> * https://lore.kernel.org/all/20260213120836.81283-1-yiconghui@gmail.com/T/#mfdbc7f97e91ca5731b51b69c8cf8173cb0b2fb3e
>>
>> Yicong Hui (3):
>>   drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors
>>   drm/syncobj: Add DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR ioctl flag
>>   drm/syncobj/doc: Remove starter task from todo list
>>
>>  Documentation/gpu/todo.rst    | 16 ------------
>>  drivers/gpu/drm/drm_syncobj.c | 49 ++++++++++++++++++++++++++++++-----
>>  include/uapi/drm/drm.h        | 11 ++++++++
>>  3 files changed, 54 insertions(+), 22 deletions(-)
>>
>> -- 
>> 2.53.0
>>

Re: [RFC PATCH v3 1/3] drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors

[Christian König]

On 2/27/26 01:23, Matthew Brost wrote:
...
>> @@ -1702,16 +1708,28 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>  				point = dma_fence_is_signaled(last_signaled) ?
>>  					last_signaled->seqno :
>>  					to_dma_fence_chain(last_signaled)->prev_seqno;
>> +
>> +				error = dma_fence_get_status(last_signaled);
>>  			}
>>  			dma_fence_put(last_signaled);
>>  		} else {
>>  			point = 0;
>> +			error = fence ? dma_fence_get_status(fence) : 0;
> 
> dma_fence_get_status returns 0 (unsignaled), 1 (signaled with no error),
> or fence->error (signaled with error != 0).
> 
> Is it intentional to return 1 to user space for a signaled fence? What
> if a driver sets fence->error to 1?
> 
> Side note: the fence error kernel doc says fence->error is only valid if
> < 0, but dma_fence_get_status doesn’t enforce that.

dma_fence_get_status() enforces this with a WARN_ON().

> Also, returning fence->error directly to user space seems like a massive
> problem. Right now, drivers can set fence->error to whatever they want,
> but now this gets reported to user space and suddenly has meaning. Does
> user space take certain actions based on the specific error code (e.g.,
> -ECANCELED, -ETIME, etc.)? It certainly can’t, because we have no
> internal kernel standards for what fence->error actually means. Two
> different drivers could assign the same error code but mean entirely
> different things—or the opposite could be true.

That is not even remotely true. fence->error is already used in the UAPI for syncfiles for like 10years or so. Android is massively relying on that.

There is also documentation on what values drivers should use: https://elixir.bootlin.com/linux/v6.19.3/source/include/linux/dma-fence.h#L565

The error reporting was just missing from drm_syncobj and only implemented for syncfiles and that's what this patch set here is fixing.

Regards,
Christian.

> 
> Thus, without some standardization plus fixing every single driver, I
> really think the best we can report in a generic mechanism like a
> syncobj is simply “error” or “no error."
> 
> Matt
> 
>>  		}
>>  		dma_fence_put(fence);
>> +
>>  		ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
>>  		ret = ret ? -EFAULT : 0;
>>  		if (ret)
>>  			break;
>> +
>> +		if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) {
>> +			ret = copy_to_user(&handles[i], &error, sizeof(*handles));
>> +
>> +			ret = ret ? -EFAULT : 0;
>> +			if (ret)
>> +				break;
>> +		}
>>  	}
>>  	drm_syncobj_array_free(syncobjs, args->count_handles);
>>  
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 27cc159c1d27..213b4dc9b612 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -1044,6 +1044,11 @@ struct drm_syncobj_array {
>>  };
>>  
>>  #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
>> +/*
>> + * Copy the status of the fence as output into the handles array.
>> + * The handles array is overwritten by that.
>> + */
>> +#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1)
>>  struct drm_syncobj_timeline_array {
>>  	__u64 handles;
>>  	__u64 points;
>> -- 
>> 2.53.0
>>

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Yicong Hui]

On 2/25/26 1:57 PM, Christian König wrote:
> On 2/25/26 14:37, Michel Dänzer wrote:
>> On 2/25/26 14:25, Christian König wrote:
>>> On 2/25/26 13:46, Yicong Hui wrote:
>>>> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
>>>> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
>>>> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
>>>> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
>>>> codes from multiple syncobjs and abort early upon error of any of them.
>>>
>>> Patch #1 looks good enough to add my rb.
>>>
>>> Patch #2 looks good as well, but I'm not familiar enough with the code and have no time to wrap my head around it to give a review.
>>>
>>> Adding a few people on CC, maybe somebody has time to take another look.
>>>
>>>>
>>>> Based on discussions from Michel Dänzer and Christian König, and a
>>>> starter task from the DRM todo documentation.
>>>>
>>>> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for discussions
>>>> on userspace implementation.
>>>>
>>>> I have looked into adding sub test cases into syncobj_wait.c and
>>>> syncobj_timeline.c, igt-tests for this and I think I understand the
>>>> process for writing tests and submitting them, however, these ioctls
>>>> only trigger in the case that there is an error, but I am not sure what
>>>> is the best way to artifically trigger an error from userspace in order
>>>> to test that these ioctl flags work. What's the recommended way to
>>>> approach this?
>>>
>>> When Michel agrees that this is the way to go then we either need an in-kernel selftest (see directory drivers/gpu/drm/tests/) or an userspace IGT test.
>>>
>>> Not sure what is more appropriate, maybe somebody on CC has more experience with that.
>>
>> I'd advise against landing this in the kernel before there's a corresponding display server implementation making use of it, in a mergeable state.
> 
> Yeah we clearly have the rule that this can't be pushed into the kernel without userspace code as well.
> 
>> Otherwise you might end up with the kernel having to support UAPI which no real-world user space actually uses. Been there, done that myself.
>>
>>
>> I don't have the capacity to contribute anything more than advice at this point.
> 
> Oh that is sad. Do you know anybody who could work on that?
> 
> It is a clear improvement to error handling and I don't like to keep Yicong's work only on the mailing list.
> 
> Thanks,
> Christian.
> 

Hello

Is there anything else I can do? Or will we have to just leave all of this here unmerged

I have read the emails from Tvrtko and Matthew and I'm absolutely happy to send a v4 to ameliorate these issues, but there might not be a need to do so if the series won't get merged in the end

Regardless, thank you to Christian and all the maintainers for being welcoming and all your work reviewing this patch series so far!

Thanks
Yicong

Re: [RFC PATCH v3 0/3] Querying errors from drm_syncobj

[Tvrtko Ursulin]

On 05/03/2026 16:23, Yicong Hui wrote:
> On 2/25/26 1:57 PM, Christian König wrote:
>> On 2/25/26 14:37, Michel Dänzer wrote:
>>> On 2/25/26 14:25, Christian König wrote:
>>>> On 2/25/26 13:46, Yicong Hui wrote:
>>>>> This patch series adds 2 new flags, DRM_SYNCOBJ_QUERY_FLAGS_ERROR and
>>>>> DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR for 3 ioctl operations
>>>>> DRM_IOCTL_SYNCOBJ_QUERY, DRM_IOCTL_SYNCOBJ_WAIT and
>>>>> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT to allow them to batch-request error
>>>>> codes from multiple syncobjs and abort early upon error of any of 
>>>>> them.
>>>>
>>>> Patch #1 looks good enough to add my rb.
>>>>
>>>> Patch #2 looks good as well, but I'm not familiar enough with the 
>>>> code and have no time to wrap my head around it to give a review.
>>>>
>>>> Adding a few people on CC, maybe somebody has time to take another 
>>>> look.
>>>>
>>>>>
>>>>> Based on discussions from Michel Dänzer and Christian König, and a
>>>>> starter task from the DRM todo documentation.
>>>>>
>>>>> See https://gitlab.gnome.org/GNOME/mutter/-/issues/4624 for 
>>>>> discussions
>>>>> on userspace implementation.
>>>>>
>>>>> I have looked into adding sub test cases into syncobj_wait.c and
>>>>> syncobj_timeline.c, igt-tests for this and I think I understand the
>>>>> process for writing tests and submitting them, however, these ioctls
>>>>> only trigger in the case that there is an error, but I am not sure 
>>>>> what
>>>>> is the best way to artifically trigger an error from userspace in 
>>>>> order
>>>>> to test that these ioctl flags work. What's the recommended way to
>>>>> approach this?
>>>>
>>>> When Michel agrees that this is the way to go then we either need an 
>>>> in-kernel selftest (see directory drivers/gpu/drm/tests/) or an 
>>>> userspace IGT test.
>>>>
>>>> Not sure what is more appropriate, maybe somebody on CC has more 
>>>> experience with that.
>>>
>>> I'd advise against landing this in the kernel before there's a 
>>> corresponding display server implementation making use of it, in a 
>>> mergeable state.
>>
>> Yeah we clearly have the rule that this can't be pushed into the 
>> kernel without userspace code as well.
>>
>>> Otherwise you might end up with the kernel having to support UAPI 
>>> which no real-world user space actually uses. Been there, done that 
>>> myself.
>>>
>>>
>>> I don't have the capacity to contribute anything more than advice at 
>>> this point.
>>
>> Oh that is sad. Do you know anybody who could work on that?
>>
>> It is a clear improvement to error handling and I don't like to keep 
>> Yicong's work only on the mailing list.
>>
>> Thanks,
>> Christian.
>>
> 
> Hello
> 
> Is there anything else I can do? Or will we have to just leave all of 
> this here unmerged
> 
> I have read the emails from Tvrtko and Matthew and I'm absolutely happy 
> to send a v4 to ameliorate these issues, but there might not be a need 
> to do so if the series won't get merged in the end

I wasn't following closely the userspace angle of the discussion to be 
sure, nor I know enough about what are all the userspaces which may want 
to use it, but in general, if there is more than one potential 
userspace, perhaps you could try to interest some of them into the 
feature. Or add support for it yourself, submit to them and say this 
improves this or that and I have the kernel feature waiting already.

One other thing, so that your effort is not lost should someone want to 
work on it in the future, perhaps a patch for the TODO file which links 
to your latest series on lore (once all review comments are addressed) 
and noting that the kernel implementation exists but is waiting on 
userspace? Not sure if we ever done something like that but maybe we 
should to avoid work duplication in the future. And also to preserve 
some credit if someone picks up the work 1-2-3 years down the road.

Regards,

Tvrtko

> Regardless, thank you to Christian and all the maintainers for being 
> welcoming and all your work reviewing this patch series so far!
> 
> Thanks
> Yicong