> -----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..f0c3f080f5d66c733dfbfa23f38 > 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.
Thanks and Regards, Arun R Murthy -------------------
