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/nouveau/dp: retry link check once on HPD IRQ before disconnect Date: Sun, 12 Apr 2026 10:25:01 +1000 Message-ID: In-Reply-To: <20260409172126.115441-4-marek@czernohous.de> References: <20260409172126.115441-1-marek@czernohous.de> <20260409172126.115441-4-marek@czernohous.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Verdict: Questionable approach =E2=80=94 adds latency to a common hot-plu= g path for all chipsets.** The change: ```c if (bits & NVIF_CONN_EVENT_V0_IRQ) { if (nouveau_dp_link_check(nv_connector)) continue; + /* Retry once after 100ms for transient DP glitches (NV50/G94) */ + msleep(100); + if (nouveau_dp_link_check(nv_connector)) + continue; } ``` **Issue 1: Global impact despite chipset-specific comment.** The comment sa= ys "NV50/G94" but `nouveau_display_hpd_work()` is the HPD handler for **all= ** nouveau-supported hardware. Every DP connector on every supported GPU wi= ll now incur a 100ms sleep on any HPD IRQ where the first link check fails.= This should either be gated on the chipset or the comment should acknowled= ge the broader scope. **Issue 2: msleep(100) in a shared workqueue.** While sleeping in a workque= ue is technically valid, this is `nouveau_display_hpd_work` which processes= HPD events for potentially multiple connectors in a loop (the `drm_connect= or_list_iter` at the top). A 100ms stall blocks processing of other connect= ors' HPD events in the same work item. If multiple connectors fire IRQs sim= ultaneously and all fail the first link check, the cumulative delay compoun= ds. **Issue 3: Treating the symptom, not the cause.** If the NVAC DP link is tr= ansiently failing after DPMS resume, the root cause may be that link traini= ng isn't waiting long enough or that the sink isn't ready. A retry loop at = the HPD IRQ level is a blunt instrument =E2=80=94 the DP link check functio= n itself or the link training code would be a more targeted place to add re= silience. **Issue 4: No Fixes: tag.** If this fixes a user-visible regression or bug,= it should include a `Fixes:` tag identifying when the behavior was introdu= ced. Same applies to patches 1 and 2. For stable kernel backporting, `Cc: s= table@vger.kernel.org` should be considered. --- Generated by Claude Code Patch Reviewer