* [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
@ 2026-02-25 9:30 Erick Karanja
2026-02-25 10:11 ` Maxime Ripard
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Erick Karanja @ 2026-02-25 9:30 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Liu Ying,
Dmitry Baryshkov, Luca Ceresoli, Shengjiu Wang, Andy Yan,
dri-devel, linux-kernel, 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.
The driver should read these fields from connector->display_info instead of
calling the deprecated functions directly.
Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 3b77e73ac0ea..ddc4ae40d94e 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;
@@ -2482,19 +2481,9 @@ static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
dev_dbg(hdmi->dev, "failed to get edid\n");
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);
+ drm_edid_connector_update(connector, drm_edid);
+ hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
+ hdmi->sink_has_audio = connector->display_info.has_audio;
return drm_edid;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
2026-02-25 9:30 [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions Erick Karanja
@ 2026-02-25 10:11 ` Maxime Ripard
2026-02-25 18:40 ` Erick Karanja
2026-02-27 3:44 ` Claude review: " Claude Code Review Bot
2026-02-27 3:44 ` Claude Code Review Bot
2 siblings, 1 reply; 7+ messages in thread
From: Maxime Ripard @ 2026-02-25 10:11 UTC (permalink / raw)
To: Erick Karanja
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Liu Ying, Dmitry Baryshkov,
Luca Ceresoli, Shengjiu Wang, Andy Yan, dri-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]
Hi,
On Wed, Feb 25, 2026 at 12:30:50PM +0300, Erick Karanja wrote:
> 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.
> The driver should read these fields from connector->display_info instead of
> calling the deprecated functions directly.
>
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 3b77e73ac0ea..ddc4ae40d94e 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;
> @@ -2482,19 +2481,9 @@ static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
> dev_dbg(hdmi->dev, "failed to get edid\n");
> 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);
> + drm_edid_connector_update(connector, drm_edid);
> + hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
> + hdmi->sink_has_audio = connector->display_info.has_audio;
You should get rid of sink_is_hdmi and sink_has_audio, and you'll don't
need to call any function anymore, deprecated or otherwise.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
2026-02-25 10:11 ` Maxime Ripard
@ 2026-02-25 18:40 ` Erick Karanja
0 siblings, 0 replies; 7+ messages in thread
From: Erick Karanja @ 2026-02-25 18:40 UTC (permalink / raw)
To: Maxime Ripard
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Liu Ying, Dmitry Baryshkov,
Luca Ceresoli, Shengjiu Wang, Andy Yan, dri-devel, linux-kernel
On Wed, 2026-02-25 at 11:11 +0100, Maxime Ripard wrote:
> Hi,
>
> On Wed, Feb 25, 2026 at 12:30:50PM +0300, Erick Karanja wrote:
> > 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.
> > The driver should read these fields from connector->display_info
> > instead of
> > calling the deprecated functions directly.
> >
> > Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++--------------
> > 1 file changed, 3 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index 3b77e73ac0ea..ddc4ae40d94e 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;
> > @@ -2482,19 +2481,9 @@ static const struct drm_edid
> > *dw_hdmi_edid_read(struct dw_hdmi *hdmi,
> > dev_dbg(hdmi->dev, "failed to get edid\n");
> > 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);
> > + drm_edid_connector_update(connector, drm_edid);
> > + hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
> > + hdmi->sink_has_audio = connector->display_info.has_audio;
>
> You should get rid of sink_is_hdmi and sink_has_audio, and you'll
> don't
> need to call any function anymore, deprecated or otherwise.
Thank you, sending v2
>
> Maxime
^ permalink raw reply [flat|nested] 7+ messages in thread
* Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
2026-02-25 9:30 [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions Erick Karanja
2026-02-25 10:11 ` Maxime Ripard
@ 2026-02-27 3:44 ` Claude Code Review Bot
2026-02-27 3:44 ` Claude Code Review Bot
2 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-02-27 3:44 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
2026-02-25 9:30 [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions Erick Karanja
2026-02-25 10:11 ` Maxime Ripard
2026-02-27 3:44 ` Claude review: " Claude Code Review Bot
@ 2026-02-27 3:44 ` Claude Code Review Bot
2 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-02-27 3:44 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
@ 2026-02-26 12:06 Erick Karanja
2026-02-27 2:14 ` Claude review: " Claude Code Review Bot
2026-02-27 2:14 ` Claude Code Review Bot
0 siblings, 2 replies; 7+ messages in thread
From: Erick Karanja @ 2026-02-26 12:06 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Liu Ying,
Dmitry Baryshkov, Luca Ceresoli, Shengjiu Wang, Andy Yan,
dri-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
2026-02-26 12:06 [PATCH v2] " Erick Karanja
@ 2026-02-27 2:14 ` Claude Code Review Bot
2026-02-27 2:14 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-02-27 2:14 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Claude review: drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions
2026-02-26 12:06 [PATCH v2] " Erick Karanja
2026-02-27 2:14 ` Claude review: " Claude Code Review Bot
@ 2026-02-27 2:14 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-02-27 2:14 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-02-27 3:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-25 9:30 [PATCH RFT] drm/bridge: dw-hdmi: Replace calls to deprecated HDMI/audio detection functions Erick Karanja
2026-02-25 10:11 ` Maxime Ripard
2026-02-25 18:40 ` Erick Karanja
2026-02-27 3:44 ` Claude review: " Claude Code Review Bot
2026-02-27 3:44 ` Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-02-26 12:06 [PATCH v2] " Erick Karanja
2026-02-27 2:14 ` Claude review: " Claude Code Review Bot
2026-02-27 2:14 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox