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/damage-helper: Test src coord in drm_atomic_helper_check_plane_damage() Date: Thu, 04 Jun 2026 15:21:21 +1000 Message-ID: In-Reply-To: <20260530185716.65688-7-tzimmermann@suse.de> References: <20260530185716.65688-1-tzimmermann@suse.de> <20260530185716.65688-7-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 Moves the source-coordinate comparison from the runtime damage iterator into the check phase: ```c + if (!drm_rect_equals(&new_plane_state->src, &old_plane_state->src)) + new_plane_state->ignore_damage_clips = true; ``` This is the right place for it -- alongside the modeset check. The function signature now takes both old and new state: ```c void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state, + const struct drm_plane_state *old_plane_state, + struct drm_plane_state *new_plane_state) ``` The corresponding removal from `drm_atomic_helper_damage_iter_init()`: ```c - if (!iter->clips || state->ignore_damage_clips || - !drm_rect_equals(&state->src, &old_state->src)) { + if (!iter->clips || state->ignore_damage_clips) { ``` Note: the `old_state` parameter is left in `damage_iter_init` for now but documented as "Unused, pass NULL". This keeps the series bisectable. The deletion of 6 kunit tests that specifically tested src-moved scenarios is appropriate since that logic is now consolidated into the `ignore_damage_clips` flag, and the new test from patch 1 covers the flag behavior. However, the removed `drm_test_damage_iter_damage_not_visible` test checked the `visible = false` case combined with src movement -- the `visible = false` case by itself is still covered by `drm_test_damage_iter_no_damage_not_visible`, so this is fine. **No issues.** --- Generated by Claude Code Patch Reviewer