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

[Claude Code Review Bot <claude-review@example.com>]

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