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

[PATCH v2 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/

v2:
 - Re-arrange patches (Alex)
 - Re-factor code to avoid repitition in pipeline creation (Suraj)

==
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/colorop: Add destroy helper for colorop objects
  drm: Allow driver-managed destruction of 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   | 179 +++++++++++++-----
 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, 264 insertions(+), 90 deletions(-)

-- 
2.25.1

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

[Chaitanya Kumar Borah]

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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@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 v2 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>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 v2 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>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 v2 03/13] drm/vkms: Fix color pipeline enum name leak

[Louis Chauvet]

On 1/9/26 09:17, 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>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alex Hung <alex.hung@amd.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.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;
>   }

[PATCH v2 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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@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 v2 05/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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 44eb823585d2..ba19a3ab23cb 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -178,6 +178,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 a3a32f9f918c..0f5ba72c1704 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -420,6 +420,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 v2 05/13] drm/colorop: Add destroy helper for colorop objects

[Jani Nikula]

On Fri, 09 Jan 2026, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> 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>
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alex Hung <alex.hung@amd.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 44eb823585d2..ba19a3ab23cb 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -178,6 +178,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 a3a32f9f918c..0f5ba72c1704 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -420,6 +420,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.
> + */

The kernel-doc should be with the function definition, not at the
declaration, and certainly not both.

BR,
Jani.

> +void drm_colorop_destroy(struct drm_colorop *colorop);
> +
>  /**
>   * drm_colorop_index - find the index of a registered colorop
>   * @colorop: colorop to find index for

-- 
Jani Nikula, Intel

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

[Borah, Chaitanya Kumar]

On 1/12/2026 4:53 PM, Jani Nikula wrote:
> On Fri, 09 Jan 2026, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> 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>
>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Alex Hung <alex.hung@amd.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 44eb823585d2..ba19a3ab23cb 100644
>> --- a/drivers/gpu/drm/drm_colorop.c
>> +++ b/drivers/gpu/drm/drm_colorop.c
>> @@ -178,6 +178,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 a3a32f9f918c..0f5ba72c1704 100644
>> --- a/include/drm/drm_colorop.h
>> +++ b/include/drm/drm_colorop.h
>> @@ -420,6 +420,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.
>> + */
> 
> The kernel-doc should be with the function definition, not at the
> declaration, and certainly not both.
> 

Thank you, Jani, for pointing it out. I have removed it in v3.

Regards
Chaitanya

