public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames
@ 2026-03-11 19:16 Krzysztof Kozlowski
  2026-03-11 20:46 ` Claude review: " Claude Code Review Bot
  2026-03-11 20:46 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 19:16 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	Maxime Ripard, linux-arm-msm, dri-devel, freedreno, linux-kernel
  Cc: Krzysztof Kozlowski, stable

Commit 384d2b03d0a1 ("drm/msm/hdmi: make use of the drm_connector_hdmi
framework") changed the unconditional register writes in few places to
updates: read, apply mask, write.  The new code reads
REG_HDMI_INFOFRAME_CTRL1 register, applies fields/mask for
HDMI_INFOFRAME_CTRL0 register and finally writes to
HDMI_INFOFRAME_CTRL0.  This difference between CTRL1 and CTRL0 looks
unintended and may result in wrong data being written to HDMI bridge
registers.

Cc: <stable@vger.kernel.org>
Fixes: 384d2b03d0a1 ("drm/msm/hdmi: make use of the drm_connector_hdmi framework")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 3c407e275ce5..a9eb6489c520 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -153,7 +153,7 @@ static int msm_hdmi_bridge_write_avi_infoframe(struct drm_bridge *bridge,
 	for (i = 0; i < ARRAY_SIZE(buf); i++)
 		hdmi_write(hdmi, REG_HDMI_AVI_INFO(i), buf[i]);
 
-	val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL1);
+	val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);
 	val |= HDMI_INFOFRAME_CTRL0_AVI_SEND |
 		HDMI_INFOFRAME_CTRL0_AVI_CONT;
 	hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
@@ -193,7 +193,7 @@ static int msm_hdmi_bridge_write_audio_infoframe(struct drm_bridge *bridge,
 		   buffer[9] << 16 |
 		   buffer[10] << 24);
 
-	val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL1);
+	val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);
 	val |= HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND |
 		HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT |
 		HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE |
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames
  2026-03-11 19:16 [PATCH] drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames Krzysztof Kozlowski
@ 2026-03-11 20:46 ` Claude Code Review Bot
  2026-03-11 20:46 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:46 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Patches: 1
Reviewed: 2026-03-12T06:46:33.744959

---

This is a single-patch bug fix for the MSM HDMI bridge driver. The fix is correct, straightforward, and well-documented. The patch addresses a clear copy-paste or refactoring error introduced in commit `384d2b03d0a1` where a read-modify-write sequence was reading from the wrong register (`CTRL1` instead of `CTRL0`) before applying `CTRL0` field masks and writing to `CTRL0`.

**Verdict: Patch looks good. Recommend applying.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames
  2026-03-11 19:16 [PATCH] drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames Krzysztof Kozlowski
  2026-03-11 20:46 ` Claude review: " Claude Code Review Bot
@ 2026-03-11 20:46 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:46 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Bug analysis:** The original code (from commit `384d2b03d0a1`) converted unconditional register writes to read-modify-write sequences but read from `REG_HDMI_INFOFRAME_CTRL1` while applying masks for `HDMI_INFOFRAME_CTRL0` fields and writing to `REG_HDMI_INFOFRAME_CTRL0`. This means the CTRL0 register was being corrupted with bits from CTRL1, and any existing CTRL0 bits that should have been preserved were lost.

**Fix correctness:** The fix changes both occurrences to read from `REG_HDMI_INFOFRAME_CTRL0`, making the read-modify-write consistent:

```c
val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);  // was CTRL1
val |= HDMI_INFOFRAME_CTRL0_AVI_SEND |
       HDMI_INFOFRAME_CTRL0_AVI_CONT;
hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
```

and similarly for the audio infoframe path:

```c
val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);  // was CTRL1
val |= HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND |
       HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT |
       HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE |
       HDMI_INFOFRAME_CTRL0_AUDIO_INFO_UPDATE;
hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
```

Looking at the post-patch source (lines 156-164), the pattern is now consistent with the immediately following block that correctly does read-modify-write on CTRL1 for CTRL1-specific fields (`HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE`).

**Minor observation:** The `|=` operation (rather than clearing bits first with a mask) means these "send/cont" bits are only ever set, never cleared through this path. This is fine since the `clear_avi_infoframe` / `clear_audio_infoframe` functions are called at the start of each write function to handle the clearing.

**Tags:** Fixes tag, Cc: stable, and Signed-off-by are all present and appropriate.

**No issues found.** 

Reviewed-by recommendation: This patch is correct and ready to merge.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-11 20:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 19:16 [PATCH] drm/msm/hdmi: Fix wrong CTRL1 register used in writing info frames Krzysztof Kozlowski
2026-03-11 20:46 ` Claude review: " Claude Code Review Bot
2026-03-11 20:46 ` 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