On 12/17/2025 6:36 AM, Alex Hung wrote:
On 12/16/25 11:19, Nícolas F. R. A. Prado wrote:
On Fri, 2025-11-14 at 17:01 -0700, Alex Hung wrote:
From: Harry Wentland <[email protected]>
Add a new drm_colorop with DRM_COLOROP_1D_CURVE with two subtypes:
DRM_COLOROP_1D_CURVE_SRGB_EOTF and
DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF.
Reviewed-by: Simon Ser <[email protected]>
Reviewed-by: Louis Chauvet <[email protected]>
Signed-off-by: Harry Wentland <[email protected]>
Co-developed-by: Alex Hung <[email protected]>
Signed-off-by: Alex Hung <[email protected]>
Reviewed-by: Daniel Stone <[email protected]>
Reviewed-by: Melissa Wen <[email protected]>
Reviewed-by: Sebastian Wick <[email protected]>
---
[..]
diff --git a/drivers/gpu/drm/drm_colorop.c
b/drivers/gpu/drm/drm_colorop.c
index 1459a28c7e7b..6fbc3c284d33 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
[..]
+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_mode_config *config = &dev->mode_config;
+ struct drm_property *prop;
+ int ret = 0;
+
+ ret = drm_mode_object_add(dev, &colorop->base,
DRM_MODE_OBJECT_COLOROP);
+ if (ret)
+ return ret;
+
+ colorop->base.properties = &colorop->properties;
+ colorop->dev = dev;
+ colorop->type = type;
+ colorop->plane = plane;
+
+ list_add_tail(&colorop->head, &config->colorop_list);
+ colorop->index = config->num_colorop++;
Hi Alex,
I know this series has already been merged, but I was looking through
the code together with Ariel and we noticed that while this init
function adds the colorop to the list in the drm_mode_config, it
doesn't remove it in the error paths below, and I believe it should.
Does that make sense?
Hi Nicolas,
drm_colorop_pipeline_destroy() calls drm_colorop_cleanup() to delete it.
After drm_colorop_pipeline_destroy is called the entire pipeline will be
freed.
void drm_colorop_cleanup(struct drm_colorop *colorop)
{
...
list_del(&colorop->head);
config->num_colorop--;
...
}
For example, amdgpu calls drm_plane_colorop_*_init functions (which call
drm_plane_colorop_init themselves) to create a pipeline. If any of
colorop creation fails, amdgpu calls drm_colorop_pipeline_destroy to
destroy the entire pipeline.
In the end, we either have a good pipeline or none.
There is one caveat though drm_colorop_pipeline_destroy() destroys all
the colorop in the mode_config. That means, that pipeline creation for
any plane fails, all colorops (even from successfully created pipelines)
are destroyed too. However, that may well be your intention here, I just
thought I would point it out.
For i915/xe, I decided to destroy colorops only for the failed pipeline.
https://lore.kernel.org/intel-gfx/[email protected]/T/#m143bb249df288cf88123d5d66283918f3317ecc2
Anyway, all these are low probability scenarios.
==
Chaitanya
Thanks,
Nicolas
+
+ /* add properties */
+
+ /* type */
+ prop = drm_property_create_enum(dev,
+ DRM_MODE_PROP_IMMUTABLE,
+ "TYPE",
drm_colorop_type_enum_list,
+ ARRAY_SIZE(drm_colorop_type_
enum_list));
+
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->type_property = prop;
+
+ drm_object_attach_property(&colorop->base,
+ colorop->type_property,
+ colorop->type);
+
+ return ret;
+}
