[PATCH 00/13] drm: Color pipeline teardown and follow-up fixes/improvements

[PATCH 00/13] drm: Color pipeline teardown and follow-up fixes/improvements

[Chaitanya Kumar Borah]

This series contains follow-up fixes and improvements for the DRM color
pipeline infrastructure that was introduced in v6.19.[1][2]

The central handling of clean up of colorop from the mode_config list
is missing. While vkms calls drm_colorop_pipeline_destroy() in vkms_destroy(),
amd driver calls it only during failure of the init path and i915/xe driver
does not call it at all. This means amd and intel leaks these objects on
driver removal.

This series adds the teardown of mode_config.colorop_list in drm_mode_config_cleanup().
Since, i915/xe sub-classes the drm_colorop within intel_colorop it was not enough
to just use drm_colorop_pipeline_destroy(). Therefore, this series

- Introduces driver-managed destruction for drm_colorop objects and
  updates core helpers to use driver-provided destroy callbacks.
- Ensures all colorop objects are correctly torn down during
  mode_config cleanup and driver removal.

In addition to that following changes are made in the series
- Fixes enum name lifetime leaks in color pipeline init in i915, amdgpu_dm, and vkms
- Corrects the ordering of the 3D LUT block in the i915 plane color pipeline
- Refactors i915 plane color pipeline initialization to reliably clean
  up partially constructed pipelines on failure.

Thanks for taking a look. Feedback is welcome.

[1] https://lore.kernel.org/dri-devel/cbe00ac4-a535-47d3-813a-e2eda7e9b991@amd.com/
[2] https://lore.kernel.org/intel-gfx/20251203085211.3663374-1-uma.shankar@intel.com/

==
Chaitanya

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Alex Hung <alex.hung@amd.com>
Cc: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Melissa Wen <mwen@igalia.com>
Cc: Simon Ser <contact@emersion.fr>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Chaitanya Kumar Borah (13):
  drm/i915/color: Place 3D LUT after CSC in plane color pipeline
  drm/amd/display: Fix color pipeline enum name leak
  drm/vkms: Fix color pipeline enum name leak
  drm/i915/display: Fix color pipeline enum name leak
  drm: Allow driver-managed destruction of colorop objects
  drm/colorop: Add destroy helper for colorop objects
  drm/amd/display: Hook up colorop destroy helper for plane pipelines
  drm/vkms: Hook up colorop destroy helper for plane pipelines
  drm/i915/display: Hook up intel_colorop_destroy
  drm: Clean up colorop objects during mode_config cleanup
  drm/vkms: Remove drm_colorop_pipeline_destroy() from vkms_destroy()
  drm/colorop: Use destroy callback for color pipeline teardown
  drm/i915/color: Add failure handling in plane color pipeline init

 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c |  31 +++-
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  13 +-
 drivers/gpu/drm/drm_colorop.c                 |  46 +++--
 drivers/gpu/drm/drm_mode_config.c             |   6 +
 .../drm/i915/display/intel_color_pipeline.c   | 161 +++++++++++++-----
 drivers/gpu/drm/i915/display/intel_colorop.c  |   6 +
 drivers/gpu/drm/i915/display/intel_colorop.h  |   1 +
 drivers/gpu/drm/vkms/vkms_colorop.c           |  31 ++--
 drivers/gpu/drm/vkms/vkms_drv.c               |   1 -
 include/drm/drm_colorop.h                     |  40 ++++-
 10 files changed, 249 insertions(+), 87 deletions(-)

-- 
2.25.1

[PATCH 01/13] drm/i915/color: Place 3D LUT after CSC in plane color pipeline

[Chaitanya Kumar Borah]

Commit 65db7a1f9cf7 ("drm/i915/color: Add 3D LUT to color pipeline")
introduced the 3D LUT block before the CSC stage. This ordering is
incorrect.

Move the 3D LUT block to its correct position in the plane color
pipeline:

  [Pre-CSC] -> [CSC] -> [3DLUT] -> [Post-CSC]

Fixes: 65db7a1f9cf7 ("drm/i915/color: Add 3D LUT to color pipeline")
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/i915/display/intel_color_pipeline.c    | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 942d9b9c93ce..684641c8323b 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -39,6 +39,15 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	/* TODO: handle failures and clean up */
 	prev_op = &colorop->base;
 
+	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
+	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
+					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	if (ret)
+		return ret;
+
+	drm_colorop_set_next_property(prev_op, &colorop->base);
+	prev_op = &colorop->base;
+
 	if (DISPLAY_VER(display) >= 35 &&
 	    intel_color_crtc_has_3dlut(display, pipe) &&
 	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
@@ -55,15 +64,6 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 		prev_op = &colorop->base;
 	}
 
-	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
-	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
-					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
-	if (ret)
-		return ret;
-
-	drm_colorop_set_next_property(prev_op, &colorop->base);
-	prev_op = &colorop->base;
-
 	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
 	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
 						  PLANE_GAMMA_SIZE,
-- 
2.25.1

[PATCH 02/13] drm/amd/display: Fix color pipeline enum name leak

[Chaitanya Kumar Borah]

dm_plane_init_colorops() allocates enum names for color pipelines.
These are eventually passed to drm_property_create_enum() which create
its own copies of the string. Free the strings after initialization
is done.

Also, allocate color pipeline enum names only after successfully creating
color pipeline.

Fixes: 9ba25915efba ("drm/amd/display: Add support for sRGB EOTF in DEGAM block")
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c   |  4 +++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 13 +++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index d585618b8064..a2de3bba8346 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -79,7 +79,6 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 
 	list->type = ops[i]->base.id;
-	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[i]->base.id);
 
 	i++;
 
@@ -197,6 +196,9 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 
 	drm_colorop_set_next_property(ops[i-1], ops[i]);
+
+	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);
+
 	return 0;
 
 cleanup:
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 2e3ee78999d9..7c4496fb4b9d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1790,12 +1790,13 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
 static int
 dm_plane_init_colorops(struct drm_plane *plane)
 {
-	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
+	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
 	struct drm_device *dev = plane->dev;
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	struct dc *dc = adev->dm.dc;
 	int len = 0;
-	int ret;
+	int ret = 0;
+	int i;
 
 	if (plane->type == DRM_PLANE_TYPE_CURSOR)
 		return 0;
@@ -1806,7 +1807,7 @@ dm_plane_init_colorops(struct drm_plane *plane)
 		if (ret) {
 			drm_err(plane->dev, "Failed to create color pipeline for plane %d: %d\n",
 				plane->base.id, ret);
-			return ret;
+			goto out;
 		}
 		len++;
 
@@ -1814,7 +1815,11 @@ dm_plane_init_colorops(struct drm_plane *plane)
 		drm_plane_create_color_pipeline_property(plane, pipelines, len);
 	}
 
-	return 0;
+out:
+	for (i = 0; i < len; i++)
+		kfree(pipelines[i].name);
+
+	return ret;
 }
 #endif
 
-- 
2.25.1

[PATCH 03/13] drm/vkms: Fix color pipeline enum name leak

[Chaitanya Kumar Borah]

vkms_initialize_colorops() allocates enum names for color pipelines,
which are copied by drm_property_create_enum(). The temporary strings
were not freed, resulting in a memory leak.

Allocate enum names only after successful pipeline construction and
free them on all exit paths

Fixes: c1e578bd08da ("drm/vkms: Add enumerated 1D curve colorop")
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/vkms/vkms_colorop.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
index 5c3ffc78aea0..d03a1f2e9c41 100644
--- a/drivers/gpu/drm/vkms/vkms_colorop.c
+++ b/drivers/gpu/drm/vkms/vkms_colorop.c
@@ -37,7 +37,6 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 
 	list->type = ops[i]->base.id;
-	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[i]->base.id);
 
 	i++;
 
@@ -88,6 +87,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 
 	drm_colorop_set_next_property(ops[i - 1], ops[i]);
 
+	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);
+
 	return 0;
 
 cleanup:
@@ -103,18 +104,18 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 
 int vkms_initialize_colorops(struct drm_plane *plane)
 {
-	struct drm_prop_enum_list pipeline;
-	int ret;
+	struct drm_prop_enum_list pipeline = {};
+	int ret = 0;
 
 	/* Add color pipeline */
 	ret = vkms_initialize_color_pipeline(plane, &pipeline);
 	if (ret)
-		return ret;
+		goto out;
 
 	/* Create COLOR_PIPELINE property and attach */
 	ret = drm_plane_create_color_pipeline_property(plane, &pipeline, 1);
-	if (ret)
-		return ret;
 
-	return 0;
+	kfree(pipeline.name);
+out:
+	return ret;
 }
-- 
2.25.1

[PATCH 04/13] drm/i915/display: Fix color pipeline enum name leak

[Chaitanya Kumar Borah]

intel_color_pipeline_plane_init() allocates enum names for color
pipelines, which are copied by drm_property_create_enum(). The temporary
strings were not freed, resulting in a memory leak.

Allocate enum names only after successful pipeline construction and free
them on all exit paths.

Fixes: ef105316819d ("drm/i915/color: Create a transfer function color pipeline")
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/i915/display/intel_color_pipeline.c    | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 684641c8323b..04af552b3648 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -34,7 +34,6 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 		return ret;
 
 	list->type = colorop->base.base.id;
-	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", colorop->base.base.id);
 
 	/* TODO: handle failures and clean up */
 	prev_op = &colorop->base;
@@ -74,6 +73,8 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 
 	drm_colorop_set_next_property(prev_op, &colorop->base);
 
+	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
+
 	return 0;
 }
 
@@ -81,9 +82,10 @@ int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
 {
 	struct drm_device *dev = plane->dev;
 	struct intel_display *display = to_intel_display(dev);
-	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
+	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
 	int len = 0;
-	int ret;
+	int ret = 0;
+	int i;
 
 	/* Currently expose pipeline only for HDR planes */
 	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id))
@@ -92,8 +94,14 @@ int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
 	/* Add pipeline consisting of transfer functions */
 	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);
 	if (ret)
-		return ret;
+		goto out;
 	len++;
 
-	return drm_plane_create_color_pipeline_property(plane, pipelines, len);
+	ret = drm_plane_create_color_pipeline_property(plane, pipelines, len);
+
+	for (i = 0; i < len; i++)
+		kfree(pipelines[i].name);
+
+out:
+	return ret;
 }
-- 
2.25.1

[PATCH 05/13] drm: Allow driver-managed destruction of colorop objects

[Chaitanya Kumar Borah]

Some drivers might want to embed struct drm_colorop inside
driver-specific objects, similar to planes or CRTCs. In such
cases, freeing only the drm_colorop is incorrect.

Add a drm_colorop_funcs callback to allow drivers to provide a destroy
hook that cleans up the full enclosing object. Make changes in helper
functions to accept helper functions as argument. Pass NULL for now
to retain current behavior.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 18 ++++++-----
 drivers/gpu/drm/drm_colorop.c                 | 31 +++++++++++++------
 .../drm/i915/display/intel_color_pipeline.c   |  8 ++---
 drivers/gpu/drm/vkms/vkms_colorop.c           | 10 +++---
 include/drm/drm_colorop.h                     | 30 +++++++++++++++---
 5 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index a2de3bba8346..dfdb4fb4219f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -72,7 +72,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
 					      amdgpu_dm_supported_degam_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
@@ -89,7 +89,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL, DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
 
@@ -104,7 +104,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
+					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
 
@@ -120,7 +121,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 			goto cleanup;
 		}
 
-		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
+		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
 						amdgpu_dm_supported_shaper_tfs,
 						DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		if (ret)
@@ -137,7 +138,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 			goto cleanup;
 		}
 
-		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, MAX_COLOR_LUT_ENTRIES,
+		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
+							MAX_COLOR_LUT_ENTRIES,
 							DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 							DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		if (ret)
@@ -154,7 +156,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 			goto cleanup;
 		}
 
-		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, LUT3D_SIZE,
+		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL, LUT3D_SIZE,
 					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
 					DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		if (ret)
@@ -172,7 +174,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
 					      amdgpu_dm_supported_blnd_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
@@ -189,7 +191,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, MAX_COLOR_LUT_ENTRIES,
+	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL, MAX_COLOR_LUT_ENTRIES,
 						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 44eb823585d2..efe61bdd9b24 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -93,7 +93,8 @@ static const struct drm_prop_enum_list drm_colorop_lut3d_interpolation_list[] =
 /* Init Helpers */
 
 static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
-				  struct drm_plane *plane, enum drm_colorop_type type,
+				  struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				  enum drm_colorop_type type,
 				  uint32_t flags)
 {
 	struct drm_mode_config *config = &dev->mode_config;
@@ -109,6 +110,7 @@ static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *co
 	colorop->type = type;
 	colorop->plane = plane;
 	colorop->next = NULL;
+	colorop->funcs = funcs;
 
 	list_add_tail(&colorop->head, &config->colorop_list);
 	colorop->index = config->num_colorop++;
@@ -203,6 +205,7 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
  * @dev: DRM device
  * @colorop: The drm_colorop object to initialize
  * @plane: The associated drm_plane
+ * @funcs: control functions for the new colorop
  * @supported_tfs: A bitfield of supported drm_plane_colorop_curve_1d_init enum values,
  *                 created using BIT(curve_type) and combined with the OR '|'
  *                 operator.
@@ -210,7 +213,8 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
  * @return zero on success, -E value on failure
  */
 int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,
-				    struct drm_plane *plane, u64 supported_tfs, uint32_t flags)
+				    struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				    u64 supported_tfs, uint32_t flags)
 {
 	struct drm_prop_enum_list enum_list[DRM_COLOROP_1D_CURVE_COUNT];
 	int i, len;
@@ -231,7 +235,7 @@ int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *
 		return -EINVAL;
 	}
 
-	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_1D_CURVE, flags);
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_1D_CURVE, flags);
 	if (ret)
 		return ret;
 
@@ -288,20 +292,23 @@ static int drm_colorop_create_data_prop(struct drm_device *dev, struct drm_color
  * @dev: DRM device
  * @colorop: The drm_colorop object to initialize
  * @plane: The associated drm_plane
+ * @funcs: control functions for new colorop
  * @lut_size: LUT size supported by driver
  * @interpolation: 1D LUT interpolation type
  * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
  * @return zero on success, -E value on failure
  */
 int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop *colorop,
-					struct drm_plane *plane, uint32_t lut_size,
+					struct drm_plane *plane,
+					const struct drm_colorop_funcs *funcs,
+					uint32_t lut_size,
 					enum drm_colorop_lut1d_interpolation_type interpolation,
 					uint32_t flags)
 {
 	struct drm_property *prop;
 	int ret;
 
-	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_1D_LUT, flags);
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_1D_LUT, flags);
 	if (ret)
 		return ret;
 
@@ -339,11 +346,12 @@ int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_color
 EXPORT_SYMBOL(drm_plane_colorop_curve_1d_lut_init);
 
 int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
-				   struct drm_plane *plane, uint32_t flags)
+				   struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				   uint32_t flags)
 {
 	int ret;
 
-	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_CTM_3X4, flags);
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_CTM_3X4, flags);
 	if (ret)
 		return ret;
 
@@ -363,16 +371,18 @@ EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
  * @dev: DRM device
  * @colorop: The drm_colorop object to initialize
  * @plane: The associated drm_plane
+ * @funcs: control functions for the new colorop
  * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
  * @return zero on success, -E value on failure
  */
 int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop *colorop,
-				struct drm_plane *plane, uint32_t flags)
+				struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				uint32_t flags)
 {
 	struct drm_property *prop;
 	int ret;
 
-	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_MULTIPLIER, flags);
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_MULTIPLIER, flags);
 	if (ret)
 		return ret;
 
@@ -391,6 +401,7 @@ EXPORT_SYMBOL(drm_plane_colorop_mult_init);
 
 int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *colorop,
 				 struct drm_plane *plane,
+				 const struct drm_colorop_funcs *funcs,
 				 uint32_t lut_size,
 				 enum drm_colorop_lut3d_interpolation_type interpolation,
 				 uint32_t flags)
@@ -398,7 +409,7 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
 	struct drm_property *prop;
 	int ret;
 
-	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_3D_LUT, flags);
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_3D_LUT, flags);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 04af552b3648..d3d73d60727c 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -25,7 +25,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 
 	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
 
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
+	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, NULL,
 						  PLANE_DEGAMMA_SIZE,
 						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
@@ -39,7 +39,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	prev_op = &colorop->base;
 
 	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
-	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
+	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, NULL,
 					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		return ret;
@@ -52,7 +52,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
 		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
 
-		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane, 17,
+		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane, NULL, 17,
 						   DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
 						   true);
 		if (ret)
@@ -64,7 +64,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	}
 
 	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
+	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, NULL,
 						  PLANE_GAMMA_SIZE,
 						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
index d03a1f2e9c41..9e9dd0494628 100644
--- a/drivers/gpu/drm/vkms/vkms_colorop.c
+++ b/drivers/gpu/drm/vkms/vkms_colorop.c
@@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
@@ -48,7 +48,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
+					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
 
@@ -64,7 +65,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
+					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
 
@@ -80,7 +82,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index a3a32f9f918c..45d1b1d3faf9 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -187,6 +187,19 @@ struct drm_colorop_state {
 	struct drm_atomic_state *state;
 };
 
+/**
+ * struct drm_colorop_funcs - driver colorop control functions
+ */
+struct drm_colorop_funcs {
+	/**
+	 * @destroy:
+	 *
+	 * Clean up colorop resources. This is called at driver unload time
+	 * through drm_mode_config_cleanup()
+	 */
+	void (*destroy)(struct drm_colorop *colorop);
+};
+
 /**
  * struct drm_colorop - DRM color operation control structure
  *
@@ -362,6 +375,8 @@ struct drm_colorop {
 	 */
 	struct drm_property *next_property;
 
+	/** @funcs: colorop control functions */
+	const struct drm_colorop_funcs *funcs;
 };
 
 #define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
@@ -390,17 +405,22 @@ void drm_colorop_pipeline_destroy(struct drm_device *dev);
 void drm_colorop_cleanup(struct drm_colorop *colorop);
 
 int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,
-				    struct drm_plane *plane, u64 supported_tfs, uint32_t flags);
+				    struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				    u64 supported_tfs, uint32_t flags);
 int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop *colorop,
-					struct drm_plane *plane, uint32_t lut_size,
+					struct drm_plane *plane,
+					const struct drm_colorop_funcs *funcs,
+					uint32_t lut_size,
 					enum drm_colorop_lut1d_interpolation_type interpolation,
 					uint32_t flags);
 int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
-				   struct drm_plane *plane, uint32_t flags);
+				   struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				   uint32_t flags);
 int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop *colorop,
-				struct drm_plane *plane, uint32_t flags);
+				struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
+				uint32_t flags);
 int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *colorop,
-				 struct drm_plane *plane,
+				 struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
 				 uint32_t lut_size,
 				 enum drm_colorop_lut3d_interpolation_type interpolation,
 				 uint32_t flags);
-- 
2.25.1

[PATCH 06/13] drm/colorop: Add destroy helper for colorop objects

[Chaitanya Kumar Borah]

Add a helper that performs common cleanup and frees the
associated object. This can be used by drivers if they do not
require any driver-specific teardown.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/drm_colorop.c | 12 ++++++++++++
 include/drm/drm_colorop.h     | 10 ++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index efe61bdd9b24..4b27804bb0bd 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -180,6 +180,18 @@ void drm_colorop_cleanup(struct drm_colorop *colorop)
 }
 EXPORT_SYMBOL(drm_colorop_cleanup);
 
