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/colorop: make lut(1/3)d_interpolation props correctly behave as mutable Date: Thu, 04 Jun 2026 12:16:44 +1000 Message-ID: In-Reply-To: <20260602215743.914265-3-mwen@igalia.com> References: <20260602215743.914265-1-mwen@igalia.com> <20260602215743.914265-3-mwen@igalia.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 **Author:** Melissa Wen This is the key data-model fix. It moves `lut1d_interpolation` and `lut3d_i= nterpolation` from `struct drm_colorop` (the immutable base object) to `str= uct drm_colorop_state` (the per-atomic-commit mutable state). **Changes reviewed:** - **`include/drm/drm_colorop.h`**: Fields move from `drm_colorop` to `drm_c= olorop_state`. Clean move, correct types preserved. - **`drm_atomic_uapi.c`**: `drm_atomic_colorop_set_property()` and `drm_ato= mic_colorop_get_property()` now read/write `state->lut1d_interpolation` / `= state->lut3d_interpolation` instead of `colorop->lut1d_interpolation` / `co= lorop->lut3d_interpolation`. Correct. - **`drm_atomic.c` (`drm_atomic_colorop_print_state`)**: Debug printing now= reads from `state->` instead of `colorop->`. Correct. - **`drm_colorop.c`**: The `__drm_colorop_state_reset()` function (called `= __drm_colorop_state_init` in the mbox diff context lines =E2=80=94 presumab= ly renamed in the base tree) now initializes the interpolation fields from = the property defaults via `drm_object_property_get_default_value()`. This f= ollows the existing pattern for `curve_1d_type`. The `colorop->lut1d_interp= olation =3D interpolation` assignments in `drm_plane_colorop_curve_1d_lut_i= nit()` and `drm_plane_colorop_3dlut_init()` are correctly removed since the= field no longer exists on `drm_colorop`. **Potential concern:** The interpolation fields are unconditionally present= on every `drm_colorop_state`, even for colorop types that don't use them (= e.g., CTM_3X4, MULTIPLIER). This wastes a few bytes per state object but is= harmless and consistent with how other fields like `data` are handled. **Verdict:** Correct. No issues. --- --- Generated by Claude Code Patch Reviewer