Claude review: drm/bridge: ite-it66121: Select HDMI or DVI mode based on sink type

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Correctness: The core logic is correct**

The approach of checking `connector->display_info.is_hdmi` and conditionally setting the mode/AVI infoframes is the right fix:

```c
if (connector->display_info.is_hdmi) {
	mode = IT66121_HDMI_MODE_HDMI;
	avi_pkt = IT66121_AVI_INFO_PKT_ON | IT66121_AVI_INFO_PKT_RPT;
}
```

For DVI sinks, `mode` stays `IT66121_HDMI_MODE_DVI` (0) and `avi_pkt` stays 0, which correctly disables HDMI mode and AVI infoframes.

**Issue: `connector->display_info.is_hdmi` accessed outside mutex**

```c
struct drm_connector *connector = ctx->connector;

if (connector->display_info.is_hdmi) {
	...
}

mutex_lock(&ctx->lock);
```

The `display_info.is_hdmi` check happens before `ctx->lock` is taken. While `display_info` is typically stable once the EDID has been read (it's set during `drm_edid_connector_update()` which happens at detect time), and `ctx->connector` was just set in the calling function, this is not a serious concern in practice — the lock here is protecting the regmap writes, not the connector state. But it's worth noting the code reads `ctx->connector` without the lock.

**Issue: Duplicate writes in `.mode_set` still not addressed**

As mentioned in patch 1, the `.mode_set` callback still unconditionally writes `IT66121_HDMI_MODE_HDMI` and enables AVI infoframes. So the sequence during a mode change is:
1. `.mode_set` writes HDMI mode + AVI infoframes ON (always)
2. `.atomic_enable` writes DVI mode + AVI infoframes OFF (for DVI sinks)

While the end state is correct, the DVI monitor momentarily receives HDMI-mode signaling between steps 1 and 2. This could cause transient issues depending on the monitor. Removing the writes from `.mode_set` would eliminate this.

**Good: `IT66121_HDMI_MODE_DVI` define**

```c
#define IT66121_HDMI_MODE_DVI			0
```

Adding a named constant for the DVI mode value (0) instead of using a bare 0 is good practice and makes the code self-documenting.

**Observation: AVI infoframe content still written in `.mode_set` for DVI**

Even after this series, the `.mode_set` callback still writes AVI infoframe *content* to the `IT66121_AVIINFO_DB1_REG` registers and the checksum register for DVI sinks. While the AVI infoframe *transmission* is disabled (the PKT register is cleared in `.atomic_enable`), writing infoframe content to hardware for a DVI connection is wasted work. This is not a bug — just unnecessary register writes. A cleaner approach would be to skip the AVI infoframe preparation in `.mode_set` for DVI sinks, but that would require `connector` to be available in `.mode_set`, which is the whole problem this series is solving. So this is acceptable as-is; the infoframe content simply goes unused.

**Summary**: The series fixes a real bug correctly but needs the duplicate register writes removed from `.mode_set` in patch 1 (or a clear explanation in the commit message for why they're intentionally kept). The enable ordering (unmute before TX mode set) is also worth reconsidering.

---
Generated by Claude Code Patch Reviewer