+/**
+ * drm_colorop_destroy() - Helper for colorop destruction
+ *
+ * @colorop: colorop to destroy
+ */
+void drm_colorop_destroy(struct drm_colorop *colorop)
+{
+	drm_colorop_cleanup(colorop);
+	kfree(colorop);
+}
+EXPORT_SYMBOL(drm_colorop_destroy);
+
 /**
  * drm_colorop_pipeline_destroy - Helper for color pipeline destruction
  *
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 45d1b1d3faf9..ee6454b08b2d 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -440,6 +440,16 @@ void drm_colorop_atomic_destroy_state(struct drm_colorop *colorop,
  */
 void drm_colorop_reset(struct drm_colorop *colorop);
 
+/**
+ * drm_colorop_destroy - destroy colorop
+ * @colorop: drm colorop
+ *
+ * Destroys @colorop by performing common DRM cleanup and freeing the
+ * colorop object. This can be used by drivers if they do not
+ * require any driver-specific teardown.
+ */
+void drm_colorop_destroy(struct drm_colorop *colorop);
+
 /**
  * drm_colorop_index - find the index of a registered colorop
  * @colorop: colorop to find index for
-- 
2.25.1

[PATCH 07/13] drm/amd/display: Hook up colorop destroy helper for plane pipelines

[Chaitanya Kumar Borah]

Provide a drm_colorop_funcs instance for amdgpu_dm color pipeline
objects and hook up the common drm_colorop_destroy() helper as the
destroy callback.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 25 +++++++++++++------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index dfdb4fb4219f..5130962193d9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -55,6 +55,10 @@ const u64 amdgpu_dm_supported_blnd_tfs =
 
 #define LUT3D_SIZE		17
 
+static const struct drm_colorop_funcs dm_colorop_funcs = {
+	.destroy = drm_colorop_destroy,
+};
+
 int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list)
 {
 	struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
@@ -72,7 +76,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs,
 					      amdgpu_dm_supported_degam_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
@@ -89,7 +93,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL, DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, &dm_colorop_funcs,
+					  DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
 
@@ -104,7 +109,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
+	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
+					     &dm_colorop_funcs,
 					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
@@ -121,7 +127,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 			goto cleanup;
 		}
 
-		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
+		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs,
 						amdgpu_dm_supported_shaper_tfs,
 						DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		if (ret)
@@ -138,7 +144,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 			goto cleanup;
 		}
 
-		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
+		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
+							&dm_colorop_funcs,
 							MAX_COLOR_LUT_ENTRIES,
 							DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 							DRM_COLOROP_FLAG_ALLOW_BYPASS);
@@ -156,7 +163,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 			goto cleanup;
 		}
 
-		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL, LUT3D_SIZE,
+		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane,
+					&dm_colorop_funcs, LUT3D_SIZE,
 					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
 					DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		if (ret)
@@ -174,7 +182,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs,
 					      amdgpu_dm_supported_blnd_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
@@ -191,7 +199,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL, MAX_COLOR_LUT_ENTRIES,
+	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, &dm_colorop_funcs,
+						  MAX_COLOR_LUT_ENTRIES,
 						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
-- 
2.25.1

[PATCH 08/13] drm/vkms: Hook up colorop destroy helper for plane pipelines

[Chaitanya Kumar Borah]

Provide a drm_colorop_funcs instance for vkms color pipeline
objects and hook up the common drm_colorop_destroy() helper as the
destroy callback.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/vkms/vkms_colorop.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
index 9e9dd0494628..ba826ad384b7 100644
--- a/drivers/gpu/drm/vkms/vkms_colorop.c
+++ b/drivers/gpu/drm/vkms/vkms_colorop.c
@@ -12,6 +12,10 @@ static const u64 supported_tfs =
 	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF);
 
+static const struct drm_colorop_funcs vkms_colorop_funcs = {
+	.destroy = drm_colorop_destroy,
+};
+
 #define MAX_COLOR_PIPELINE_OPS 4
 
 static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list)
@@ -31,7 +35,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &vkms_colorop_funcs,
+					      supported_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
@@ -48,7 +53,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
+	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, &vkms_colorop_funcs,
 					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
@@ -65,7 +70,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
+	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, &vkms_colorop_funcs,
 					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
@@ -82,7 +87,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
 		goto cleanup;
 	}
 
-	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
+	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &vkms_colorop_funcs,
+					      supported_tfs,
 					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		goto cleanup;
-- 
2.25.1

[PATCH 09/13] drm/i915/display: Hook up intel_colorop_destroy

[Chaitanya Kumar Borah]

i915 embeds struct drm_colorop inside struct intel_colorop, so the
default drm_colorop_destroy() helper cannot be used. Add an
intel_colorop_destroy() helper that performs common DRM cleanup and
frees intel_colorop object.

This ensures correct teardown of plane color pipeline objects.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color_pipeline.c | 13 +++++++++----
 drivers/gpu/drm/i915/display/intel_colorop.c        |  6 ++++++
 drivers/gpu/drm/i915/display/intel_colorop.h        |  1 +
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index d3d73d60727c..8fecc53540ba 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -13,6 +13,10 @@
 #define PLANE_DEGAMMA_SIZE 128
 #define PLANE_GAMMA_SIZE 32
 
+static const struct drm_colorop_funcs intel_colorop_funcs = {
+	.destroy = intel_colorop_destroy,
+};
+
 static
 int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_enum_list *list,
 				     enum pipe pipe)
@@ -25,7 +29,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 
 	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
 
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, NULL,
+	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, &intel_colorop_funcs,
 						  PLANE_DEGAMMA_SIZE,
 						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
@@ -39,7 +43,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	prev_op = &colorop->base;
 
 	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
-	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, NULL,
+	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, &intel_colorop_funcs,
 					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
 	if (ret)
 		return ret;
@@ -52,7 +56,8 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
 		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
 
-		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane, NULL, 17,
+		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
+						   &intel_colorop_funcs, 17,
 						   DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
 						   true);
 		if (ret)
@@ -64,7 +69,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	}
 
 	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, NULL,
+	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, &intel_colorop_funcs,
 						  PLANE_GAMMA_SIZE,
 						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
diff --git a/drivers/gpu/drm/i915/display/intel_colorop.c b/drivers/gpu/drm/i915/display/intel_colorop.c
index f2fc0d8780ce..fae4383f8518 100644
--- a/drivers/gpu/drm/i915/display/intel_colorop.c
+++ b/drivers/gpu/drm/i915/display/intel_colorop.c
@@ -33,3 +33,9 @@ struct intel_colorop *intel_colorop_create(enum intel_color_block id)
 
 	return colorop;
 }
+
+void intel_colorop_destroy(struct drm_colorop *colorop)
+{
+	drm_colorop_cleanup(colorop);
+	kfree(to_intel_colorop(colorop));
+}
diff --git a/drivers/gpu/drm/i915/display/intel_colorop.h b/drivers/gpu/drm/i915/display/intel_colorop.h
index 21d58eb9f3d0..79824506bbab 100644
--- a/drivers/gpu/drm/i915/display/intel_colorop.h
+++ b/drivers/gpu/drm/i915/display/intel_colorop.h
@@ -11,5 +11,6 @@
 struct intel_colorop *to_intel_colorop(struct drm_colorop *colorop);
 struct intel_colorop *intel_colorop_alloc(void);
 struct intel_colorop *intel_colorop_create(enum intel_color_block id);
+void intel_colorop_destroy(struct drm_colorop *colorop);
 
 #endif /* __INTEL_COLOROP_H__ */
-- 
2.25.1

[PATCH 10/13] drm: Clean up colorop objects during mode_config cleanup

[Chaitanya Kumar Borah]

Tear down all registered drm_colorop objects during
drm_mode_config_cleanup() by invoking their destroy callbacks.

This ensures proper cleanup of color pipeline objects during DRM device
removal.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/drm_mode_config.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index d12db9b0bab8..84ae8a23a367 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -524,6 +524,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	struct drm_property *property, *pt;
 	struct drm_property_blob *blob, *bt;
 	struct drm_plane *plane, *plt;
+	struct drm_colorop *colorop, *copt;
 
 	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
 				 head) {
@@ -553,6 +554,11 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 		drm_property_destroy(dev, property);
 	}
 
+	list_for_each_entry_safe(colorop, copt, &dev->mode_config.colorop_list,
+				 head) {
+		colorop->funcs->destroy(colorop);
+	}
+
 	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
 				 head) {
 		plane->funcs->destroy(plane);
-- 
2.25.1

[PATCH 11/13] drm/vkms: Remove drm_colorop_pipeline_destroy() from vkms_destroy()

[Chaitanya Kumar Borah]

Now that colorops are cleaned from drm_mode_config_cleanup(), remove
drm_colorop_pipeline_destroy() from  vkms_destroy().

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index dd1402f43773..351e17b5885c 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -258,7 +258,6 @@ void vkms_destroy(struct vkms_config *config)
 
 	fdev = config->dev->faux_dev;
 
-	drm_colorop_pipeline_destroy(&config->dev->drm);
 	drm_dev_unregister(&config->dev->drm);
 	drm_atomic_helper_shutdown(&config->dev->drm);
 	devres_release_group(&fdev->dev, NULL);
-- 
2.25.1

[PATCH 12/13] drm/colorop: Use destroy callback for color pipeline teardown

[Chaitanya Kumar Borah]

Switch drm_colorop_pipeline_destroy() to use the driver-provided
destroy callback instead of directly calling drm_colorop_cleanup()
and freeing the object.

This allows drivers that embed struct drm_colorop in driver-specific
objects to perform correct teardown.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/drm_colorop.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 4b27804bb0bd..fafe45b93ff8 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -205,8 +205,7 @@ void drm_colorop_pipeline_destroy(struct drm_device *dev)
 	struct drm_colorop *colorop, *next;
 
 	list_for_each_entry_safe(colorop, next, &config->colorop_list, head) {
-		drm_colorop_cleanup(colorop);
-		kfree(colorop);
+		colorop->funcs->destroy(colorop);
 	}
 }
 EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
-- 
2.25.1

[PATCH 13/13] drm/i915/color: Add failure handling in plane color pipeline init

[Chaitanya Kumar Borah]

The plane color pipeline initialization built up multiple colorop blocks
inline, but did not reliably clean up partially constructed pipelines
when an intermediate step failed. This could lead to leaked colorop
objects and fragile error handling as the pipeline grows.

Refactor the pipeline construction to use a common helper for adding
colorop blocks. This centralizes allocation, initialization, and
teardown logic, allowing the caller to reliably unwind all previously
created colorops on failure.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/i915/display/intel_color_pipeline.c   | 142 ++++++++++++------
 1 file changed, 100 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 8fecc53540ba..035ec3f022cd 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -2,6 +2,8 @@
 /*
  * Copyright © 2025 Intel Corporation
  */
+#include <drm/drm_print.h>
+
 #include "intel_color.h"
 #include "intel_colorop.h"
 #include "intel_color_pipeline.h"
@@ -10,6 +12,7 @@
 #include "skl_universal_plane.h"
 
 #define MAX_COLOR_PIPELINES 1
+#define MAX_COLOROP 4
 #define PLANE_DEGAMMA_SIZE 128
 #define PLANE_GAMMA_SIZE 32
 
@@ -18,69 +21,124 @@ static const struct drm_colorop_funcs intel_colorop_funcs = {
 };
 
 static
-int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_enum_list *list,
-				     enum pipe pipe)
+struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *plane,
+							     struct intel_colorop *prev,
+							     enum intel_color_block id)
 {
 	struct drm_device *dev = plane->dev;
-	struct intel_display *display = to_intel_display(dev);
-	struct drm_colorop *prev_op;
 	struct intel_colorop *colorop;
 	int ret;
 
-	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
-
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, &intel_colorop_funcs,
-						  PLANE_DEGAMMA_SIZE,
-						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
-						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
+	colorop = intel_colorop_create(id);
+
+	if (IS_ERR(colorop))
+		return colorop;
+
+	switch (id) {
+	case INTEL_PLANE_CB_PRE_CSC_LUT:
+		ret = drm_plane_colorop_curve_1d_lut_init(dev,
+							  &colorop->base, plane,
+							  &intel_colorop_funcs,
+							  PLANE_DEGAMMA_SIZE,
+							  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
+							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		break;
+	case INTEL_PLANE_CB_CSC:
+		ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
+						     &intel_colorop_funcs,
+						     DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		break;
+	case INTEL_PLANE_CB_3DLUT:
+		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
+						   &intel_colorop_funcs, 17,
+						   DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
+						   true);
+		break;
+	case INTEL_PLANE_CB_POST_CSC_LUT:
+		ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
+							  &intel_colorop_funcs,
+							  PLANE_GAMMA_SIZE,
+							  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
+							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		break;
+	default:
+		drm_err(plane->dev, "Invalid colorop id [%d]", id);
+		ret = -EINVAL;
+	}
 
 	if (ret)
-		return ret;
+		goto cleanup;
 
-	list->type = colorop->base.base.id;
+	if (prev)
+		drm_colorop_set_next_property(&prev->base, &colorop->base);
 
-	/* TODO: handle failures and clean up */
-	prev_op = &colorop->base;
+	return colorop;
 
-	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
-	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, &intel_colorop_funcs,
-					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
-	if (ret)
-		return ret;
+cleanup:
+	intel_colorop_destroy(&colorop->base);
+	return ERR_PTR(ret);
+}
 
-	drm_colorop_set_next_property(prev_op, &colorop->base);
-	prev_op = &colorop->base;
+static
+int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_enum_list *list,
+				     enum pipe pipe)
+{
+	struct drm_device *dev = plane->dev;
+	struct intel_display *display = to_intel_display(dev);
+	struct intel_colorop *colorop[MAX_COLOROP];
+	int ret = 0;
+	int i = 0;
 
-	if (DISPLAY_VER(display) >= 35 &&
-	    intel_color_crtc_has_3dlut(display, pipe) &&
-	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
-		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
+	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, NULL,
+							    INTEL_PLANE_CB_PRE_CSC_LUT);
+
+	if (IS_ERR(colorop[i])) {
+		ret = PTR_ERR(colorop[i]);
+		goto cleanup;
+	}
+	i++;
 
-		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
-						   &intel_colorop_funcs, 17,
-						   DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
-						   true);
-		if (ret)
-			return ret;
 
-		drm_colorop_set_next_property(prev_op, &colorop->base);
+	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i - 1],
+							    INTEL_PLANE_CB_CSC);
 
-		prev_op = &colorop->base;
+	if (IS_ERR(colorop[i])) {
+		ret = PTR_ERR(colorop[i]);
+		goto cleanup;
 	}
 
-	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
-	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, &intel_colorop_funcs,
-						  PLANE_GAMMA_SIZE,
-						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
-						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
-	if (ret)
-		return ret;
+	i++;
 
-	drm_colorop_set_next_property(prev_op, &colorop->base);
+	if (DISPLAY_VER(display) >= 35 &&
+	    intel_color_crtc_has_3dlut(display, pipe) &&
+	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
+		colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i - 1],
+								    INTEL_PLANE_CB_3DLUT);
+
+		if (IS_ERR(colorop[i])) {
+			ret = PTR_ERR(colorop[i]);
+			goto cleanup;
+		}
+		i++;
+	}
 
-	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
+	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i - 1],
+							    INTEL_PLANE_CB_POST_CSC_LUT);
+	if (IS_ERR(colorop[i])) {
+		ret = PTR_ERR(colorop[i]);
+		goto cleanup;
+	}
+	i++;
+
+	list->type = colorop[0]->base.base.id;
+	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", colorop[0]->base.base.id);
 
 	return 0;
+
+cleanup:
+	while (--i >= 0)
+		intel_colorop_destroy(&colorop[i]->base);
+	return ret;
 }
 
 int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
-- 
2.25.1

RE: [PATCH 01/13] drm/i915/color: Place 3D LUT after CSC in plane color pipeline

[Kandpal, Suraj]

>
> Subject: [PATCH 01/13] drm/i915/color: Place 3D LUT after CSC in plane color
> pipeline
> 
> Commit 65db7a1f9cf7 ("drm/i915/color: Add 3D LUT to color pipeline")

This line is not needed since Fixes tells us what introduces the problematic change.
Maybe add why this ordering was incorrect.
With that addressed,
LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> introduced the 3D LUT block before the CSC stage. This ordering is incorrect.
> 
> Move the 3D LUT block to its correct position in the plane color
> pipeline:
> 
>   [Pre-CSC] -> [CSC] -> [3DLUT] -> [Post-CSC]
> 
> Fixes: 65db7a1f9cf7 ("drm/i915/color: Add 3D LUT to color pipeline")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../drm/i915/display/intel_color_pipeline.c    | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 942d9b9c93ce..684641c8323b 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -39,6 +39,15 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  	/* TODO: handle failures and clean up */
>  	prev_op = &colorop->base;
> 
> +	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	if (ret)
> +		return ret;
> +
> +	drm_colorop_set_next_property(prev_op, &colorop->base);
> +	prev_op = &colorop->base;
> +
>  	if (DISPLAY_VER(display) >= 35 &&
>  	    intel_color_crtc_has_3dlut(display, pipe) &&
>  	    plane->type == DRM_PLANE_TYPE_PRIMARY) { @@ -55,15 +64,6
> @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> drm_prop_en
>  		prev_op = &colorop->base;
>  	}
> 
> -	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> -
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		return ret;
> -
> -	drm_colorop_set_next_property(prev_op, &colorop->base);
> -	prev_op = &colorop->base;
> -
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
>  	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
>  						  PLANE_GAMMA_SIZE,
> --
> 2.25.1

RE: [PATCH 04/13] drm/i915/display: Fix color pipeline enum name leak

[Kandpal, Suraj]

> Subject: [PATCH 04/13] drm/i915/display: Fix color pipeline enum name leak
> 
> intel_color_pipeline_plane_init() allocates enum names for color pipelines,
> which are copied by drm_property_create_enum(). The temporary strings were
> not freed, resulting in a memory leak.
> 
> Allocate enum names only after successful pipeline construction and free them
> on all exit paths.
> 
> Fixes: ef105316819d ("drm/i915/color: Create a transfer function color
> pipeline")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  .../drm/i915/display/intel_color_pipeline.c    | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 684641c8323b..04af552b3648 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -34,7 +34,6 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  		return ret;
> 
>  	list->type = colorop->base.base.id;
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", colorop-
> >base.base.id);
> 
>  	/* TODO: handle failures and clean up */
>  	prev_op = &colorop->base;
> @@ -74,6 +73,8 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
> 
>  	drm_colorop_set_next_property(prev_op, &colorop->base);
> 
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
> +
>  	return 0;
>  }
> 
> @@ -81,9 +82,10 @@ int intel_color_pipeline_plane_init(struct drm_plane
> *plane, enum pipe pipe)  {
>  	struct drm_device *dev = plane->dev;
>  	struct intel_display *display = to_intel_display(dev);
> -	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
> +	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
>  	int len = 0;
> -	int ret;
> +	int ret = 0;
> +	int i;
> 
>  	/* Currently expose pipeline only for HDR planes */
>  	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id)) @@ -92,8
> +94,14 @@ int intel_color_pipeline_plane_init(struct drm_plane *plane, enum
> pipe pipe)
>  	/* Add pipeline consisting of transfer functions */
>  	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);
>  	if (ret)
> -		return ret;
> +		goto out;
>  	len++;
> 
> -	return drm_plane_create_color_pipeline_property(plane, pipelines,
> len);
> +	ret = drm_plane_create_color_pipeline_property(plane, pipelines, len);
> +
> +	for (i = 0; i < len; i++)
> +		kfree(pipelines[i].name);
> +
> +out:
> +	return ret;
>  }
> --
> 2.25.1

RE: [PATCH 05/13] drm: Allow driver-managed destruction of colorop objects

[Kandpal, Suraj]

