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: Sat, 16 May 2026 15:34:45 +1000 Message-ID: In-Reply-To: <20260511122421.114014-5-tzimmermann@suse.de> References: <20260511122421.114014-1-tzimmermann@suse.de> <20260511122421.114014-5-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 moves the `drm_rect_equals(&state->src, &old_state->src)` check from `drm_atomic_helper_damage_iter_init()` (commit time) to `drm_atomic_helper_check_plane_damage()` (check time). This is the right place for it because: - The check requires the old plane state, which should be available during atomic_check but shouldn't need to be threaded through to commit-time helpers. - Setting `ignore_damage_clips` at check time gives a consistent view to all downstream consumers. The function signature changes from `(state, plane_state)` to `(state, old_plane_state, new_plane_state)` which is consistent. **Test removals**: 6 tests are dropped (`*_src_moved`, `*_fractional_src_moved`, `*_damage_not_visible` with src moved). This is appropriate because those tests were testing the src-comparison logic inside `damage_iter_init`, which no longer exists there. The `drm_test_damage_iter_damage_ignore` test from patch 1 covers the equivalent behavior via the flag. However, I note the `drm_test_damage_iter_damage_not_visible` test is also removed here. This test had `mock->state.visible = false` *and* moved src coords. The `visible = false` case is still covered by the existing `drm_test_damage_iter_no_damage_not_visible` test (which doesn't move src), so no coverage gap. --- Generated by Claude Code Patch Reviewer