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

[RFC PATCH v2 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.


---
Changes:
v2:
* 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/dma-buf/dma-fence-chain.c | 52 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_syncobj.c     | 46 ++++++++++++++++++++++++---
 include/linux/dma-fence-chain.h   |  1 +
 include/uapi/drm/drm.h            |  2 ++
 5 files changed, 96 insertions(+), 21 deletions(-)

-- 
2.53.0

[RFC PATCH v2 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>
---
 drivers/dma-buf/dma-fence-chain.c | 52 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_syncobj.c     | 21 ++++++++++++-
 include/linux/dma-fence-chain.h   |  1 +
 include/uapi/drm/drm.h            |  1 +
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
index a8a90acf4f34..076d78b73379 100644
--- a/drivers/dma-buf/dma-fence-chain.c
+++ b/drivers/dma-buf/dma-fence-chain.c
@@ -76,6 +76,58 @@ struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_chain_walk);
 
+/**
+ * dma_fence_chain_find_error - find the latest error
+ * @fence: current chain node
+ *
+ * Walk the chain repeatedly until reaches a fence with error, or the
+ * end of the fence chain. Does not garbage collect.
+ *
+ * Returns the first error it finds in the chain.
+ */
+int64_t dma_fence_chain_find_error(struct dma_fence *fence)
+{
+	struct dma_fence_chain *chain, *prev_chain;
+	struct dma_fence *prev;
+	int64_t error = 0;
+
+	chain = to_dma_fence_chain(fence);
+	if (!chain)
+		return fence->error;
+
+	if (chain->fence->error)
+		return chain->fence->error;
+
+	while ((prev = dma_fence_chain_get_prev(chain))) {
+		prev_chain = to_dma_fence_chain(prev);
+
+		if (prev_chain) {
+
+			if (prev_chain->fence->error) {
+				error = prev_chain->fence->error;
+				dma_fence_put(prev);
+				break;
+			}
+
+			chain = prev_chain;
+		} else {
+
+			if (prev->error)
+				error = prev->error;
+			dma_fence_put(prev);
+			break;
+		}
+
+
+		dma_fence_put(prev);
+
+	}
+
+
+	return error;
+}
+EXPORT_SYMBOL(dma_fence_chain_find_error);
+
 /**
  * dma_fence_chain_find_seqno - find fence chain node by seqno
  * @pfence: pointer to the chain node where to start
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 2d4ab745fdad..322f64b72775 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);
+	uint64_t __user *handles = u64_to_user_ptr(args->handles);
 	uint32_t i;
 	int ret;
 
 	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)
@@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 		uint64_t point;
 
 		fence = drm_syncobj_fence_get(syncobjs[i]);
+
+		if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) {
+			int64_t error = 0;
+
+			if (fence)
+				error = dma_fence_chain_find_error(fence);
+
+			ret = copy_to_user(&handles[i], &error, sizeof(int64_t));
+			ret = ret ? -EFAULT : 0;
+			if (ret) {
+				dma_fence_put(fence);
+				break;
+			}
+
+		}
+
 		chain = to_dma_fence_chain(fence);
 		if (chain) {
 			struct dma_fence *iter, *last_signaled =
diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
index 68c3c1e41014..b4ada124d7b6 100644
--- a/include/linux/dma-fence-chain.h
+++ b/include/linux/dma-fence-chain.h
@@ -122,6 +122,7 @@ static inline void dma_fence_chain_free(struct dma_fence_chain *chain)
 	     iter = dma_fence_chain_walk(iter))
 
 struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence);
+int64_t dma_fence_chain_find_error(struct dma_fence *fence);
 int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno);
 void dma_fence_chain_init(struct dma_fence_chain *chain,
 			  struct dma_fence *prev,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 27cc159c1d27..2640cc0a09fe 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -1044,6 +1044,7 @@ struct drm_syncobj_array {
 };
 
 #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
+#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1) /* fill out error codes if they are found */
 struct drm_syncobj_timeline_array {
 	__u64 handles;
 	__u64 points;
-- 
2.53.0

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

[Christian König]

On 2/20/26 03:26, 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>
> ---
>  drivers/dma-buf/dma-fence-chain.c | 52 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_syncobj.c     | 21 ++++++++++++-
>  include/linux/dma-fence-chain.h   |  1 +
>  include/uapi/drm/drm.h            |  1 +
>  4 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
> index a8a90acf4f34..076d78b73379 100644
> --- a/drivers/dma-buf/dma-fence-chain.c
> +++ b/drivers/dma-buf/dma-fence-chain.c
> @@ -76,6 +76,58 @@ struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence)
>  }
>  EXPORT_SYMBOL(dma_fence_chain_walk);
>  
> +/**
> + * dma_fence_chain_find_error - find the latest error
> + * @fence: current chain node
> + *
> + * Walk the chain repeatedly until reaches a fence with error, or the
> + * end of the fence chain. Does not garbage collect.
> + *
> + * Returns the first error it finds in the chain.
> + */
> +int64_t dma_fence_chain_find_error(struct dma_fence *fence)
> +{
> +	struct dma_fence_chain *chain, *prev_chain;
> +	struct dma_fence *prev;
> +	int64_t error = 0;
> +
> +	chain = to_dma_fence_chain(fence);
> +	if (!chain)
> +		return fence->error;
> +
> +	if (chain->fence->error)
> +		return chain->fence->error;
> +
> +	while ((prev = dma_fence_chain_get_prev(chain))) {
> +		prev_chain = to_dma_fence_chain(prev);
> +
> +		if (prev_chain) {
> +
> +			if (prev_chain->fence->error) {
> +				error = prev_chain->fence->error;
> +				dma_fence_put(prev);
> +				break;
> +			}
> +
> +			chain = prev_chain;
> +		} else {
> +
> +			if (prev->error)
> +				error = prev->error;
> +			dma_fence_put(prev);
> +			break;
> +		}
> +
> +
> +		dma_fence_put(prev);
> +
> +	}
> +
> +
> +	return error;
> +}
> +EXPORT_SYMBOL(dma_fence_chain_find_error);

