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, 04 Jun 2026 14:13:48 +1000 Message-ID: In-Reply-To: <20260601114756.51953-2-tzimmermann@suse.de> References: <20260601114756.51953-1-tzimmermann@suse.de> <20260601114756.51953-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 correctly replaces the hand-rolled plane validation with the standard `drm_atomic_helper_check_plane_state()` helper. The key changes: ```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` is correct for a primary plane that must fully cover the CRTC. - `can_update_disabled=true` is the right call -- the original code returned early when `!crtc_state->enable`, but letting the helper still initialize plane state is cleaner and safer. - The switch from `drm_atomic_get_crtc_state()` to `drm_atomic_get_new_crtc_state()` is a subtle but correct improvement: the old call would add the CRTC to the atomic state (potentially forcing a modeset), while the new call only retrieves it if already present. `drm_atomic_helper_check_plane_state()` handles `new_crtc_state == NULL` correctly by treating the plane as not visible. - The visibility check (`!new_plane_state->visible`) gates the stride validation below, which is correct since there's no point checking stride on an invisible plane. The commit message is thorough and the Fixes/Cc:stable tags are appropriate. --- Generated by Claude Code Patch Reviewer