Claude review: drm/i915/audio: skip ELD query for force-off connectors

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

Patch Review

**Locking issue — `intel_audio_connector_is_forced_off()` called without the audio mutex, but that's fine for this data; however, `intel_attached_encoder()` access needs consideration:**

The patch moves the `mutex_unlock(&display->audio.mutex)` *before* calling `intel_audio_connector_is_forced_off()`. The function iterates the connector list (safe via `drm_connector_list_iter`) and reads `connector->encoder` (via `intel_attached_encoder`) and `connector->base.force`. The `force` field is set at boot from the video= parameter and is effectively immutable after that, so reading it locklessly is fine. The `connector->encoder` pointer is similarly stable for forced-off connectors (they won't get hotplugged). So this is probably safe in practice, though a comment noting why no lock is needed would help a future reader.

**MST edge case:**

With MST (DisplayPort Multi-Stream), a single physical port can have multiple connectors. The new function returns `true` if *any* connector on the port is forced off. If a user forces off one MST connector but another MST connector on the same port is active, this would incorrectly suppress the ELD query for the entire port. This is unlikely in practice (users don't typically force off individual MST connectors), but worth considering whether the check should require *all* connectors on the port to be forced off, not just one.

**Unnecessary `#include` addition:**

The diff adds `#include "intel_display.h"` at line 39, but `intel_display.h` is already included at line 39 in the patched file (the original file already has it). Looking at the diff context more carefully — the diff shows adding `intel_display.h` between `intel_de.h` and `intel_display_types.h`. Checking the actual file confirms `intel_display.h` was **already present** before the patch. This include addition is redundant and should be dropped.

**Semantic question — should the return be 0 or -EINVAL?**

The patch changes the error path to return 0 with `*enabled = false` for forced-off connectors. This is a reasonable choice that tells the audio driver "nothing is connected, stop asking." But the existing callers of `get_eld` in the HDA driver might distinguish between "not connected" (0 + disabled) and "error" (-EINVAL) in meaningful ways. The commit message explains this is desired behavior, and it does match what happens for physically disconnected ports with no active encoder, so the semantics are correct.

**Minor: the debug message is now printed without the audio mutex held:**

```c
		mutex_unlock(&display->audio.mutex);
		if (intel_audio_connector_is_forced_off(display, port)) {
			*enabled = false;
			return 0;
		}
		drm_dbg_kms(display->drm, "Not valid for port %c\n",
			    port_name(port));
		return -EINVAL;
```

The `drm_dbg_kms` call was previously inside the mutex region (before the original `mutex_unlock`). Moving it outside the mutex is harmless — the debug print doesn't access any protected state — but it's worth noting as a side-effect of this refactoring.

**Summary:**
- The redundant `#include "intel_display.h"` should be dropped.
- The MST edge case (one forced-off connector vs. all forced-off on a port) deserves a mention in the commit message or a comment, even if it's not a practical concern.
- The locking is safe but subtle — worth a brief inline comment.
- Otherwise the patch is correct and addresses a real issue cleanly.

---
Generated by Claude Code Patch Reviewer