[V7 16/45] drm/colorop: Add 3x4 CTM type
Louis Chauvet
louis.chauvet at bootlin.com
Tue Feb 25 11:20:23 UTC 2025
Le 20/12/2024 à 05:33, Alex Hung a écrit :
> From: Harry Wentland <harry.wentland at amd.com>
>
> This type is used to support a 3x4 matrix in colorops. A 3x4
> matrix uses the last column as a "bias" column. Some HW exposes
> support for 3x4. The calculation looks like:
>
> out matrix in
> |R| |0 1 2 3 | | R |
> |G| = |4 5 6 7 | x | G |
> |B| |8 9 10 11| | B |
> |1.0|
>
> This is also the first colorop where we need a blob property to
> program the property. For that we'll introduce a new DATA
> property that can be used by all colorop TYPEs requiring a
> blob. The way a DATA blob is read depends on the TYPE of
> the colorop.
>
> We only create the DATA property for property types that
> need it.
>
> Signed-off-by: Alex Hung <alex.hung at amd.com>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> ---
>
> v6:
> - take ref for DATA blob in duplicate_state func (Xaver Hugl)
>
> v5:
> - Add function signature for init (Sebastian)
> - Fix kernel-doc
>
> v4:
> - Create helper function for creating 3x4 CTM colorop
> - Fix CTM indexes in comment (Pekka)
>
> drivers/gpu/drm/drm_atomic.c | 14 ++++++++++-
> drivers/gpu/drm/drm_atomic_uapi.c | 29 +++++++++++++++++++++
> drivers/gpu/drm/drm_colorop.c | 42 +++++++++++++++++++++++++++++++
> include/drm/drm_colorop.h | 21 ++++++++++++++++
> include/uapi/drm/amdgpu_drm.h | 9 -------
> include/uapi/drm/drm_mode.h | 24 +++++++++++++++++-
> 6 files changed, 128 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 35136987d5e8..c58663327e6b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -787,7 +787,19 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
> drm_printf(p, "colorop[%u]:\n", colorop->base.id);
> drm_printf(p, "\ttype=%s\n", drm_get_colorop_type_name(colorop->type));
> drm_printf(p, "\tbypass=%u\n", state->bypass);
> - drm_printf(p, "\tcurve_1d_type=%s\n", drm_get_colorop_curve_1d_type_name(state->curve_1d_type));
> +
> + switch (colorop->type) {
> + case DRM_COLOROP_1D_CURVE:
> + drm_printf(p, "\tcurve_1d_type=%s\n",
> + drm_get_colorop_curve_1d_type_name(state->curve_1d_type));
> + break;
> + case DRM_COLOROP_CTM_3X4:
> + drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data->base.id : 0);
> + break;
> + default:
> + break;
> + }
> +
As suggested by Simon, you could add this switch in a previous patch, so
you could avoid editing the same line twice.
With or without this change:
Reviewed-by: Louis Chauvet <louis.chauvet at bootlin.com>
> drm_printf(p, "\tnext=%d\n", colorop->next ? colorop->next->base.id : 0);
> }
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index f0c22abcc28f..7bc4978e5441 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -692,6 +692,30 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> return 0;
> }
>
> +static int drm_atomic_color_set_data_property(struct drm_colorop *colorop,
> + struct drm_colorop_state *state,
> + struct drm_property *property, uint64_t val)
> +{
> + ssize_t elem_size = -1;
> + ssize_t size = -1;
> + bool replaced = false;
> +
> + switch (colorop->type) {
> + case DRM_COLOROP_CTM_3X4:
> + size = sizeof(struct drm_color_ctm_3x4);
> + break;
> + default:
> + /* should never get here */
> + return -EINVAL;
> + }
> +
> + return drm_property_replace_blob_from_id(colorop->dev,
> + &state->data,
> + val,
> + size,
> + elem_size,
> + &replaced);
> +}
>
> static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
> struct drm_colorop_state *state, struct drm_file *file_priv,
> @@ -701,6 +725,9 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
> state->bypass = val;
> } else if (property == colorop->curve_1d_type_property) {
> state->curve_1d_type = val;
> + } else if (property == colorop->data_property) {
> + return drm_atomic_color_set_data_property(colorop,
> + state, property, val);
> } else {
> drm_dbg_atomic(colorop->dev,
> "[COLOROP:%d:%d] unknown property [PROP:%d:%s]]\n",
> @@ -723,6 +750,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
> *val = state->bypass;
> } else if (property == colorop->curve_1d_type_property) {
> *val = state->curve_1d_type;
> + } else if (property == colorop->data_property) {
> + *val = (state->data) ? state->data->base.id : 0;
> } else {
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 954acd09673a..7d4b29c0a0cc 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -64,6 +64,7 @@
>
> static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
> { DRM_COLOROP_1D_CURVE, "1D Curve" },
> + { DRM_COLOROP_CTM_3X4, "3x4 Matrix"}
> };
>
> static const char * const colorop_curve_1d_type_names[] = {
> @@ -202,11 +203,51 @@ int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *coloro
> }
> EXPORT_SYMBOL(drm_colorop_curve_1d_init);
>
> +static int drm_colorop_create_data_prop(struct drm_device *dev, struct drm_colorop *colorop)
> +{
> + struct drm_property *prop;
> +
> + /* data */
> + prop = drm_property_create(dev, DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
> + "DATA", 0);
> + if (!prop)
> + return -ENOMEM;
> +
> + colorop->data_property = prop;
> + drm_object_attach_property(&colorop->base,
> + colorop->data_property,
> + 0);
> +
> + return 0;
> +}
> +
> +int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
> + struct drm_plane *plane)
> +{
> + int ret;
> +
> + ret = drm_colorop_init(dev, colorop, plane, DRM_COLOROP_CTM_3X4);
> + if (ret)
> + return ret;
> +
> + ret = drm_colorop_create_data_prop(dev, colorop);
> + if (ret)
> + return ret;
> +
> + drm_colorop_reset(colorop);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_colorop_ctm_3x4_init);
> +
> static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop,
> struct drm_colorop_state *state)
> {
> memcpy(state, colorop->state, sizeof(*state));
>
> + if (state->data)
> + drm_property_blob_get(state->data);
> +
> state->bypass = true;
> }
>
> @@ -288,6 +329,7 @@ void drm_colorop_reset(struct drm_colorop *colorop)
>
> static const char * const colorop_type_name[] = {
> [DRM_COLOROP_1D_CURVE] = "1D Curve",
> + [DRM_COLOROP_CTM_3X4] = "3x4 Matrix"
> };
>
> const char *drm_get_colorop_type_name(enum drm_colorop_type type)
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> index 2f0572be37bb..e9f5c1adc2fe 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -96,6 +96,14 @@ struct drm_colorop_state {
> */
> enum drm_colorop_curve_1d_type curve_1d_type;
>
> + /**
> + * @data:
> + *
> + * Data blob for any TYPE that requires such a blob. The
> + * interpretation of the blob is TYPE-specific.
> + */
> + struct drm_property_blob *data;
> +
> /** @state: backpointer to global drm_atomic_state */
> struct drm_atomic_state *state;
> };
> @@ -200,6 +208,17 @@ struct drm_colorop {
> */
> struct drm_property *curve_1d_type_property;
>
> + /**
> + * @data_property:
> + *
> + * blob property for any TYPE that requires a blob of data,
> + * such as 1DLUT, CTM, 3DLUT, etc.
> + *
> + * The way this blob is interpreted depends on the TYPE of
> + * this
> + */
> + struct drm_property *data_property;
> +
> /**
> * @next_property:
> *
> @@ -236,6 +255,8 @@ static inline struct drm_colorop *drm_colorop_find(struct drm_device *dev,
>
> int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop,
> struct drm_plane *plane, u64 supported_tfs);
> +int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop,
> + struct drm_plane *plane);
>
> struct drm_colorop_state *
> drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop);
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index efe5de6ce208..e44362e74fa1 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -1283,15 +1283,6 @@ struct drm_amdgpu_info_gpuvm_fault {
> #define AMDGPU_FAMILY_GC_11_5_0 150 /* GC 11.5.0 */
> #define AMDGPU_FAMILY_GC_12_0_0 152 /* GC 12.0.0 */
>
> -/* FIXME wrong namespace! */
> -struct drm_color_ctm_3x4 {
> - /*
> - * Conversion matrix with 3x4 dimensions in S31.32 sign-magnitude
> - * (not two's complement!) format.
> - */
> - __u64 matrix[12];
> -};
> -
> #if defined(__cplusplus)
> }
> #endif
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 53985d2b7eea..6fc1ce24800a 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -847,6 +847,20 @@ struct drm_color_ctm {
> __u64 matrix[9];
> };
>
> +struct drm_color_ctm_3x4 {
> + /*
> + * Conversion matrix with 3x4 dimensions in S31.32 sign-magnitude
> + * (not two's complement!) format.
> + *
> + * out matrix in
> + * |R| |0 1 2 3 | | R |
> + * |G| = |4 5 6 7 | x | G |
> + * |B| |8 9 10 11| | B |
> + * |1.0|
> + */
> + __u64 matrix[12];
> +};
> +
> struct drm_color_lut {
> /*
> * Values are mapped linearly to 0.0 - 1.0 range, with 0x0 == 0.0 and
> @@ -872,7 +886,15 @@ enum drm_colorop_type {
> * A 1D curve that is being applied to all color channels. The
> * curve is specified via the CURVE_1D_TYPE colorop property.
> */
> - DRM_COLOROP_1D_CURVE
> + DRM_COLOROP_1D_CURVE,
> +
> + /**
> + * @DRM_COLOROP_CTM_3X4:
> + *
> + * A 3x4 matrix. Its values are specified via the
> + * &drm_color_ctm_3x4 struct provided via the DATA property.
> + */
> + DRM_COLOROP_CTM_3X4,
> };
>
> /**
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
More information about the amd-gfx
mailing list