Claude review: drm: Send per-connector hotplug events

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

Patch Review

**Verdict: Looks good, with minor observations.**

**Design approach — correct and clean:**

The key insight in v9 is decomposing `drm_kms_helper_hotplug_event()` (which calls `drm_sysfs_hotplug_event()` + `drm_client_dev_hotplug()`) into its two parts. The `drm_sysfs_connector_hotplug_event()` calls are safe to make inside the `mode_config.mutex` since they only do `kobject_uevent_env()`, while `drm_client_dev_hotplug()` is deferred to after the lock is released. This avoids the deadlock from earlier versions and avoids needing a temporary allocation for connector IDs.

**`output_poll_execute` changes:**

The poll path now calls `drm_sysfs_connector_hotplug_event(connector)` inline within the loop (under the mutex), and defers the `drm_client_dev_hotplug(dev)` call to after `mutex_unlock()`:

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

and later:

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

This is correct. The `delayed_event` handling has been moved from before the lock acquisition to after the lock release in the `out:` label. This is a subtle but important change:

- **Observation 1 (minor):** In the original code, `delayed_event` was consumed before the `goto out` on `!drm_kms_helper_poll`, meaning a delayed event would still fire `drm_kms_helper_hotplug_event()`. In the new code, the `delayed_event` check is at the `out:` label, so it's also reached from that path — behavior is preserved. Good.

- **Observation 2 (minor):** In the original code, `delayed_event` was consumed before the `mutex_trylock` failure path (`goto out` with `repoll = true`). In the new code, the same applies — if the trylock fails, we still check `delayed_event` at `out:`, which is correct and actually slightly better since we don't lose the delayed event on trylock failure. In the old code, a trylock failure after consuming `delayed_event` would fire the hotplug event. The new code does the same via the `out:` label path. Correct.

- **Observation 3 (race on `delayed_event`):** The `delayed_event` flag is read and cleared without holding the `mode_config.mutex` (it's in the `out:` block after the unlock). This is the same as the original code (which also read it before acquiring the lock), so it's not a regression. The flag is set under the poll work context which is single-threaded, and races with `drm_helper_probe_single_connector_modes` setting it are benign (worst case: an extra poll cycle). Fine.

**`drm_helper_hpd_irq_event` changes:**

The old code tracked `first_changed_connector` and a `changed` count, then dispatched either `drm_kms_helper_connector_hotplug_event()` for single-connector changes or `drm_kms_helper_hotplug_event()` for multi-connector changes. The new code simply calls `drm_sysfs_connector_hotplug_event()` for each changed connector within the loop, then calls `drm_client_dev_hotplug()` once after the lock is released:

```c
 		if (check_connector_changed(connector)) {
-			if (!first_changed_connector) {
-				drm_connector_get(connector);
-				first_changed_connector = connector;
-			}
-			changed++;
+			changed = true;
+			drm_sysfs_connector_hotplug_event(connector);
 		}
```

This is a nice simplification — the `first_changed_connector` / refcount dance is eliminated entirely. The `drm_sysfs_connector_hotplug_event()` call is safe under `mode_config.mutex` since it just does a `kobject_uevent_env()`.

- **Observation 4 (behavioral change in multi-connector case):** Previously, if 3 connectors changed simultaneously, userspace got 1 generic `HOTPLUG=1` uevent (without `CONNECTOR=`). Now it gets 3 separate `HOTPLUG=1 CONNECTOR=<id>` uevents plus a single `drm_client_dev_hotplug()`. This is intentional per the cover letter and is strictly better for userspace — more information, no loss of the old signal. Well-designed compositors already handle `CONNECTOR=` uevents. Older compositors that ignore `CONNECTOR=` still see `HOTPLUG=1` and re-probe everything, so backwards compatibility is maintained.

- **Observation 5 (multiple `drm_client_dev_hotplug` calls):** In both paths, `drm_client_dev_hotplug()` is called exactly once if anything changed, which is correct and avoids redundant fbdev/fbcon restores.

**No issues found.** The patch is a clean improvement that simplifies the code while providing better information to userspace. The locking split is correct — uevent delivery inside the lock, client notification outside.

---
Generated by Claude Code Patch Reviewer