From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/atomic-helpers: Evaluate plane damage after atomic_check Date: Sat, 16 May 2026 15:34:44 +1000 Message-ID: In-Reply-To: <20260511122421.114014-3-tzimmermann@suse.de> References: <20260511122421.114014-1-tzimmermann@suse.de> <20260511122421.114014-3-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Correctness**: This adds a second call to `drm_atomic_helper_check_plane_damage()` at the end of `drm_atomic_helper_check_planes()`, after all plane/CRTC atomic_check callbacks have run. This is correct because drivers like ast, gud, ingenic, and mgag200 may set `crtc_state->mode_changed = true` in their atomic_check, which needs to be caught for modeset detection. Note that at this point in the series, there are now *two* calls to `drm_atomic_helper_check_plane_damage()` for every plane -- one before atomic_check (existing) and one after (new). This is intentional as a transitional state; the pre-check call is removed in patch 8. The function is idempotent with respect to `ignore_damage_clips` (it only sets it to true, never clears it), so the double-call is safe. **Minor**: The braces around the single-statement for_each loop body are unnecessary per kernel coding style: ```c for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { drm_atomic_helper_check_plane_damage(state, new_plane_state); } ``` Could be: ```c for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) drm_atomic_helper_check_plane_damage(state, new_plane_state); ``` Not a blocker. --- Generated by Claude Code Patch Reviewer