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?

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;
> +}

Reply via email to