> Subject: [PATCH 05/13] drm: Allow driver-managed destruction of colorop
> objects
> 
> Some drivers might want to embed struct drm_colorop inside driver-specific
> objects, similar to planes or CRTCs. In such cases, freeing only the drm_colorop
> is incorrect.
> 
> Add a drm_colorop_funcs callback to allow drivers to provide a destroy hook
> that cleans up the full enclosing object. Make changes in helper functions to
> accept helper functions as argument. Pass NULL for now to retain current
> behavior.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 18 ++++++-----
>  drivers/gpu/drm/drm_colorop.c                 | 31 +++++++++++++------
>  .../drm/i915/display/intel_color_pipeline.c   |  8 ++---
>  drivers/gpu/drm/vkms/vkms_colorop.c           | 10 +++---
>  include/drm/drm_colorop.h                     | 30 +++++++++++++++---
>  5 files changed, 66 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index a2de3bba8346..dfdb4fb4219f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -72,7 +72,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> 
> amdgpu_dm_supported_degam_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> @@ -89,7 +89,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_mult_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL,
> +DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -104,7 +104,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -120,7 +121,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> NULL,
> 
> 	amdgpu_dm_supported_shaper_tfs,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -137,7 +138,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> MAX_COLOR_LUT_ENTRIES,
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> NULL,
> +
> 	MAX_COLOR_LUT_ENTRIES,
> 
> 	DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -154,7 +156,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane,
> LUT3D_SIZE,
> +		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL,
> +LUT3D_SIZE,
> 
> 	DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -172,7 +174,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>  					      amdgpu_dm_supported_blnd_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> @@ -189,7 +191,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> MAX_COLOR_LUT_ENTRIES,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
> +MAX_COLOR_LUT_ENTRIES,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 44eb823585d2..efe61bdd9b24 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -93,7 +93,8 @@ static const struct drm_prop_enum_list
> drm_colorop_lut3d_interpolation_list[] =
>  /* Init Helpers */
> 
>  static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				  struct drm_plane *plane, enum
> drm_colorop_type type,
> +				  struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				  enum drm_colorop_type type,
>  				  uint32_t flags)
>  {
>  	struct drm_mode_config *config = &dev->mode_config; @@ -109,6
> +110,7 @@ static int drm_plane_colorop_init(struct drm_device *dev, struct
> drm_colorop *co
>  	colorop->type = type;
>  	colorop->plane = plane;
>  	colorop->next = NULL;
> +	colorop->funcs = funcs;
> 
>  	list_add_tail(&colorop->head, &config->colorop_list);
>  	colorop->index = config->num_colorop++; @@ -203,6 +205,7 @@
> EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>   * @dev: DRM device
>   * @colorop: The drm_colorop object to initialize
>   * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
>   * @supported_tfs: A bitfield of supported drm_plane_colorop_curve_1d_init
> enum values,
>   *                 created using BIT(curve_type) and combined with the OR '|'
>   *                 operator.
> @@ -210,7 +213,8 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>   * @return zero on success, -E value on failure
>   */
>  int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -				    struct drm_plane *plane, u64 supported_tfs,
> uint32_t flags)
> +				    struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				    u64 supported_tfs, uint32_t flags)
>  {
>  	struct drm_prop_enum_list
> enum_list[DRM_COLOROP_1D_CURVE_COUNT];
>  	int i, len;
> @@ -231,7 +235,7 @@ int drm_plane_colorop_curve_1d_init(struct
> drm_device *dev, struct drm_colorop *
>  		return -EINVAL;
>  	}
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_1D_CURVE, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_1D_CURVE, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -288,20 +292,23 @@ static int drm_colorop_create_data_prop(struct
> drm_device *dev, struct drm_color
>   * @dev: DRM device
>   * @colorop: The drm_colorop object to initialize
>   * @plane: The associated drm_plane
> + * @funcs: control functions for new colorop
>   * @lut_size: LUT size supported by driver
>   * @interpolation: 1D LUT interpolation type
>   * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>   * @return zero on success, -E value on failure
>   */
>  int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -					struct drm_plane *plane, uint32_t
> lut_size,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs
> *funcs,
> +					uint32_t lut_size,
>  					enum
> drm_colorop_lut1d_interpolation_type interpolation,
>  					uint32_t flags)
>  {
>  	struct drm_property *prop;
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_1D_LUT, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_1D_LUT, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -339,11 +346,12 @@ int drm_plane_colorop_curve_1d_lut_init(struct
> drm_device *dev, struct drm_color
> EXPORT_SYMBOL(drm_plane_colorop_curve_1d_lut_init);
> 
>  int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -				   struct drm_plane *plane, uint32_t flags)
> +				   struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				   uint32_t flags)
>  {
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_CTM_3X4, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_CTM_3X4, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -363,16 +371,18 @@
> EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
>   * @dev: DRM device
>   * @colorop: The drm_colorop object to initialize
>   * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
>   * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>   * @return zero on success, -E value on failure
>   */
>  int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				struct drm_plane *plane, uint32_t flags)
> +				struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				uint32_t flags)
>  {
>  	struct drm_property *prop;
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_MULTIPLIER, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_MULTIPLIER, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -391,6 +401,7 @@ EXPORT_SYMBOL(drm_plane_colorop_mult_init);
> 
>  int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
>  				 struct drm_plane *plane,
> +				 const struct drm_colorop_funcs *funcs,
>  				 uint32_t lut_size,
>  				 enum drm_colorop_lut3d_interpolation_type
> interpolation,
>  				 uint32_t flags)
> @@ -398,7 +409,7 @@ int drm_plane_colorop_3dlut_init(struct drm_device
> *dev, struct drm_colorop *col
>  	struct drm_property *prop;
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_3D_LUT, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_3D_LUT, flags);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 04af552b3648..d3d73d60727c 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -25,7 +25,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
> +NULL,
>  						  PLANE_DEGAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); @@ -39,7 +39,7 @@ int
> _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  	prev_op = &colorop->base;
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> NULL,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		return ret;
> @@ -52,7 +52,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>  		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> plane, 17,
> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> plane, NULL,
> +17,
> 
> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>  						   true);
>  		if (ret)
> @@ -64,7 +64,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  	}
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
> +NULL,
>  						  PLANE_GAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); diff --git
> a/drivers/gpu/drm/vkms/vkms_colorop.c
> b/drivers/gpu/drm/vkms/vkms_colorop.c
> index d03a1f2e9c41..9e9dd0494628 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +supported_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> @@ -48,7 +48,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -64,7 +65,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -80,7 +82,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +supported_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index
> a3a32f9f918c..45d1b1d3faf9 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -187,6 +187,19 @@ struct drm_colorop_state {
>  	struct drm_atomic_state *state;
>  };
> 
> +/**
> + * struct drm_colorop_funcs - driver colorop control functions  */
> +struct drm_colorop_funcs {
> +	/**
> +	 * @destroy:
> +	 *
> +	 * Clean up colorop resources. This is called at driver unload time
> +	 * through drm_mode_config_cleanup()
> +	 */
> +	void (*destroy)(struct drm_colorop *colorop); };
> +
>  /**
>   * struct drm_colorop - DRM color operation control structure
>   *
> @@ -362,6 +375,8 @@ struct drm_colorop {
>  	 */
>  	struct drm_property *next_property;
> 
> +	/** @funcs: colorop control functions */
> +	const struct drm_colorop_funcs *funcs;
>  };
> 
>  #define obj_to_colorop(x) container_of(x, struct drm_colorop, base) @@ -
> 390,17 +405,22 @@ void drm_colorop_pipeline_destroy(struct drm_device
> *dev);  void drm_colorop_cleanup(struct drm_colorop *colorop);
> 
>  int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -				    struct drm_plane *plane, u64 supported_tfs,
> uint32_t flags);
> +				    struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				    u64 supported_tfs, uint32_t flags);
>  int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -					struct drm_plane *plane, uint32_t
> lut_size,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs
> *funcs,
> +					uint32_t lut_size,
>  					enum
> drm_colorop_lut1d_interpolation_type interpolation,
>  					uint32_t flags);
>  int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -				   struct drm_plane *plane, uint32_t flags);
> +				   struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				   uint32_t flags);
>  int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				struct drm_plane *plane, uint32_t flags);
> +				struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				uint32_t flags);
>  int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				 struct drm_plane *plane,
> +				 struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
>  				 uint32_t lut_size,
>  				 enum drm_colorop_lut3d_interpolation_type
> interpolation,
>  				 uint32_t flags);
> --
> 2.25.1

RE: [PATCH 06/13] drm/colorop: Add destroy helper for colorop objects

[Kandpal, Suraj]

> Subject: [PATCH 06/13] drm/colorop: Add destroy helper for colorop objects
> 
> Add a helper that performs common cleanup and frees the associated object.
> This can be used by drivers if they do not require any driver-specific teardown.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/drm_colorop.c | 12 ++++++++++++
>  include/drm/drm_colorop.h     | 10 ++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index efe61bdd9b24..4b27804bb0bd 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -180,6 +180,18 @@ void drm_colorop_cleanup(struct drm_colorop
> *colorop)  }  EXPORT_SYMBOL(drm_colorop_cleanup);
> 
> +/**
> + * drm_colorop_destroy() - Helper for colorop destruction
> + *
> + * @colorop: colorop to destroy
> + */
> +void drm_colorop_destroy(struct drm_colorop *colorop) {
> +	drm_colorop_cleanup(colorop);
> +	kfree(colorop);
> +}
> +EXPORT_SYMBOL(drm_colorop_destroy);
> +
>  /**
>   * drm_colorop_pipeline_destroy - Helper for color pipeline destruction
>   *
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index
> 45d1b1d3faf9..ee6454b08b2d 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -440,6 +440,16 @@ void drm_colorop_atomic_destroy_state(struct
> drm_colorop *colorop,
>   */
>  void drm_colorop_reset(struct drm_colorop *colorop);
> 
> +/**
> + * drm_colorop_destroy - destroy colorop
> + * @colorop: drm colorop
> + *
> + * Destroys @colorop by performing common DRM cleanup and freeing the
> + * colorop object. This can be used by drivers if they do not
> + * require any driver-specific teardown.
> + */
> +void drm_colorop_destroy(struct drm_colorop *colorop);
> +
>  /**
>   * drm_colorop_index - find the index of a registered colorop
>   * @colorop: colorop to find index for
> --
> 2.25.1

RE: [PATCH 09/13] drm/i915/display: Hook up intel_colorop_destroy

[Kandpal, Suraj]

> Subject: [PATCH 09/13] drm/i915/display: Hook up intel_colorop_destroy
> 
> i915 embeds struct drm_colorop inside struct intel_colorop, so the default
> drm_colorop_destroy() helper cannot be used. Add an
> intel_colorop_destroy() helper that performs common DRM cleanup and frees
> intel_colorop object.
> 
> This ensures correct teardown of plane color pipeline objects.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_color_pipeline.c | 13 +++++++++----
>  drivers/gpu/drm/i915/display/intel_colorop.c        |  6 ++++++
>  drivers/gpu/drm/i915/display/intel_colorop.h        |  1 +
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index d3d73d60727c..8fecc53540ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -13,6 +13,10 @@
>  #define PLANE_DEGAMMA_SIZE 128
>  #define PLANE_GAMMA_SIZE 32
> 
> +static const struct drm_colorop_funcs intel_colorop_funcs = {
> +	.destroy = intel_colorop_destroy,
> +};
> +
>  static
>  int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> drm_prop_enum_list *list,
>  				     enum pipe pipe)
> @@ -25,7 +29,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane, NULL,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
> +&intel_colorop_funcs,
>  						  PLANE_DEGAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); @@ -39,7 +43,7 @@ int
> _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  	prev_op = &colorop->base;
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +&intel_colorop_funcs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		return ret;
> @@ -52,7 +56,8 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>  		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> plane, NULL, 17,
> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> plane,
> +						   &intel_colorop_funcs, 17,
> 
> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>  						   true);
>  		if (ret)
> @@ -64,7 +69,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  	}
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane, NULL,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane,
> +&intel_colorop_funcs,
>  						  PLANE_GAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); diff --git
> a/drivers/gpu/drm/i915/display/intel_colorop.c
> b/drivers/gpu/drm/i915/display/intel_colorop.c
> index f2fc0d8780ce..fae4383f8518 100644
> --- a/drivers/gpu/drm/i915/display/intel_colorop.c
> +++ b/drivers/gpu/drm/i915/display/intel_colorop.c
> @@ -33,3 +33,9 @@ struct intel_colorop *intel_colorop_create(enum
> intel_color_block id)
> 
>  	return colorop;
>  }
> +
> +void intel_colorop_destroy(struct drm_colorop *colorop) {
> +	drm_colorop_cleanup(colorop);
> +	kfree(to_intel_colorop(colorop));
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_colorop.h
> b/drivers/gpu/drm/i915/display/intel_colorop.h
> index 21d58eb9f3d0..79824506bbab 100644
> --- a/drivers/gpu/drm/i915/display/intel_colorop.h
> +++ b/drivers/gpu/drm/i915/display/intel_colorop.h
> @@ -11,5 +11,6 @@
>  struct intel_colorop *to_intel_colorop(struct drm_colorop *colorop);  struct
> intel_colorop *intel_colorop_alloc(void);  struct intel_colorop
> *intel_colorop_create(enum intel_color_block id);
> +void intel_colorop_destroy(struct drm_colorop *colorop);
> 
>  #endif /* __INTEL_COLOROP_H__ */
> --
> 2.25.1

RE: [PATCH 10/13] drm: Clean up colorop objects during mode_config cleanup

[Kandpal, Suraj]

> Subject: [PATCH 10/13] drm: Clean up colorop objects during mode_config
> cleanup
> 
> Tear down all registered drm_colorop objects during
> drm_mode_config_cleanup() by invoking their destroy callbacks.
> 
> This ensures proper cleanup of color pipeline objects during DRM device
> removal.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/drm_mode_config.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index d12db9b0bab8..84ae8a23a367 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -524,6 +524,7 @@ void drm_mode_config_cleanup(struct drm_device
> *dev)
>  	struct drm_property *property, *pt;
>  	struct drm_property_blob *blob, *bt;
>  	struct drm_plane *plane, *plt;
> +	struct drm_colorop *colorop, *copt;
> 
>  	list_for_each_entry_safe(encoder, enct, &dev-
> >mode_config.encoder_list,
>  				 head) {
> @@ -553,6 +554,11 @@ void drm_mode_config_cleanup(struct drm_device
> *dev)
>  		drm_property_destroy(dev, property);
>  	}
> 
> +	list_for_each_entry_safe(colorop, copt, &dev-
> >mode_config.colorop_list,
> +				 head) {
> +		colorop->funcs->destroy(colorop);
> +	}
> +
>  	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
>  				 head) {
>  		plane->funcs->destroy(plane);
> --
> 2.25.1

RE: [PATCH 12/13] drm/colorop: Use destroy callback for color pipeline teardown

[Kandpal, Suraj]

> Subject: [PATCH 12/13] drm/colorop: Use destroy callback for color pipeline
> teardown
> 
> Switch drm_colorop_pipeline_destroy() to use the driver-provided destroy
> callback instead of directly calling drm_colorop_cleanup() and freeing the
> object.
> 
> This allows drivers that embed struct drm_colorop in driver-specific objects to
> perform correct teardown.

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/drm_colorop.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 4b27804bb0bd..fafe45b93ff8 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -205,8 +205,7 @@ void drm_colorop_pipeline_destroy(struct drm_device
> *dev)
>  	struct drm_colorop *colorop, *next;
> 
>  	list_for_each_entry_safe(colorop, next, &config->colorop_list, head) {
> -		drm_colorop_cleanup(colorop);
> -		kfree(colorop);
> +		colorop->funcs->destroy(colorop);
>  	}
>  }
>  EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
> --
> 2.25.1

RE: [PATCH 13/13] drm/i915/color: Add failure handling in plane color pipeline init

[Kandpal, Suraj]

> Subject: [PATCH 13/13] drm/i915/color: Add failure handling in plane color
> pipeline init
> 
> The plane color pipeline initialization built up multiple colorop blocks inline,
> but did not reliably clean up partially constructed pipelines when an
> intermediate step failed. This could lead to leaked colorop objects and fragile
> error handling as the pipeline grows.
> 
> Refactor the pipeline construction to use a common helper for adding colorop
> blocks. This centralizes allocation, initialization, and teardown logic, allowing
> the caller to reliably unwind all previously created colorops on failure.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../drm/i915/display/intel_color_pipeline.c   | 142 ++++++++++++------
>  1 file changed, 100 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 8fecc53540ba..035ec3f022cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -2,6 +2,8 @@
>  /*
>   * Copyright © 2025 Intel Corporation
>   */
> +#include <drm/drm_print.h>
> +
>  #include "intel_color.h"
>  #include "intel_colorop.h"
>  #include "intel_color_pipeline.h"
> @@ -10,6 +12,7 @@
>  #include "skl_universal_plane.h"
> 
>  #define MAX_COLOR_PIPELINES 1
> +#define MAX_COLOROP 4
>  #define PLANE_DEGAMMA_SIZE 128
>  #define PLANE_GAMMA_SIZE 32
> 
> @@ -18,69 +21,124 @@ static const struct drm_colorop_funcs
> intel_colorop_funcs = {  };
> 
>  static
> -int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> drm_prop_enum_list *list,
> -				     enum pipe pipe)
> +struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct
> drm_plane *plane,
> +							     struct
> intel_colorop *prev,
> +							     enum
> intel_color_block id)
>  {

Seems like you just added a new function and then changed the function 
_intel_color_pipeline_plane_init but git format-patch messed this patch up bit
Maybe try use --patience option to make this patch more readable.
 
>  	struct drm_device *dev = plane->dev;
> -	struct intel_display *display = to_intel_display(dev);
> -	struct drm_colorop *prev_op;
>  	struct intel_colorop *colorop;
>  	int ret;
> 
> -	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
> -
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane, &intel_colorop_funcs,
> -						  PLANE_DEGAMMA_SIZE,
> -
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> -
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	colorop = intel_colorop_create(id);
> +
> +	if (IS_ERR(colorop))
> +		return colorop;
> +
> +	switch (id) {
> +	case INTEL_PLANE_CB_PRE_CSC_LUT:
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev,
> +							  &colorop->base,
> plane,
> +
> &intel_colorop_funcs,
> +
> PLANE_DEGAMMA_SIZE,
> +
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		break;
> +	case INTEL_PLANE_CB_CSC:
> +		ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base,
> plane,
> +						     &intel_colorop_funcs,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		break;
> +	case INTEL_PLANE_CB_3DLUT:
> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> plane,
> +						   &intel_colorop_funcs, 17,
> +
> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> +						   true);
> +		break;
> +	case INTEL_PLANE_CB_POST_CSC_LUT:
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop-
> >base, plane,
> +
> &intel_colorop_funcs,
> +
> PLANE_GAMMA_SIZE,
> +
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		break;
> +	default:
> +		drm_err(plane->dev, "Invalid colorop id [%d]", id);
> +		ret = -EINVAL;
> +	}
> 
>  	if (ret)
> -		return ret;
> +		goto cleanup;
> 
> -	list->type = colorop->base.base.id;
> +	if (prev)
> +		drm_colorop_set_next_property(&prev->base, &colorop-
> >base);
> 
> -	/* TODO: handle failures and clean up */
> -	prev_op = &colorop->base;
> +	return colorop;
> 
> -	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> &intel_colorop_funcs,
> -
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		return ret;
> +cleanup:
> +	intel_colorop_destroy(&colorop->base);
> +	return ERR_PTR(ret);
> +}
> 
> -	drm_colorop_set_next_property(prev_op, &colorop->base);
> -	prev_op = &colorop->base;
> +static
> +int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> drm_prop_enum_list *list,
> +				     enum pipe pipe)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct intel_display *display = to_intel_display(dev);
> +	struct intel_colorop *colorop[MAX_COLOROP];
> +	int ret = 0;
> +	int i = 0;
> 
> -	if (DISPLAY_VER(display) >= 35 &&
> -	    intel_color_crtc_has_3dlut(display, pipe) &&
> -	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
> -		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, NULL,
> +
> INTEL_PLANE_CB_PRE_CSC_LUT);
> +
> +	if (IS_ERR(colorop[i])) {
> +		ret = PTR_ERR(colorop[i]);
> +		goto cleanup;
> +	}
> +	i++;

