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: it6505: disable HDCP retry when KSV list timeout Date: Tue, 10 Mar 2026 12:28:48 +1000 Message-ID: In-Reply-To: <20260309-disable-hdcp-auto-retry-v1-1-591a1d75a3a6@ite.com.tw> References: <20260309-disable-hdcp-auto-retry-v1-1-591a1d75a3a6@ite.com.tw> <20260309-disable-hdcp-auto-retry-v1-1-591a1d75a3a6@ite.com.tw> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Change 1 =E2=80=94 `it6505_hdcp_wait_ksv_list` timeout path (line 2241):** ```c timeout: it6505_stop_hdcp(it6505); ``` This is correct. When the KSV list wait times out after 5 seconds, retrying= authentication would just repeat the same timeout. Stopping HDCP is the ri= ght thing to do. Note that this `timeout` label is also reached when `it650= 5_hdcp_part2_ksvlist_check()` fails (line 2237-2238), so the KSV list check= failure case also now stops instead of retrying =E2=80=94 this is also rea= sonable behavior. **Change 2 =E2=80=94 `it6505_irq_hdcp_fail` (line 2599):** ```c it6505->hdcp_status =3D HDCP_AUTH_IDLE; it6505_show_hdcp_info(it6505); it6505_stop_hdcp(it6505); ``` This one is more nuanced. The HDCP fail interrupt can fire for various reas= ons, not just KSV list timeout on problematic converters. Unconditionally s= topping HDCP on *any* failure means: - **Transient failures** (e.g., noise on the link during authentication) wi= ll now permanently disable HDCP for the session rather than retrying. The H= DCP spec generally expects implementations to be able to retry authenticati= on. - **Content protection policy**: If userspace has requested HDCP via the `C= ontent Protection` DRM property, failing permanently without re-attempting = may violate the expected behavior. Userspace would need to explicitly toggl= e the property to trigger a new attempt. **Suggestion**: Consider whether `it6505_irq_hdcp_fail` should use a limite= d retry count (e.g., retry 2-3 times before giving up) rather than stopping= on the first failure. The KSV list timeout path change is clearly correct,= but the general HDCP failure path could benefit from a bounded retry strat= egy. At minimum, the commit message should acknowledge that this also chang= es behavior for non-KSV-timeout HDCP failures. **Minor note**: `it6505_stop_hdcp()` calls `cancel_delayed_work(&it6505->hd= cp_work)` =E2=80=94 calling this from `it6505_irq_hdcp_fail` (which runs in= an IRQ/work context) should be fine since `cancel_delayed_work` is safe to= call from any context, but it's worth noting that if the hdcp_work is curr= ently running, it won't wait for it to finish (unlike `cancel_delayed_work_= sync`). This matches the existing pattern in the driver, so it's not a new = concern. --- Generated by Claude Code Patch Reviewer