On 2/25/25 03:13, Louis Chauvet wrote:
Le 20/12/2024 à 05:33, Alex Hung a écrit :From: Harry Wentland <harry.wentland@xxxxxxx> 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. Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx> Co-developed-by: Alex Hung <alex.hung@xxxxxxx> Signed-off-by: Alex Hung <alex.hung@xxxxxxx> --- v5: - Add drm_get_colorop_curve_1d_type_name in header - Add drm_colorop_init - Set default curve - Add kernel docs v4: - Use drm_colorop_curve_1d_type_enum_list to get name (Pekka) - Create separate init function for 1D curve - Pass supported TFs into 1D curve init function drivers/gpu/drm/drm_atomic_uapi.c | 18 ++-- drivers/gpu/drm/drm_colorop.c | 134 ++++++++++++++++++++++++++++++ include/drm/drm_colorop.h | 60 +++++++++++++ 3 files changed, 207 insertions(+), 5 deletions(-)diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/ drm_atomic_uapi.cindex 59fc25b59100..9a5dbf0a1306 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c@@ -648,11 +648,17 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,struct drm_colorop_state *state, struct drm_file *file_priv, struct drm_property *property, uint64_t val) { - drm_dbg_atomic(colorop->dev, - "[COLOROP:%d] unknown property [PROP:%d:%s]]\n", - colorop->base.id, - property->base.id, property->name); - return -EINVAL; + if (property == colorop->curve_1d_type_property) { + state->curve_1d_type = val; + } else { + drm_dbg_atomic(colorop->dev, + "[COLOROP:%d:%d] unknown property [PROP:%d:%s]]\n", + colorop->base.id, colorop->type, + property->base.id, property->name); + return -EINVAL; + } + + return 0; } static int@@ -662,6 +668,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,{ if (property == colorop->type_property) { *val = colorop->type; + } else if (property == colorop->curve_1d_type_property) { + *val = state->curve_1d_type; } else { return -EINVAL; }diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/ drm_colorop.cindex 1459a28c7e7b..a42de0aa48e1 100644 --- a/drivers/gpu/drm/drm_colorop.c +++ b/drivers/gpu/drm/drm_colorop.c @@ -31,6 +31,123 @@ #include "drm_crtc_internal.h" +static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = { + { DRM_COLOROP_1D_CURVE, "1D Curve" }, +}; + +static const char * const colorop_curve_1d_type_names[] = { + [DRM_COLOROP_1D_CURVE_SRGB_EOTF] = "sRGB EOTF", + [DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF] = "sRGB Inverse EOTF", +}; + + +/* Init Helpers */ ++static int drm_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++; + + /* 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));I think this function belongs to the previous patch "Add TYPE property".
This function is only called by the first colorop. Some pieces of the code in this function are introduced with the first colorop (1D curve) so it makes sense to include it here.
+ + if (!prop) + return -ENOMEM; + + colorop->type_property = prop; + + drm_object_attach_property(&colorop->base, + colorop->type_property, + colorop->type); + + return ret; +} + +/** + * drm_colorop_curve_1d_init - Initialize a DRM_COLOROP_1D_CURVE + * + * @dev: DRM device + * @colorop: The drm_colorop object to initialize + * @plane: The associated drm_plane+ * @supported_tfs: A bitfield of supported drm_colorop_curve_1d_init enum values, + * created using BIT(curve_type) and combined with the OR '|'+ * operator. + * @return zero on success, -E value on failure + */+int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,+ struct drm_plane *plane, u64 supported_tfs) +{ + struct drm_prop_enum_list enum_list[DRM_COLOROP_1D_CURVE_COUNT]; + int i, len; + + struct drm_property *prop; + int ret; + + if (!supported_tfs) { + drm_err(dev,+ "No supported TFs for new 1D curve colorop on [PLANE:%d: %s]\n",+ plane->base.id, plane->name); + return -EINVAL; + } + + if ((supported_tfs & -BIT(DRM_COLOROP_1D_CURVE_COUNT)) != 0) { + drm_err(dev, "Unknown TF provided on [PLANE:%d:%s]\n", + plane->base.id, plane->name); + return -EINVAL; + } + + ret = drm_colorop_init(dev, colorop, plane, DRM_COLOROP_1D_CURVE); + if (ret) + return ret; + + len = 0; + for (i = 0; i < DRM_COLOROP_1D_CURVE_COUNT; i++) { + if ((supported_tfs & BIT(i)) == 0) + continue; + + enum_list[len].type = i; + enum_list[len].name = colorop_curve_1d_type_names[i]; + len++; + } + + if (WARN_ON(len <= 0)) + return -EINVAL; + + + /* initialize 1D curve only attribute */+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC, "CURVE_1D_TYPE",+ enum_list, len); + if (!prop) + return -ENOMEM; + + colorop->curve_1d_type_property = prop;+ drm_object_attach_property(&colorop->base, colorop- >curve_1d_type_property,+ enum_list[0].type); + drm_colorop_reset(colorop); + + return 0; +} +EXPORT_SYMBOL(drm_colorop_curve_1d_init); +static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop,struct drm_colorop_state *state) {@@ -70,7 +187,16 @@ void drm_colorop_atomic_destroy_state(struct drm_colorop *colorop, static void __drm_colorop_state_reset(struct drm_colorop_state *colorop_state,struct drm_colorop *colorop) { + u64 val; + colorop_state->colorop = colorop; + + if (colorop->curve_1d_type_property) { + drm_object_property_get_default_value(&colorop->base, + colorop->curve_1d_type_property, + &val); + colorop_state->curve_1d_type = val; + } } /**@@ -114,3 +240,11 @@ const char *drm_get_colorop_type_name(enum drm_colorop_type typereturn colorop_type_name[type];Probably a dumb question: can't we use drm_colorop_type_enum_list instead of colorop_type_name in the drm_get_colorop_type_name function? So we will avoid duplicating the string "1D Curve".
Using drm_colorop_type_enum_list in drm_get_colorop_type_name needs enumerating the list every time (extra CPU cycles) unlike using colorop_type_name[].
}Thanks, Louis Chauvet [...]
