On 2/13/26 13:08, Yicong Hui wrote: > Add DRM_IOCTL_SYNCOBJ_QUERY_ERROR to allow userspace to query the error > status of a fence held by a timeline/binary syncobj.
Not bad for a first try, but quite a bunch of general comments. First of all to get that merged you need to point out a Mesa merge request where this interface is actually used in userspace, so that we can look at the full solution. > Signed-off-by: Yicong Hui <[email protected]> > --- > drivers/gpu/drm/drm_internal.h | 2 ++ > drivers/gpu/drm/drm_ioctl.c | 2 ++ > drivers/gpu/drm/drm_syncobj.c | 22 ++++++++++++++++++++++ > include/uapi/drm/drm.h | 13 +++++++++++++ > 4 files changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index f893b1e3a596..d4d722983544 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -285,6 +285,8 @@ int drm_syncobj_timeline_signal_ioctl(struct drm_device > *dev, void *data, > struct drm_file *file_private); > int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_private); > +int drm_syncobj_query_error_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file_private); > > /* drm_framebuffer.c */ > void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent, > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index ff193155129e..61b114a6a65f 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -732,6 +732,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, > DRM_MASTER), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, > DRM_MASTER), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, > DRM_MASTER), > + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY_ERROR, > drm_syncobj_query_error_ioctl, > + DRM_RENDER_ALLOW), My educated guess is that userspace doesn't want to call this IOCTL separately because of performance reasons. Instead add some additional flag to DRM_SYNCOBJ_WAIT_FLAGS_* so that the IOCTL aborts the wait and returns an error as soon as it sees any fence with an error. Another DRM_SYNCOBJ_QUERY_FLAGS_* is potentially also useful to query the error on a number of drm_syncobjs at the same time. But in general since this is not a HW feature the userspace developers need to voice their requirements and explain how they want to have that implemented. So adding Michel on CC, I've briefly discussed that topic with him on XDC last year. Thanks, Christian. > }; > > #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE(drm_ioctls) > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index 2d4ab745fdad..2152cd029070 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -1717,3 +1717,25 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, > void *data, > > return ret; > } > + > +int drm_syncobj_query_error_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file_private) > +{ > + struct drm_syncobj_error *args = data; > + struct dma_fence *fence; > + int ret; > + > + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > + return -EOPNOTSUPP; > + > + ret = drm_syncobj_find_fence(file_private, args->handle, args->point, > 0, &fence); > + > + if (ret) > + return ret; > + > + args->error = fence->error; > + > + dma_fence_put(fence); > + > + return 0; > +} > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 27cc159c1d27..087c0f2120ec 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -1051,6 +1051,11 @@ struct drm_syncobj_timeline_array { > __u32 flags; > }; > > +struct drm_syncobj_error { > + __u32 handle; > + __s32 error; > + __u64 point; > +}; > > /* Query current scanout sequence number */ > struct drm_crtc_get_sequence { > @@ -1363,6 +1368,14 @@ extern "C" { > */ > #define DRM_IOCTL_GEM_CHANGE_HANDLE DRM_IOWR(0xD2, struct > drm_gem_change_handle) > > +/** > + * DRM_IOCTL_SYNCOBJ_QUERY_ERROR - Query the error code from a failed > drm_syncobj > + * > + * This ioctl provides userspace a way to query the error code of a binary > and > + * timeline drm_syncobj in the case that the submission fails. > + */ > +#define DRM_IOCTL_SYNCOBJ_QUERY_ERROR DRM_IOWR(0xD3, struct > drm_syncobj_error) > + > /* > * Device specific ioctls should only be in their respective headers > * The device specific ioctl range is from 0x40 to 0x9f.
