[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

[PATCH v3] 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)

v3: Correct if condition

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..1e15cb42d50b 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_FORCE_OFF) {
+		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-27T13:57:30.178462

---

This is a single well-motivated patch that fixes a real behavioral gap in the DRM probe helper infrastructure. The problem is clear: `drm_helper_probe_single_connector_modes()` (line 595) already respects `connector->force` and skips hardware detection when a connector is forced, but `drm_helper_probe_detect()` does not. Callers that go through `drm_helper_probe_detect()` directly — most notably i915's `intel_hotplug_detect_connector()` — perform unnecessary hardware probes (AUX/DPCD reads, DDC transactions) on connectors the user has explicitly forced off via `video=`.

The fix is minimal, correct in scope, and well-placed. The decision to only short-circuit `DRM_FORCE_OFF` (and not `DRM_FORCE_ON` / `DRM_FORCE_ON_DIGITAL`) is well-reasoned and explicitly documented in the commit message.

**Recommendation: Looks good to merge.**

---
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

**Correctness: Good**

The early return at line 404-408 is placed before both code paths (`!ctx` and the locked path), so it covers all callers uniformly. The return value `connector_status_disconnected` is consistent with what `drm_helper_probe_single_connector_modes()` sets at line 600 for `DRM_FORCE_OFF`.

**Epoch counter behavior: Acceptable**

The early return bypasses the `epoch_counter` increment at line 419-420. This is fine because:
- For the initial boot probe, `connector->status` starts as `connector_status_unknown`, so the first call returning `connector_status_disconnected` would trigger an epoch bump in the `!ctx` path (via `drm_helper_probe_detect_ctx` at line 377-378). However, with this patch the early return skips that bump. In practice, this is a non-issue: the polling path at line 795 already skips forced connectors entirely (`if (connector->force) continue`), and `drm_helper_probe_single_connector_modes()` at line 595 handles forced connectors before calling `drm_helper_probe_detect()`.
- The i915 caller at `intel_hotplug.c:322` checks `old_epoch_counter != connector->base.epoch_counter` at line 326, but since the connector is forced off, no epoch change is expected and `INTEL_HOTPLUG_UNCHANGED` will be returned, which is the correct result.

**One minor observation on callers:**

The `drm_helper_hpd_irq_event()` caller at line 994 does `connector->status = drm_helper_probe_detect(...)` unconditionally (no `!connector->force` guard like i915 has at line 323). With this patch, a forced-off connector receiving an HPD IRQ would now get `connector_status_disconnected` assigned directly, which is the correct status for a forced-off connector. Previously it would have run the full detect and potentially set `connector_status_connected`, which would then be overridden later by `drm_helper_probe_single_connector_modes()`. So this is actually a slight improvement for that path too.

**Style: Clean**

The debug message format `[CONNECTOR:%d:%s] forced off, skipping detect` follows the existing DRM debug conventions in the file.

**Commit message: Well-written**

Clear problem description, explicit rationale for why `DRM_FORCE_ON`/`DRM_FORCE_ON_DIGITAL` are not short-circuited, and includes test evidence. The version history is helpful.

**Minor nit (non-blocking):** The commit message has a missing closing backtick on the test command line:
```
With `video=DP-2:d drm.debug=0x04, Linux logs:
```
should be:
```
With `video=DP-2:d drm.debug=0x04`, Linux logs:
```

**Verdict: Reviewed-by worthy.** The patch is correct, minimal, and well-explained.

---
Generated by Claude Code Patch Reviewer

[PATCH] 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 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.

This is 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
    [   11.701245] [drm:drm_connector_init_only [drm]] cmdline mode for connector DP-2  0x0@60Hz
    […]
    [   12.102009] i915 0000:00:02.0: [drm:drm_helper_probe_detect [drm_kms_helper]] [CONNECTOR:130:DP-2] force=1, skipping detect

Assisted-by: Claude Sonnet 4.6
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/gpu/drm/drm_probe_helper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index d4dc8cb45bce..bc513f24bc28 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -401,6 +401,17 @@ 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] force=%d, skipping detect\n",
+			     connector->base.id, connector->name,
+			     connector->force);
+		if (connector->force == DRM_FORCE_ON ||
+		    connector->force == DRM_FORCE_ON_DIGITAL)
+			return connector_status_connected;
+		else
+			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: 2
Reviewed: 2026-05-27T14:37:43.812300

---

This is a single-patch fix for a real and well-identified issue: `video=<connector>:d` on the kernel command line correctly sets `connector->force` at init time, but `drm_helper_probe_detect()` has no guard for it. This means callers like i915's `intel_hotplug_detect_connector()` (which goes through `check_connector_changed()` → `drm_helper_probe_detect()`) still run a full hardware probe on forced-off connectors.

The fix is the right approach — it mirrors the existing pattern in `drm_helper_probe_single_connector_modes()` (line 589) and the existing `connector->force` skip in `output_poll_execute()` (line 789). The commit message is well-written with good motivation and test evidence.

There are a couple of concerns worth raising.

---
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

**Correctness — epoch counter not updated on first call (minor issue)**

When `drm_helper_probe_detect()` is called via the `ctx != NULL` path, the existing code bumps `epoch_counter` when the returned status differs from `connector->status`:

```c
if (ret != connector->status)
    connector->epoch_counter += 1;
```

The new early return skips this entirely. On the very first call (when `connector->status` is `connector_status_unknown` and force is `DRM_FORCE_OFF`), the returned `connector_status_disconnected` differs from the stored status but the epoch counter is not bumped. This means callers like `check_connector_changed()` (line 988) won't see the status transition:

```c
connector->status = drm_helper_probe_detect(connector, NULL, false);
if (old_epoch_counter == connector->epoch_counter) {
    ...
    return false;
}
```

In practice this is likely benign — the initial status update for forced connectors typically happens through `drm_helper_probe_single_connector_modes()` which sets `connector->status` directly (line 592-594) before ever reaching this path. But it would be more robust to update the epoch counter in the early-return path too, matching the existing pattern. Consider:

```c
if (connector->force) {
    int status;

    drm_dbg_kms(dev, "[CONNECTOR:%d:%s] force=%d, skipping detect\n",
                connector->base.id, connector->name,
                connector->force);
    if (connector->force == DRM_FORCE_ON ||
        connector->force == DRM_FORCE_ON_DIGITAL)
        status = connector_status_connected;
    else
        status = connector_status_disconnected;
    if (status != connector->status)
        connector->epoch_counter += 1;
    return status;
}
```

**The `ctx == NULL` path — `drm_helper_probe_detect_ctx()` also has epoch counter logic**

When `ctx` is NULL, the code falls through to `drm_helper_probe_detect_ctx()` which has its own `epoch_counter` bump at line 377-378. The new early return is placed *before* the `ctx` dispatch, so it correctly covers both paths. Good.

**Style nit — `else` after `return`**

The `else` on line 108 is unnecessary since the `if` branch returns:

```c
if (connector->force == DRM_FORCE_ON ||
    connector->force == DRM_FORCE_ON_DIGITAL)
    return connector_status_connected;
else
    return connector_status_disconnected;
```

This does match the pattern used in `drm_helper_probe_single_connector_modes()` (line 590-594) which also uses if/else, so it's consistent with existing code style. Not a blocker, but it could be simplified to just:

```c
if (connector->force == DRM_FORCE_ON ||
    connector->force == DRM_FORCE_ON_DIGITAL)
    return connector_status_connected;
return connector_status_disconnected;
```

**Debug message format — `force=%d`**

Printing the raw enum value (`force=%d`) is functional but not immediately readable in logs. The log output `force=1` means `DRM_FORCE_OFF` (since `UNSPECIFIED=0, OFF=1, ON=2, ON_DIGITAL=3`). This is fine for developers who know the enum, but a human-readable string would be friendlier. Minor nit, not a blocker.

**Overall assessment**: The patch is correct and addresses a real issue. The epoch counter concern is the only substantive point — it should either be handled or explicitly called out as intentionally skipped. With that addressed, this is a clean, well-motivated fix.

---
Generated by Claude Code Patch Reviewer