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@xxxxxxxxx/T/#m6ab4f94a19c769193895d7728383f84e452cbbfa
> * Went from adding a new ioctl to implementing flags for existing
> ones.
> 
> v1:
> * https://lore.kernel.org/all/20260213120836.81283-1-yiconghui@xxxxxxxxx/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(-)
>