Thanks for the review!
On 29-10-2025 12:32, Kandpal, Suraj wrote:
Subject: [PATCH v6 1/5] drm: Define user readable error codes for atomic ioctl
There can be multiple reasons for a failure in atomic_ioctl. Most often in these
error conditions -EINVAL is returned. User/Compositor would have to blindly
take a call on failure of this ioctl so as to use ALLOW_MODESET or any. It would
Or Any ? I think you need to rephrase
Done!
be good if user/compositor gets a readable error code on failure so they can
take proper corrections in the next commit.
The struct drm_mode_atomic is being passed by the user/compositor which
holds the properties for modeset/flip. Reusing the same struct for returning
the error code in case of failure can save by creating a new uapi/interface for
returning the error code.
This sentence seems a little weird can you correct this.
Done!
The element 'reserved' in the struct drm_mode_atomic is used for returning
the user readable error code. This points to the struct
In that case why leave the element name as 'reserved' should that be changed
Good thought!
There are drm clients checking for this reserved variable to be NULL as
its unused.
Once this patch is merged, then will a follow-on patch to change this
variable name.
drm_mode_atomic_err_code. Failure reasons have been initialized in
DRM_MODE_ATOMIC_FAILURE_REASON.
I don't see the code for this here.
Done! Updated!
Signed-off-by: Arun R Murthy <[email protected]>
---
include/uapi/drm/drm_mode.h | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index
1e0e02a79b5c8db200cf9fd35edfe163d701cbd5..1e9eeae472e74bbd1b5e0b6f7
9f9782cafaf5b6e 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -45,6 +45,7 @@ extern "C" {
#define DRM_CONNECTOR_NAME_LEN 32
#define DRM_DISPLAY_MODE_LEN 32
#define DRM_PROP_NAME_LEN 32
+#define DRM_MODE_ATOMIC_FAILURE_STRING_LEN 128
#define DRM_MODE_TYPE_BUILTIN (1<<0) /* deprecated */
#define DRM_MODE_TYPE_CLOCK_C ((1<<1) | DRM_MODE_TYPE_BUILTIN)
/* deprecated */
@@ -1205,6 +1206,46 @@ struct drm_mode_destroy_dumb {
DRM_MODE_ATOMIC_NONBLOCK |\
DRM_MODE_ATOMIC_ALLOW_MODESET)
+/**
+ * enum drm_mode_atomic_err_code - error codes for failures in
+atomic_ioctl
+ * @DRM_MODE_ATOMIC_INVALID_API_USAGE: invallid API
usage(DRM_ATOMIC not
+ * enabled, invalid falg, page_flip event
+ * with test-only, etc)
+ * @DRM_MODE_ATOMIC_CRTC_NEED_FULL_MODESET: Need full modeset
Should this be just be CRTC_NEED_MODESET to make it differ from the below
definition.
Taken care of to match the error code/function in the KMD.
on this
+crtc
+ * @DRM_MODE_ATOMIC_NEED_FULL_MODESET: Need full modeset on all
+connected crtc's
+ * @DRM_MODE_ATOMIC_ASYN_NOTSUPP_PLANE: Aync flip not supported
Typos here
* DRM_MODE_ATOMIC_ASYNC_NOT_SUPP_PLANE
* Async
Done!
Thanks and Regards,
Arun R Murthy
-------------------
Regards,
Suraj Kandpal
on this
+plane
+ * DRM_MODE_ATOMIC_ASYNC_MODIFIER_NOT_SUPP: Modifier not
supported by
+async flip
+ * @DRM_MODE_ATOMIC_ASYNC_PROP_CHANGED: Property changed in
async flip
+*/ enum drm_mode_atomic_failure_codes {
+ DRM_MODE_ATOMIC_INVALID_API_USAGE,
+ DRM_MODE_ATOMIC_CRTC_NEED_FULL_MODESET,
+ DRM_MODE_ATOMIC_NEED_FULL_MODESET,
+ DRM_MODE_ATOMIC_ASYNC_NOT_SUPP_PLANE,
+ DRM_MODE_ATOMIC_ASYNC_MODIFIER_NOT_SUPP,
+ DRM_MODE_ATOMIC_ASYNC_PROP_CHANGED,
+};
+
+/**
+ * drm_mode_atomic_err_code - struct to store the error code
+ *
+ * pointer to this struct will be stored in reserved variable of
+ * struct drm_mode_atomic to report the failure cause to the user.
+ *
+ * @failure_code: error codes defined in enum
+drm_moide_atomic_failure_code
+ * @failure_string_ptr: pointer to user readable error message string
+ * @failure_obj_ptr: pointer to the drm_object that caused error
+ * @reserved: reserved for future use
+ * @count_objs: count of drm_objects if multiple drm_objects caused
+error */ struct drm_mode_atomic_err_code {
+ __u64 failure_code;
+ __u64 failure_objs_ptr;
+ __u64 reserved;
+ __u32 count_objs;
+ char failure_string[DRM_MODE_ATOMIC_FAILURE_STRING_LEN];
+};
+
struct drm_mode_atomic {
__u32 flags;
__u32 count_objs;
--
2.25.1
