On Wed, 8 Nov 2023 11:36:27 -0500
Harry Wentland <harry.wentl...@amd.com> wrote:

> Add a read-only TYPE property. The TYPE specifies the colorop
> type, such as enumerated curve, 1D LUT, CTM, 3D LUT, PWL LUT,
> etc.
> 
> v3:
>  - Make TYPE a range property
>  - Move enum drm_colorop_type to uapi header
>  - Fix drm_get_colorop_type_name description
> 
> For now we're only introducing an enumerated 1D LUT type to
> illustrate the concept.
> 
> Signed-off-by: Harry Wentland <harry.wentl...@amd.com>
> ---
>  drivers/gpu/drm/drm_atomic.c      |  4 +--
>  drivers/gpu/drm/drm_atomic_uapi.c |  8 +++++-
>  drivers/gpu/drm/drm_colorop.c     | 43 ++++++++++++++++++++++++++++++-
>  include/drm/drm_colorop.h         | 17 +++++++++++-
>  include/uapi/drm/drm_mode.h       |  4 +++
>  5 files changed, 71 insertions(+), 5 deletions(-)
> 

...

>  static int drm_atomic_set_writeback_fb_for_connector(
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index a295ab96aee1..902b13e300fd 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -32,12 +32,17 @@
>  
>  /* TODO big colorop doc, including properties, etc. */
>  
> +static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
> +     { DRM_COLOROP_1D_CURVE, "1D Curve" },
> +};
> +
>  /* Init Helpers */
>  
>  int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
> -                  struct drm_plane *plane)
> +                  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);
> @@ -46,12 +51,28 @@ int drm_colorop_init(struct drm_device *dev, struct 
> drm_colorop *colorop,
>  
>       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++;
>  
>       /* add properties */
> +
> +     /* type */
> +     prop = drm_property_create_range(dev,
> +                                     DRM_MODE_PROP_IMMUTABLE,
> +                                     "TYPE", type, type);

Why was this made a range property?

My review comment for the doc patch is wrong, the doc should mention
enum drm_colorop_type.

> +
> +     if (!prop)
> +             return -ENOMEM;
> +
> +     colorop->type_property = prop;
> +
> +     drm_object_attach_property(&colorop->base,
> +                                colorop->type_property,
> +                                colorop->type);
> +
>       return ret;
>  }

...

> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 4e3251ff894a..0ebf0f480dc8 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -858,6 +858,10 @@ struct drm_color_lut {
>       __u16 reserved;
>  };
>  
> +enum drm_colorop_type {
> +     DRM_COLOROP_1D_CURVE
> +};


Thanks,
pq

Attachment: pgpCxmwQGJzYF.pgp
Description: OpenPGP digital signature

Reply via email to