Claude review: drm/tegra: Enable cmu for Tegra186 and Tegra194

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

Patch Review

**Commit message tags (must fix):**

The `Co-developed-by` tag requires a corresponding `Signed-off-by` immediately after it per kernel documentation (Documentation/process/submitting-patches.rst). Currently:

```
Co-developed-by: Kurt Kiefer <kekiefer@gmail.com>
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
```

Should be:

```
Co-developed-by: Kurt Kiefer <kekiefer@gmail.com>
Signed-off-by: Kurt Kiefer <kekiefer@gmail.com>
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
```

**dc.c -- LUT table and probe:**

The `default_srgb_lut` table has exactly 1025 entries, matching the `OUTPUT_LUT_SIZE_SIZE_1025` hardware setting. Good.

The LUT entry packing looks correct:
```c
r = default_srgb_lut[i];
dc->cmu_output_lut[i] = (r << 32) | (r << 16) | r;
```
This applies the same sRGB curve to R, G, and B channels within a 64-bit entry (three 16-bit fields in the lower 48 bits). This matches what's expected for a uniform gamma correction.

Use of `dmam_alloc_coherent` is appropriate -- the LUT must persist for the device's lifetime and this avoids manual cleanup. No issues here.

**dc.h -- Register definitions:**

The new `CMU_ENABLE_ENABLE` is correctly placed as a bit flag for `DC_DISP_DISP_COLOR_CONTROL` (0x430), alongside the existing `BASE_COLOR_SIZE_*` and `DITHER_CONTROL_*` definitions. Bit 20 doesn't conflict with any existing masks (`BASE_COLOR_SIZE_MASK` is bits 0-3, `DITHER_CONTROL_MASK` is bits 8-9).

The new nvdisplay-specific registers:
```c
#define DC_DISP_CORE_HEAD_SET_CONTROL_OUTPUT_LUT    0x431
#define DC_DISP_COREPVT_HEAD_SET_OUTPUT_LUT_BASE    0x432
#define DC_DISP_COREPVT_HEAD_SET_OUTPUT_LUT_BASE_HI 0x433
```
These overlap with legacy registers (`DC_DISP_SHIFT_CLOCK_OPTIONS` at 0x431, `DC_DISP_DATA_ENABLE_OPTIONS` at 0x432, `DC_DISP_SERIAL_INTERFACE_OPTIONS` at 0x433), which is expected since nvdisplay (Tegra186+) uses a different register layout at the same offsets. The placement after the existing `/* Tegra186 and later */` section at the `DC_DISP_CORE_SOR_SET_CONTROL` block is correct. A brief comment like `/* Tegra186+ CMU output LUT registers */` would aid readability but is not strictly necessary.

**sor.c -- HDMI enable path:**

The insertion point is well-chosen. The CMU setup is inserted between the mask-clearing and the `switch(state->bpc)`:

```c
value = tegra_dc_readl(dc, DC_DISP_DISP_COLOR_CONTROL);
value &= ~DITHER_CONTROL_MASK;
value &= ~BASE_COLOR_SIZE_MASK;
+   if (dc->soc->has_nvdisplay && dc->cmu_output_lut) {
+       /* ... program LUT base & control ... */
+       value |= CMU_ENABLE_ENABLE;
+   }

    switch (state->bpc) {
```

This is clean -- `CMU_ENABLE_ENABLE` (bit 20) is accumulated into `value` alongside the color size bits, and all are written together at the existing `tegra_dc_writel(dc, value, DC_DISP_DISP_COLOR_CONTROL)` at the end of the switch. No bit conflicts.

**sor.c -- DP enable path:**

```c
+   if (dc->soc->has_nvdisplay && dc->cmu_output_lut) {
+       value = tegra_dc_readl(dc, DC_DISP_DISP_COLOR_CONTROL);
+       tegra_dc_writel(dc, lower_32_bits(dc->cmu_output_phys),
+                       DC_DISP_COREPVT_HEAD_SET_OUTPUT_LUT_BASE);
+       ...
+       value |= CMU_ENABLE_ENABLE;
+       tegra_dc_writel(dc, value, DC_DISP_DISP_COLOR_CONTROL);
+   }
```

This is a standalone read-modify-write, which is correct for the DP path since there's no existing color control manipulation there (confirmed: `DC_DISP_DISP_COLOR_CONTROL` is only touched in the HDMI enable path at lines 2557/2584 of the current tree).

**Code duplication (minor):** The LUT base/control/CMU-enable programming sequence is nearly identical between the HDMI and DP paths (~8 lines duplicated). Consider extracting a helper like `tegra_dc_setup_cmu(dc)` in dc.c to avoid this. This is a style preference, not a correctness issue.

**No disable path (observation):** CMU is enabled but never explicitly disabled on encoder disable. This is acceptable because: (a) CMU is always desired on nvdisplay hardware (matching vendor behavior), (b) the LUT buffer persists via dmam_alloc_coherent, and (c) CMU is re-programmed on every enable. However, if future work adds runtime CMU control or drm color management properties, a disable path would be needed.

**Summary of required changes:**
1. Add missing `Signed-off-by: Kurt Kiefer <kekiefer@gmail.com>` after the `Co-developed-by` tag.

**Suggestions (optional):**
1. Extract the duplicated LUT programming into a small helper function.
2. Add a brief comment above the new nvdisplay register definitions noting they overlap with legacy register addresses.

---
Generated by Claude Code Patch Reviewer