Claude review: drm/i915/color: Program Pre-CSC registers for SDR

Claude Code Review Bot
2026-04-12T03:07:24Z

Patch Review

**Issue (medium)**: Hardcoded `plane = plane - 3` offset:
```c
/*
* First 3 planes are HDR, so reduce by 3 to get to the right
* SDR plane offset
*/
plane = plane - 3;
```

This assumes planes 0-2 are always HDR and SDR planes always start at index 3. While true for current hardware (`icl_hdr_plane_mask()` returns `BIT(PLANE_1) | BIT(PLANE_2) | BIT(PLANE_3)` = planes 0,1,2), this is a magic number that will silently break on future platforms with different layouts. Consider using a named constant or deriving the offset from `icl_hdr_plane_mask()` (e.g., `hweight8(icl_hdr_plane_mask())`).

There is also no defensive check that `plane >= 3` before the subtraction. While `icl_is_hdr_plane()` guards the callers, an explicit `drm_WARN_ON` would catch future misuse.

**Observation**: SDR LUT uses 16-bit precision (`1 << 16`) vs HDR's 24-bit (`1 << 24`), and writes to non-ENH register variants (`PLANE_PRE_CSC_GAMC_INDEX` vs `PLANE_PRE_CSC_GAMC_INDEX_ENH`). This matches the hardware.