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/display: scdc_helper: Add HDMI 2.0 scrambling management helpers Date: Thu, 04 Jun 2026 13:33:47 +1000 Message-ID: In-Reply-To: <20260602-dw-hdmi-qp-scramb-v7-4-445eb54ee1ed@collabora.com> References: <20260602-dw-hdmi-qp-scramb-v7-0-445eb54ee1ed@collabora.com> <20260602-dw-hdmi-qp-scramb-v7-4-445eb54ee1ed@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 This is the core of the series. A few observations: 1. **`drm_scdc_stop_scrambling()` ignores the return value of `scrambler_disable()`:** ```c return hdmi->funcs->scrambler_disable(connector); ``` This does return it, but the caller in `dw_hdmi_qp_bridge_atomic_disable()` (patch 14) ignores the return value of `drm_scdc_stop_scrambling()`. During disable, failing to turn off source-side scrambling is arguably non-fatal, but logging the failure would be prudent. 2. **`drm_scdc_reset_crtc()` reads SCDC before checking if scrambling is actually needed:** The function reads `SCDC_TMDS_CONFIG` from the sink via I2C, which can take non-trivial time during every hotplug reconnect. This is only reached when `scrambler_supported && plugged && scrambler_enabled`, so it's gated correctly. 3. **Race window in `drm_scdc_try_scrambling_setup()`:** The function checks `READ_ONCE(connector->hdmi.scrambler_enabled)` before scheduling work, but between the SCDC writes and the schedule call, the work could have been cancelled by a concurrent `drm_scdc_stop_scrambling()`. This appears safe because `cancel_delayed_work_sync()` in stop_scrambling will wait for any in-flight callback, and the monitor callback re-checks `scrambler_enabled` before acting. --- Generated by Claude Code Patch Reviewer