Claude review: drm: Send per-connector hotplug events

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

Patch Review

**Verdict: Correct, with minor observations.**

This patch modifies two functions: `output_poll_execute()` and `drm_helper_hpd_irq_event()`.

#### `output_poll_execute()` changes

**`!drm_kms_helper_poll` path (polling disabled):**

```c
if (!drm_kms_helper_poll) {
    if (dev->mode_config.poll_running) {
        drm_kms_helper_disable_hpd(dev);
        dev->mode_config.poll_running = false;
    }

    scoped_guard(mutex, &dev->mode_config.mutex) {
        changed = dev->mode_config.delayed_event;
        dev->mode_config.delayed_event = false;
    }

    if (changed)
        drm_kms_helper_hotplug_event(dev);

    return;
}
```

This is the v10 fix. Previously, `delayed_event` was read/cleared *before* the `!drm_kms_helper_poll` check and outside any lock, racing with the writer in `drm_helper_probe_single_connector_modes()` (which sets it under `mode_config.mutex`, line 568/627 of the same file). The `scoped_guard` is the right fix.

Note: this path falls back to `drm_kms_helper_hotplug_event(dev)` (a generic, non-connector-specific event), because `delayed_event` is a bare `bool` that doesn't carry connector information. This is acceptable — the polling loop isn't running here so there's no connector to identify, and a generic hotplug is better than none.

**`mutex_trylock` failure path:**

```c
if (!mutex_trylock(&dev->mode_config.mutex)) {
    schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
    return;
}
```

Old code set `repoll = true` and fell through to the `out:` label. New code reschedules directly and returns. Functionally equivalent, and cleaner since it removes the `goto out` pattern.

**Connector iteration loop:**

```c
drm_sysfs_connector_hotplug_event(connector);
changed = true;
```

`drm_sysfs_connector_hotplug_event()` calls `kobject_uevent_env()` while `mode_config.mutex` is held. This is intentional — the comment at line 621-625 explains the deadlock concern is about "fb helpers" (i.e., `drm_client_dev_hotplug`), not about the sysfs uevent itself. `kobject_uevent_env` does a netlink broadcast which could be slow under memory pressure, but that's a latency concern, not a correctness issue.

**`delayed_event` handling after the loop:**

```c
if (dev->mode_config.delayed_event) {
    dev->mode_config.delayed_event = false;
    changed = true;
    drm_sysfs_hotplug_event(dev);
}
```

This picks up probes that happened between poll cycles (e.g., userspace snooped via `drm_helper_probe_single_connector_modes`). It sends a generic (non-connector-specific) uevent because the `delayed_event` bool doesn't identify which connector. If the poll loop already sent a connector-specific uevent for the same connector, userspace sees both — slightly redundant but harmless.

**After mutex release:**

```c
if (changed)
    drm_client_dev_hotplug(dev);
```

Only `drm_client_dev_hotplug()` runs outside the lock (it takes `clientlist_mutex` and calls into fb helpers). This is the correct split that avoids the deadlock. The uevent was already sent inside the lock.

**Minor observation:** If multiple connectors changed, `drm_client_dev_hotplug(dev)` is still called only once at the end rather than per-connector. This is fine — the function doesn't take a connector argument and just notifies all registered DRM clients.

#### `drm_helper_hpd_irq_event()` changes

```c
if (check_connector_changed(connector)) {
    changed = true;
    drm_sysfs_connector_hotplug_event(connector);
}
```

The old code tracked `first_changed_connector` with a refcount get/put and branched on `changed == 1` vs `changed > 0` to decide between connector-specific and generic hotplug. The new code is much simpler: send a connector-specific uevent for *every* changed connector inside the loop. The `drm_connector_get`/`drm_connector_put` dance is gone since we no longer need to stash a connector reference across the lock boundary.

```c
if (changed)
    drm_client_dev_hotplug(dev);
```

Again, client hotplug is deferred until after `mode_config.mutex` is released. Correct.

**Return type change:** `int changed` → `bool changed`. The function returns `bool`, so this is a cleanup. Note that `first_changed_connector` is gone, which also removes the intermediate `drm_connector_get()` ref — no leak risk.

#### Overall assessment

No correctness bugs found. The locking discipline is sound. The split of "sysfs uevent under lock / client hotplug after lock" is well-justified by the existing deadlock comment and the fb helper lock ordering. The `!drm_kms_helper_poll` path fix in v10 (reading `delayed_event` under the mutex) addresses a real race. The code is simpler than what it replaces.

---
Generated by Claude Code Patch Reviewer