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/hibmc: Use drm_atomic_helper_check_plane_state() Date: Thu, 23 Apr 2026 10:03:22 +1000 Message-ID: In-Reply-To: <20260420121130.200133-2-tzimmermann@suse.de> References: <20260420121130.200133-1-tzimmermann@suse.de> <20260420121130.200133-2-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 **Status: Good** This replaces a hand-rolled plane state check with the standard `drm_atomic_helper_check_plane_state()` helper. The conversion is correct: ```c ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state, DRM_PLANE_NO_SCALING, DRM_PLANE_NO_SCALING, false, true); ``` - `can_position=false` (4th arg) correctly enforces that the plane must cover the full CRTC, preserving the original behavior where the plane had to match the display exactly. - `can_update_disabled=true` (5th arg) allows check to proceed even with a disabled CRTC, which is cleaner than the old early return on `!crtc_state->enable`. - Using `drm_atomic_get_new_crtc_state()` (read-only) instead of the old `drm_atomic_get_crtc_state()` (which adds/locks) is the correct modern pattern. The CRTC state is always already present at this point because `drm_atomic_helper_check_modeset()` runs before `drm_atomic_helper_check_planes()`. The commit message notes that the visibility check is slightly relaxed (plane can now be *larger* than the CRTC, not just exactly equal). This is a correct behavioral change and well-documented. The 128-byte pitch alignment check is preserved, which is important for hardware correctness. No issues. --- Generated by Claude Code Patch Reviewer