> BR,
> Jani.
> 
>> +void drm_colorop_destroy(struct drm_colorop *colorop);
>> +
>>   /**
>>    * drm_colorop_index - find the index of a registered colorop
>>    * @colorop: colorop to find index for
> 

[PATCH v2 06/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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 ba19a3ab23cb..4b27804bb0bd 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++;
@@ -215,6 +217,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.
@@ -222,7 +225,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;
@@ -243,7 +247,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;
 
@@ -300,20 +304,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;
 
@@ -351,11 +358,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;
 
@@ -375,16 +383,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;
 
@@ -403,6 +413,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)
@@ -410,7 +421,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 0f5ba72c1704..ee6454b08b2d 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 v2 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>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 v2 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>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 v2 08/13] drm/vkms: Hook up colorop destroy helper for plane pipelines

[Louis Chauvet]

On 1/9/26 09:17, 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>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alex Hung <alex.hung@amd.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.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;

[PATCH v2 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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@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 1d84933f05aa..9e54f51cfad8 100644
--- a/drivers/gpu/drm/i915/display/intel_colorop.c
+++ b/drivers/gpu/drm/i915/display/intel_colorop.c
@@ -35,3 +35,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 9276eee6e75a..638baf67d98d 100644
--- a/drivers/gpu/drm/i915/display/intel_colorop.h
+++ b/drivers/gpu/drm/i915/display/intel_colorop.h
@@ -13,5 +13,6 @@ struct intel_colorop;
 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 v2 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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 v2 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>
Reviewed-by: Uma Shankar <uma.shankar@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 434c295f44ba..95020765c4c2 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -259,7 +259,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 v2 11/13] drm/vkms: Remove drm_colorop_pipeline_destroy() from vkms_destroy()

[Louis Chauvet]

On 1/9/26 09:17, Chaitanya Kumar Borah wrote:
> 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>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.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 434c295f44ba..95020765c4c2 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -259,7 +259,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);

[PATCH v2 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>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alex Hung <alex.hung@amd.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 v2 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.

v2:
 - Refactor code to avoid repetition (Suraj)

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/i915/display/intel_color_pipeline.c   | 164 +++++++++++++-----
 1 file changed, 117 insertions(+), 47 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..1b8d504fa9f2 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
 
@@ -17,70 +20,137 @@ static const struct drm_colorop_funcs intel_colorop_funcs = {
 	.destroy = intel_colorop_destroy,
 };
 
+/*
+ * 3DLUT can be bound to all three HDR planes. However, even with the latest
+ * color pipeline UAPI, there is no good way to represent a HW block which
+ * can be shared/attached at different stages of the pipeline. So right now,
+ * we expose 3DLUT only attached with the primary plane.
+ *
+ * That way we don't confuse the userspace with opaque commit failures
+ * on trying to enable it on multiple planes which would otherwise make
+ * the pipeline totally unusable.
+ */
+static const enum intel_color_block nvl_primary_plane_pipeline[] = {
+	INTEL_PLANE_CB_PRE_CSC_LUT,
+	INTEL_PLANE_CB_CSC,
+	INTEL_PLANE_CB_3DLUT,
+	INTEL_PLANE_CB_POST_CSC_LUT,
+};
+
+static const enum intel_color_block hdr_plane_pipeline[] = {
+	INTEL_PLANE_CB_PRE_CSC_LUT,
+	INTEL_PLANE_CB_CSC,
+	INTEL_PLANE_CB_POST_CSC_LUT,
+};
+
+static bool plane_has_3dlut(struct intel_display *display, enum pipe pipe,
+			    struct drm_plane *plane)
+{
+	return (DISPLAY_VER(display) >= 35 &&
+		intel_color_crtc_has_3dlut(display, pipe) &&
+		plane->type == DRM_PLANE_TYPE_PRIMARY);
+}
+
 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);
