public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH RESEND] drm/bridge: it6505: disable HDCP retry when KSV list timeout
@ 2026-03-09  9:47 Hermes Wu via B4 Relay
  2026-03-10  2:28 ` Claude review: " Claude Code Review Bot
  2026-03-10  2:28 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Hermes Wu via B4 Relay @ 2026-03-09  9:47 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Pet.Weng, Kenneth.Hung, treapking, dri-devel, linux-kernel,
	Hermes Wu

From: Hermes Wu <Hermes.wu@ite.com.tw>

Some DP to HDMI converters fail to report KSV list within the 6-second
HDCP window, causing the IT6505 to repeatedly restart authentication.
This results in continuous flickering on the connected HDMI monitor.

Disable automatic HDCP retry when KSV list timeout. This breaks the
re-auth loop and significantly improves stability/user experience with
problematic converters.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index a094803ba7aa4e12165fcde432d4e6417fbf3676..1c1e6b78fadbf29da00a68e26671ce5b54ee1557 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2238,7 +2238,7 @@ static void it6505_hdcp_wait_ksv_list(struct work_struct *work)
 		return;
 
 timeout:
-	it6505_start_hdcp(it6505);
+	it6505_stop_hdcp(it6505);
 }
 
 static void it6505_hdcp_work(struct work_struct *work)
@@ -2596,7 +2596,7 @@ static void it6505_irq_hdcp_fail(struct it6505 *it6505)
 	DRM_DEV_DEBUG_DRIVER(dev, "hdcp fail interrupt");
 	it6505->hdcp_status = HDCP_AUTH_IDLE;
 	it6505_show_hdcp_info(it6505);
-	it6505_start_hdcp(it6505);
+	it6505_stop_hdcp(it6505);
 }
 
 static void it6505_irq_aux_cmd_fail(struct it6505 *it6505)

---
base-commit: 38feb171b3f92d77e8061fafb5ddfffc2c13b672
change-id: 20260112-disable-hdcp-auto-retry-6a8d00f6ba29

Best regards,
-- 
Hermes Wu <Hermes.wu@ite.com.tw>



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm/bridge: it6505: disable HDCP retry when KSV list timeout
  2026-03-09  9:47 [PATCH RESEND] drm/bridge: it6505: disable HDCP retry when KSV list timeout Hermes Wu via B4 Relay
@ 2026-03-10  2:28 ` Claude Code Review Bot
  2026-03-10  2:28 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-10  2:28 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/bridge: it6505: disable HDCP retry when KSV list timeout
Author: Hermes Wu via B4 Relay <devnull+Hermes.wu.ite.com.tw@kernel.org>
Patches: 1
Reviewed: 2026-03-10T12:28:47.941159

---

This is a single, straightforward patch that fixes a real user-visible problem (continuous flickering) caused by the IT6505 bridge driver endlessly retrying HDCP authentication when KSV list times out or HDCP fails with certain DP-to-HDMI converters. The fix is simple: replace `it6505_start_hdcp()` with `it6505_stop_hdcp()` in two error paths, breaking the re-authentication loop.

The change is small, well-scoped, and the commit message clearly explains both the problem and the fix. The approach is reasonable — when HDCP cannot succeed (due to a problematic downstream converter), stopping gracefully is better than retrying indefinitely.

**Verdict: Looks good overall, with one concern worth discussing.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm/bridge: it6505: disable HDCP retry when KSV list timeout
  2026-03-09  9:47 [PATCH RESEND] drm/bridge: it6505: disable HDCP retry when KSV list timeout Hermes Wu via B4 Relay
  2026-03-10  2:28 ` Claude review: " Claude Code Review Bot
@ 2026-03-10  2:28 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-10  2:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Change 1 — `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 right thing to do. Note that this `timeout` label is also reached when `it6505_hdcp_part2_ksvlist_check()` fails (line 2237-2238), so the KSV list check failure case also now stops instead of retrying — this is also reasonable behavior.

**Change 2 — `it6505_irq_hdcp_fail` (line 2599):**

```c
	it6505->hdcp_status = 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 reasons, not just KSV list timeout on problematic converters. Unconditionally stopping HDCP on *any* failure means:

- **Transient failures** (e.g., noise on the link during authentication) will now permanently disable HDCP for the session rather than retrying. The HDCP spec generally expects implementations to be able to retry authentication.
- **Content protection policy**: If userspace has requested HDCP via the `Content Protection` DRM property, failing permanently without re-attempting may violate the expected behavior. Userspace would need to explicitly toggle the property to trigger a new attempt.

**Suggestion**: Consider whether `it6505_irq_hdcp_fail` should use a limited 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 strategy. At minimum, the commit message should acknowledge that this also changes behavior for non-KSV-timeout HDCP failures.

**Minor note**: `it6505_stop_hdcp()` calls `cancel_delayed_work(&it6505->hdcp_work)` — 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 currently 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-10  2:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-09  9:47 [PATCH RESEND] drm/bridge: it6505: disable HDCP retry when KSV list timeout Hermes Wu via B4 Relay
2026-03-10  2:28 ` Claude review: " Claude Code Review Bot
2026-03-10  2:28 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox