> 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 > 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. > 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 > 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. > > 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. > 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 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