-
-	if (ret)
-		return ret;
-
-	list->type = colorop->base.base.id;
-
-	/* 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, &intel_colorop_funcs,
-					     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) {
-		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
-
+	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);
-		if (ret)
-			return ret;
-
-		drm_colorop_set_next_property(prev_op, &colorop->base);
-
-		prev_op = &colorop->base;
+		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;
 	}
 
-	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;
+		goto cleanup;
 
-	drm_colorop_set_next_property(prev_op, &colorop->base);
+	if (prev)
+		drm_colorop_set_next_property(&prev->base, &colorop->base);
 
-	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
+	return colorop;
+
+cleanup:
+	intel_colorop_destroy(&colorop->base);
+	return ERR_PTR(ret);
+}
+
+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];
+	struct intel_colorop *prev = NULL;
+	const enum intel_color_block *pipeline;
+	int pipeline_len;
+	int ret = 0;
+	int i;
+
+	if (plane_has_3dlut(display, pipe, plane)) {
+		pipeline = nvl_primary_plane_pipeline;
+		pipeline_len = ARRAY_SIZE(nvl_primary_plane_pipeline);
+	} else {
+		pipeline = hdr_plane_pipeline;
+		pipeline_len = ARRAY_SIZE(hdr_plane_pipeline);
+	}
+
+	for (i = 0; i < pipeline_len; i++) {
+		colorop[i] = intel_color_pipeline_plane_add_colorop(plane, prev,
+								    pipeline[i]);
+		if (IS_ERR(colorop[i])) {
+			ret = PTR_ERR(colorop[i]);
+			goto cleanup;
+		}
+
+		prev = colorop[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 v2 13/13] drm/i915/color: Add failure handling in plane color pipeline init

[Kandpal, Suraj]

> Subject: [PATCH v2 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.
> 
> v2:
>  - Refactor code to avoid repetition (Suraj)
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  .../drm/i915/display/intel_color_pipeline.c   | 164 +++++++++++++-----
>  1 file changed, 117 insertions(+), 47 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..1b8d504fa9f2 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
> 
> @@ -17,70 +20,137 @@ static const struct drm_colorop_funcs
> intel_colorop_funcs = {
>  	.destroy = intel_colorop_destroy,
>  };
> 
> +/*
> + * 3DLUT can be bound to all three HDR planes. However, even with the
> +latest
> + * color pipeline UAPI, there is no good way to represent a HW block
> +which
> + * can be shared/attached at different stages of the pipeline. So right
> +now,
> + * we expose 3DLUT only attached with the primary plane.
> + *
> + * That way we don't confuse the userspace with opaque commit failures
> + * on trying to enable it on multiple planes which would otherwise make
> + * the pipeline totally unusable.
> + */
> +static const enum intel_color_block nvl_primary_plane_pipeline[] = {

Use the official code name that would be xe3plpd 

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

> +	INTEL_PLANE_CB_PRE_CSC_LUT,
> +	INTEL_PLANE_CB_CSC,
> +	INTEL_PLANE_CB_3DLUT,
> +	INTEL_PLANE_CB_POST_CSC_LUT,
> +};
> +
> +static const enum intel_color_block hdr_plane_pipeline[] = {
> +	INTEL_PLANE_CB_PRE_CSC_LUT,
> +	INTEL_PLANE_CB_CSC,
> +	INTEL_PLANE_CB_POST_CSC_LUT,
> +};
> +
> +static bool plane_has_3dlut(struct intel_display *display, enum pipe pipe,
> +			    struct drm_plane *plane)
> +{
> +	return (DISPLAY_VER(display) >= 35 &&
> +		intel_color_crtc_has_3dlut(display, pipe) &&
> +		plane->type == DRM_PLANE_TYPE_PRIMARY); }
> +
>  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);
> -
> -	if (ret)
> -		return ret;
> -
> -	list->type = colorop->base.base.id;
> -
> -	/* 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,
> &intel_colorop_funcs,
> -
> 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) {
> -		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
> -
> +	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);
> -		if (ret)
> -			return ret;
> -
> -		drm_colorop_set_next_property(prev_op, &colorop->base);
> -
> -		prev_op = &colorop->base;
> +		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;
>  	}
> 
> -	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;
> +		goto cleanup;
> 
> -	drm_colorop_set_next_property(prev_op, &colorop->base);
> +	if (prev)
> +		drm_colorop_set_next_property(&prev->base, &colorop-
> >base);
> 
> -	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", list->type);
> +	return colorop;
> +
> +cleanup:
> +	intel_colorop_destroy(&colorop->base);
> +	return ERR_PTR(ret);
> +}
> +
> +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];
> +	struct intel_colorop *prev = NULL;
> +	const enum intel_color_block *pipeline;
> +	int pipeline_len;
> +	int ret = 0;
> +	int i;
> +
> +	if (plane_has_3dlut(display, pipe, plane)) {
> +		pipeline = nvl_primary_plane_pipeline;
> +		pipeline_len = ARRAY_SIZE(nvl_primary_plane_pipeline);
> +	} else {
> +		pipeline = hdr_plane_pipeline;
> +		pipeline_len = ARRAY_SIZE(hdr_plane_pipeline);
> +	}
> +
> +	for (i = 0; i < pipeline_len; i++) {
> +		colorop[i] = intel_color_pipeline_plane_add_colorop(plane,
> prev,
> +								    pipeline[i]);
> +		if (IS_ERR(colorop[i])) {
> +			ret = PTR_ERR(colorop[i]);
> +			goto cleanup;
> +		}
> +
> +		prev = colorop[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