From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/colorop: make lut(1/3)d_interpolation mutable
Date: Thu, 07 May 2026 13:05:38 +1000
Message-ID: <review-patch4-20260506192633.16066-5-mwen@igalia.com>
In-Reply-To: <20260506192633.16066-5-mwen@igalia.com>
References: <20260506192633.16066-1-mwen@igalia.com> <20260506192633.16066-5-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

**Assessment: Good, well-structured**

This patch moves `lut1d_interpolation` and `lut3d_interpolation` from `stru=
ct drm_colorop` (the immutable hardware object) to `struct drm_colorop_stat=
e` (the per-commit mutable state). This is the right pattern =E2=80=94 prop=
erties that userspace can change per atomic commit must live in the state s=
truct.

The changes are consistent:
- **drm_colorop.h**: Fields moved from `drm_colorop` to `drm_colorop_state`
- **drm_atomic.c**: Print state reads from `state->lut1d_interpolation` / `=
state->lut3d_interpolation` instead of `colorop->...`
- **drm_atomic_uapi.c**: Set/get property uses `state->...` instead of `col=
orop->...`
- **drm_colorop.c**: Default values initialized in `__drm_colorop_state_res=
et()` via `drm_object_property_get_default_value()`, and the assignment `co=
lorop->lut1d_interpolation =3D interpolation` removed from init functions

The initialization in `__drm_colorop_state_reset()` correctly mirrors the p=
attern already used for `curve_1d_type`:

```c
if (colorop->lut1d_interpolation_property) {
    drm_object_property_get_default_value(&colorop->base,
                          colorop->lut1d_interpolation_property,
                          &val);
    colorop_state->lut1d_interpolation =3D val;
}
```

Since `__drm_atomic_helper_colorop_duplicate_state()` does a `memcpy` of th=
e entire state, the new fields will be correctly preserved during state dup=
lication without additional code.

**Note**: The return value of `drm_object_property_get_default_value()` is =
not checked for errors (unlike the existing `curve_1d_type` case which uses=
 `if (!drm_object_property_get_default_value(...))`). The existing code for=
 `curve_1d_type` wraps the call in an `if (!drm_object_property_get_default=
_value(...))` condition. The new code for `lut1d_interpolation` and `lut3d_=
interpolation` does not check the return value =E2=80=94 the value will be =
used even if the call fails. This is inconsistent with the existing pattern=
 but practically harmless since the property existence check (`if (colorop-=
>lut1d_interpolation_property)`) ensures the property exists and the defaul=
t value lookup should succeed.

---
Generated by Claude Code Patch Reviewer