I see a lot of repeated code maybe we can get this done via a loop
static const enum intel_colorop_type pipeline[] = {
        INTEL_PLANE_CB_PRE_CSC_LUT,
        INTEL_PLANE_CB_CSC,
        INTEL_PLANE_CB_3DLUT,
        INTEL_PLANE_CB_POST_CSC_LUT,
};

for (n = 0; n < ARRAY_SIZE(pipeline); n++) {
        if (pipeline[n] == INTEL_PLANE_CB_3DLUT &&
            (DISPLAY_VER(display) < 35 ||
             plane->type != DRM_PLANE_TYPE_PRIMARY ||
             !intel_color_crtc_has_3dlut(display, pipe)))
                continue;

        ret = add_plane_colorop(plane, colorop, &i, &prev, pipeline[n]);
        if (ret)
                goto cleanup;
}

Where add_plane_colorop is 

static int
add_plane_colorop(struct drm_plane *plane,
                  struct intel_colorop **colorop,
                  int *i,
                  struct intel_colorop **prev,
                  enum intel_colorop_type type)
{
        colorop[*i] = intel_color_pipeline_plane_add_colorop(plane, *prev, type);
        if (IS_ERR(colorop[*i]))
                return PTR_ERR(colorop[*i]);

        *prev = colorop[*i];
        (*i)++;

        return 0;
} 

Regards,
Suraj Kandpal 

> 
> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> plane,
> -						   &intel_colorop_funcs, 17,
> -
> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> -						   true);
> -		if (ret)
> -			return ret;
> 
> -		drm_colorop_set_next_property(prev_op, &colorop->base);
> +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i -
> 1],
> +
> INTEL_PLANE_CB_CSC);
> 
> -		prev_op = &colorop->base;
> +	if (IS_ERR(colorop[i])) {
> +		ret = PTR_ERR(colorop[i]);
> +		goto cleanup;
>  	}
> 
> -	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> plane, &intel_colorop_funcs,
> -						  PLANE_GAMMA_SIZE,
> -
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> -
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		return ret;
> +	i++;
> 
> -	drm_colorop_set_next_property(prev_op, &colorop->base);
> +	if (DISPLAY_VER(display) >= 35 &&
> +	    intel_color_crtc_has_3dlut(display, pipe) &&
> +	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
> +		colorop[i] = intel_color_pipeline_plane_add_colorop(plane,
> colorop[i - 1],
> +
> INTEL_PLANE_CB_3DLUT);
> +
> +		if (IS_ERR(colorop[i])) {
> +			ret = PTR_ERR(colorop[i]);
> +			goto cleanup;
> +		}
> +		i++;
> +	}
> 
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
> +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i -
> 1],
> +
> INTEL_PLANE_CB_POST_CSC_LUT);
> +	if (IS_ERR(colorop[i])) {
> +		ret = PTR_ERR(colorop[i]);
> +		goto cleanup;
> +	}
> +	i++;
> +
> +	list->type = colorop[0]->base.base.id;
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d",
> +colorop[0]->base.base.id);
> 
>  	return 0;
> +
> +cleanup:
> +	while (--i >= 0)
> +		intel_colorop_destroy(&colorop[i]->base);
> +	return ret;
>  }
> 
>  int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
> --
> 2.25.1

RE: [PATCH 01/13] drm/i915/color: Place 3D LUT after CSC in plane color pipeline

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 01/13] drm/i915/color: Place 3D LUT after CSC in plane color
> pipeline
> 
> Commit 65db7a1f9cf7 ("drm/i915/color: Add 3D LUT to color pipeline") introduced
> the 3D LUT block before the CSC stage. This ordering is incorrect.
> 
> Move the 3D LUT block to its correct position in the plane color
> pipeline:
> 
>   [Pre-CSC] -> [CSC] -> [3DLUT] -> [Post-CSC]

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Fixes: 65db7a1f9cf7 ("drm/i915/color: Add 3D LUT to color pipeline")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../drm/i915/display/intel_color_pipeline.c    | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 942d9b9c93ce..684641c8323b 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -39,6 +39,15 @@ int _intel_color_pipeline_plane_init(struct drm_plane
> *plane, struct drm_prop_en
>  	/* TODO: handle failures and clean up */
>  	prev_op = &colorop->base;
> 
> +	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	if (ret)
> +		return ret;
> +
> +	drm_colorop_set_next_property(prev_op, &colorop->base);
> +	prev_op = &colorop->base;
> +
>  	if (DISPLAY_VER(display) >= 35 &&
>  	    intel_color_crtc_has_3dlut(display, pipe) &&
>  	    plane->type == DRM_PLANE_TYPE_PRIMARY) { @@ -55,15 +64,6
> @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> drm_prop_en
>  		prev_op = &colorop->base;
>  	}
> 
> -	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> -
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		return ret;
> -
> -	drm_colorop_set_next_property(prev_op, &colorop->base);
> -	prev_op = &colorop->base;
> -
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
>  	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
>  						  PLANE_GAMMA_SIZE,
> --
> 2.25.1

RE: [PATCH 02/13] drm/amd/display: Fix color pipeline enum name leak

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 02/13] drm/amd/display: Fix color pipeline enum name leak
> 
> dm_plane_init_colorops() allocates enum names for color pipelines.
> These are eventually passed to drm_property_create_enum() which create its own
> copies of the string. Free the strings after initialization is done.
> 
> Also, allocate color pipeline enum names only after successfully creating color
> pipeline.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

@harry.wentland@amd.com, @alex.hung@amd.com Please ack if looks ok to you as well.

Regards,
Uma Shankar

> Fixes: 9ba25915efba ("drm/amd/display: Add support for sRGB EOTF in DEGAM
> block")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c   |  4 +++-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 13 +++++++++----
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index d585618b8064..a2de3bba8346 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -79,7 +79,6 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
> 
>  	list->type = ops[i]->base.id;
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[i]-
> >base.id);
> 
>  	i++;
> 
> @@ -197,6 +196,9 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
> 
>  	drm_colorop_set_next_property(ops[i-1], ops[i]);
> +
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d",
> +ops[0]->base.id);
> +
>  	return 0;
> 
>  cleanup:
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 2e3ee78999d9..7c4496fb4b9d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1790,12 +1790,13 @@ dm_atomic_plane_get_property(struct drm_plane
> *plane,  static int  dm_plane_init_colorops(struct drm_plane *plane)  {
> -	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
> +	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
>  	struct drm_device *dev = plane->dev;
>  	struct amdgpu_device *adev = drm_to_adev(dev);
>  	struct dc *dc = adev->dm.dc;
>  	int len = 0;
> -	int ret;
> +	int ret = 0;
> +	int i;
> 
>  	if (plane->type == DRM_PLANE_TYPE_CURSOR)
>  		return 0;
> @@ -1806,7 +1807,7 @@ dm_plane_init_colorops(struct drm_plane *plane)
>  		if (ret) {
>  			drm_err(plane->dev, "Failed to create color pipeline for
> plane %d: %d\n",
>  				plane->base.id, ret);
> -			return ret;
> +			goto out;
>  		}
>  		len++;
> 
> @@ -1814,7 +1815,11 @@ dm_plane_init_colorops(struct drm_plane *plane)
>  		drm_plane_create_color_pipeline_property(plane, pipelines, len);
>  	}
> 
> -	return 0;
> +out:
> +	for (i = 0; i < len; i++)
> +		kfree(pipelines[i].name);
> +
> +	return ret;
>  }
>  #endif
> 
> --
> 2.25.1

RE: [PATCH 03/13] drm/vkms: Fix color pipeline enum name leak

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 03/13] drm/vkms: Fix color pipeline enum name leak
> 
> vkms_initialize_colorops() allocates enum names for color pipelines, which are
> copied by drm_property_create_enum(). The temporary strings were not freed,
> resulting in a memory leak.
> 
> Allocate enum names only after successful pipeline construction and free them on
> all exit paths

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Fixes: c1e578bd08da ("drm/vkms: Add enumerated 1D curve colorop")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/vkms/vkms_colorop.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c
> b/drivers/gpu/drm/vkms/vkms_colorop.c
> index 5c3ffc78aea0..d03a1f2e9c41 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -37,7 +37,6 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
> 
>  	list->type = ops[i]->base.id;
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[i]-
> >base.id);
> 
>  	i++;
> 
> @@ -88,6 +87,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
> 
>  	drm_colorop_set_next_property(ops[i - 1], ops[i]);
> 
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d",
> +ops[0]->base.id);
> +
>  	return 0;
> 
>  cleanup:
> @@ -103,18 +104,18 @@ static int vkms_initialize_color_pipeline(struct
> drm_plane *plane, struct drm_pr
> 
>  int vkms_initialize_colorops(struct drm_plane *plane)  {
> -	struct drm_prop_enum_list pipeline;
> -	int ret;
> +	struct drm_prop_enum_list pipeline = {};
> +	int ret = 0;
> 
>  	/* Add color pipeline */
>  	ret = vkms_initialize_color_pipeline(plane, &pipeline);
>  	if (ret)
> -		return ret;
> +		goto out;
> 
>  	/* Create COLOR_PIPELINE property and attach */
>  	ret = drm_plane_create_color_pipeline_property(plane, &pipeline, 1);
> -	if (ret)
> -		return ret;
> 
> -	return 0;
> +	kfree(pipeline.name);
> +out:
> +	return ret;
>  }
> --
> 2.25.1

RE: [PATCH 04/13] drm/i915/display: Fix color pipeline enum name leak

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 04/13] drm/i915/display: Fix color pipeline enum name leak
> 
> intel_color_pipeline_plane_init() allocates enum names for color pipelines, which
> are copied by drm_property_create_enum(). The temporary strings were not freed,
> resulting in a memory leak.
> 
> Allocate enum names only after successful pipeline construction and free them on
> all exit paths.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Fixes: ef105316819d ("drm/i915/color: Create a transfer function color pipeline")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../drm/i915/display/intel_color_pipeline.c    | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 684641c8323b..04af552b3648 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -34,7 +34,6 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
>  		return ret;
> 
>  	list->type = colorop->base.base.id;
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", colorop-
> >base.base.id);
> 
>  	/* TODO: handle failures and clean up */
>  	prev_op = &colorop->base;
> @@ -74,6 +73,8 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
> 
>  	drm_colorop_set_next_property(prev_op, &colorop->base);
> 
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
> +
>  	return 0;
>  }
> 
> @@ -81,9 +82,10 @@ int intel_color_pipeline_plane_init(struct drm_plane *plane,
> enum pipe pipe)  {
>  	struct drm_device *dev = plane->dev;
>  	struct intel_display *display = to_intel_display(dev);
> -	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
> +	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
>  	int len = 0;
> -	int ret;
> +	int ret = 0;
> +	int i;
> 
>  	/* Currently expose pipeline only for HDR planes */
>  	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id)) @@ -92,8 +94,14
> @@ int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
>  	/* Add pipeline consisting of transfer functions */
>  	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);
>  	if (ret)
> -		return ret;
> +		goto out;
>  	len++;
> 
> -	return drm_plane_create_color_pipeline_property(plane, pipelines, len);
> +	ret = drm_plane_create_color_pipeline_property(plane, pipelines, len);
> +
> +	for (i = 0; i < len; i++)
> +		kfree(pipelines[i].name);
> +
> +out:
> +	return ret;
>  }
> --
> 2.25.1

RE: [PATCH 05/13] drm: Allow driver-managed destruction of colorop objects

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 05/13] drm: Allow driver-managed destruction of colorop objects
> 
> Some drivers might want to embed struct drm_colorop inside driver-specific
> objects, similar to planes or CRTCs. In such cases, freeing only the drm_colorop
> is incorrect.
> 
> Add a drm_colorop_funcs callback to allow drivers to provide a destroy hook that
> cleans up the full enclosing object. Make changes in helper functions to accept
> helper functions as argument. Pass NULL for now to retain current behavior.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 18 ++++++-----
>  drivers/gpu/drm/drm_colorop.c                 | 31 +++++++++++++------
>  .../drm/i915/display/intel_color_pipeline.c   |  8 ++---
>  drivers/gpu/drm/vkms/vkms_colorop.c           | 10 +++---
>  include/drm/drm_colorop.h                     | 30 +++++++++++++++---
>  5 files changed, 66 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index a2de3bba8346..dfdb4fb4219f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -72,7 +72,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>  					      amdgpu_dm_supported_degam_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> @@ -89,7 +89,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_mult_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL,
> +DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -104,7 +104,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -120,7 +121,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> 
> 	amdgpu_dm_supported_shaper_tfs,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -137,7 +138,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> MAX_COLOR_LUT_ENTRIES,
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> NULL,
> +
> 	MAX_COLOR_LUT_ENTRIES,
> 
> 	DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -154,7 +156,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane,
> LUT3D_SIZE,
> +		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL,
> +LUT3D_SIZE,
> 
> 	DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -172,7 +174,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>  					      amdgpu_dm_supported_blnd_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> @@ -189,7 +191,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> MAX_COLOR_LUT_ENTRIES,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
> +MAX_COLOR_LUT_ENTRIES,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c index
> 44eb823585d2..efe61bdd9b24 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -93,7 +93,8 @@ static const struct drm_prop_enum_list
> drm_colorop_lut3d_interpolation_list[] =
>  /* Init Helpers */
> 
>  static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				  struct drm_plane *plane, enum
> drm_colorop_type type,
> +				  struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				  enum drm_colorop_type type,
>  				  uint32_t flags)
>  {
>  	struct drm_mode_config *config = &dev->mode_config; @@ -109,6
> +110,7 @@ static int drm_plane_colorop_init(struct drm_device *dev, struct
> drm_colorop *co
>  	colorop->type = type;
>  	colorop->plane = plane;
>  	colorop->next = NULL;
> +	colorop->funcs = funcs;
> 
>  	list_add_tail(&colorop->head, &config->colorop_list);
>  	colorop->index = config->num_colorop++; @@ -203,6 +205,7 @@
> EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>   * @dev: DRM device
>   * @colorop: The drm_colorop object to initialize
>   * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
>   * @supported_tfs: A bitfield of supported drm_plane_colorop_curve_1d_init enum
> values,
>   *                 created using BIT(curve_type) and combined with the OR '|'
>   *                 operator.
> @@ -210,7 +213,8 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>   * @return zero on success, -E value on failure
>   */
>  int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				    struct drm_plane *plane, u64 supported_tfs,
> uint32_t flags)
> +				    struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				    u64 supported_tfs, uint32_t flags)
>  {
>  	struct drm_prop_enum_list
> enum_list[DRM_COLOROP_1D_CURVE_COUNT];
>  	int i, len;
> @@ -231,7 +235,7 @@ int drm_plane_colorop_curve_1d_init(struct drm_device
> *dev, struct drm_colorop *
>  		return -EINVAL;
>  	}
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_1D_CURVE, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_1D_CURVE, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -288,20 +292,23 @@ static int drm_colorop_create_data_prop(struct
> drm_device *dev, struct drm_color
>   * @dev: DRM device
>   * @colorop: The drm_colorop object to initialize
>   * @plane: The associated drm_plane
> + * @funcs: control functions for new colorop
>   * @lut_size: LUT size supported by driver
>   * @interpolation: 1D LUT interpolation type
>   * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>   * @return zero on success, -E value on failure
>   */
>  int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -					struct drm_plane *plane, uint32_t lut_size,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs *funcs,
> +					uint32_t lut_size,
>  					enum
> drm_colorop_lut1d_interpolation_type interpolation,
>  					uint32_t flags)
>  {
>  	struct drm_property *prop;
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_1D_LUT, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_1D_LUT, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -339,11 +346,12 @@ int drm_plane_colorop_curve_1d_lut_init(struct
> drm_device *dev, struct drm_color
> EXPORT_SYMBOL(drm_plane_colorop_curve_1d_lut_init);
> 
>  int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				   struct drm_plane *plane, uint32_t flags)
> +				   struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				   uint32_t flags)
>  {
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_CTM_3X4, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_CTM_3X4, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -363,16 +371,18 @@ EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
>   * @dev: DRM device
>   * @colorop: The drm_colorop object to initialize
>   * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
>   * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>   * @return zero on success, -E value on failure
>   */
>  int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				struct drm_plane *plane, uint32_t flags)
> +				struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				uint32_t flags)
>  {
>  	struct drm_property *prop;
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_MULTIPLIER, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_MULTIPLIER, flags);
>  	if (ret)
>  		return ret;
> 
> @@ -391,6 +401,7 @@ EXPORT_SYMBOL(drm_plane_colorop_mult_init);
> 
>  int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
>  				 struct drm_plane *plane,
> +				 const struct drm_colorop_funcs *funcs,
>  				 uint32_t lut_size,
>  				 enum drm_colorop_lut3d_interpolation_type
> interpolation,
>  				 uint32_t flags)
> @@ -398,7 +409,7 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev,
> struct drm_colorop *col
>  	struct drm_property *prop;
>  	int ret;
> 
> -	ret = drm_plane_colorop_init(dev, colorop, plane,
> DRM_COLOROP_3D_LUT, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> +DRM_COLOROP_3D_LUT, flags);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 04af552b3648..d3d73d60727c 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -25,7 +25,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +NULL,
>  						  PLANE_DEGAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); @@ -39,7 +39,7 @@ int
> _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  	prev_op = &colorop->base;
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, NULL,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		return ret;
> @@ -52,7 +52,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
>  	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>  		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
> 17,
> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
> NULL,
> +17,
> 
> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>  						   true);
>  		if (ret)
> @@ -64,7 +64,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
>  	}
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +NULL,
>  						  PLANE_GAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); diff --git
> a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
> index d03a1f2e9c41..9e9dd0494628 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +supported_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> @@ -48,7 +48,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -64,7 +65,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -80,7 +82,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +supported_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index
> a3a32f9f918c..45d1b1d3faf9 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -187,6 +187,19 @@ struct drm_colorop_state {
>  	struct drm_atomic_state *state;
>  };
> 
> +/**
> + * struct drm_colorop_funcs - driver colorop control functions  */
> +struct drm_colorop_funcs {
> +	/**
> +	 * @destroy:
> +	 *
> +	 * Clean up colorop resources. This is called at driver unload time
> +	 * through drm_mode_config_cleanup()
> +	 */
> +	void (*destroy)(struct drm_colorop *colorop); };
> +
>  /**
>   * struct drm_colorop - DRM color operation control structure
>   *
> @@ -362,6 +375,8 @@ struct drm_colorop {
>  	 */
>  	struct drm_property *next_property;
> 
> +	/** @funcs: colorop control functions */
> +	const struct drm_colorop_funcs *funcs;
>  };
> 
>  #define obj_to_colorop(x) container_of(x, struct drm_colorop, base) @@ -390,17
> +405,22 @@ void drm_colorop_pipeline_destroy(struct drm_device *dev);  void
> drm_colorop_cleanup(struct drm_colorop *colorop);
> 
>  int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				    struct drm_plane *plane, u64 supported_tfs,
> uint32_t flags);
> +				    struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				    u64 supported_tfs, uint32_t flags);
>  int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> -					struct drm_plane *plane, uint32_t lut_size,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs *funcs,
> +					uint32_t lut_size,
>  					enum
> drm_colorop_lut1d_interpolation_type interpolation,
>  					uint32_t flags);
>  int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				   struct drm_plane *plane, uint32_t flags);
> +				   struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				   uint32_t flags);
>  int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				struct drm_plane *plane, uint32_t flags);
> +				struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
> +				uint32_t flags);
>  int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> -				 struct drm_plane *plane,
> +				 struct drm_plane *plane, const struct
> drm_colorop_funcs *funcs,
>  				 uint32_t lut_size,
>  				 enum drm_colorop_lut3d_interpolation_type
> interpolation,
>  				 uint32_t flags);
> --
> 2.25.1

