From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm: Introduce DRM_CAP_ATOMIC_ERROR_REPORTING
Date: Wed, 01 Apr 2026 07:55:28 +1000
Message-ID: <review-patch7-20260331-atomic-v11-7-6a1df7ec5af8@intel.com>
In-Reply-To: <20260331-atomic-v11-7-6a1df7ec5af8@intel.com>
References: <20260331-atomic-v11-0-6a1df7ec5af8@intel.com> <20260331-atomic-v11-7-6a1df7ec5af8@intel.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

```c
case DRM_CAP_ATOMIC_ERROR_REPORTING:
    req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) && 1;
    break;
```

**`&& 1` is redundant.** `drm_core_check_feature()` already returns a boolean-like value. Just use:
```c
req->value = drm_core_check_feature(dev, DRIVER_ATOMIC);
```

**This cap is always true for all atomic drivers.** That means every atomic driver now implicitly "supports" error reporting, even though only the DRM core and i915 async flip paths actually set error codes. All other error paths will return `DRM_MODE_ATOMIC_UNSPECIFIED_ERROR` with an empty string. The capability should probably be opt-in per driver until meaningful coverage exists.

**Missing documentation:** The `DRM_CAP_ATOMIC_ERROR_REPORTING` constant should be documented alongside the description of how userspace should use the `reserved` field. There's no documentation in the UAPI header explaining the protocol: allocate `struct drm_mode_atomic_err_code`, put its address in `reserved`, call the ioctl, check the struct on failure.

---

**Summary of required fixes before merge:**
1. Fix UAPI typos (`ATTOMIC`, `VIOLOATION`, etc.) -- these are permanent
2. Fix the memory leak in `drm_mode_atomic_add_error_msg()` (missing `kfree`)
3. Add NULL check for `kvasprintf` return
4. Initialize `error_code_ptr = NULL`
5. Remove the spammy debug message when `!arg->reserved`
6. Move `copy_to_user` after the `-EDEADLK` retry logic
7. The `reserved` field repurposing needs more careful ABI design -- consider a flag bit to opt into error reporting rather than assuming non-zero `reserved` is always a valid pointer
8. Remove `&& 1` from the capability check

---
Generated by Claude Code Patch Reviewer