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: Add ->detect_ctx hook and drm_bridge_detect_ctx() Date: Wed, 25 Mar 2026 07:36:46 +1000 Message-ID: In-Reply-To: <20260323-dw-hdmi-qp-scramb-v4-1-1d99df84883e@collabora.com> References: <20260323-dw-hdmi-qp-scramb-v4-0-1d99df84883e@collabora.com> <20260323-dw-hdmi-qp-scramb-v4-1-1d99df84883e@collabora.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Overall: Looks good.** The locking pattern (internal `drm_modeset_acquire= _ctx` with EDEADLK retry when no external ctx is provided) follows establis= hed DRM conventions. One concern: - **Return type mixing:** `drm_bridge_detect_ctx()` returns `int`, which ca= n be either a `drm_connector_status` enum value or a negative error code. T= he fallback path at line 282 calls `bridge->funcs->detect()` which returns = `enum drm_connector_status`. This works because the enum values are small n= on-negative integers, but it's worth documenting this mixing more explicitl= y. The docstring mentions it can return `-EDEADLK` only when `@ctx is set`,= but the internal ctx path silently maps errors to `connector_status_unknow= n` (line 274-275), which could mask real errors. Consider at least a `dev_w= arn` or `drm_dbg` when a negative return is swallowed. - **connection_mutex locking:** The function locks `connection_mutex` in th= e internal-ctx path (line 263). This is correct for the bridge-connector us= e case, but the docstring for `detect_ctx` in `drm_bridge.h` says `connecti= on_mutex will always be locked` =E2=80=94 this is only true when called fro= m bridge-connector, not when called directly with `ctx =3D NULL` from other= paths (like `drm_bridge_connector_get_modes_edid` which passes `NULL`). Ac= tually, looking at the code again, the NULL-ctx path *does* lock it (line 2= 63-264), so the guarantee holds. Good. --- Generated by Claude Code Patch Reviewer