[PATCH v2 3/4] drm/atomic: Return user readable error in atomic_ioctl

[Murthy, Arun R]

On 30-07-2025 18:44, Harry Wentland wrote:
>
> On 2025-07-30 06:16, Arun R Murthy wrote:
>> Add user readable error codes for failure cases in drm_atomic_ioctl() so
>> that user can decode the error code and take corrective measurements.
>>
> Thanks for doing this work.
>
>
>> @@ -1541,6 +1548,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>>   		ret = drm_atomic_commit(state);
>>   	}
>>   
>> +	/* update the error code if any error to allow user handling it */
>> +	arg->reserved = state->error_code;
> Once we do this we paint ourselves into a corner.
>
> It's nice that we have this reserved field since it allows us to
> extend the atomic ioctl without the need to define a new one.
>
> When we discussed this at the Display Next Hackfest [1] we came
> to the conclusion that we would want:
>
> - an enum to show what is the issue
> - a string that can be logged to display verbose information
>    about the failure
> - an optional array of KMS object IDs participating in the failure
>
> We could define these in a new struct
>
> struct drm_mode_atomic2 {
> 	__u64 failure_reason;
> 	__u64 failure_string;
> 	__u32 drm_object_ids[MAX_FAILURE_OBJECT_IDS]
> 	__u64 reserved;
> }
>
> that we link to via the drm_mode_atomic->reserved field.
>
> Your approach of flags, as opposed to an enum, would allow reporting
> of multiple failures. Do we think drivers would ever make use of it?
> Normally check short-circuits when a failure/limitation is hit.
Thanks for the feedback. As pointed it would be good to have a struct 
pointed by the reserved variable, so that we can further extend the scope.
W.r.t the enum, I feel its better to have bit notification as we can 
convey multiple errors if any.

Understand that the driver at present will return on the first error but 
upon adding this user readable error code can extend the driver to check 
add the properties provided by user and report error at once, so that 
multiple failure iterations can be overridden.

Having obj_id would be a good one, but when the flow goes to the vendor 
specific driver, they may not have the link to the obj_id where a 
failure is occurred. But still good to have so that at the early stage 
in set_prop, sanity check failures can be reported with obj_id so that 
it would be easy for the user to correct them in the next atomic ioctl.

Will take care of these feedback in my next revision of patch set.

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