From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/i915/color: Program Pre-CSC registers for SDR Date: Mon, 09 Mar 2026 08:26:53 +1000 Message-ID: In-Reply-To: <20260306165307.3233194-8-chaitanya.kumar.borah@intel.com> References: <20260306165307.3233194-1-chaitanya.kumar.borah@intel.com> <20260306165307.3233194-8-chaitanya.kumar.borah@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **BUG =E2=80=94 Wrong LUT extract function for SDR pre-CSC.** The function = receives `const struct drm_color_lut32 *pre_csc_lut` (32-bit per-channel va= lues) but uses `drm_color_lut_extract()`: ```c lut_val =3D drm_color_lut_extract(pre_csc_lut[i].green, 16); ``` `drm_color_lut_extract` expects a 16-bit input (from `struct drm_color_lut`= ), scaling from `[0, 2^16-1]` to `[0, 2^precision-1]`. But `drm_color_lut32= .green` is a `u32` in the range `[0, 2^32-1]`. This will produce incorrect = values for any input above 65535. Should use `drm_color_lut32_extract(pre_c= sc_lut[i].green, 16)` instead. **Concern =E2=80=94 Hardcoded plane offset `plane =3D plane - 3`:** ```c /* * First 3 planes are HDR, so reduce by 3 to get to the right * SDR plane offset */ plane =3D plane - 3; ``` This hardcodes the assumption that exactly 3 HDR planes precede SDR planes.= While this matches current hardware (`PLANE_1`, `PLANE_2`, `PLANE_3` are H= DR per `icl_hdr_plane_mask()`), it's fragile. The `_MMIO_PLANE_GAMC` macro = uses `_PIPE(plane, a, b)` which only supports two planes (plane 0 and plane= 1). After subtracting 3, `PLANE_4` becomes index 1, `PLANE_5` becomes inde= x 2, etc. But `_PIPE` only handles index 0 and 1. If there are more than 2 = SDR planes, this will access wrong registers. The code should add a comment= explaining which SDR planes this supports and add a `WARN_ON` for unexpect= ed plane IDs. --- Generated by Claude Code Patch Reviewer