* [PATCH] drm/probe-helper: signal hotplug for force-enabled connectors on HPD
@ 2026-03-28 15:25 Miguel Sedek
2026-03-29 12:40 ` Miguel Sedek
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Miguel Sedek @ 2026-03-28 15:25 UTC (permalink / raw)
To: dri-devel; +Cc: maarten.lankhorst, mripard, tzimmermann, Miguel Sedek
When a connector has DRM_FORCE_ON (set via video=CONNECTOR:e),
check_connector_changed() calls the driver's detect() callback.
Well-behaved drivers (e.g. NVIDIA's open kernel module) check
connector->force and return connector_status_connected regardless
of actual hardware state. This means the epoch counter never
increments and no uevent reaches userspace.
However, the HPD interrupt itself indicates a physical state
change on the link. A common scenario is an HDMI-connected TV
switching to built-in apps (Netflix, etc.), which causes the TV's
HDMI receiver to stop listening to TMDS signals. When the TV
returns to the HDMI input, the GPU needs to re-negotiate the link,
but no component in the stack detects this because the forced
status masks the physical change.
Add an epoch counter increment for force-enabled connectors after
detect() returns. This ensures the hotplug uevent reaches
userspace compositors so they can trigger a modeset for link
re-training.
Note that detect() is still called normally before the epoch
bump. This preserves driver-side effects like scrambler
re-initialization on vc4/i915. This approach differs from a
prior patch [1] which skipped detect() entirely for forced
connectors and was rejected due to regression concerns on
scrambler re-init.
The software polling path (output_poll_execute) already skips
forced connectors with an explicit guard. This patch addresses
only the hardware HPD IRQ path, which was previously unguarded.
[1] https://patchwork.kernel.org/project/dri-devel/patch/20220826091121.389315-1-mrodin@de.adit-jv.com/
Tested with:
- GPU: NVIDIA RTX 5070 Ti (nvidia-open 595.58.03)
- Display: Samsung 75" 4K TV via HDMI-A-1 (force-enabled)
- Kernel: 6.19.10 (CachyOS)
- Compositor: KWin 6.6.3 (Wayland)
Signed-off-by: Miguel Sedek <miguelsedek@gmail.com>
---
drm_probe_helper.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drm_probe_helper.c b/drm_probe_helper.c
index 92f2176..b7d60e2 100644
--- a/drm_probe_helper.c
+++ b/drm_probe_helper.c
@@ -984,6 +984,23 @@ static bool check_connector_changed(struct drm_connector *connector)
old_epoch_counter = connector->epoch_counter;
connector->status = drm_helper_probe_detect(connector, NULL, false);
+ /*
+ * For force-enabled connectors, the driver's detect() returns the
+ * forced status, so the epoch counter never increments. But an HPD
+ * event on a forced connector still indicates a physical link state
+ * change (e.g. a TV switching inputs). Signal the change to
+ * userspace so the compositor can trigger a modeset for link
+ * re-training. Note: detect() is still called above so driver-side
+ * effects (scrambler re-init, etc.) are preserved.
+ */
+ if (connector->force && old_epoch_counter == connector->epoch_counter) {
+ connector->epoch_counter += 1;
+ drm_dbg_kms(dev,
+ "[CONNECTOR:%d:%s] HPD on force-enabled connector, signaling link re-train\n",
+ connector->base.id, connector->name);
+ return true;
+ }
+
if (old_epoch_counter == connector->epoch_counter) {
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Same epoch counter %llu\n",
connector->base.id,
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/probe-helper: signal hotplug for force-enabled connectors on HPD
2026-03-28 15:25 [PATCH] drm/probe-helper: signal hotplug for force-enabled connectors on HPD Miguel Sedek
@ 2026-03-29 12:40 ` Miguel Sedek
2026-03-31 7:46 ` Claude review: " Claude Code Review Bot
2026-03-31 7:46 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Miguel Sedek @ 2026-03-29 12:40 UTC (permalink / raw)
To: dri-devel; +Cc: maarten.lankhorst, mripard, tzimmermann, Miguel Sedek
Withdrawing this patch. The root cause turned out to be a userspace
script running SimulateUserActivity in a loop, which prevented DPMS
from ever cycling. With DPMS working correctly, the HDMI link
re-negotiates on wake without any kernel changes needed.
Apologies for the noise.
Miguel Sedek
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: drm/probe-helper: signal hotplug for force-enabled connectors on HPD
2026-03-28 15:25 [PATCH] drm/probe-helper: signal hotplug for force-enabled connectors on HPD Miguel Sedek
2026-03-29 12:40 ` Miguel Sedek
@ 2026-03-31 7:46 ` Claude Code Review Bot
2026-03-31 7:46 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-31 7:46 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**1. Malformed diff path**
The diff header shows:
```
diff --git a/drm_probe_helper.c b/drm_probe_helper.c
```
This should be `a/drivers/gpu/drm/drm_probe_helper.c`. This means `git am` will fail to apply the patch. The author likely ran `git diff` from inside `drivers/gpu/drm/` rather than from the repository root.
**2. The premise is driver-specific, not universal**
The commit message states:
> Well-behaved drivers (e.g. NVIDIA's open kernel module) check connector->force and return connector_status_connected regardless of actual hardware state.
But this is not a universal DRM convention. The DRM core's own force handling lives in `drm_helper_probe_single_connector_modes()` (the full probe path at `drm_probe_helper.c:589-606`), which explicitly checks `connector->force` and overrides the status. The detect-only path called by `check_connector_changed()` does **not** check `connector->force` — it goes straight to the driver's `detect()` callback via `detect_connector_status()`. Whether a driver respects `connector->force` in its own detect callback is driver-specific behavior, not a DRM convention. Many drivers will return the actual physical status from detect(), meaning the epoch counter **would** change and this patch's code path would never trigger.
This makes the patch's effect inconsistent across drivers — it only fires when the driver happens to check `connector->force` in its detect callback.
**3. Unconditional epoch bump on every HPD is too aggressive**
```c
if (connector->force && old_epoch_counter == connector->epoch_counter) {
connector->epoch_counter += 1;
...
return true;
}
```
Every HPD event on a forced connector will now trigger a uevent and potentially a full modeset. HPD lines can be noisy — spurious events, signal settling, etc. This could cause unnecessary modesetting storms. There's no debouncing or consideration of whether the link actually needs re-training. The commit message's scenario (TV switching inputs) is valid, but the solution is a blanket that catches all HPD events.
**4. Early return skips existing debug logging**
The patch returns `true` before reaching the existing code at line 999 that logs the epoch counter change:
```c
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Changed epoch counter %llu => %llu\n", ...);
```
While the patch adds its own debug print, it uses different format/wording, making debug log analysis inconsistent between the forced and non-forced paths.
**5. Alternative approach to consider**
Since this is really about the nvidia-open driver's `detect()` honoring `connector->force`, it might be more appropriate to either:
- Fix the nvidia driver to not blindly return forced status from its `detect()` callback (the DRM core handles forcing elsewhere), or
- Have `detect_connector_status()` itself understand forced connectors if this is truly meant to be a core behavior.
Modifying epoch counters outside of `drm_helper_probe_detect()` creates a second code path for epoch management, which is fragile.
**6. Minor: the fix should probably also apply to `drm_helper_hpd_irq_event()`**
The commit message says "This patch addresses only the hardware HPD IRQ path," but `check_connector_changed()` is used by the per-connector `drm_connector_helper_hpd_irq_event()`. The multi-connector `drm_helper_hpd_irq_event()` (line 807) has its own inline detect+epoch logic and would have the same issue but is not addressed.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread* Claude review: drm/probe-helper: signal hotplug for force-enabled connectors on HPD
2026-03-28 15:25 [PATCH] drm/probe-helper: signal hotplug for force-enabled connectors on HPD Miguel Sedek
2026-03-29 12:40 ` Miguel Sedek
2026-03-31 7:46 ` Claude review: " Claude Code Review Bot
@ 2026-03-31 7:46 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-31 7:46 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/probe-helper: signal hotplug for force-enabled connectors on HPD
Author: Miguel Sedek <miguelsedek@gmail.com>
Patches: 2
Reviewed: 2026-03-31T17:46:44.640883
---
This is a single patch that forces an epoch counter bump for force-enabled connectors on the HPD IRQ path, so that userspace receives a uevent even though the connector status doesn't change. The problem is real — a forced connector can mask physical link state changes — but the patch has several issues with the approach and patch formatting that need addressing.
The diff header is malformed (missing `drivers/gpu/drm/` prefix), the fix is arguably too broad (every HPD on any forced connector will trigger a modeset, including spurious events), and the premise relies on driver-specific behavior that isn't universal.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-31 7:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-28 15:25 [PATCH] drm/probe-helper: signal hotplug for force-enabled connectors on HPD Miguel Sedek
2026-03-29 12:40 ` Miguel Sedek
2026-03-31 7:46 ` Claude review: " Claude Code Review Bot
2026-03-31 7:46 ` 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