RE: [PATCH 06/13] drm/colorop: Add destroy helper for colorop objects

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 06/13] drm/colorop: Add destroy helper for colorop objects
> 
> Add a helper that performs common cleanup and frees the associated object. This
> can be used by drivers if they do not require any driver-specific teardown.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/drm_colorop.c | 12 ++++++++++++
>  include/drm/drm_colorop.h     | 10 ++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c index
> efe61bdd9b24..4b27804bb0bd 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -180,6 +180,18 @@ void drm_colorop_cleanup(struct drm_colorop *colorop)
> }  EXPORT_SYMBOL(drm_colorop_cleanup);
> 
> +/**
> + * drm_colorop_destroy() - Helper for colorop destruction
> + *
> + * @colorop: colorop to destroy
> + */
> +void drm_colorop_destroy(struct drm_colorop *colorop) {
> +	drm_colorop_cleanup(colorop);
> +	kfree(colorop);
> +}
> +EXPORT_SYMBOL(drm_colorop_destroy);
> +
>  /**
>   * drm_colorop_pipeline_destroy - Helper for color pipeline destruction
>   *
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index
> 45d1b1d3faf9..ee6454b08b2d 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -440,6 +440,16 @@ void drm_colorop_atomic_destroy_state(struct
> drm_colorop *colorop,
>   */
>  void drm_colorop_reset(struct drm_colorop *colorop);
> 
> +/**
> + * drm_colorop_destroy - destroy colorop
> + * @colorop: drm colorop
> + *
> + * Destroys @colorop by performing common DRM cleanup and freeing the
> + * colorop object. This can be used by drivers if they do not
> + * require any driver-specific teardown.
> + */
> +void drm_colorop_destroy(struct drm_colorop *colorop);
> +
>  /**
>   * drm_colorop_index - find the index of a registered colorop
>   * @colorop: colorop to find index for
> --
> 2.25.1

RE: [PATCH 07/13] drm/amd/display: Hook up colorop destroy helper for plane pipelines

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 07/13] drm/amd/display: Hook up colorop destroy helper for
> plane pipelines
> 
> Provide a drm_colorop_funcs instance for amdgpu_dm color pipeline objects and
> hook up the common drm_colorop_destroy() helper as the destroy callback.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 25 +++++++++++++------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index dfdb4fb4219f..5130962193d9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -55,6 +55,10 @@ const u64 amdgpu_dm_supported_blnd_tfs =
> 
>  #define LUT3D_SIZE		17
> 
> +static const struct drm_colorop_funcs dm_colorop_funcs = {
> +	.destroy = drm_colorop_destroy,
> +};
> +
>  int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct
> drm_prop_enum_list *list)  {
>  	struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS]; @@ -72,7 +76,7
> @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct
> drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +&dm_colorop_funcs,
>  					      amdgpu_dm_supported_degam_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> @@ -89,7 +93,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL,
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, &dm_colorop_funcs,
> +
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> 
> @@ -104,7 +109,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> +					     &dm_colorop_funcs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> @@ -121,7 +127,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +&dm_colorop_funcs,
> 
> 	amdgpu_dm_supported_shaper_tfs,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -138,7 +144,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> NULL,
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> +							&dm_colorop_funcs,
> 
> 	MAX_COLOR_LUT_ENTRIES,
> 
> 	DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
> @@ -156,7 +163,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  			goto cleanup;
>  		}
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL,
> LUT3D_SIZE,
> +		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane,
> +					&dm_colorop_funcs, LUT3D_SIZE,
> 
> 	DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> 
> 	DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		if (ret)
> @@ -174,7 +182,7 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +&dm_colorop_funcs,
>  					      amdgpu_dm_supported_blnd_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> @@ -191,7 +199,8 @@ int amdgpu_dm_initialize_default_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
> MAX_COLOR_LUT_ENTRIES,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> &dm_colorop_funcs,
> +						  MAX_COLOR_LUT_ENTRIES,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
> --
> 2.25.1

RE: [PATCH 08/13] drm/vkms: Hook up colorop destroy helper for plane pipelines

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 08/13] drm/vkms: Hook up colorop destroy helper for plane
> pipelines
> 
> Provide a drm_colorop_funcs instance for vkms color pipeline objects and hook up
> the common drm_colorop_destroy() helper as the destroy callback.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/vkms/vkms_colorop.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c
> b/drivers/gpu/drm/vkms/vkms_colorop.c
> index 9e9dd0494628..ba826ad384b7 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -12,6 +12,10 @@ static const u64 supported_tfs =
>  	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
>  	BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF);
> 
> +static const struct drm_colorop_funcs vkms_colorop_funcs = {
> +	.destroy = drm_colorop_destroy,
> +};
> +
>  #define MAX_COLOR_PIPELINE_OPS 4
> 
>  static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct
> drm_prop_enum_list *list) @@ -31,7 +35,8 @@ static int
> vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> &vkms_colorop_funcs,
> +					      supported_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> @@ -48,7 +53,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> +&vkms_colorop_funcs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> @@ -65,7 +70,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> +&vkms_colorop_funcs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> @@ -82,7 +87,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane
> *plane, struct drm_pr
>  		goto cleanup;
>  	}
> 
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> &vkms_colorop_funcs,
> +					      supported_tfs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		goto cleanup;
> --
> 2.25.1

RE: [PATCH 09/13] drm/i915/display: Hook up intel_colorop_destroy

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 09/13] drm/i915/display: Hook up intel_colorop_destroy
> 
> i915 embeds struct drm_colorop inside struct intel_colorop, so the default
> drm_colorop_destroy() helper cannot be used. Add an
> intel_colorop_destroy() helper that performs common DRM cleanup and frees
> intel_colorop object.
> 
> This ensures correct teardown of plane color pipeline objects.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color_pipeline.c | 13 +++++++++----
>  drivers/gpu/drm/i915/display/intel_colorop.c        |  6 ++++++
>  drivers/gpu/drm/i915/display/intel_colorop.h        |  1 +
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index d3d73d60727c..8fecc53540ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -13,6 +13,10 @@
>  #define PLANE_DEGAMMA_SIZE 128
>  #define PLANE_GAMMA_SIZE 32
> 
> +static const struct drm_colorop_funcs intel_colorop_funcs = {
> +	.destroy = intel_colorop_destroy,
> +};
> +
>  static
>  int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> drm_prop_enum_list *list,
>  				     enum pipe pipe)
> @@ -25,7 +29,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
> 
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> NULL,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +&intel_colorop_funcs,
>  						  PLANE_DEGAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); @@ -39,7 +43,7 @@ int
> _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  	prev_op = &colorop->base;
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +&intel_colorop_funcs,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  	if (ret)
>  		return ret;
> @@ -52,7 +56,8 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
>  	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>  		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> 
> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
> NULL, 17,
> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane,
> +						   &intel_colorop_funcs, 17,
> 
> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>  						   true);
>  		if (ret)
> @@ -64,7 +69,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane,
> struct drm_prop_en
>  	}
> 
>  	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> NULL,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +&intel_colorop_funcs,
>  						  PLANE_GAMMA_SIZE,
> 
> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> 
> DRM_COLOROP_FLAG_ALLOW_BYPASS); diff --git
> a/drivers/gpu/drm/i915/display/intel_colorop.c
> b/drivers/gpu/drm/i915/display/intel_colorop.c
> index f2fc0d8780ce..fae4383f8518 100644
> --- a/drivers/gpu/drm/i915/display/intel_colorop.c
> +++ b/drivers/gpu/drm/i915/display/intel_colorop.c
> @@ -33,3 +33,9 @@ struct intel_colorop *intel_colorop_create(enum
> intel_color_block id)
> 
>  	return colorop;
>  }
> +
> +void intel_colorop_destroy(struct drm_colorop *colorop) {
> +	drm_colorop_cleanup(colorop);
> +	kfree(to_intel_colorop(colorop));
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_colorop.h
> b/drivers/gpu/drm/i915/display/intel_colorop.h
> index 21d58eb9f3d0..79824506bbab 100644
> --- a/drivers/gpu/drm/i915/display/intel_colorop.h
> +++ b/drivers/gpu/drm/i915/display/intel_colorop.h
> @@ -11,5 +11,6 @@
>  struct intel_colorop *to_intel_colorop(struct drm_colorop *colorop);  struct
> intel_colorop *intel_colorop_alloc(void);  struct intel_colorop
> *intel_colorop_create(enum intel_color_block id);
> +void intel_colorop_destroy(struct drm_colorop *colorop);
> 
>  #endif /* __INTEL_COLOROP_H__ */
> --
> 2.25.1

RE: [PATCH 10/13] drm: Clean up colorop objects during mode_config cleanup

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 10/13] drm: Clean up colorop objects during mode_config
> cleanup
> 
> Tear down all registered drm_colorop objects during
> drm_mode_config_cleanup() by invoking their destroy callbacks.
> 
> This ensures proper cleanup of color pipeline objects during DRM device removal.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/drm_mode_config.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index d12db9b0bab8..84ae8a23a367 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -524,6 +524,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  	struct drm_property *property, *pt;
>  	struct drm_property_blob *blob, *bt;
>  	struct drm_plane *plane, *plt;
> +	struct drm_colorop *colorop, *copt;
> 
>  	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
>  				 head) {
> @@ -553,6 +554,11 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  		drm_property_destroy(dev, property);
>  	}
> 
> +	list_for_each_entry_safe(colorop, copt, &dev->mode_config.colorop_list,
> +				 head) {
> +		colorop->funcs->destroy(colorop);
> +	}
> +
>  	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
>  				 head) {
>  		plane->funcs->destroy(plane);
> --
> 2.25.1

RE: [PATCH 11/13] drm/vkms: Remove drm_colorop_pipeline_destroy() from vkms_destroy()

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 11/13] drm/vkms: Remove drm_colorop_pipeline_destroy() from
> vkms_destroy()
> 
> Now that colorops are cleaned from drm_mode_config_cleanup(), remove
> drm_colorop_pipeline_destroy() from  vkms_destroy().

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/vkms/vkms_drv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index dd1402f43773..351e17b5885c 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -258,7 +258,6 @@ void vkms_destroy(struct vkms_config *config)
> 
>  	fdev = config->dev->faux_dev;
> 
> -	drm_colorop_pipeline_destroy(&config->dev->drm);
>  	drm_dev_unregister(&config->dev->drm);
>  	drm_atomic_helper_shutdown(&config->dev->drm);
>  	devres_release_group(&fdev->dev, NULL);
> --
> 2.25.1

RE: [PATCH 12/13] drm/colorop: Use destroy callback for color pipeline teardown

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Sent: Friday, December 19, 2025 12:26 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> nfraprado@collabora.com; ville.syrjala@linux.intel.com; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: [PATCH 12/13] drm/colorop: Use destroy callback for color pipeline
> teardown
> 
> Switch drm_colorop_pipeline_destroy() to use the driver-provided destroy callback
> instead of directly calling drm_colorop_cleanup() and freeing the object.
> 
> This allows drivers that embed struct drm_colorop in driver-specific objects to
> perform correct teardown.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/drm_colorop.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c index
> 4b27804bb0bd..fafe45b93ff8 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -205,8 +205,7 @@ void drm_colorop_pipeline_destroy(struct drm_device
> *dev)
>  	struct drm_colorop *colorop, *next;
> 
>  	list_for_each_entry_safe(colorop, next, &config->colorop_list, head) {
> -		drm_colorop_cleanup(colorop);
> -		kfree(colorop);
> +		colorop->funcs->destroy(colorop);
>  	}
>  }
>  EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
> --
> 2.25.1

RE: [PATCH 13/13] drm/i915/color: Add failure handling in plane color pipeline init

[Shankar, Uma]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Tuesday, January 6, 2026 9:25 AM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; dri-
> devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: harry.wentland@amd.com; jani.nikula@linux.intel.com;
> louis.chauvet@bootlin.com; mwen@igalia.com; contact@emersion.fr;
> alex.hung@amd.com; daniels@collabora.com; Shankar, Uma
> <uma.shankar@intel.com>; nfraprado@collabora.com;
> ville.syrjala@linux.intel.com; Roper, Matthew D <matthew.d.roper@intel.com>;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: RE: [PATCH 13/13] drm/i915/color: Add failure handling in plane color
> pipeline init
> 
> > Subject: [PATCH 13/13] drm/i915/color: Add failure handling in plane
> > color pipeline init
> >
> > The plane color pipeline initialization built up multiple colorop
> > blocks inline, but did not reliably clean up partially constructed
> > pipelines when an intermediate step failed. This could lead to leaked
> > colorop objects and fragile error handling as the pipeline grows.
> >
> > Refactor the pipeline construction to use a common helper for adding
> > colorop blocks. This centralizes allocation, initialization, and
> > teardown logic, allowing the caller to reliably unwind all previously created
> colorops on failure.
> >
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > ---
> >  .../drm/i915/display/intel_color_pipeline.c   | 142 ++++++++++++------
> >  1 file changed, 100 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> > b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> > index 8fecc53540ba..035ec3f022cd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> > @@ -2,6 +2,8 @@
> >  /*
> >   * Copyright © 2025 Intel Corporation
> >   */
> > +#include <drm/drm_print.h>
> > +
> >  #include "intel_color.h"
> >  #include "intel_colorop.h"
> >  #include "intel_color_pipeline.h"
> > @@ -10,6 +12,7 @@
> >  #include "skl_universal_plane.h"
> >
> >  #define MAX_COLOR_PIPELINES 1
> > +#define MAX_COLOROP 4
> >  #define PLANE_DEGAMMA_SIZE 128
> >  #define PLANE_GAMMA_SIZE 32
> >
> > @@ -18,69 +21,124 @@ static const struct drm_colorop_funcs
> > intel_colorop_funcs = {  };
> >
> >  static
> > -int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> > drm_prop_enum_list *list,
> > -				     enum pipe pipe)
> > +struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct
> > drm_plane *plane,
> > +							     struct
> > intel_colorop *prev,
> > +							     enum
> > intel_color_block id)
> >  {
> 
> Seems like you just added a new function and then changed the function
> _intel_color_pipeline_plane_init but git format-patch messed this patch up bit
> Maybe try use --patience option to make this patch more readable.

Yeah the refactor created some weirdness but logically looks ok.
Check if it can be made more readable. Maybe break it in 2 patches if required,
one to introduce the helper and next using the new helper.

> >  	struct drm_device *dev = plane->dev;
> > -	struct intel_display *display = to_intel_display(dev);
> > -	struct drm_colorop *prev_op;
> >  	struct intel_colorop *colorop;
> >  	int ret;
> >
> > -	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
> > -
> > -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> > plane, &intel_colorop_funcs,
> > -						  PLANE_DEGAMMA_SIZE,
> > -
> > DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> > -
> > DRM_COLOROP_FLAG_ALLOW_BYPASS);
> > +	colorop = intel_colorop_create(id);
> > +
> > +	if (IS_ERR(colorop))
> > +		return colorop;
> > +
> > +	switch (id) {
> > +	case INTEL_PLANE_CB_PRE_CSC_LUT:
> > +		ret = drm_plane_colorop_curve_1d_lut_init(dev,
> > +							  &colorop->base,
> > plane,
> > +
> > &intel_colorop_funcs,
> > +
> > PLANE_DEGAMMA_SIZE,
> > +
> > DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> > +
> > DRM_COLOROP_FLAG_ALLOW_BYPASS);
> > +		break;
> > +	case INTEL_PLANE_CB_CSC:
> > +		ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base,
> > plane,
> > +						     &intel_colorop_funcs,
> > +
> > DRM_COLOROP_FLAG_ALLOW_BYPASS);
> > +		break;
> > +	case INTEL_PLANE_CB_3DLUT:
> > +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> > plane,
> > +						   &intel_colorop_funcs, 17,
> > +
> > DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> > +						   true);
> > +		break;
> > +	case INTEL_PLANE_CB_POST_CSC_LUT:
> > +		ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop-
> > >base, plane,
> > +
> > &intel_colorop_funcs,
> > +
> > PLANE_GAMMA_SIZE,
> > +
> > DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> > +
> > DRM_COLOROP_FLAG_ALLOW_BYPASS);
> > +		break;
> > +	default:
> > +		drm_err(plane->dev, "Invalid colorop id [%d]", id);
> > +		ret = -EINVAL;
> > +	}
> >
> >  	if (ret)
> > -		return ret;
> > +		goto cleanup;
> >
> > -	list->type = colorop->base.base.id;
> > +	if (prev)
> > +		drm_colorop_set_next_property(&prev->base, &colorop-
> > >base);
> >
> > -	/* TODO: handle failures and clean up */
> > -	prev_op = &colorop->base;
> > +	return colorop;
> >
> > -	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> > -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> > &intel_colorop_funcs,
> > -
> > DRM_COLOROP_FLAG_ALLOW_BYPASS);
> > -	if (ret)
> > -		return ret;
> > +cleanup:
> > +	intel_colorop_destroy(&colorop->base);
> > +	return ERR_PTR(ret);
> > +}
> >
> > -	drm_colorop_set_next_property(prev_op, &colorop->base);
> > -	prev_op = &colorop->base;
> > +static
> > +int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
> > drm_prop_enum_list *list,
> > +				     enum pipe pipe)
> > +{
> > +	struct drm_device *dev = plane->dev;
> > +	struct intel_display *display = to_intel_display(dev);
> > +	struct intel_colorop *colorop[MAX_COLOROP];
> > +	int ret = 0;
> > +	int i = 0;
> >
> > -	if (DISPLAY_VER(display) >= 35 &&
> > -	    intel_color_crtc_has_3dlut(display, pipe) &&
> > -	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> > +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, NULL,
> > +
> > INTEL_PLANE_CB_PRE_CSC_LUT);
> > +
> > +	if (IS_ERR(colorop[i])) {
> > +		ret = PTR_ERR(colorop[i]);
> > +		goto cleanup;
> > +	}
> > +	i++;
> 
> I see a lot of repeated code maybe we can get this done via a loop static const
> enum intel_colorop_type pipeline[] = {
>         INTEL_PLANE_CB_PRE_CSC_LUT,
>         INTEL_PLANE_CB_CSC,
>         INTEL_PLANE_CB_3DLUT,
>         INTEL_PLANE_CB_POST_CSC_LUT,
> };
> 
> for (n = 0; n < ARRAY_SIZE(pipeline); n++) {
>         if (pipeline[n] == INTEL_PLANE_CB_3DLUT &&
>             (DISPLAY_VER(display) < 35 ||
>              plane->type != DRM_PLANE_TYPE_PRIMARY ||
>              !intel_color_crtc_has_3dlut(display, pipe)))
>                 continue;
> 
>         ret = add_plane_colorop(plane, colorop, &i, &prev, pipeline[n]);
>         if (ret)
>                 goto cleanup;
> }
> 
> Where add_plane_colorop is
> 
> static int
> add_plane_colorop(struct drm_plane *plane,
>                   struct intel_colorop **colorop,
>                   int *i,
>                   struct intel_colorop **prev,
>                   enum intel_colorop_type type) {
>         colorop[*i] = intel_color_pipeline_plane_add_colorop(plane, *prev, type);
>         if (IS_ERR(colorop[*i]))
>                 return PTR_ERR(colorop[*i]);
> 
>         *prev = colorop[*i];
>         (*i)++;
> 
>         return 0;
> }

This can be explored, we can create pipeline wise array entries and traverse them in loop
to make it clearer and more concise. To create a specific pipeline, we can just pass the array.

Overall changes in the series look good to me. Thanks for working on fixing the leaks.

Regards,
Uma Shankar

> >
> > -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
> > plane,
> > -						   &intel_colorop_funcs, 17,
> > -
> > DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> > -						   true);
> > -		if (ret)
> > -			return ret;
> >
> > -		drm_colorop_set_next_property(prev_op, &colorop->base);
> > +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i
> > +-
> > 1],
> > +
> > INTEL_PLANE_CB_CSC);
> >
> > -		prev_op = &colorop->base;
> > +	if (IS_ERR(colorop[i])) {
> > +		ret = PTR_ERR(colorop[i]);
> > +		goto cleanup;
> >  	}
> >
> > -	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> > -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
> > plane, &intel_colorop_funcs,
> > -						  PLANE_GAMMA_SIZE,
> > -
> > DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> > -
> > DRM_COLOROP_FLAG_ALLOW_BYPASS);
> > -	if (ret)
> > -		return ret;
> > +	i++;
> >
> > -	drm_colorop_set_next_property(prev_op, &colorop->base);
> > +	if (DISPLAY_VER(display) >= 35 &&
> > +	    intel_color_crtc_has_3dlut(display, pipe) &&
> > +	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > +		colorop[i] = intel_color_pipeline_plane_add_colorop(plane,
> > colorop[i - 1],
> > +
> > INTEL_PLANE_CB_3DLUT);
> > +
> > +		if (IS_ERR(colorop[i])) {
> > +			ret = PTR_ERR(colorop[i]);
> > +			goto cleanup;
> > +		}
> > +		i++;
> > +	}
> >
> > -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
> > +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i
> > +-
> > 1],
> > +
> > INTEL_PLANE_CB_POST_CSC_LUT);
> > +	if (IS_ERR(colorop[i])) {
> > +		ret = PTR_ERR(colorop[i]);
> > +		goto cleanup;
> > +	}
> > +	i++;
> > +
> > +	list->type = colorop[0]->base.base.id;
> > +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d",
> > +colorop[0]->base.base.id);
> >
> >  	return 0;
> > +
> > +cleanup:
> > +	while (--i >= 0)
> > +		intel_colorop_destroy(&colorop[i]->base);
> > +	return ret;
> >  }
> >
> >  int intel_color_pipeline_plane_init(struct drm_plane *plane, enum
> > pipe pipe)
> > --
> > 2.25.1

