public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: ite-it66121: Disable HDMI audio capture stream
@ 2026-03-12 19:33 Sen Wang
  2026-03-13  3:51 ` Claude review: " Claude Code Review Bot
  2026-03-13  3:51 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Sen Wang @ 2026-03-12 19:33 UTC (permalink / raw)
  To: ple, neil.armstrong
  Cc: andrzej.hajda, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
	devarsht, s-jain1, j-sahu, dri-devel, linux-kernel, Sen Wang

The IT66121 is an HDMI transmitter and does not support audio capture.
However, the driver registers hdmi-codec without setting no_i2s_capture,
causing the codec to advertise a non-functional capture stream. Userspace
audio servers such as PulseAudio then attempt to open this capture
device, resulting in errors.

Set no_i2s_capture and no_spdif_capture in hdmi_codec_pdata so that the
hdmi-codec driver only registers playback, matching the actual hardware
capability.

Signed-off-by: Sen Wang <sen@ti.com>
---
 drivers/gpu/drm/bridge/ite-it66121.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 329fa47a5701..a1b2c3d4e5f6 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -1476,6 +1476,8 @@ static int it66121_audio_codec_init(struct it66121_ctx *ctx, struct device *dev)
 		.i2s = 1, /* Only i2s support for now */
 		.spdif = 0,
 		.max_i2s_channels = 8,
+		.no_i2s_capture = 1,
+		.no_spdif_capture = 1,
 		.no_capture_mute = 1,
 	};

--
2.34.1

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

* Claude review: drm/bridge: ite-it66121: Disable HDMI audio capture stream
  2026-03-12 19:33 [PATCH] drm/bridge: ite-it66121: Disable HDMI audio capture stream Sen Wang
  2026-03-13  3:51 ` Claude review: " Claude Code Review Bot
@ 2026-03-13  3:51 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-13  3:51 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/bridge: ite-it66121: Disable HDMI audio capture stream
Author: Sen Wang <sen@ti.com>
Patches: 1
Reviewed: 2026-03-13T13:51:27.552579

---

This is a clean, single-patch fix for a real-world problem. The IT66121 is an HDMI transmitter, so it has no audio capture capability, yet the driver was registering a capture stream through hdmi-codec. This causes PulseAudio (and likely PipeWire) to attempt to open the non-functional capture device, producing errors. The fix is straightforward, correct, and follows the same pattern already used by the `tda998x` bridge driver.

**Verdict: Looks good to me.**

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/bridge: ite-it66121: Disable HDMI audio capture stream
  2026-03-12 19:33 [PATCH] drm/bridge: ite-it66121: Disable HDMI audio capture stream Sen Wang
@ 2026-03-13  3:51 ` Claude Code Review Bot
  2026-03-13  3:51 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-13  3:51 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness:** The patch correctly sets both `no_i2s_capture` and `no_spdif_capture` to `1` in the `hdmi_codec_pdata` initializer at `ite-it66121.c:1475-1476`. Both fields are valid members of `struct hdmi_codec_pdata` (defined in `include/sound/hdmi-codec.h:126,129`). Since the driver already sets `.spdif = 0` (SPDIF disabled entirely) and `.i2s = 1`, adding `.no_i2s_capture = 1` is the important one here, while `.no_spdif_capture = 1` is a belt-and-suspenders addition that doesn't hurt.

**Placement:** The new fields are placed logically between `.max_i2s_channels` and `.no_capture_mute`, matching the struct field order in the header, which is good style.

**Precedent:** The `tda998x` bridge driver (`drivers/gpu/drm/bridge/tda998x_drv.c:1177`) already uses `.no_i2s_capture = 1` in the same manner, confirming this is the established pattern.

**Commit message:** Clear and well-written — explains the hardware limitation, the symptom (PulseAudio errors), and the fix.

**Minor observation:** Since `.spdif = 0` already means SPDIF is disabled, setting `.no_spdif_capture = 1` is technically redundant but harmless and defensive. This is fine.

**No issues found.** 

Reviewed-by quality: this patch is ready to merge as-is.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-03-13  3:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-12 19:33 [PATCH] drm/bridge: ite-it66121: Disable HDMI audio capture stream Sen Wang
2026-03-13  3:51 ` Claude review: " Claude Code Review Bot
2026-03-13  3:51 ` 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