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

[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

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

[Jani Nikula]

On Tue, 26 May 2026, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> 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.

Note that drm_helper_probe_single_connector_modes() still calls the
->force hook, which is crucial.

Returning early for force off may be fine, but force on will still need
to try to read the EDID. And the EDID reads return early for force off.

Side note, please drop the excessive backticks and the ellipsis from
commit messages.

BR,
Jani.

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

-- 
Jani Nikula, Intel

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