Re: [PATCH 02/13] drm/amd/display: Fix color pipeline enum name leak

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> dm_plane_init_colorops() allocates enum names for color pipelines.
> These are eventually passed to drm_property_create_enum() which create
> its own copies of the string. Free the strings after initialization
> is done.
> 
> Also, allocate color pipeline enum names only after successfully creating
> color pipeline.
> 
> Fixes: 9ba25915efba ("drm/amd/display: Add support for sRGB EOTF in DEGAM block")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c   |  4 +++-
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 13 +++++++++----
>   2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index d585618b8064..a2de3bba8346 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -79,7 +79,6 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   
>   	list->type = ops[i]->base.id;
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[i]->base.id);
>   
>   	i++;
>   
> @@ -197,6 +196,9 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   
>   	drm_colorop_set_next_property(ops[i-1], ops[i]);
> +
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);
> +
>   	return 0;
>   
>   cleanup:
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 2e3ee78999d9..7c4496fb4b9d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1790,12 +1790,13 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
>   static int
>   dm_plane_init_colorops(struct drm_plane *plane)
>   {
> -	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
> +	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
>   	struct drm_device *dev = plane->dev;
>   	struct amdgpu_device *adev = drm_to_adev(dev);
>   	struct dc *dc = adev->dm.dc;
>   	int len = 0;
> -	int ret;
> +	int ret = 0;
> +	int i;
>   
>   	if (plane->type == DRM_PLANE_TYPE_CURSOR)
>   		return 0;
> @@ -1806,7 +1807,7 @@ dm_plane_init_colorops(struct drm_plane *plane)
>   		if (ret) {
>   			drm_err(plane->dev, "Failed to create color pipeline for plane %d: %d\n",
>   				plane->base.id, ret);
> -			return ret;
> +			goto out;
>   		}
>   		len++;
>   
> @@ -1814,7 +1815,11 @@ dm_plane_init_colorops(struct drm_plane *plane)
>   		drm_plane_create_color_pipeline_property(plane, pipelines, len);
>   	}
>   
> -	return 0;
> +out:
> +	for (i = 0; i < len; i++)
> +		kfree(pipelines[i].name);
> +
> +	return ret;
>   }
>   #endif
>   

Re: [PATCH 03/13] drm/vkms: Fix color pipeline enum name leak

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> vkms_initialize_colorops() allocates enum names for color pipelines,
> which are copied by drm_property_create_enum(). The temporary strings
> were not freed, resulting in a memory leak.
> 
> Allocate enum names only after successful pipeline construction and
> free them on all exit paths
> 
> Fixes: c1e578bd08da ("drm/vkms: Add enumerated 1D curve colorop")
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   drivers/gpu/drm/vkms/vkms_colorop.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
> index 5c3ffc78aea0..d03a1f2e9c41 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -37,7 +37,6 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   
>   	list->type = ops[i]->base.id;
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[i]->base.id);
>   
>   	i++;
>   
> @@ -88,6 +87,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   
>   	drm_colorop_set_next_property(ops[i - 1], ops[i]);
>   
> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);
> +
>   	return 0;
>   
>   cleanup:
> @@ -103,18 +104,18 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   
>   int vkms_initialize_colorops(struct drm_plane *plane)
>   {
> -	struct drm_prop_enum_list pipeline;
> -	int ret;
> +	struct drm_prop_enum_list pipeline = {};
> +	int ret = 0;
>   
>   	/* Add color pipeline */
>   	ret = vkms_initialize_color_pipeline(plane, &pipeline);
>   	if (ret)
> -		return ret;
> +		goto out;
>   
>   	/* Create COLOR_PIPELINE property and attach */
>   	ret = drm_plane_create_color_pipeline_property(plane, &pipeline, 1);
> -	if (ret)
> -		return ret;
>   
> -	return 0;
> +	kfree(pipeline.name);
> +out:
> +	return ret;
>   }

Re: [PATCH 05/13] drm: Allow driver-managed destruction of colorop objects

[Alex Hung]

It is a nitpick, but is it better to have patch 6 first, and then 
separate changes to drm to a single patch, and move the changes in 
amdgpu, vkms and i915 in the following corresponding patches (7, 8 and 9)?

We can avoid passing NULL to struct drm_colorop_funcs *funcs.

Otherwise it looks good to me.

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> Some drivers might want to embed struct drm_colorop inside
> driver-specific objects, similar to planes or CRTCs. In such
> cases, freeing only the drm_colorop is incorrect.
> 
> Add a drm_colorop_funcs callback to allow drivers to provide a destroy
> hook that cleans up the full enclosing object. Make changes in helper
> functions to accept helper functions as argument. Pass NULL for now
> to retain current behavior.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 18 ++++++-----
>   drivers/gpu/drm/drm_colorop.c                 | 31 +++++++++++++------
>   .../drm/i915/display/intel_color_pipeline.c   |  8 ++---
>   drivers/gpu/drm/vkms/vkms_colorop.c           | 10 +++---
>   include/drm/drm_colorop.h                     | 30 +++++++++++++++---
>   5 files changed, 66 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index a2de3bba8346..dfdb4fb4219f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -72,7 +72,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>   					      amdgpu_dm_supported_degam_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
> @@ -89,7 +89,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL, DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
>   
> @@ -104,7 +104,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
>   
> @@ -120,7 +121,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   			goto cleanup;
>   		}
>   
> -		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>   						amdgpu_dm_supported_shaper_tfs,
>   						DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   		if (ret)
> @@ -137,7 +138,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   			goto cleanup;
>   		}
>   
> -		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, MAX_COLOR_LUT_ENTRIES,
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
> +							MAX_COLOR_LUT_ENTRIES,
>   							DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>   							DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   		if (ret)
> @@ -154,7 +156,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   			goto cleanup;
>   		}
>   
> -		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, LUT3D_SIZE,
> +		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL, LUT3D_SIZE,
>   					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>   					DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   		if (ret)
> @@ -172,7 +174,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>   					      amdgpu_dm_supported_blnd_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
> @@ -189,7 +191,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, MAX_COLOR_LUT_ENTRIES,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL, MAX_COLOR_LUT_ENTRIES,
>   						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>   						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 44eb823585d2..efe61bdd9b24 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -93,7 +93,8 @@ static const struct drm_prop_enum_list drm_colorop_lut3d_interpolation_list[] =
>   /* Init Helpers */
>   
>   static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				  struct drm_plane *plane, enum drm_colorop_type type,
> +				  struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				  enum drm_colorop_type type,
>   				  uint32_t flags)
>   {
>   	struct drm_mode_config *config = &dev->mode_config;
> @@ -109,6 +110,7 @@ static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *co
>   	colorop->type = type;
>   	colorop->plane = plane;
>   	colorop->next = NULL;
> +	colorop->funcs = funcs;
>   
>   	list_add_tail(&colorop->head, &config->colorop_list);
>   	colorop->index = config->num_colorop++;
> @@ -203,6 +205,7 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>    * @dev: DRM device
>    * @colorop: The drm_colorop object to initialize
>    * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
>    * @supported_tfs: A bitfield of supported drm_plane_colorop_curve_1d_init enum values,
>    *                 created using BIT(curve_type) and combined with the OR '|'
>    *                 operator.
> @@ -210,7 +213,8 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>    * @return zero on success, -E value on failure
>    */
>   int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				    struct drm_plane *plane, u64 supported_tfs, uint32_t flags)
> +				    struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				    u64 supported_tfs, uint32_t flags)
>   {
>   	struct drm_prop_enum_list enum_list[DRM_COLOROP_1D_CURVE_COUNT];
>   	int i, len;
> @@ -231,7 +235,7 @@ int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *
>   		return -EINVAL;
>   	}
>   
> -	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_1D_CURVE, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_1D_CURVE, flags);
>   	if (ret)
>   		return ret;
>   
> @@ -288,20 +292,23 @@ static int drm_colorop_create_data_prop(struct drm_device *dev, struct drm_color
>    * @dev: DRM device
>    * @colorop: The drm_colorop object to initialize
>    * @plane: The associated drm_plane
> + * @funcs: control functions for new colorop
>    * @lut_size: LUT size supported by driver
>    * @interpolation: 1D LUT interpolation type
>    * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>    * @return zero on success, -E value on failure
>    */
>   int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop *colorop,
> -					struct drm_plane *plane, uint32_t lut_size,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs *funcs,
> +					uint32_t lut_size,
>   					enum drm_colorop_lut1d_interpolation_type interpolation,
>   					uint32_t flags)
>   {
>   	struct drm_property *prop;
>   	int ret;
>   
> -	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_1D_LUT, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_1D_LUT, flags);
>   	if (ret)
>   		return ret;
>   
> @@ -339,11 +346,12 @@ int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_color
>   EXPORT_SYMBOL(drm_plane_colorop_curve_1d_lut_init);
>   
>   int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				   struct drm_plane *plane, uint32_t flags)
> +				   struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				   uint32_t flags)
>   {
>   	int ret;
>   
> -	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_CTM_3X4, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_CTM_3X4, flags);
>   	if (ret)
>   		return ret;
>   
> @@ -363,16 +371,18 @@ EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
>    * @dev: DRM device
>    * @colorop: The drm_colorop object to initialize
>    * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
>    * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>    * @return zero on success, -E value on failure
>    */
>   int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				struct drm_plane *plane, uint32_t flags)
> +				struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				uint32_t flags)
>   {
>   	struct drm_property *prop;
>   	int ret;
>   
> -	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_MULTIPLIER, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_MULTIPLIER, flags);
>   	if (ret)
>   		return ret;
>   
> @@ -391,6 +401,7 @@ EXPORT_SYMBOL(drm_plane_colorop_mult_init);
>   
>   int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *colorop,
>   				 struct drm_plane *plane,
> +				 const struct drm_colorop_funcs *funcs,
>   				 uint32_t lut_size,
>   				 enum drm_colorop_lut3d_interpolation_type interpolation,
>   				 uint32_t flags)
> @@ -398,7 +409,7 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
>   	struct drm_property *prop;
>   	int ret;
>   
> -	ret = drm_plane_colorop_init(dev, colorop, plane, DRM_COLOROP_3D_LUT, flags);
> +	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_3D_LUT, flags);
>   	if (ret)
>   		return ret;
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 04af552b3648..d3d73d60727c 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -25,7 +25,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>   
>   	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
>   
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, NULL,
>   						  PLANE_DEGAMMA_SIZE,
>   						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>   						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
> @@ -39,7 +39,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>   	prev_op = &colorop->base;
>   
>   	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, NULL,
>   					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		return ret;
> @@ -52,7 +52,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>   	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>   		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
>   
> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane, 17,
> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, plane, NULL, 17,
>   						   DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>   						   true);
>   		if (ret)
> @@ -64,7 +64,7 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>   	}
>   
>   	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, plane, NULL,
>   						  PLANE_GAMMA_SIZE,
>   						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>   						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
> index d03a1f2e9c41..9e9dd0494628 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
> @@ -48,7 +48,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
>   
> @@ -64,7 +65,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
>   
> @@ -80,7 +82,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> index a3a32f9f918c..45d1b1d3faf9 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -187,6 +187,19 @@ struct drm_colorop_state {
>   	struct drm_atomic_state *state;
>   };
>   
> +/**
> + * struct drm_colorop_funcs - driver colorop control functions
> + */
> +struct drm_colorop_funcs {
> +	/**
> +	 * @destroy:
> +	 *
> +	 * Clean up colorop resources. This is called at driver unload time
> +	 * through drm_mode_config_cleanup()
> +	 */
> +	void (*destroy)(struct drm_colorop *colorop);
> +};
> +
>   /**
>    * struct drm_colorop - DRM color operation control structure
>    *
> @@ -362,6 +375,8 @@ struct drm_colorop {
>   	 */
>   	struct drm_property *next_property;
>   
> +	/** @funcs: colorop control functions */
> +	const struct drm_colorop_funcs *funcs;
>   };
>   
>   #define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
> @@ -390,17 +405,22 @@ void drm_colorop_pipeline_destroy(struct drm_device *dev);
>   void drm_colorop_cleanup(struct drm_colorop *colorop);
>   
>   int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				    struct drm_plane *plane, u64 supported_tfs, uint32_t flags);
> +				    struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				    u64 supported_tfs, uint32_t flags);
>   int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop *colorop,
> -					struct drm_plane *plane, uint32_t lut_size,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs *funcs,
> +					uint32_t lut_size,
>   					enum drm_colorop_lut1d_interpolation_type interpolation,
>   					uint32_t flags);
>   int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				   struct drm_plane *plane, uint32_t flags);
> +				   struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				   uint32_t flags);
>   int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				struct drm_plane *plane, uint32_t flags);
> +				struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
> +				uint32_t flags);
>   int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *colorop,
> -				 struct drm_plane *plane,
> +				 struct drm_plane *plane, const struct drm_colorop_funcs *funcs,
>   				 uint32_t lut_size,
>   				 enum drm_colorop_lut3d_interpolation_type interpolation,
>   				 uint32_t flags);

