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

[Alex Hung]

On 3/31/26 06:19, Pekka Paalanen wrote:
On Mon, 30 Mar 2026 09:33:53 -0300
Melissa Wen <m...@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_uehh3...@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-664d97e86...@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 <m...@igalia.com>
Suggested-by: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
Signed-off-by: Alex Hung <alex.h...@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)