| Message ID | 20231108163647.106853-9-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Color Pipeline API w/ VKMS | expand |
On Wed, 8 Nov 2023 11:36:27 -0500 Harry Wentland <harry.wentland@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.wentland@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
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 6390443f1819..59e1797d1ca8 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -634,8 +634,8 @@ drm_atomic_get_colorop_state(struct drm_atomic_state *state, state->colorops[index].new_state = colorop_state; colorop_state->state = state; - drm_dbg_atomic(colorop->dev, "Added [COLOROP:%d] %p state to %p\n", - colorop->base.id, colorop_state, state); + drm_dbg_atomic(colorop->dev, "Added [COLOROP:%d:%d] %p state to %p\n", + colorop->base.id, colorop->type, colorop_state, state); return colorop_state; } diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 21da1b327ee9..f22bd8671236 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -682,7 +682,13 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop, const struct drm_colorop_state *state, struct drm_property *property, uint64_t *val) { - return -EINVAL; + if (property == colorop->type_property) { + *val = colorop->type; + } else { + return -EINVAL; + } + + return 0; } 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); + + if (!prop) + return -ENOMEM; + + colorop->type_property = prop; + + drm_object_attach_property(&colorop->base, + colorop->type_property, + colorop->type); + return ret; } EXPORT_SYMBOL(drm_colorop_init); @@ -150,3 +171,23 @@ void drm_colorop_reset(struct drm_colorop *colorop) __drm_colorop_reset(colorop, colorop->state); } EXPORT_SYMBOL(drm_colorop_reset); + + +static const char * const colorop_type_name[] = { + [DRM_COLOROP_1D_CURVE] = "1D Curve", +}; + +/** + * drm_get_colorop_type_name - return a string for colorop type + * @type: colorop type to compute name of + * + * In contrast to the other drm_get_*_name functions this one here returns a + * const pointer and hence is threadsafe. + */ +const char *drm_get_colorop_type_name(enum drm_colorop_type type) +{ + if (WARN_ON(type >= ARRAY_SIZE(colorop_type_name))) + return "unknown"; + + return colorop_type_name[type]; +} diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index 3dd169b0317d..0386440afe40 100644 --- a/include/drm/drm_colorop.h +++ b/include/drm/drm_colorop.h @@ -103,6 +103,21 @@ struct drm_colorop { /** @properties: property tracking for this plane */ struct drm_object_properties properties; + /** + * @type: + * + * Read-only + * Type of color operation + */ + enum drm_colorop_type type; + + /** + * @type_property: + * + * Read-only "TYPE" property for specifying the type of + * this color operation. The type is enum drm_colorop_type. + */ + struct drm_property *type_property; }; #define obj_to_colorop(x) container_of(x, struct drm_colorop, base) @@ -127,7 +142,7 @@ static inline struct drm_colorop *drm_colorop_find(struct drm_device *dev, } 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_colorop_state * drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop); 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 +}; + /** * struct hdr_metadata_infoframe - HDR Metadata Infoframe Data. *
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.wentland@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(-)