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: Switch to using ->detect_ctx hook Date: Wed, 25 Mar 2026 07:36:46 +1000 Message-ID: In-Reply-To: <20260323-dw-hdmi-qp-scramb-v4-2-1d99df84883e@collabora.com> References: <20260323-dw-hdmi-qp-scramb-v4-0-1d99df84883e@collabora.com> <20260323-dw-hdmi-qp-scramb-v4-2-1d99df84883e@collabora.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Overall: Looks correct but has a subtle concern.** - **`drm_bridge_connector_get_modes_edid` calls `detect_ctx` with NULL ctx (line 303):** This means the internal locking path in `drm_bridge_detect_ctx()` will be used. For dw-hdmi-qp's `detect_ctx` implementation (patch 3), this could trigger `dw_hdmi_qp_reset_crtc()` which calls `drm_bridge_helper_reset_crtc()`. Doing a full CRTC reset inside `get_modes` seems unexpected and potentially problematic. However, this only triggers when `scramb_enabled` is true and the sink's SCDC state mismatches, so in practice during `get_modes` the pipeline should already be in a consistent state. - **Removal of `.detect` from `drm_bridge_connector_funcs`:** The old `drm_bridge_connector_detect` is completely removed, and only `detect_ctx` is provided as a helper func. This means any code path that called `connector->funcs->detect()` directly will now get NULL. This should be fine since DRM core checks for `helper_funcs->detect_ctx` first (as of recent kernels), but it's a behavioral change worth being aware of for any out-of-tree users. --- Generated by Claude Code Patch Reviewer