Re: [PATCH] drm/colorop: Mark interpolation properties as immutable and atomic

[Pekka Paalanen <pekka.paalanen@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 4084 bytes --]

On Mon, 30 Mar 2026 09:33:53 -0300
Melissa Wen <mwen@igalia.com> wrote:

> On 26/03/2026 21:39, Alex Hung wrote:
> > LUT1D_INTERPOLATION and LUT3D_INTERPOLATION are read-only properties but
> > were missing DRM_MODE_PROP_IMMUTABLE and DRM_MODE_PROP_ATOMIC flags.  
> + Xaver
> 
> So, I double checked previous conversation about these two interpolation 
> properties.
> AFAIU, we agreed (?) on not making it immutable because it could become 
> mutable if a HW allows it.
> - 
> https://lore.kernel.org/dri-devel/CAFZQkGyj7=N2UcbBnjv7Az3oHSD2LXaAx5wzccpSt_UEhH3NZw@mail.gmail.com/

Simon's rationale there makes sense to me.

> So, what's the best approach to inform userspace that they cannot change 
> this property?
> Perhaps the driver has to reject if in the end the property is immutable (?)

If the enum is exposed with a single value only, then obviously
userspace cannot program any other value.

Each driver needs to customize every enum for what they actually
support, anyway.


Thanks,
pq

> 
> Melissa
> >
> > Add the correct flags and remove the now-unreachable set_property
> > handlers in drm_atomic_colorop_set_property().
> >
> > Link: https://lore.kernel.org/amd-gfx/dbb27ec4-cdc6-4ead-9daf-664d97e86cd0@intel.com/
> > Fixes: 7fa3ee8c0a79 ("drm/colorop: Define LUT_1D interpolation")
> > Fixes: db971856bbe0 ("drm/colorop: Add 3D LUT support to color pipeline")
> > Suggested-by: Melissa Wen <mwen@igalia.com>
> > Suggested-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > Signed-off-by: Alex Hung <alex.hung@amd.com>
> > ---
> >   drivers/gpu/drm/drm_atomic_uapi.c | 4 ----
> >   drivers/gpu/drm/drm_colorop.c     | 6 ++++--
> >   2 files changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 713fa9e81732..e831894d5a51 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -758,8 +758,6 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
> >   			state->bypass = val;
> >   			*replaced = true;
> >   		}
> > -	} else if (property == colorop->lut1d_interpolation_property) {
> > -		colorop->lut1d_interpolation = val;
> >   	} else if (property == colorop->curve_1d_type_property) {
> >   		if (state->curve_1d_type != val) {
> >   			state->curve_1d_type = val;
> > @@ -770,8 +768,6 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
> >   			state->multiplier = val;
> >   			*replaced = true;
> >   		}
> > -	} else if (property == colorop->lut3d_interpolation_property) {
> > -		colorop->lut3d_interpolation = val;
> >   	} else if (property == colorop->data_property) {
> >   		return drm_atomic_color_set_data_property(colorop, state,
> >   							  property, val,
> > diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> > index 398cc81ae588..34998f393f2e 100644
> > --- a/drivers/gpu/drm/drm_colorop.c
> > +++ b/drivers/gpu/drm/drm_colorop.c
> > @@ -317,7 +317,8 @@ int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_color
> >   	colorop->size = lut_size;
> >   
> >   	/* interpolation */
> > -	prop = drm_property_create_enum(dev, 0, "LUT1D_INTERPOLATION",
> > +	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_IMMUTABLE,
> > +					"LUT1D_INTERPOLATION",
> >   					drm_colorop_lut1d_interpolation_list,
> >   					ARRAY_SIZE(drm_colorop_lut1d_interpolation_list));
> >   	if (!prop)
> > @@ -413,7 +414,8 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
> >   	colorop->size = lut_size;
> >   
> >   	/* interpolation */
> > -	prop = drm_property_create_enum(dev, 0, "LUT3D_INTERPOLATION",
> > +	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_IMMUTABLE,
> > +					"LUT3D_INTERPOLATION",
> >   					drm_colorop_lut3d_interpolation_list,
> >   					ARRAY_SIZE(drm_colorop_lut3d_interpolation_list));
> >   	if (!prop)  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]