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/bridge_connector: Convert to atomic_create_state Date: Sun, 22 Mar 2026 03:32:29 +1000 Message-ID: In-Reply-To: <20260320-drm-mode-config-init-v2-20-c63f1134e76c@kernel.org> References: <20260320-drm-mode-config-init-v2-0-c63f1134e76c@kernel.org> <20260320-drm-mode-config-init-v2-20-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 Converts `drm_bridge_connector` to use `atomic_create_state` instead of `reset`. The old `drm_bridge_connector_reset()` called `drm_atomic_helper_connector_reset()` then `__drm_atomic_helper_connector_hdmi_state_init()`. The new version calls `drm_atomic_helper_connector_create_state()` then adds HDMI init. **Note**: The `.reset` callback is removed entirely from `drm_bridge_connector_funcs`. This means `drm_mode_config_reset()` will no longer create state for bridge connectors (it falls through to the `else if (atomic_create_state)` path instead). This is fine for the initial state, but if anyone calls `drm_mode_config_reset()` during suspend/resume to re-create connector states, the bridge connector won't get a new state since there's no `reset` anymore. The `drm_mode_config_reset` fallback to `atomic_create_state` added in patch 13 should handle this, but it doesn't free the old state first -- unlike `drm_atomic_helper_connector_reset()` which does `kfree(connector->state)` before creating a new one. This means calling `drm_mode_config_reset()` on a bridge connector after this conversion would leak the old state and overwrite the pointer. However, the `drm_mode_config_reset` fallback code checks nothing (`if (connector->funcs->atomic_create_state)`) and will always create a new state, so this is indeed a leak if `reset()` is called with existing state. Since `drm_mode_config_reset()` is documented for suspend/resume use, this is a concern. Has Laurent's R-b. The concern above may be acceptable if bridge connectors are never expected to go through `drm_mode_config_reset()` during suspend/resume, but it should be documented or the fallback path in `drm_mode_config_reset()` should handle existing state (free old before creating new). --- Generated by Claude Code Patch Reviewer