[PATCH v2] drm/probe-helper: honour connector->force in drm_helper_probe_detect()

[PATCH v2] drm/probe-helper: honour connector->force in drm_helper_probe_detect()

[Paul Menzel]

With `video=DP-2:d drm.debug=0x04` the connectors are correctly forced off
at init time:

    [9.742468] [drm] forcing DP-2 connector off

but intel_dp_detect() is still called immediately after:

    [9.908738] [drm:intel_dp_detect] [CONNECTOR:130:DP-2]
    [9.912982] [drm:intel_hotplug_detect_connector] [CONNECTOR:130:DP-2] status updated from unknown to unknown (epoch counter 0->1)

drm_helper_probe_single_connector_modes() already short-circuits when
connector->force is set, returning the forced status without calling any
detect callbacks.  drm_helper_probe_detect() however has no such guard, so
callers that go through it directly – such as i915’s
intel_hotplug_detect_connector() – still run the full hardware probe (AUX
DPCD read, GMBus DDC transaction, ...) even when the user has forced a
connector off via the `video=` command-line parameter.

Add the same connector->force check for forced off connectors at the top of
drm_helper_probe_detect(), so that the hardware probe is skipped regardless
of the entry point.  This makes video=<connector>:d effective at boot time,
not just for userspace GETCONNECTOR ioctls.

DRM_FORCE_ON and DRM_FORCE_ON_DIGITAL are intentionally not short-
circuited here: for those cases the detect callback must still run to
initialise sink capabilities such as DPCD link rate and lane count.

Tested on a Dell XPS 13 9360 (Intel Kaby Lake-U GT2 [HD Graphics 620]
[8086:5916]). With `video=DP-2:d drm.debug=0x04, Linux logs:

    [   11.701243] [drm] forcing DP-2 connector off
    [...]
    [   12.102009] i915 0000:00:02.0: [drm:drm_helper_probe_detect [drm_kms_helper]] [CONNECTOR:130:DP-2] forced off, skipping detect

v2: Do not return early for DRM_FORCE_ON and DRM_FORCE_ON_DIGITAL (Jani,
gemini/gemini-3.1-pro-preview)

Assisted-by: Claude Sonnet 4.6
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---


 drivers/gpu/drm/drm_probe_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index d4dc8cb45bce..b657ae109f98 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -401,6 +401,12 @@ drm_helper_probe_detect(struct drm_connector *connector,
 	struct drm_device *dev = connector->dev;
 	int ret;
 
+	if (connector->force) {
+		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] forced off, skipping detect\n",
+			    connector->base.id, connector->name);
+		return connector_status_disconnected;
+	}
+
 	if (!ctx)
 		return drm_helper_probe_detect_ctx(connector, force);
 
-- 
2.53.0

Claude review: drm/probe-helper: honour connector->force in drm_helper_probe_detect()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/probe-helper: honour connector->force in drm_helper_probe_detect()
Author: Paul Menzel <pmenzel@molgen.mpg.de>
Patches: 1
Reviewed: 2026-05-27T14:18:39.983377

---

This is a single patch (v2) that adds a short-circuit to `drm_helper_probe_detect()` for connectors forced off via the `video=` kernel command-line parameter. The motivation is sound — there's a real asymmetry where `drm_helper_probe_single_connector_modes()` (line 589) already skips detection for forced connectors, but `drm_helper_probe_detect()` does not, causing unnecessary hardware probes (AUX DPCD reads, etc.) for connectors the user explicitly disabled.

The patch has one significant correctness bug: the condition `if (connector->force)` is true for **all** non-`DRM_FORCE_UNSPECIFIED` values including `DRM_FORCE_ON` and `DRM_FORCE_ON_DIGITAL`, despite the commit message explicitly claiming those are "intentionally not short-circuited." This directly contradicts the v2 changelog note addressing Jani's feedback.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/probe-helper: honour connector->force in drm_helper_probe_detect()

[Claude Code Review Bot]

Patch Review

**Bug: condition does not match the stated intent (critical)**

The patch adds:
```c
if (connector->force) {
    drm_dbg_kms(dev, "[CONNECTOR:%d:%s] forced off, skipping detect\n",
                connector->base.id, connector->name);
    return connector_status_disconnected;
}
```

`connector->force` is an `enum drm_connector_force` with values:
- `DRM_FORCE_UNSPECIFIED` = 0
- `DRM_FORCE_OFF` = 1
- `DRM_FORCE_ON` = 2
- `DRM_FORCE_ON_DIGITAL` = 3

Since C treats any non-zero value as truthy, `if (connector->force)` matches `DRM_FORCE_OFF`, `DRM_FORCE_ON`, **and** `DRM_FORCE_ON_DIGITAL` — all three return `connector_status_disconnected`. This directly contradicts the commit message which says "DRM_FORCE_ON and DRM_FORCE_ON_DIGITAL are intentionally not short-circuited here" and the v2 changelog "Do not return early for DRM_FORCE_ON and DRM_FORCE_ON_DIGITAL."

The fix should be:
```c
if (connector->force == DRM_FORCE_OFF) {
```

**Epoch counter not incremented**

When `drm_helper_probe_detect()` returns normally (no early return), lines 413-414 handle bumping the epoch counter:
```c
if (ret != connector->status)
    connector->epoch_counter += 1;
```

The new early return at the top bypasses this. For callers like `check_connector_changed()` (line 988) and the poll work (line 807) that compare epoch counters to detect changes, the early return means a forced-off connector whose status was previously `connector_status_unknown` (initial state) will return `connector_status_disconnected` but never bump the epoch counter. The caller will see no change and may not fire the hotplug event to inform userspace.

The i915 caller `intel_hotplug_detect_connector()` (line 322) similarly uses the epoch counter. In practice this may be fine since the status will stabilize quickly, but it's a behavioral inconsistency worth noting.

**The `ctx == NULL` path already has its own issues**

When `ctx` is NULL, the function delegates to `drm_helper_probe_detect_ctx()` which also lacks any force check. The new guard covers both paths, which is correct.

**i915 already partially handles this**

Worth noting: the i915 caller at `intel_hotplug.c:323` already has `if (!connector->base.force) connector->base.status = status;` — so it avoids overwriting the status for forced connectors. The patch eliminates the unnecessary hardware probe itself, which is the right thing to do, but the existing guard means the practical impact on i915 was already limited to wasted I/O (AUX/DDC transactions), not incorrect status reporting.

**Debug message is misleading for the buggy condition**

The debug message says "forced off, skipping detect" but with the current condition it would also print for `DRM_FORCE_ON` connectors, which would be confusing. This becomes moot once the condition is fixed to `== DRM_FORCE_OFF`.

**Summary:** Good idea, real problem being solved, but the patch as written has the condition inverted from its stated intent — it will break `DRM_FORCE_ON` and `DRM_FORCE_ON_DIGITAL` connectors by returning disconnected for them. The fix is straightforward: change `if (connector->force)` to `if (connector->force == DRM_FORCE_OFF)`.

---
Generated by Claude Code Patch Reviewer