[PATCH] drm/i915/audio: skip ELD query for force-off connectors

[PATCH] drm/i915/audio: skip ELD query for force-off connectors

[Paul Menzel]

When a connector is forced off via the `video=` command-line parameter
(`video=<connector>:d`), `intel_audio_component_get_eld()` returns -EINVAL
for that port because `find_audio_state()` finds no active encoder.  The
audio driver interprets this as an error rather than a clean “no monitor
present” reply, and, on the Dell XPS 13 9360, at `drm.debug=0x04` the
repeated calls generate the log below:

    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port B
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port B
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port B
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port C
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port C
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port C
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port D
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port D
    i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port D

Add `intel_audio_connector_is_forced_off()` which scans the connector list
to check whether any connector on the given port has
`force == DRM_FORCE_OFF`. When the caller finds no audio state and the
port’s connector is forced off, report `*enabled = false` and return 0 so
the audio driver sees a clean “not connected” answer and stops querying
that port.

With `video=DP-2:d drm.debug=0x04` the unneeded calls are gone (port C):

    [   90.458772] i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port B
    [   90.459649] i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port B
    [   90.460547] i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port B
    [   90.461581] i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port D
    [   90.462509] i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port D
    [   90.463385] i915 0000:00:02.0: [drm:intel_audio_component_get_eld [i915]] Not valid for port D

Assisted-by: Claude Sonnet 4.6
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 29 +++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 081627e0d917..7864b9a485d5 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -36,6 +36,7 @@
 #include "intel_cdclk.h"
 #include "intel_crtc.h"
 #include "intel_de.h"
+#include "intel_display.h"
 #include "intel_display_types.h"
 #include "intel_display_wa.h"
 #include "intel_lpe_audio.h"
@@ -1211,6 +1212,28 @@ static int intel_audio_component_sync_audio_rate(struct device *kdev, int port,
 	return err;
 }
 
+static bool intel_audio_connector_is_forced_off(struct intel_display *display,
+						int port)
+{
+	struct drm_connector_list_iter conn_iter;
+	struct intel_connector *connector;
+	bool forced_off = false;
+
+	drm_connector_list_iter_begin(display->drm, &conn_iter);
+	for_each_intel_connector_iter(connector, &conn_iter) {
+		struct intel_encoder *encoder = intel_attached_encoder(connector);
+
+		if (encoder && encoder->port == port &&
+		    connector->base.force == DRM_FORCE_OFF) {
+			forced_off = true;
+			break;
+		}
+	}
+	drm_connector_list_iter_end(&conn_iter);
+
+	return forced_off;
+}
+
 static int intel_audio_component_get_eld(struct device *kdev, int port,
 					 int cpu_transcoder, bool *enabled,
 					 unsigned char *buf, int max_bytes)
@@ -1223,9 +1246,13 @@ static int intel_audio_component_get_eld(struct device *kdev, int port,
 
 	audio_state = find_audio_state(display, port, cpu_transcoder);
 	if (!audio_state) {
+		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));
-		mutex_unlock(&display->audio.mutex);
 		return -EINVAL;
 	}
 
-- 
2.53.0

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

[Claude Code Review Bot]

Overall Series Review

Subject: drm/i915/audio: skip ELD query for force-off connectors
Author: Paul Menzel <pmenzel@molgen.mpg.de>
Patches: 1
Reviewed: 2026-05-27T14:32:39.446168

---

This is a single patch that addresses a real annoyance: when a connector is forced off via `video=<connector>:d`, the HDA audio driver repeatedly calls `get_eld()` for that port, getting `-EINVAL` each time, which produces noisy debug logs. The fix adds a helper that checks if any connector on the port is forced off, and returns a clean "not connected" (`*enabled = false`, return 0) instead of an error.

The approach is reasonable and the commit message is well-written with clear before/after log evidence. However, there is a **locking concern** and a **semantic question** worth discussing.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

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