Re: [PATCH 06/13] drm/colorop: Add destroy helper for colorop objects

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> Add a helper that performs common cleanup and frees the
> associated object. This can be used by drivers if they do not
> require any driver-specific teardown.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   drivers/gpu/drm/drm_colorop.c | 12 ++++++++++++
>   include/drm/drm_colorop.h     | 10 ++++++++++
>   2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index efe61bdd9b24..4b27804bb0bd 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -180,6 +180,18 @@ void drm_colorop_cleanup(struct drm_colorop *colorop)
>   }
>   EXPORT_SYMBOL(drm_colorop_cleanup);
>   
> +/**
> + * drm_colorop_destroy() - Helper for colorop destruction
> + *
> + * @colorop: colorop to destroy
> + */
> +void drm_colorop_destroy(struct drm_colorop *colorop)
> +{
> +	drm_colorop_cleanup(colorop);
> +	kfree(colorop);
> +}
> +EXPORT_SYMBOL(drm_colorop_destroy);
> +
>   /**
>    * drm_colorop_pipeline_destroy - Helper for color pipeline destruction
>    *
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> index 45d1b1d3faf9..ee6454b08b2d 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -440,6 +440,16 @@ void drm_colorop_atomic_destroy_state(struct drm_colorop *colorop,
>    */
>   void drm_colorop_reset(struct drm_colorop *colorop);
>   
> +/**
> + * drm_colorop_destroy - destroy colorop
> + * @colorop: drm colorop
> + *
> + * Destroys @colorop by performing common DRM cleanup and freeing the
> + * colorop object. This can be used by drivers if they do not
> + * require any driver-specific teardown.
> + */
> +void drm_colorop_destroy(struct drm_colorop *colorop);
> +
>   /**
>    * drm_colorop_index - find the index of a registered colorop
>    * @colorop: colorop to find index for

Re: [PATCH 07/13] drm/amd/display: Hook up colorop destroy helper for plane pipelines

[Alex Hung]

See my nitpick comments for patch 6

Otherwise it looks to me.

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> Provide a drm_colorop_funcs instance for amdgpu_dm color pipeline
> objects and hook up the common drm_colorop_destroy() helper as the
> destroy callback.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 25 +++++++++++++------
>   1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index dfdb4fb4219f..5130962193d9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -55,6 +55,10 @@ const u64 amdgpu_dm_supported_blnd_tfs =
>   
>   #define LUT3D_SIZE		17
>   
> +static const struct drm_colorop_funcs dm_colorop_funcs = {
> +	.destroy = drm_colorop_destroy,
> +};
> +
>   int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list)
>   {
>   	struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
> @@ -72,7 +76,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs,
>   					      amdgpu_dm_supported_degam_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
> @@ -89,7 +93,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL, DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +	ret = drm_plane_colorop_mult_init(dev, ops[i], plane, &dm_colorop_funcs,
> +					  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
>   
> @@ -104,7 +109,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane,
> +					     &dm_colorop_funcs,
>   					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
> @@ -121,7 +127,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   			goto cleanup;
>   		}
>   
> -		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs,
>   						amdgpu_dm_supported_shaper_tfs,
>   						DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   		if (ret)
> @@ -138,7 +144,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   			goto cleanup;
>   		}
>   
> -		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL,
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane,
> +							&dm_colorop_funcs,
>   							MAX_COLOR_LUT_ENTRIES,
>   							DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>   							DRM_COLOROP_FLAG_ALLOW_BYPASS);
> @@ -156,7 +163,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   			goto cleanup;
>   		}
>   
> -		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL, LUT3D_SIZE,
> +		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane,
> +					&dm_colorop_funcs, LUT3D_SIZE,
>   					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>   					DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   		if (ret)
> @@ -174,7 +182,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &dm_colorop_funcs,
>   					      amdgpu_dm_supported_blnd_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
> @@ -191,7 +199,8 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, NULL, MAX_COLOR_LUT_ENTRIES,
> +	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, &dm_colorop_funcs,
> +						  MAX_COLOR_LUT_ENTRIES,
>   						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>   						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)

Re: [PATCH 08/13] drm/vkms: Hook up colorop destroy helper for plane pipelines

[Alex Hung]

See my nitpick comments for patch 6

Otherwise it looks to me.

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> Provide a drm_colorop_funcs instance for vkms color pipeline
> objects and hook up the common drm_colorop_destroy() helper as the
> destroy callback.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   drivers/gpu/drm/vkms/vkms_colorop.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c
> index 9e9dd0494628..ba826ad384b7 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -12,6 +12,10 @@ static const u64 supported_tfs =
>   	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF);
>   
> +static const struct drm_colorop_funcs vkms_colorop_funcs = {
> +	.destroy = drm_colorop_destroy,
> +};
> +
>   #define MAX_COLOR_PIPELINE_OPS 4
>   
>   static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list)
> @@ -31,7 +35,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &vkms_colorop_funcs,
> +					      supported_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
> @@ -48,7 +53,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, &vkms_colorop_funcs,
>   					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
> @@ -65,7 +70,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, &vkms_colorop_funcs,
>   					     DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;
> @@ -82,7 +87,8 @@ static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pr
>   		goto cleanup;
>   	}
>   
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, supported_tfs,
> +	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, &vkms_colorop_funcs,
> +					      supported_tfs,
>   					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
>   	if (ret)
>   		goto cleanup;

Re: [PATCH 10/13] drm: Clean up colorop objects during mode_config cleanup

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> Tear down all registered drm_colorop objects during
> drm_mode_config_cleanup() by invoking their destroy callbacks.
> 
> This ensures proper cleanup of color pipeline objects during DRM device
> removal.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   drivers/gpu/drm/drm_mode_config.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index d12db9b0bab8..84ae8a23a367 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -524,6 +524,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>   	struct drm_property *property, *pt;
>   	struct drm_property_blob *blob, *bt;
>   	struct drm_plane *plane, *plt;
> +	struct drm_colorop *colorop, *copt;
>   
>   	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
>   				 head) {
> @@ -553,6 +554,11 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>   		drm_property_destroy(dev, property);
>   	}
>   
> +	list_for_each_entry_safe(colorop, copt, &dev->mode_config.colorop_list,
> +				 head) {
> +		colorop->funcs->destroy(colorop);
> +	}
> +
>   	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
>   				 head) {
>   		plane->funcs->destroy(plane);

Re: [PATCH 12/13] drm/colorop: Use destroy callback for color pipeline teardown

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
> Switch drm_colorop_pipeline_destroy() to use the driver-provided
> destroy callback instead of directly calling drm_colorop_cleanup()
> and freeing the object.
> 
> This allows drivers that embed struct drm_colorop in driver-specific
> objects to perform correct teardown.
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   drivers/gpu/drm/drm_colorop.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 4b27804bb0bd..fafe45b93ff8 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -205,8 +205,7 @@ void drm_colorop_pipeline_destroy(struct drm_device *dev)
>   	struct drm_colorop *colorop, *next;
>   
>   	list_for_each_entry_safe(colorop, next, &config->colorop_list, head) {
> -		drm_colorop_cleanup(colorop);
> -		kfree(colorop);
> +		colorop->funcs->destroy(colorop);
>   	}
>   }
>   EXPORT_SYMBOL(drm_colorop_pipeline_destroy);

Re: [PATCH 05/13] drm: Allow driver-managed destruction of colorop objects

[Borah, Chaitanya Kumar]

On 1/7/2026 5:32 AM, Alex Hung wrote:
> It is a nitpick, but is it better to have patch 6 first, and then 
> separate changes to drm to a single patch, and move the changes in 
> amdgpu, vkms and i915 in the following corresponding patches (7, 8 and 9)?
> 
> We can avoid passing NULL to struct drm_colorop_funcs *funcs.

Thank you very much, Alex, for the review.

I structured the patches this way to keep each patch buildable and to 
keep the existing behavior intact until all changes are in place.

Once the prototypes of the init functions are changed, all call sites 
need to be updated. I couldn’t find a way to keep the DRM changes 
isolated while also preserving per-patch buildability. If I am missing 
something here, please let me know. I can respin accordingly.

In any case, I can move patch 6 ahead of this one.

==
Chaitanya

> 
> Otherwise it looks good to me.
> 
> Reviewed-by: Alex Hung <alex.hung@amd.com>
> 
> On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
>> Some drivers might want to embed struct drm_colorop inside
>> driver-specific objects, similar to planes or CRTCs. In such
>> cases, freeing only the drm_colorop is incorrect.
>>
>> Add a drm_colorop_funcs callback to allow drivers to provide a destroy
>> hook that cleans up the full enclosing object. Make changes in helper
>> functions to accept helper functions as argument. Pass NULL for now
>> to retain current behavior.
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>   .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 18 ++++++-----
>>   drivers/gpu/drm/drm_colorop.c                 | 31 +++++++++++++------
>>   .../drm/i915/display/intel_color_pipeline.c   |  8 ++---
>>   drivers/gpu/drm/vkms/vkms_colorop.c           | 10 +++---
>>   include/drm/drm_colorop.h                     | 30 +++++++++++++++---
>>   5 files changed, 66 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c 
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>> index a2de3bba8346..dfdb4fb4219f 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>> @@ -72,7 +72,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>>                             amdgpu_dm_supported_degam_tfs,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>> @@ -89,7 +89,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_mult_init(dev, ops[i], plane, 
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +    ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL, 
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           goto cleanup;
>> @@ -104,7 +104,8 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, 
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
>> +                         DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           goto cleanup;
>> @@ -120,7 +121,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>               goto cleanup;
>>           }
>> -        ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
>> +        ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>>                           amdgpu_dm_supported_shaper_tfs,
>>                           DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>           if (ret)
>> @@ -137,7 +138,8 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>               goto cleanup;
>>           }
>> -        ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, 
>> MAX_COLOR_LUT_ENTRIES,
>> +        ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, 
>> NULL,
>> +                            MAX_COLOR_LUT_ENTRIES,
>>                               DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>                               DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>           if (ret)
>> @@ -154,7 +156,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>               goto cleanup;
>>           }
>> -        ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, 
>> LUT3D_SIZE,
>> +        ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL, 
>> LUT3D_SIZE,
>>                       DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>>                       DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>           if (ret)
>> @@ -172,7 +174,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>>                             amdgpu_dm_supported_blnd_tfs,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>> @@ -189,7 +191,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, 
>> MAX_COLOR_LUT_ENTRIES,
>> +    ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, 
>> NULL, MAX_COLOR_LUT_ENTRIES,
>>                             DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/ 
>> drm_colorop.c
>> index 44eb823585d2..efe61bdd9b24 100644
>> --- a/drivers/gpu/drm/drm_colorop.c
>> +++ b/drivers/gpu/drm/drm_colorop.c
>> @@ -93,7 +93,8 @@ static const struct drm_prop_enum_list 
>> drm_colorop_lut3d_interpolation_list[] =
>>   /* Init Helpers */
>>   static int drm_plane_colorop_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                  struct drm_plane *plane, enum drm_colorop_type type,
>> +                  struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                  enum drm_colorop_type type,
>>                     uint32_t flags)
>>   {
>>       struct drm_mode_config *config = &dev->mode_config;
>> @@ -109,6 +110,7 @@ static int drm_plane_colorop_init(struct 
>> drm_device *dev, struct drm_colorop *co
>>       colorop->type = type;
>>       colorop->plane = plane;
>>       colorop->next = NULL;
>> +    colorop->funcs = funcs;
>>       list_add_tail(&colorop->head, &config->colorop_list);
>>       colorop->index = config->num_colorop++;
>> @@ -203,6 +205,7 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>>    * @dev: DRM device
>>    * @colorop: The drm_colorop object to initialize
>>    * @plane: The associated drm_plane
>> + * @funcs: control functions for the new colorop
>>    * @supported_tfs: A bitfield of supported 
>> drm_plane_colorop_curve_1d_init enum values,
>>    *                 created using BIT(curve_type) and combined with 
>> the OR '|'
>>    *                 operator.
>> @@ -210,7 +213,8 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>>    * @return zero on success, -E value on failure
>>    */
>>   int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                    struct drm_plane *plane, u64 supported_tfs, 
>> uint32_t flags)
>> +                    struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                    u64 supported_tfs, uint32_t flags)
>>   {
>>       struct drm_prop_enum_list enum_list[DRM_COLOROP_1D_CURVE_COUNT];
>>       int i, len;
>> @@ -231,7 +235,7 @@ int drm_plane_colorop_curve_1d_init(struct 
>> drm_device *dev, struct drm_colorop *
>>           return -EINVAL;
>>       }
>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>> DRM_COLOROP_1D_CURVE, flags);
>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>> DRM_COLOROP_1D_CURVE, flags);
>>       if (ret)
>>           return ret;
>> @@ -288,20 +292,23 @@ static int drm_colorop_create_data_prop(struct 
>> drm_device *dev, struct drm_color
>>    * @dev: DRM device
>>    * @colorop: The drm_colorop object to initialize
>>    * @plane: The associated drm_plane
>> + * @funcs: control functions for new colorop
>>    * @lut_size: LUT size supported by driver
>>    * @interpolation: 1D LUT interpolation type
>>    * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>>    * @return zero on success, -E value on failure
>>    */
>>   int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, 
>> struct drm_colorop *colorop,
>> -                    struct drm_plane *plane, uint32_t lut_size,
>> +                    struct drm_plane *plane,
>> +                    const struct drm_colorop_funcs *funcs,
>> +                    uint32_t lut_size,
>>                       enum drm_colorop_lut1d_interpolation_type 
>> interpolation,
>>                       uint32_t flags)
>>   {
>>       struct drm_property *prop;
>>       int ret;
>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>> DRM_COLOROP_1D_LUT, flags);
>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>> DRM_COLOROP_1D_LUT, flags);
>>       if (ret)
>>           return ret;
>> @@ -339,11 +346,12 @@ int drm_plane_colorop_curve_1d_lut_init(struct 
>> drm_device *dev, struct drm_color
>>   EXPORT_SYMBOL(drm_plane_colorop_curve_1d_lut_init);
>>   int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                   struct drm_plane *plane, uint32_t flags)
>> +                   struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                   uint32_t flags)
>>   {
>>       int ret;
>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>> DRM_COLOROP_CTM_3X4, flags);
>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>> DRM_COLOROP_CTM_3X4, flags);
>>       if (ret)
>>           return ret;
>> @@ -363,16 +371,18 @@ EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
>>    * @dev: DRM device
>>    * @colorop: The drm_colorop object to initialize
>>    * @plane: The associated drm_plane
>> + * @funcs: control functions for the new colorop
>>    * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>>    * @return zero on success, -E value on failure
>>    */
>>   int drm_plane_colorop_mult_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                struct drm_plane *plane, uint32_t flags)
>> +                struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                uint32_t flags)
>>   {
>>       struct drm_property *prop;
>>       int ret;
>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>> DRM_COLOROP_MULTIPLIER, flags);
>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>> DRM_COLOROP_MULTIPLIER, flags);
>>       if (ret)
>>           return ret;
>> @@ -391,6 +401,7 @@ EXPORT_SYMBOL(drm_plane_colorop_mult_init);
>>   int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>>                    struct drm_plane *plane,
>> +                 const struct drm_colorop_funcs *funcs,
>>                    uint32_t lut_size,
>>                    enum drm_colorop_lut3d_interpolation_type 
>> interpolation,
>>                    uint32_t flags)
>> @@ -398,7 +409,7 @@ int drm_plane_colorop_3dlut_init(struct drm_device 
>> *dev, struct drm_colorop *col
>>       struct drm_property *prop;
>>       int ret;
>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>> DRM_COLOROP_3D_LUT, flags);
>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>> DRM_COLOROP_3D_LUT, flags);
>>       if (ret)
>>           return ret;
>> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/ 
>> drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> index 04af552b3648..d3d73d60727c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> @@ -25,7 +25,7 @@ int _intel_color_pipeline_plane_init(struct 
>> drm_plane *plane, struct drm_prop_en
>>       colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
>> -    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>> plane,
>> +    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>> plane, NULL,
>>                             PLANE_DEGAMMA_SIZE,
>>                             DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> @@ -39,7 +39,7 @@ int _intel_color_pipeline_plane_init(struct 
>> drm_plane *plane, struct drm_prop_en
>>       prev_op = &colorop->base;
>>       colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, 
>> NULL,
>>                            DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           return ret;
>> @@ -52,7 +52,7 @@ int _intel_color_pipeline_plane_init(struct 
>> drm_plane *plane, struct drm_prop_en
>>           plane->type == DRM_PLANE_TYPE_PRIMARY) {
>>           colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
>> -        ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, 
>> plane, 17,
>> +        ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, 
>> plane, NULL, 17,
>>                              DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>>                              true);
>>           if (ret)
>> @@ -64,7 +64,7 @@ int _intel_color_pipeline_plane_init(struct 
>> drm_plane *plane, struct drm_prop_en
>>       }
>>       colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
>> -    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>> plane,
>> +    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>> plane, NULL,
>>                             PLANE_GAMMA_SIZE,
>>                             DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/ 
>> vkms/vkms_colorop.c
>> index d03a1f2e9c41..9e9dd0494628 100644
>> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
>> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
>> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, 
>> supported_tfs,
>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, 
>> supported_tfs,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           goto cleanup;
>> @@ -48,7 +48,8 @@ static int vkms_initialize_color_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, 
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
>> +                         DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           goto cleanup;
>> @@ -64,7 +65,8 @@ static int vkms_initialize_color_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, 
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
>> +                         DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           goto cleanup;
>> @@ -80,7 +82,7 @@ static int vkms_initialize_color_pipeline(struct 
>> drm_plane *plane, struct drm_pr
>>           goto cleanup;
>>       }
>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, 
>> supported_tfs,
>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, 
>> supported_tfs,
>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>       if (ret)
>>           goto cleanup;
>> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
>> index a3a32f9f918c..45d1b1d3faf9 100644
>> --- a/include/drm/drm_colorop.h
>> +++ b/include/drm/drm_colorop.h
>> @@ -187,6 +187,19 @@ struct drm_colorop_state {
>>       struct drm_atomic_state *state;
>>   };
>> +/**
>> + * struct drm_colorop_funcs - driver colorop control functions
>> + */
>> +struct drm_colorop_funcs {
>> +    /**
>> +     * @destroy:
>> +     *
>> +     * Clean up colorop resources. This is called at driver unload time
>> +     * through drm_mode_config_cleanup()
>> +     */
>> +    void (*destroy)(struct drm_colorop *colorop);
>> +};
>> +
>>   /**
>>    * struct drm_colorop - DRM color operation control structure
>>    *
>> @@ -362,6 +375,8 @@ struct drm_colorop {
>>        */
>>       struct drm_property *next_property;
>> +    /** @funcs: colorop control functions */
>> +    const struct drm_colorop_funcs *funcs;
>>   };
>>   #define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
>> @@ -390,17 +405,22 @@ void drm_colorop_pipeline_destroy(struct 
>> drm_device *dev);
>>   void drm_colorop_cleanup(struct drm_colorop *colorop);
>>   int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                    struct drm_plane *plane, u64 supported_tfs, 
>> uint32_t flags);
>> +                    struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                    u64 supported_tfs, uint32_t flags);
>>   int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, 
>> struct drm_colorop *colorop,
>> -                    struct drm_plane *plane, uint32_t lut_size,
>> +                    struct drm_plane *plane,
>> +                    const struct drm_colorop_funcs *funcs,
>> +                    uint32_t lut_size,
>>                       enum drm_colorop_lut1d_interpolation_type 
>> interpolation,
>>                       uint32_t flags);
>>   int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                   struct drm_plane *plane, uint32_t flags);
>> +                   struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                   uint32_t flags);
>>   int drm_plane_colorop_mult_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                struct drm_plane *plane, uint32_t flags);
>> +                struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>> +                uint32_t flags);
>>   int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> -                 struct drm_plane *plane,
>> +                 struct drm_plane *plane, const struct 
>> drm_colorop_funcs *funcs,
>>                    uint32_t lut_size,
>>                    enum drm_colorop_lut3d_interpolation_type 
>> interpolation,
>>                    uint32_t flags);
> 

Re: [PATCH 05/13] drm: Allow driver-managed destruction of colorop objects

[Alex Hung]

On 1/8/26 07:11, Borah, Chaitanya Kumar wrote:
> 
> On 1/7/2026 5:32 AM, Alex Hung wrote:
>> It is a nitpick, but is it better to have patch 6 first, and then 
>> separate changes to drm to a single patch, and move the changes in 
>> amdgpu, vkms and i915 in the following corresponding patches (7, 8 and 
>> 9)?
>>
>> We can avoid passing NULL to struct drm_colorop_funcs *funcs.
> 
> Thank you very much, Alex, for the review.
> 
> I structured the patches this way to keep each patch buildable and to 
> keep the existing behavior intact until all changes are in place.
> 
> Once the prototypes of the init functions are changed, all call sites 
> need to be updated. I couldn’t find a way to keep the DRM changes 
> isolated while also preserving per-patch buildability. If I am missing 
> something here, please let me know. I can respin accordingly.
> 
> In any case, I can move patch 6 ahead of this one.

Sounds good to me.


