[PATCH v2] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions

[PATCH v2] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions

[Erick Karanja]

Replace calls to the deprecated drm_detect_hdmi_monitor() and
drm_detect_monitor_audio() functions. Calling drm_edid_connector_update()
updates connector->display_info.is_hdmi and connector->display_info.has_audio.

Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
Changes since v1:
	- Get rid of sink_is_hdmi and sink_has_audio as a
	  later call to drm_edid_connector_update() updates
	  connector display info.

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 3b77e73ac0ea..ba88120de1b8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2472,7 +2472,6 @@ static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
 						struct drm_connector *connector)
 {
 	const struct drm_edid *drm_edid;
-	const struct edid *edid;
 
 	if (!hdmi->ddc)
 		return NULL;
@@ -2483,19 +2482,6 @@ static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
 		return NULL;
 	}
 
-	/*
-	 * FIXME: This should use connector->display_info.is_hdmi and
-	 * connector->display_info.has_audio from a path that has read the EDID
-	 * and called drm_edid_connector_update().
-	 */
-	edid = drm_edid_raw(drm_edid);
-
-	dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
-		edid->width_cm, edid->height_cm);
-
-	hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
-	hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
-
 	return drm_edid;
 }
 
-- 
2.51.0

Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
Author: Erick Karanja <karanja99erick@gmail.com>
Patches: 1
Reviewed: 2026-02-27T12:14:28.991376

---

This is a single patch (v2) that attempts to remove deprecated `drm_detect_hdmi_monitor()` and `drm_detect_monitor_audio()` calls from the dw-hdmi bridge driver. The idea is correct in principle — `drm_edid_connector_update()` is already called after `dw_hdmi_edid_read()` returns (at line 2508), and it populates `connector->display_info.is_hdmi` and `connector->display_info.has_audio`. However, **the patch is incomplete and will cause a functional regression**: it removes the assignments to `hdmi->sink_is_hdmi` and `hdmi->sink_has_audio` but does **not** update the multiple consumers of those fields to use `connector->display_info` instead.

**NAK as-is.** The remaining usages will read zero-initialized `false` values, meaning:
- HDMI sinks will always be treated as DVI (no infoframes, no limited range RGB)
- Audio will never be enabled
- The struct fields themselves become dead code

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions

[Claude Code Review Bot]

Patch Review

**Subject line is misleading.** The commit message says "Replace calls to deprecated...functions", but the patch only *removes* the old calls — it does not replace anything. The consumers are left reading stale (always-false) fields.

**Critical: Remaining consumers not updated.** After this patch, the following code still references the now-never-assigned fields:

1. `dw-hdmi.c:2061` — `hdmi_av_composer()`:
   ```c
   inv_val |= hdmi->sink_is_hdmi ?
       HDMI_FC_INVIDCONF_DVI_MODEZ_HDMI_MODE :
       HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE;
   ```
   Will always select DVI mode.

2. `dw-hdmi.c:2294` — `dw_hdmi_setup()`:
   ```c
   hdmi->hdmi_data.rgb_limited_range = hdmi->sink_is_hdmi &&
       drm_default_rgb_quant_range(mode) ==
       HDMI_QUANTIZATION_RANGE_LIMITED;
   ```
   Will always set `rgb_limited_range = false`.

3. `dw-hdmi.c:2316` — `dw_hdmi_setup()`:
   ```c
   if (hdmi->sink_has_audio) {
   ```
   Audio will never be configured.

4. `dw-hdmi.c:2325` — `dw_hdmi_setup()`:
   ```c
   if (hdmi->sink_is_hdmi) {
   ```
   AVI InfoFrame, vendor-specific infoframe, and DRM infoframe configuration will be skipped entirely.

**What a correct patch would need to do:**
- Remove the `sink_is_hdmi` and `sink_has_audio` fields from `struct dw_hdmi` (lines 161-162).
- Change all four consumer sites to read from `connector->display_info.is_hdmi` and `connector->display_info.has_audio` instead. Note that `dw_hdmi_setup()` and `hdmi_av_composer()` already receive a `const struct drm_connector *connector` parameter (or `const struct drm_display_info *display`), so the information is accessible — it just needs to be wired up.

**Additional note on the bridge path:** When dw-hdmi operates via `dw_hdmi_bridge_edid_read()` (line 2990), the bridge framework calls `drm_edid_connector_update()` on behalf of the caller, so the approach is sound — but only if the consumers are updated.

