Re: [PATCH v3 4/4] drm/i915/display: Error codes for async flip failures

[Murthy, Arun R]

On 22-08-2025 17:01, Maarten Lankhorst wrote:
Hey,
I'm not entirely sold on the design, it's way more complicated than it should 
be, it should be trivial to add any amount of error messages.

Replace return -EINVAL; with return drm_atomic_error_einval(state, class, 
"string");
Where class may be an enum, but in a way more generic way than currently specified, for example "invalid use of api", 
"requires modeset", "invalid arguments", "driver limitations", "async flip not possible".

The drm_atomic_error_einval() would set class and str as appropriate, and then 
return -EINVAL.
Looks good to me, but again this state, string will have to be filled to 
the struct and the address of the struct to be sent in the reserved 
field to the user.
So background for choosing this struct [1]

That's probably all we should need here.

Kind regards,
~Maarten

Den 2025-08-22 kl. 09:00, skrev Arun R Murthy:
For failures in async flip atomic check/commit path return user readable
error codes in struct drm_atomic_state.

Signed-off-by: Arun R Murthy <arun.r.mur...@intel.com>
---
  drivers/gpu/drm/i915/display/intel_display.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 
c1a3a95c65f0b66c24ddd64f47dfdc67bbde86c9..5e23f4fc747bd01fa05eba63661bf7279b083317
 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5950,6 +5950,7 @@ static int intel_async_flip_check_uapi(struct 
intel_atomic_state *state,
                drm_dbg_kms(display->drm,
                            "[CRTC:%d:%s] modeset required\n",
                            crtc->base.base.id, crtc->base.name);
+               state->base.error_code->failure_flags = 
DRM_MODE_ATOMIC_CRTC_NEED_FULL_MODESET;
                return -EINVAL;
        }
  
@@ -6019,6 +6020,7 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
                drm_dbg_kms(display->drm,
                            "[CRTC:%d:%s] modeset required\n",
                            crtc->base.base.id, crtc->base.name);
+               state->base.error_code->failure_flags = 
DRM_MODE_ATOMIC_CRTC_NEED_FULL_MODESET;
                return -EINVAL;
        }
  
@@ -6061,6 +6063,8 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
                                    plane->base.base.id, plane->base.name,
                                    &new_plane_state->hw.fb->format->format,
                                    new_plane_state->hw.fb->modifier);
+                       state->base.error_code->failure_flags =
+                               DRM_MODE_ATOMIC_ASYNC_MODIFIER_NOT_SUPPORTED;
                        return -EINVAL;
                }
  

Thanks and Regards,
Arun R Murthy
-------------------
[1] https://hackmd.io/f3bDn3kyRUalLn4LbMfCVQ#Commit-Failure-Feedback