That is superfluous, only signaled fences can have an error.

> +
>  /**
>   * dma_fence_chain_find_seqno - find fence chain node by seqno
>   * @pfence: pointer to the chain node where to start
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 2d4ab745fdad..322f64b72775 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);
> +	uint64_t __user *handles = u64_to_user_ptr(args->handles);
>  	uint32_t i;
>  	int ret;
>  
>  	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)
> @@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  		uint64_t point;

Make a local variable "int error" here.

>  
>  		fence = drm_syncobj_fence_get(syncobjs[i]);
> +
> +		if (args->flags & DRM_SYNCOBJ_QUERY_FLAGS_ERROR) {

That should probably be just another functionality before the existing copy, but see below after the "if (chain)".

> +			int64_t error = 0;

The error code is an int and only 32bits.

> +
> +			if (fence)
> +				error = dma_fence_chain_find_error(fence);
> +
> +			ret = copy_to_user(&handles[i], &error, sizeof(int64_t));

The handles are also only 32bits!

> +			ret = ret ? -EFAULT : 0;
> +			if (ret) {
> +				dma_fence_put(fence);
> +				break;
> +			}
> +
> +		}
> +
>  		chain = to_dma_fence_chain(fence);
>  		if (chain) {

In this code path whenever point is assigned something do a "error = dma_fence_get_status(fence);" and then eventually copy the error to userspace after copying the point.

>  			struct dma_fence *iter, *last_signaled =
> diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
> index 68c3c1e41014..b4ada124d7b6 100644
> --- a/include/linux/dma-fence-chain.h
> +++ b/include/linux/dma-fence-chain.h
> @@ -122,6 +122,7 @@ static inline void dma_fence_chain_free(struct dma_fence_chain *chain)
>  	     iter = dma_fence_chain_walk(iter))
>  
>  struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence);
> +int64_t dma_fence_chain_find_error(struct dma_fence *fence);
>  int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno);
>  void dma_fence_chain_init(struct dma_fence_chain *chain,
>  			  struct dma_fence *prev,
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 27cc159c1d27..2640cc0a09fe 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -1044,6 +1044,7 @@ struct drm_syncobj_array {
>  };
>  
>  #define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */
> +#define DRM_SYNCOBJ_QUERY_FLAGS_ERROR (1 << 1) /* fill out error codes if they are found */

First of all comments please never after a define! The existing code has that already messed up.

Then the new define needs more documentation. Something like:

/*
 * Copy the status of the fence as output into the handles array.
 * The handles array is overwritten by that.
 */

