> -----Original Message----- > From: Murthy, Arun R <[email protected]> > Sent: Tuesday, February 17, 2026 9:36 AM > To: Kandpal, Suraj <[email protected]>; Maarten Lankhorst > <[email protected]>; Maxime Ripard > <[email protected]>; Thomas Zimmermann <[email protected]>; > David Airlie <[email protected]>; Simona Vetter <[email protected]>; Jani > Nikula <[email protected]>; Vivi, Rodrigo <[email protected]>; > Joonas Lahtinen <[email protected]>; Tvrtko Ursulin > <[email protected]>; [email protected]; [email protected]; > Shankar, Uma <[email protected]>; [email protected]; > Kumar, Naveen1 <[email protected]>; Yella, Ramya Krishna > <[email protected]> > Cc: [email protected]; [email protected]; intel- > [email protected] > Subject: RE: [PATCH v9 5/7] drm/atomic: Return user readable error in > atomic_ioctl > > > > -----Original Message----- > > From: Kandpal, Suraj <[email protected]> > > Sent: Friday, February 13, 2026 2:54 PM > > To: Murthy, Arun R <[email protected]>; Maarten Lankhorst > > <[email protected]>; Maxime Ripard > > <[email protected]>; Thomas Zimmermann <[email protected]>; > David > > Airlie <[email protected]>; Simona Vetter <[email protected]>; Jani > > Nikula <[email protected]>; Vivi, Rodrigo > > <[email protected]>; Joonas Lahtinen > > <[email protected]>; Tvrtko Ursulin > > <[email protected]>; [email protected]; [email protected]; > > Shankar, Uma <[email protected]>; [email protected]; > > Kumar, > > Naveen1 <[email protected]>; Yella, Ramya Krishna > > <[email protected]> > > Cc: [email protected]; [email protected]; > > intel- [email protected]; Murthy, Arun R > > <[email protected]> > > Subject: RE: [PATCH v9 5/7] drm/atomic: Return user readable error in > > atomic_ioctl > > > > > Subject: [PATCH v9 5/7] drm/atomic: Return user readable error in > > > atomic_ioctl > > > > > > Add user readable error codes for failure cases in > > > drm_atomic_ioctl() so that user can decode the error code and take > corrective measurements. > > > > > > v8: Replaced DRM_MODE_ATOMIC_ASYNC_NOT_SUPP_PLANE, > > > DRM_MODE_ATOMIC_ASYNC_MODIFIER_NOT_SUPP with > > INVALID_API_USAGE > > > (Xaver) > > > v9: Move free atomic_state on error to patch 3 (Suraj) > > > > > > Signed-off-by: Arun R Murthy <[email protected]> > > > --- > > > drivers/gpu/drm/drm_atomic_uapi.c | 58 > > > +++++++++++++++++++++++++++++-- > > > -------- > > > 1 file changed, 44 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > > b/drivers/gpu/drm/drm_atomic_uapi.c > > > index > > > > > > bcd12b6eac4f497d2edb8581d9fb0fd54cbef827..f0c3f080f5d66c733dfbfa23f3 > 8 > > a > > > 22132193adec 100644 > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > > @@ -1196,6 +1196,11 @@ int drm_atomic_set_property(struct > > > drm_atomic_state *state, > > > ret = drm_atomic_connector_get_property(connector, > > > connector_state, > > > prop, > > > &old_val); > > > ret = drm_atomic_check_prop_changes(ret, old_val, > > prop_value, > > > prop); > > > + if (ret) { > > > + drm_mode_atomic_add_error_msg(&state- > > > >error_code, > > > + > > > DRM_MODE_ATOMIC_ASYNC_PROP_CHANGED, > > > + "property change > > > not allowed in async flip"); > > > + } > > > break; > > > } > > > > > > @@ -1218,6 +1223,11 @@ int drm_atomic_set_property(struct > > > drm_atomic_state *state, > > > ret = drm_atomic_crtc_get_property(crtc, crtc_state, > > > prop, &old_val); > > > ret = drm_atomic_check_prop_changes(ret, old_val, > > prop_value, > > > prop); > > > + if (ret) { > > > + drm_mode_atomic_add_error_msg(&state- > > > >error_code, > > > + > > > DRM_MODE_ATOMIC_ASYNC_PROP_CHANGED, > > > + "property change > > > not allowed in async flip"); > > > + } > > > break; > > > } > > > > > > @@ -1256,9 +1266,10 @@ int drm_atomic_set_property(struct > > > drm_atomic_state *state, > > > ret = plane_funcs- > > > >atomic_async_check(plane, state, true); > > > > > > if (ret) { > > > - drm_dbg_atomic(prop->dev, > > > - "[PLANE:%d:%s] does not > > > support async flips\n", > > > - obj->id, plane->name); > > > + > > > drm_mode_atomic_add_error_msg(&state->error_code, > > > + > > > DRM_MODE_ATOMIC_INVALID_API_USAGE, > > > + > > > "[PLANE:%d:%s] does not support async flip", > > > + obj->id, > > > plane->name); > > > break; > > > } > > > } > > > @@ -1568,6 +1579,7 @@ int drm_mode_atomic_ioctl(struct drm_device > > *dev, > > > struct drm_atomic_state *state; > > > struct drm_modeset_acquire_ctx ctx; > > > struct drm_out_fence_state *fence_state; > > > + struct drm_mode_atomic_err_code __user *error_code_ptr; > > > int ret = 0; > > > unsigned int i, j, num_fences = 0; > > > bool async_flip = false; > > > @@ -1576,6 +1588,14 @@ int drm_mode_atomic_ioctl(struct > drm_device > > > *dev, > > > if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) > > > return -EOPNOTSUPP; > > > > > > + if (!arg->reserved) > > > + drm_dbg_atomic(dev, > > > + "memory not allocated for drm_atomic error > > > reporting\n"); > > > + else > > > + /* Update the error code if any error to allow user handling it > > > */ > > > + error_code_ptr = (struct drm_mode_atomic_err_code __user > > > *) > > > + (unsigned long)arg->reserved; > > > + > > > state = drm_atomic_state_alloc(dev); > > > if (!state) > > > return -ENOMEM; > > > @@ -1584,11 +1604,16 @@ int drm_mode_atomic_ioctl(struct > drm_device > > > *dev, > > > state->acquire_ctx = &ctx; > > > state->allow_modeset = !!(arg->flags & > > > DRM_MODE_ATOMIC_ALLOW_MODESET); > > > > > > + memset(&state->error_code, 0, sizeof(*error_code_ptr)); > > > + > > > /* disallow for userspace that has not enabled atomic cap (even > > > * though this may be a bit overkill, since legacy userspace > > > * wouldn't know how to call this ioctl) > > > */ > > > if (!file_priv->atomic) { > > > + drm_mode_atomic_add_error_msg(&state->error_code, > > > + > > > DRM_MODE_ATOMIC_INVALID_API_USAGE, > > > + "drm atomic capability not > > > enabled"); > > > drm_dbg_atomic(dev, > > > "commit failed: atomic cap not enabled\n"); > > > ret = -EINVAL; > > > @@ -1596,21 +1621,18 @@ int drm_mode_atomic_ioctl(struct > drm_device > > > *dev, > > > } > > > > > > if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS) { > > > - drm_dbg_atomic(dev, "commit failed: invalid flag\n"); > > > - ret = -EINVAL; > > > - goto out; > > > - } > > > - > > > - if (arg->reserved) { > > > - drm_dbg_atomic(dev, "commit failed: reserved field set\n"); > > > + drm_mode_atomic_add_error_msg(&state->error_code, > > > + > > > DRM_MODE_ATOMIC_INVALID_API_USAGE, > > > + "invalid flag"); > > > ret = -EINVAL; > > > goto out; > > > } > > > > > > if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) { > > > if (!dev->mode_config.async_page_flip) { > > > - drm_dbg_atomic(dev, > > > - "commit failed: > > > DRM_MODE_PAGE_FLIP_ASYNC not supported\n"); > > > + drm_mode_atomic_add_error_msg(&state- > > > >error_code, > > > + > > > DRM_MODE_ATOMIC_INVALID_API_USAGE, > > > + > > > "DRM_MODE_PAGE_FLIP_ASYNC not supported with ATOMIC > > > +ioctl"); > > > ret = -EINVAL; > > > goto out; > > > } > > > @@ -1621,8 +1643,9 @@ int drm_mode_atomic_ioctl(struct drm_device > > *dev, > > > /* can't test and expect an event at the same time. */ > > > if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) && > > > (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) { > > > - drm_dbg_atomic(dev, > > > - "commit failed: page-flip event requested with > > > test-only commit\n"); > > > + drm_mode_atomic_add_error_msg(&state->error_code, > > > + > > > DRM_MODE_ATOMIC_INVALID_API_USAGE, > > > + "page-flip event requested with > > > test-only commit"); > > > ret = -EINVAL; > > > goto out; > > > } > > > @@ -1725,6 +1748,13 @@ int drm_mode_atomic_ioctl(struct > drm_device > > > *dev, > > > } > > > > > > out: > > > + /* Update the error code if any error to allow user handling it */ > > > + if (ret < 0 && arg->reserved) { > > > + if (copy_to_user(error_code_ptr, &state->error_code, > > > + sizeof(state->error_code))) > > > > Corner case what if you end up here right after allocating a state and > > come here because of one of the conditions Which were previously > > checked before state allocation. And then this copy to user fails. > > Then we return without freeing the state, fences etc. Should this be > > done in a cleaner way. > > > Can as well change this return -EFAULT to just ret = -EFAULT and allow the > code flow to continue to clear the atomic state and drop the locks acquired. >
Yes that makes sense With that fixed Reviewed-by: Suraj Kandpal <[email protected]> > Thanks and Regards, > Arun R Murthy > -------------------