**Minor:** The removed `dev_dbg` logging of EDID width/height (useful for debug) could be kept or moved, but this is cosmetic and not a blocker.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
Author: Erick Karanja <karanja99erick@gmail.com>
Patches: 3
Reviewed: 2026-02-27T13:44:34.670495

---

This is a single-patch series attempting to address a FIXME comment already present in the dw-hdmi driver. The goal — replacing deprecated `drm_detect_hdmi_monitor()` / `drm_detect_monitor_audio()` with `connector->display_info` fields populated by `drm_edid_connector_update()` — is correct and desirable. However, the implementation has a significant problem: it introduces a **duplicate call** to `drm_edid_connector_update()` in the connector path, and also drops useful debug logging. The patch needs rework.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions

[Claude Code Review Bot]

Patch Review

**Concept: Correct.** The existing code has a FIXME comment (lines 2485-2489 in the current tree) that explicitly asks for this exact change. Removing the deprecated `drm_detect_hdmi_monitor()` and `drm_detect_monitor_audio()` in favor of `connector->display_info.is_hdmi` / `.has_audio` is the right direction.

**Problem 1: Duplicate `drm_edid_connector_update()` in the connector path.**

The patch adds `drm_edid_connector_update(connector, drm_edid)` inside `dw_hdmi_edid_read()`. But `dw_hdmi_edid_read()` has **two callers**:

1. `dw_hdmi_connector_get_modes()` (line 2520) — which **already calls** `drm_edid_connector_update()` at line 2522:
   ```c
   drm_edid = dw_hdmi_edid_read(hdmi, connector);
   drm_edid_connector_update(connector, drm_edid);  /* existing call */
   ```

2. `dw_hdmi_bridge_edid_read()` (line 3009) — which returns the EDID to the **bridge connector framework**, which then calls `drm_edid_connector_update()` itself in `drm_bridge_connector_get_modes_edid()` at `drm_bridge_connector.c:311`.

So with this patch, `drm_edid_connector_update()` will be called **twice** in both code paths — once inside `dw_hdmi_edid_read()` and once by the caller/framework. While calling it twice is not a crash-inducing bug, it's wasteful and indicates the wrong approach.

**The proper fix** should keep the `drm_edid_connector_update()` call where it already is (in the callers/framework) and just change the `dw_hdmi_edid_read()` function to read from `connector->display_info` **after** `dw_hdmi_edid_read()` returns and `drm_edid_connector_update()` has been called. This likely means moving the `sink_is_hdmi`/`sink_has_audio` assignment out of `dw_hdmi_edid_read()` and into each caller, after their respective `drm_edid_connector_update()` calls.

**Problem 2: Lost debug logging.**

The patch removes:
```c
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
        edid->width_cm, edid->height_cm);
```

This debug message is useful for driver developers and should be preserved. It can be rewritten to use `connector->display_info` fields (e.g., `connector->display_info.width_mm`) rather than raw EDID fields, or simply kept as-is using `drm_edid_raw()`.

**Problem 3: Ordering concern.**

Even ignoring the duplicate call, the patch places `drm_edid_connector_update()` *before* returning the `drm_edid` to the caller. In `dw_hdmi_connector_get_modes()`, the caller then calls it again. But more importantly, the bridge path (`dw_hdmi_bridge_edid_read`) just returns `drm_edid` and the framework calls `drm_edid_connector_update()` again. The intent of the FIXME was to read `display_info` **after** the normal update path has run, not to add an early extra call.

**Suggested approach:** Instead of adding `drm_edid_connector_update()` inside `dw_hdmi_edid_read()`, refactor so that `sink_is_hdmi` and `sink_has_audio` are set in each caller *after* `drm_edid_connector_update()` has been called:

```c
/* In dw_hdmi_connector_get_modes(): */
drm_edid = dw_hdmi_edid_read(hdmi, connector);
drm_edid_connector_update(connector, drm_edid);   /* already exists */
hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
hdmi->sink_has_audio = connector->display_info.has_audio;
```

And similarly handle the bridge path (which is trickier since the framework makes the call, so you may need to set these fields in the bridge `atomic_enable` or `mode_set` callback where `display_info` is already populated).

**Minor:** The commit message and subject are clear and well-written. The `RFT` (Request For Testing) tag is appropriate given this is untested.

---
Generated by Claude Code Patch Reviewer