Claude review: gpu/drm: tegra: add DSI support for Tegra20/Tegra30

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

Patch Review

**Overall: Good patch, well-structured refactoring.**

This patch introduces `struct tegra_dsi_config` with two boolean flags and associates per-SoC configs via `of_match_data`. The code movement from `tegra_dsi_pad_calibrate()` into `tegra_dsi_pad_enable()` is clean and logical — the Tegra114+ pad init sequence (zeroing PAD_CONTROL_0..4, then writing slew/preemp settings) was always done before `tegra_mipi_start_calibration()`, and for Tegra20/30 that MIPI calibration path won't be used anyway (handled by CSI per the cover letter).

**Positive notes:**

- Using `device_get_match_data()` with an `ENODEV` check is the preferred modern pattern.
- `devm_clk_get_optional()` for `clk_lp` is correct — `clk_prepare_enable(NULL)` / `clk_disable_unprepare(NULL)` are documented no-ops in the kernel clock API, so the runtime suspend/resume paths at lines 1103 and 1136 are safe without any additional guards.
- The separate `tegra30_dsi_config` (identical to `tegra20_dsi_config` today but with a forward-looking comment about PLL_D/PLL_D2 muxing) is reasonable.
- New compatibles in `host1x_drm_subdevs[]` are inserted in the correct sorted position (between `-dc` and `-hdmi` for each SoC generation).

**Minor comments:**

1. The comment added at the top of `tegra_dsi_setup_clocks()`:

```c
+	/*
+	 * Tegra124+ uses a clock gate, not a mux, so this step
+	 * should be redundant for configuration; yet, DSI refuses
+	 * to work without it.
+	 */
+
 	parent = clk_get_parent(dsi->clk);
```

This comment describes a Tegra124+ quirk but sits inside a function that is now only called when `has_mux_parent_clk == true` (i.e., Tegra114+). The comment is useful context and matches the `TODO` on `tegra124_dsi_config`, so this is fine — but the blank line after the closing `*/` is a bit unusual for kernel style.

2. The `tegra20_dsi_config` and `tegra30_dsi_config` structs have identical content. This is a deliberate choice (documented by the comment on tegra30's struct) to allow independent future evolution. No issue, just noting it.

3. The Tegra20/30 pad control else-branch in `tegra_dsi_pad_enable()`:

```c
+	} else {
+		value = DSI_PAD_CONTROL_LPUPADJ(0x1) | DSI_PAD_CONTROL_LPDNADJ(0x1) |
+			DSI_PAD_CONTROL_PREEMP_EN(0x1) | DSI_PAD_CONTROL_SLEWDNADJ(0x6) |
+			DSI_PAD_CONTROL_SLEWUPADJ(0x6) | DSI_PAD_CONTROL_PDIO(0) |
+			DSI_PAD_CONTROL_PDIO_CLK(0) | DSI_PAD_CONTROL_PULLDN_ENAB(0);
+		tegra_dsi_writel(dsi, value, DSI_PAD_CONTROL_0);
+	}
```

The `PDIO(0)`, `PDIO_CLK(0)`, and `PULLDN_ENAB(0)` fields are all zero and don't contribute to the final register value — they're OR'd in as 0. Including them for documentation/completeness is acceptable, but if there was an intent to make these configurable later, it might be cleaner to omit the zero fields. This is a style nit, no functional impact.

4. New register macros in `dsi.h` — the bit positions look correct based on the Tegra TRM (e.g., `PULLDN_ENAB` at bit 28, `SLEWUPADJ` at bits [26:24], etc.). The macros share the same register offset `DSI_PAD_CONTROL_0 = 0x4b` as the Tegra114+ macros, which is correct since it's the same register address with different bit layouts per SoC generation.

---

---
Generated by Claude Code Patch Reviewer