> 
> ==
> Chaitanya
> 
>>
>> Otherwise it looks good to me.
>>
>> Reviewed-by: Alex Hung <alex.hung@amd.com>
>>
>> On 12/18/25 23:56, Chaitanya Kumar Borah wrote:
>>> Some drivers might want to embed struct drm_colorop inside
>>> driver-specific objects, similar to planes or CRTCs. In such
>>> cases, freeing only the drm_colorop is incorrect.
>>>
>>> Add a drm_colorop_funcs callback to allow drivers to provide a destroy
>>> hook that cleans up the full enclosing object. Make changes in helper
>>> functions to accept helper functions as argument. Pass NULL for now
>>> to retain current behavior.
>>>
>>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>> ---
>>>   .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 18 ++++++-----
>>>   drivers/gpu/drm/drm_colorop.c                 | 31 +++++++++++++------
>>>   .../drm/i915/display/intel_color_pipeline.c   |  8 ++---
>>>   drivers/gpu/drm/vkms/vkms_colorop.c           | 10 +++---
>>>   include/drm/drm_colorop.h                     | 30 +++++++++++++++---
>>>   5 files changed, 66 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/ 
>>> amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/ 
>>> amdgpu_dm_colorop.c
>>> index a2de3bba8346..dfdb4fb4219f 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>>> @@ -72,7 +72,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
>>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>>>                             amdgpu_dm_supported_degam_tfs,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>> @@ -89,7 +89,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_mult_init(dev, ops[i], plane, 
>>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>> +    ret = drm_plane_colorop_mult_init(dev, ops[i], plane, NULL, 
>>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           goto cleanup;
>>> @@ -104,7 +104,8 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, 
>>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
>>> +                         DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           goto cleanup;
>>> @@ -120,7 +121,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>               goto cleanup;
>>>           }
>>> -        ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
>>> +        ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>>>                           amdgpu_dm_supported_shaper_tfs,
>>>                           DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>           if (ret)
>>> @@ -137,7 +138,8 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>               goto cleanup;
>>>           }
>>> -        ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], 
>>> plane, MAX_COLOR_LUT_ENTRIES,
>>> +        ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], 
>>> plane, NULL,
>>> +                            MAX_COLOR_LUT_ENTRIES,
>>>                               DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>>                               DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>           if (ret)
>>> @@ -154,7 +156,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>               goto cleanup;
>>>           }
>>> -        ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, 
>>> LUT3D_SIZE,
>>> +        ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, NULL, 
>>> LUT3D_SIZE,
>>>                       DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>>>                       DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>           if (ret)
>>> @@ -172,7 +174,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
>>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL,
>>>                             amdgpu_dm_supported_blnd_tfs,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>> @@ -189,7 +191,7 @@ int amdgpu_dm_initialize_default_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, 
>>> MAX_COLOR_LUT_ENTRIES,
>>> +    ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, 
>>> NULL, MAX_COLOR_LUT_ENTRIES,
>>>                             DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/ 
>>> drm_colorop.c
>>> index 44eb823585d2..efe61bdd9b24 100644
>>> --- a/drivers/gpu/drm/drm_colorop.c
>>> +++ b/drivers/gpu/drm/drm_colorop.c
>>> @@ -93,7 +93,8 @@ static const struct drm_prop_enum_list 
>>> drm_colorop_lut3d_interpolation_list[] =
>>>   /* Init Helpers */
>>>   static int drm_plane_colorop_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                  struct drm_plane *plane, enum drm_colorop_type type,
>>> +                  struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                  enum drm_colorop_type type,
>>>                     uint32_t flags)
>>>   {
>>>       struct drm_mode_config *config = &dev->mode_config;
>>> @@ -109,6 +110,7 @@ static int drm_plane_colorop_init(struct 
>>> drm_device *dev, struct drm_colorop *co
>>>       colorop->type = type;
>>>       colorop->plane = plane;
>>>       colorop->next = NULL;
>>> +    colorop->funcs = funcs;
>>>       list_add_tail(&colorop->head, &config->colorop_list);
>>>       colorop->index = config->num_colorop++;
>>> @@ -203,6 +205,7 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>>>    * @dev: DRM device
>>>    * @colorop: The drm_colorop object to initialize
>>>    * @plane: The associated drm_plane
>>> + * @funcs: control functions for the new colorop
>>>    * @supported_tfs: A bitfield of supported 
>>> drm_plane_colorop_curve_1d_init enum values,
>>>    *                 created using BIT(curve_type) and combined with 
>>> the OR '|'
>>>    *                 operator.
>>> @@ -210,7 +213,8 @@ EXPORT_SYMBOL(drm_colorop_pipeline_destroy);
>>>    * @return zero on success, -E value on failure
>>>    */
>>>   int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                    struct drm_plane *plane, u64 supported_tfs, 
>>> uint32_t flags)
>>> +                    struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                    u64 supported_tfs, uint32_t flags)
>>>   {
>>>       struct drm_prop_enum_list enum_list[DRM_COLOROP_1D_CURVE_COUNT];
>>>       int i, len;
>>> @@ -231,7 +235,7 @@ int drm_plane_colorop_curve_1d_init(struct 
>>> drm_device *dev, struct drm_colorop *
>>>           return -EINVAL;
>>>       }
>>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>>> DRM_COLOROP_1D_CURVE, flags);
>>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>>> DRM_COLOROP_1D_CURVE, flags);
>>>       if (ret)
>>>           return ret;
>>> @@ -288,20 +292,23 @@ static int drm_colorop_create_data_prop(struct 
>>> drm_device *dev, struct drm_color
>>>    * @dev: DRM device
>>>    * @colorop: The drm_colorop object to initialize
>>>    * @plane: The associated drm_plane
>>> + * @funcs: control functions for new colorop
>>>    * @lut_size: LUT size supported by driver
>>>    * @interpolation: 1D LUT interpolation type
>>>    * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>>>    * @return zero on success, -E value on failure
>>>    */
>>>   int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, 
>>> struct drm_colorop *colorop,
>>> -                    struct drm_plane *plane, uint32_t lut_size,
>>> +                    struct drm_plane *plane,
>>> +                    const struct drm_colorop_funcs *funcs,
>>> +                    uint32_t lut_size,
>>>                       enum drm_colorop_lut1d_interpolation_type 
>>> interpolation,
>>>                       uint32_t flags)
>>>   {
>>>       struct drm_property *prop;
>>>       int ret;
>>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>>> DRM_COLOROP_1D_LUT, flags);
>>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>>> DRM_COLOROP_1D_LUT, flags);
>>>       if (ret)
>>>           return ret;
>>> @@ -339,11 +346,12 @@ int drm_plane_colorop_curve_1d_lut_init(struct 
>>> drm_device *dev, struct drm_color
>>>   EXPORT_SYMBOL(drm_plane_colorop_curve_1d_lut_init);
>>>   int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                   struct drm_plane *plane, uint32_t flags)
>>> +                   struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                   uint32_t flags)
>>>   {
>>>       int ret;
>>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>>> DRM_COLOROP_CTM_3X4, flags);
>>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>>> DRM_COLOROP_CTM_3X4, flags);
>>>       if (ret)
>>>           return ret;
>>> @@ -363,16 +371,18 @@ EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
>>>    * @dev: DRM device
>>>    * @colorop: The drm_colorop object to initialize
>>>    * @plane: The associated drm_plane
>>> + * @funcs: control functions for the new colorop
>>>    * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>>>    * @return zero on success, -E value on failure
>>>    */
>>>   int drm_plane_colorop_mult_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                struct drm_plane *plane, uint32_t flags)
>>> +                struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                uint32_t flags)
>>>   {
>>>       struct drm_property *prop;
>>>       int ret;
>>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>>> DRM_COLOROP_MULTIPLIER, flags);
>>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>>> DRM_COLOROP_MULTIPLIER, flags);
>>>       if (ret)
>>>           return ret;
>>> @@ -391,6 +401,7 @@ EXPORT_SYMBOL(drm_plane_colorop_mult_init);
>>>   int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>>                    struct drm_plane *plane,
>>> +                 const struct drm_colorop_funcs *funcs,
>>>                    uint32_t lut_size,
>>>                    enum drm_colorop_lut3d_interpolation_type 
>>> interpolation,
>>>                    uint32_t flags)
>>> @@ -398,7 +409,7 @@ int drm_plane_colorop_3dlut_init(struct 
>>> drm_device *dev, struct drm_colorop *col
>>>       struct drm_property *prop;
>>>       int ret;
>>> -    ret = drm_plane_colorop_init(dev, colorop, plane, 
>>> DRM_COLOROP_3D_LUT, flags);
>>> +    ret = drm_plane_colorop_init(dev, colorop, plane, funcs, 
>>> DRM_COLOROP_3D_LUT, flags);
>>>       if (ret)
>>>           return ret;
>>> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/ 
>>> drivers/gpu/drm/i915/display/intel_color_pipeline.c
>>> index 04af552b3648..d3d73d60727c 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>>> @@ -25,7 +25,7 @@ int _intel_color_pipeline_plane_init(struct 
>>> drm_plane *plane, struct drm_prop_en
>>>       colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
>>> -    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>>> plane,
>>> +    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>>> plane, NULL,
>>>                             PLANE_DEGAMMA_SIZE,
>>>                             DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>> @@ -39,7 +39,7 @@ int _intel_color_pipeline_plane_init(struct 
>>> drm_plane *plane, struct drm_prop_en
>>>       prev_op = &colorop->base;
>>>       colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
>>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
>>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, 
>>> NULL,
>>>                            DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           return ret;
>>> @@ -52,7 +52,7 @@ int _intel_color_pipeline_plane_init(struct 
>>> drm_plane *plane, struct drm_prop_en
>>>           plane->type == DRM_PLANE_TYPE_PRIMARY) {
>>>           colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
>>> -        ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, 
>>> plane, 17,
>>> +        ret = drm_plane_colorop_3dlut_init(dev, &colorop->base, 
>>> plane, NULL, 17,
>>>                              
>>> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>>>                              true);
>>>           if (ret)
>>> @@ -64,7 +64,7 @@ int _intel_color_pipeline_plane_init(struct 
>>> drm_plane *plane, struct drm_prop_en
>>>       }
>>>       colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
>>> -    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>>> plane,
>>> +    ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, 
>>> plane, NULL,
>>>                             PLANE_GAMMA_SIZE,
>>>                             DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/ 
>>> vkms/vkms_colorop.c
>>> index d03a1f2e9c41..9e9dd0494628 100644
>>> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
>>> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
>>> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, 
>>> supported_tfs,
>>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, 
>>> supported_tfs,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           goto cleanup;
>>> @@ -48,7 +48,8 @@ static int vkms_initialize_color_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, 
>>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
>>> +                         DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           goto cleanup;
>>> @@ -64,7 +65,8 @@ static int vkms_initialize_color_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, 
>>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>> +    ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane, NULL,
>>> +                         DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           goto cleanup;
>>> @@ -80,7 +82,7 @@ static int vkms_initialize_color_pipeline(struct 
>>> drm_plane *plane, struct drm_pr
>>>           goto cleanup;
>>>       }
>>> -    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, 
>>> supported_tfs,
>>> +    ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane, NULL, 
>>> supported_tfs,
>>>                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
>>>       if (ret)
>>>           goto cleanup;
>>> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
>>> index a3a32f9f918c..45d1b1d3faf9 100644
>>> --- a/include/drm/drm_colorop.h
>>> +++ b/include/drm/drm_colorop.h
>>> @@ -187,6 +187,19 @@ struct drm_colorop_state {
>>>       struct drm_atomic_state *state;
>>>   };
>>> +/**
>>> + * struct drm_colorop_funcs - driver colorop control functions
>>> + */
>>> +struct drm_colorop_funcs {
>>> +    /**
>>> +     * @destroy:
>>> +     *
>>> +     * Clean up colorop resources. This is called at driver unload time
>>> +     * through drm_mode_config_cleanup()
>>> +     */
>>> +    void (*destroy)(struct drm_colorop *colorop);
>>> +};
>>> +
>>>   /**
>>>    * struct drm_colorop - DRM color operation control structure
>>>    *
>>> @@ -362,6 +375,8 @@ struct drm_colorop {
>>>        */
>>>       struct drm_property *next_property;
>>> +    /** @funcs: colorop control functions */
>>> +    const struct drm_colorop_funcs *funcs;
>>>   };
>>>   #define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
>>> @@ -390,17 +405,22 @@ void drm_colorop_pipeline_destroy(struct 
>>> drm_device *dev);
>>>   void drm_colorop_cleanup(struct drm_colorop *colorop);
>>>   int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                    struct drm_plane *plane, u64 supported_tfs, 
>>> uint32_t flags);
>>> +                    struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                    u64 supported_tfs, uint32_t flags);
>>>   int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, 
>>> struct drm_colorop *colorop,
>>> -                    struct drm_plane *plane, uint32_t lut_size,
>>> +                    struct drm_plane *plane,
>>> +                    const struct drm_colorop_funcs *funcs,
>>> +                    uint32_t lut_size,
>>>                       enum drm_colorop_lut1d_interpolation_type 
>>> interpolation,
>>>                       uint32_t flags);
>>>   int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                   struct drm_plane *plane, uint32_t flags);
>>> +                   struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                   uint32_t flags);
>>>   int drm_plane_colorop_mult_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                struct drm_plane *plane, uint32_t flags);
>>> +                struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>> +                uint32_t flags);
>>>   int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> -                 struct drm_plane *plane,
>>> +                 struct drm_plane *plane, const struct 
>>> drm_colorop_funcs *funcs,
>>>                    uint32_t lut_size,
>>>                    enum drm_colorop_lut3d_interpolation_type 
>>> interpolation,
>>>                    uint32_t flags);
>>
> 

Re: [PATCH 13/13] drm/i915/color: Add failure handling in plane color pipeline init

[Borah, Chaitanya Kumar]

On 1/6/2026 9:25 AM, Kandpal, Suraj wrote:
>> Subject: [PATCH 13/13] drm/i915/color: Add failure handling in plane color
>> pipeline init
>>
>> The plane color pipeline initialization built up multiple colorop blocks inline,
>> but did not reliably clean up partially constructed pipelines when an
>> intermediate step failed. This could lead to leaked colorop objects and fragile
>> error handling as the pipeline grows.
>>
>> Refactor the pipeline construction to use a common helper for adding colorop
>> blocks. This centralizes allocation, initialization, and teardown logic, allowing
>> the caller to reliably unwind all previously created colorops on failure.
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>   .../drm/i915/display/intel_color_pipeline.c   | 142 ++++++++++++------
>>   1 file changed, 100 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> index 8fecc53540ba..035ec3f022cd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> @@ -2,6 +2,8 @@
>>   /*
>>    * Copyright © 2025 Intel Corporation
>>    */
>> +#include <drm/drm_print.h>
>> +
>>   #include "intel_color.h"
>>   #include "intel_colorop.h"
>>   #include "intel_color_pipeline.h"
>> @@ -10,6 +12,7 @@
>>   #include "skl_universal_plane.h"
>>
>>   #define MAX_COLOR_PIPELINES 1
>> +#define MAX_COLOROP 4
>>   #define PLANE_DEGAMMA_SIZE 128
>>   #define PLANE_GAMMA_SIZE 32
>>
>> @@ -18,69 +21,124 @@ static const struct drm_colorop_funcs
>> intel_colorop_funcs = {  };
>>
>>   static
>> -int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
>> drm_prop_enum_list *list,
>> -				     enum pipe pipe)
>> +struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct
>> drm_plane *plane,
>> +							     struct
>> intel_colorop *prev,
>> +							     enum
>> intel_color_block id)
>>   {
> 
> Seems like you just added a new function and then changed the function
> _intel_color_pipeline_plane_init but git format-patch messed this patch up bit
> Maybe try use --patience option to make this patch more readable.
>   

I used it on the v2 but did not help much :/ May be I am missing something.

>>   	struct drm_device *dev = plane->dev;
>> -	struct intel_display *display = to_intel_display(dev);
>> -	struct drm_colorop *prev_op;
>>   	struct intel_colorop *colorop;
>>   	int ret;
>>
>> -	colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT);
>> -
>> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
>> plane, &intel_colorop_funcs,
>> -						  PLANE_DEGAMMA_SIZE,
>> -
>> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>> -
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +	colorop = intel_colorop_create(id);
>> +
>> +	if (IS_ERR(colorop))
>> +		return colorop;
>> +
>> +	switch (id) {
>> +	case INTEL_PLANE_CB_PRE_CSC_LUT:
>> +		ret = drm_plane_colorop_curve_1d_lut_init(dev,
>> +							  &colorop->base,
>> plane,
>> +
>> &intel_colorop_funcs,
>> +
>> PLANE_DEGAMMA_SIZE,
>> +
>> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>> +
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +		break;
>> +	case INTEL_PLANE_CB_CSC:
>> +		ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base,
>> plane,
>> +						     &intel_colorop_funcs,
>> +
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +		break;
>> +	case INTEL_PLANE_CB_3DLUT:
>> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
>> plane,
>> +						   &intel_colorop_funcs, 17,
>> +
>> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>> +						   true);
>> +		break;
>> +	case INTEL_PLANE_CB_POST_CSC_LUT:
>> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop-
>>> base, plane,
>> +
>> &intel_colorop_funcs,
>> +
>> PLANE_GAMMA_SIZE,
>> +
>> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>> +
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> +		break;
>> +	default:
>> +		drm_err(plane->dev, "Invalid colorop id [%d]", id);
>> +		ret = -EINVAL;
>> +	}
>>
>>   	if (ret)
>> -		return ret;
>> +		goto cleanup;
>>
>> -	list->type = colorop->base.base.id;
>> +	if (prev)
>> +		drm_colorop_set_next_property(&prev->base, &colorop-
>>> base);
>>
>> -	/* TODO: handle failures and clean up */
>> -	prev_op = &colorop->base;
>> +	return colorop;
>>
>> -	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
>> -	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
>> &intel_colorop_funcs,
>> -
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> -	if (ret)
>> -		return ret;
>> +cleanup:
>> +	intel_colorop_destroy(&colorop->base);
>> +	return ERR_PTR(ret);
>> +}
>>
>> -	drm_colorop_set_next_property(prev_op, &colorop->base);
>> -	prev_op = &colorop->base;
>> +static
>> +int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
>> drm_prop_enum_list *list,
>> +				     enum pipe pipe)
>> +{
>> +	struct drm_device *dev = plane->dev;
>> +	struct intel_display *display = to_intel_display(dev);
>> +	struct intel_colorop *colorop[MAX_COLOROP];
>> +	int ret = 0;
>> +	int i = 0;
>>
>> -	if (DISPLAY_VER(display) >= 35 &&
>> -	    intel_color_crtc_has_3dlut(display, pipe) &&
>> -	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>> -		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
>> +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, NULL,
>> +
>> INTEL_PLANE_CB_PRE_CSC_LUT);
>> +
>> +	if (IS_ERR(colorop[i])) {
>> +		ret = PTR_ERR(colorop[i]);
>> +		goto cleanup;
>> +	}
>> +	i++;
> 
> I see a lot of repeated code maybe we can get this done via a loop
> static const enum intel_colorop_type pipeline[] = {
>          INTEL_PLANE_CB_PRE_CSC_LUT,
>          INTEL_PLANE_CB_CSC,
>          INTEL_PLANE_CB_3DLUT,
>          INTEL_PLANE_CB_POST_CSC_LUT,
> };
> 
> for (n = 0; n < ARRAY_SIZE(pipeline); n++) {
>          if (pipeline[n] == INTEL_PLANE_CB_3DLUT &&
>              (DISPLAY_VER(display) < 35 ||
>               plane->type != DRM_PLANE_TYPE_PRIMARY ||
>               !intel_color_crtc_has_3dlut(display, pipe)))
>                  continue;
> 
>          ret = add_plane_colorop(plane, colorop, &i, &prev, pipeline[n]);
>          if (ret)
>                  goto cleanup;
> }
> 
> Where add_plane_colorop is
> 
> static int
> add_plane_colorop(struct drm_plane *plane,
>                    struct intel_colorop **colorop,
>                    int *i,
>                    struct intel_colorop **prev,
>                    enum intel_colorop_type type)
> {
>          colorop[*i] = intel_color_pipeline_plane_add_colorop(plane, *prev, type);
>          if (IS_ERR(colorop[*i]))
>                  return PTR_ERR(colorop[*i]);
> 
>          *prev = colorop[*i];
>          (*i)++;
> 
>          return 0;
> }

Sounds like a good idea, thank you. Implemented a version of this in v2

https://lore.kernel.org/intel-gfx/20260109081728.478844-14-chaitanya.kumar.borah@intel.com/T/#u

> 
> Regards,
> Suraj Kandpal
> 
>>
>> -		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
>> plane,
>> -						   &intel_colorop_funcs, 17,
>> -
>> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>> -						   true);
>> -		if (ret)
>> -			return ret;
>>
>> -		drm_colorop_set_next_property(prev_op, &colorop->base);
>> +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i -
>> 1],
>> +
>> INTEL_PLANE_CB_CSC);
>>
>> -		prev_op = &colorop->base;
>> +	if (IS_ERR(colorop[i])) {
>> +		ret = PTR_ERR(colorop[i]);
>> +		goto cleanup;
>>   	}
>>
>> -	colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT);
>> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base,
>> plane, &intel_colorop_funcs,
>> -						  PLANE_GAMMA_SIZE,
>> -
>> DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>> -
>> DRM_COLOROP_FLAG_ALLOW_BYPASS);
>> -	if (ret)
>> -		return ret;
>> +	i++;
>>
>> -	drm_colorop_set_next_property(prev_op, &colorop->base);
>> +	if (DISPLAY_VER(display) >= 35 &&
>> +	    intel_color_crtc_has_3dlut(display, pipe) &&
>> +	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>> +		colorop[i] = intel_color_pipeline_plane_add_colorop(plane,
>> colorop[i - 1],
>> +
>> INTEL_PLANE_CB_3DLUT);
>> +
>> +		if (IS_ERR(colorop[i])) {
>> +			ret = PTR_ERR(colorop[i]);
>> +			goto cleanup;
>> +		}
>> +		i++;
>> +	}
>>
>> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
>> +	colorop[i] = intel_color_pipeline_plane_add_colorop(plane, colorop[i -
>> 1],
>> +
>> INTEL_PLANE_CB_POST_CSC_LUT);
>> +	if (IS_ERR(colorop[i])) {
>> +		ret = PTR_ERR(colorop[i]);
>> +		goto cleanup;
>> +	}
>> +	i++;
>> +
>> +	list->type = colorop[0]->base.base.id;
>> +	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d",
>> +colorop[0]->base.base.id);
>>
>>   	return 0;
>> +
>> +cleanup:
>> +	while (--i >= 0)
>> +		intel_colorop_destroy(&colorop[i]->base);
>> +	return ret;
>>   }
>>
>>   int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
>> --
>> 2.25.1
>