Regards,
Christian.

>  struct drm_syncobj_timeline_array {
>  	__u64 handles;
>  	__u64 points;

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

[Yicong Hui]

>> +
>>   /**
>>    * dma_fence_chain_find_seqno - find fence chain node by seqno
>>    * @pfence: pointer to the chain node where to start
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index 2d4ab745fdad..322f64b72775 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);
>> +	uint64_t __user *handles = u64_to_user_ptr(args->handles);
>>   	uint32_t i;
>>   	int ret;
>>   
>>   	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)
>> @@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>   		uint64_t point;
> 
> Make a local variable "int error" here.
> 
> ...
> 
>> +			int64_t error = 0;
> 
> The error code is an int and only 32bits.
Understood, will change that!

> 
>> +
>> +			if (fence)
>> +				error = dma_fence_chain_find_error(fence);
>> +
>> +			ret = copy_to_user(&handles[i], &error, sizeof(int64_t));
> 
> The handles are also only 32bits!
Ah, that's my mistake. Was thrown off by the __u64 in the struct definition but it is obvious now that it is a u32. Fixed as well!

> 
>> +			ret = ret ? -EFAULT : 0;
>> +			if (ret) {
>> +				dma_fence_put(fence);
>> +				break;
>> +			}
>> +
>> +		}
>> +
>>   		chain = to_dma_fence_chain(fence);
>>   		if (chain) {
> 
> In this code path whenever point is assigned something do a "error = dma_fence_get_status(fence);" and then eventually copy the error to userspace after copying the point.
> 

Hi! Were you thinking something that looks a little bit like this?

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

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

[Christian König]

On 2/20/26 18:05, Yicong Hui wrote:
>>> +
>>>   /**
>>>    * dma_fence_chain_find_seqno - find fence chain node by seqno
>>>    * @pfence: pointer to the chain node where to start
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index 2d4ab745fdad..322f64b72775 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);
>>> +    uint64_t __user *handles = u64_to_user_ptr(args->handles);
>>>       uint32_t i;
>>>       int ret;
>>>         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)
>>> @@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>>           uint64_t point;
>>
>> Make a local variable "int error" here.
>>
>> ...
>>
>>> +            int64_t error = 0;
>>
>> The error code is an int and only 32bits.
> Understood, will change that!
> 
>>
>>> +
>>> +            if (fence)
>>> +                error = dma_fence_chain_find_error(fence);
>>> +
>>> +            ret = copy_to_user(&handles[i], &error, sizeof(int64_t));
>>
>> The handles are also only 32bits!
> Ah, that's my mistake. Was thrown off by the __u64 in the struct definition but it is obvious now that it is a u32. Fixed as well!
> 
>>
>>> +            ret = ret ? -EFAULT : 0;
>>> +            if (ret) {
>>> +                dma_fence_put(fence);
>>> +                break;
>>> +            }
>>> +
>>> +        }
>>> +
>>>           chain = to_dma_fence_chain(fence);
>>>           if (chain) {
>>
>> In this code path whenever point is assigned something do a "error = dma_fence_get_status(fence);" and then eventually copy the error to userspace after copying the point.
>>
> 
> Hi! Were you thinking something that looks a little bit like this?

Yeah that looks like what I had in mind to.

Thanks,
Christian.

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

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

[Yicong Hui]

On 2/20/26 5:07 PM, Christian König wrote:
> On 2/20/26 18:05, Yicong Hui wrote:
>>>> +
>>>>    /**
>>>>     * dma_fence_chain_find_seqno - find fence chain node by seqno
>>>>     * @pfence: pointer to the chain node where to start
>>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>>> index 2d4ab745fdad..322f64b72775 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);
>>>> +    uint64_t __user *handles = u64_to_user_ptr(args->handles);
>>>>        uint32_t i;
>>>>        int ret;
>>>>          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)
>>>> @@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>>>            uint64_t point;
>>>
>>> Make a local variable "int error" here.
>>>
>>> ...
>>>
>>>> +            int64_t error = 0;
>>>
>>> The error code is an int and only 32bits.
>> Understood, will change that!
>>
>>>
>>>> +
>>>> +            if (fence)
>>>> +                error = dma_fence_chain_find_error(fence);
>>>> +
>>>> +            ret = copy_to_user(&handles[i], &error, sizeof(int64_t));
>>>
>>> The handles are also only 32bits!
>> Ah, that's my mistake. Was thrown off by the __u64 in the struct definition but it is obvious now that it is a u32. Fixed as well!
>>
>>>
>>>> +            ret = ret ? -EFAULT : 0;
>>>> +            if (ret) {
>>>> +                dma_fence_put(fence);
>>>> +                break;
>>>> +            }
>>>> +
>>>> +        }
>>>> +
>>>>            chain = to_dma_fence_chain(fence);
>>>>            if (chain) {
>>>
>>> In this code path whenever point is assigned something do a "error = dma_fence_get_status(fence);" and then eventually copy the error to userspace after copying the point.
>>>
>>
>> Hi! Were you thinking something that looks a little bit like this?
> 
> Yeah that looks like what I had in mind to.
> 
> Thanks,
> Christian.
> 

Hi! What should I do at this point? Send a v3, or..?

Thanks!
Yicong

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

[Christian König]

On 2/20/26 20:40, Yicong Hui wrote:
> On 2/20/26 5:07 PM, Christian König wrote:
>> On 2/20/26 18:05, Yicong Hui wrote:
>>>>> +
>>>>>    /**
>>>>>     * dma_fence_chain_find_seqno - find fence chain node by seqno
>>>>>     * @pfence: pointer to the chain node where to start
>>>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>>>> index 2d4ab745fdad..322f64b72775 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);
>>>>> +    uint64_t __user *handles = u64_to_user_ptr(args->handles);
>>>>>        uint32_t i;
>>>>>        int ret;
>>>>>          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)
>>>>> @@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>>>>            uint64_t point;
>>>>
>>>> Make a local variable "int error" here.
>>>>
>>>> ...
>>>>
>>>>> +            int64_t error = 0;
>>>>
>>>> The error code is an int and only 32bits.
>>> Understood, will change that!
>>>
>>>>
>>>>> +
>>>>> +            if (fence)
>>>>> +                error = dma_fence_chain_find_error(fence);
>>>>> +
>>>>> +            ret = copy_to_user(&handles[i], &error, sizeof(int64_t));
>>>>
>>>> The handles are also only 32bits!
>>> Ah, that's my mistake. Was thrown off by the __u64 in the struct definition but it is obvious now that it is a u32. Fixed as well!
>>>
>>>>
>>>>> +            ret = ret ? -EFAULT : 0;
>>>>> +            if (ret) {
>>>>> +                dma_fence_put(fence);
>>>>> +                break;
>>>>> +            }
>>>>> +
>>>>> +        }
>>>>> +
>>>>>            chain = to_dma_fence_chain(fence);
>>>>>            if (chain) {
>>>>
>>>> In this code path whenever point is assigned something do a "error = dma_fence_get_status(fence);" and then eventually copy the error to userspace after copying the point.
>>>>
>>>
>>> Hi! Were you thinking something that looks a little bit like this?
>>
>> Yeah that looks like what I had in mind to.
>>
>> Thanks,
>> Christian.
>>
> 
> Hi! What should I do at this point? Send a v3, or..?

You can go ahead and send a v3. But after that I think we need some userspace code using that.

Could you also look into writing IGT tests for this?

Thanks,
Christian.

> 
> Thanks!
> Yicong

[RFC PATCH v2 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>
---
 drivers/gpu/drm/drm_syncobj.c | 25 +++++++++++++++++++++----
 include/uapi/drm/drm.h        |  1 +
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 322f64b72775..fb55fd46fb84 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1139,6 +1139,13 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
 			if (!fence)
 				continue;
 
+			if ((flags & DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR) && fence->error) {
+				if (idx)
+					*idx = i;
+				timeout = fence->error;
+				goto done_waiting;
+			}
+
 			if ((flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE) ||
 			    dma_fence_is_signaled(fence) ||
 			    (!entries[i].fence_cb.func &&
@@ -1242,8 +1249,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 +1264,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 +1347,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 +1392,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 2640cc0a09fe..bc958af5a910 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -977,6 +977,7 @@ 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 */
+#define DRM_SYNCOBJ_WAIT_FLAGS_ABORT_ON_ERROR (1 << 4) /* abort upon any fence failure return err */
 struct drm_syncobj_wait {
 	__u64 handles;
 	/* absolute timeout */
-- 
2.53.0

[RFC PATCH v2 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