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

[Alex Hung <alex.hung@amd.com>]

On 3/31/26 06:19, Pekka Paalanen wrote:
> 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.

Yes this makes sense and I forgot that we discussed it before. Thanks 
Melissa and pekka

> 
>> 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.

As Melissa pointed out in the other thread, the doc said these 
properties are "Read-only" when they are not. Should we update the 
documents, such as below, to avoid confusion?

diff --git include/drm/drm_colorop.h include/drm/drm_colorop.h
index a3a32f9f918c..86d75babe30d 100644
--- include/drm/drm_colorop.h
+++ include/drm/drm_colorop.h
@@ -312,7 +312,7 @@ struct drm_colorop {
  	/**
  	 * @lut1d_interpolation_property:
  	 *
-	 * Read-only property for DRM_COLOROP_1D_LUT interpolation
+	 * Property for DRM_COLOROP_1D_LUT interpolation
  	 */
  	struct drm_property *lut1d_interpolation_property;

@@ -340,7 +340,7 @@ struct drm_colorop {
  	/**
  	 * @lut3d_interpolation_property:
  	 *
-	 * Read-only property for DRM_COLOROP_3D_LUT interpolation
+	 * Property for DRM_COLOROP_3D_LUT interpolation
  	 */
  	struct drm_property *lut3d_interpolation_property;


> 
> 
> 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)
>>
>