Re: [PATCH v5 4/5] drm/atomic: Return user readable error in atomic_ioctl

Sat, 18 Oct 2025 03:46:20 -0700 

On 01-10-2025 20:17, Louis Chauvet wrote:



Le 16/09/2025 à 09:18, Arun R Murthy a écrit :

Add user readable error codes for failure cases in drm_atomic_ioctl() so
that user can decode the error code and take corrective measurements.

Signed-off-by: Arun R Murthy <[email protected]>
---

  drivers/gpu/drm/drm_atomic_uapi.c | 71 
++++++++++++++++++++++++++++-----------

  1 file changed, 52 insertions(+), 19 deletions(-)


diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 
3ef478e717bec917d1b8803c72bbcc8d6409d745..3d4e9709e8e289cf302413e171b1336812d65c1c 
100644

--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c

@@ -1036,6 +1036,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;
          }

  @@ -1058,6 +1063,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;
          }

  @@ -1096,9 +1106,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_ASYNC_NOT_SUPP_PLANE,

+                                      "[PLANE:%d:%s] does not 
support async flip",

+                                      obj->id, plane->name);
                      break;


Thanks for this series, it is a nice contribution!


I am not sure about this change, drm_dbg will be in debug logs, but 
drm_mode_atomic_add_error_msg do a drm_err, is it expected?
This is also the case for other drm_mode_atomic_add_error_msg calls in 
this patch and the next one.



Thanks for pointing out, this was a miss from my side.
Will get it changed to debug prints.

Thanks and Regards,
Arun R Murthy
-------------------


                  }
              }
@@ -1393,6 +1404,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;
@@ -1401,6 +1413,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;

@@ -1409,31 +1429,35 @@ 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(struct 
drm_mode_atomic_err_code));

+
      /* 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_dbg_atomic(dev,
-                   "commit failed: atomic cap not enabled\n");
-        return -EINVAL;
+ drm_mode_atomic_add_error_msg(&state->error_code,
+                          DRM_MODE_ATOMIC_INVALID_API_USAGE,
+                          "DRM_ATOMIC capability not enabled");
+        ret = -EINVAL;
+        goto out;
      }
        if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS) {
-        drm_dbg_atomic(dev, "commit failed: invalid flag\n");
-        return -EINVAL;
-    }
-
-    if (arg->reserved) {
-        drm_dbg_atomic(dev, "commit failed: reserved field set\n");
-        return -EINVAL;
+ 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");

-            return -EINVAL;
+ 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;
          }
            async_flip = true;
@@ -1442,9 +1466,11 @@ 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");

-        return -EINVAL;
+ 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;
      }
    retry:
@@ -1543,6 +1569,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)))
+            return -EFAULT;
+    }
+
      if (num_fences)
          complete_signaling(dev, state, fence_state, num_fences, !ret);

















    











					Reply via email to