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/plane: Add new atomic_create_state callback Date: Sun, 22 Mar 2026 03:32:27 +1000 Message-ID: In-Reply-To: <20260320-drm-mode-config-init-v2-8-c63f1134e76c@kernel.org> References: <20260320-drm-mode-config-init-v2-0-c63f1134e76c@kernel.org> <20260320-drm-mode-config-init-v2-8-c63f1134e76c@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Adds the `atomic_create_state` callback to `drm_plane_funcs`, creates `__drm_atomic_helper_plane_create_state()` and `drm_atomic_helper_plane_create_state()` helpers. Also adds fallback logic in `drm_mode_config_reset()` to call `atomic_create_state` if `reset` isn't set. **Issue**: `__drm_atomic_helper_plane_create_state()` is a trivial one-line wrapper around `__drm_atomic_helper_plane_state_init()`: ```c void __drm_atomic_helper_plane_create_state(struct drm_plane *plane, struct drm_plane_state *state) { __drm_atomic_helper_plane_state_init(state, plane); } ``` This is unlike the crtc version (patch 10) which adds `drm_crtc_vblank_reset()`. For planes there's no extra init needed, so this wrapper adds indirection with no value. Consider whether `__drm_atomic_helper_plane_state_init` could just be called directly, or document why the wrapper exists (future-proofing for additional init steps?). **Issue in drm_mode_config_reset()**: The return value of `drm_mode_config_plane_create_state()` is silently ignored: ```c else if (plane->funcs->atomic_create_state) drm_mode_config_plane_create_state(plane); ``` Since `drm_mode_config_reset()` returns void, there's nothing to do with the error, but a `WARN_ON` or `drm_err` on failure would help debugging. This same issue applies to the crtc and connector versions added in patches 10 and 13. --- Generated by Claude Code Patch Reviewer