[PATCH V9 16/43] drm/colorop: Add 3x4 CTM type
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Mon Jun 16 11:30:23 UTC 2025
> -----Original Message-----
> From: Alex Hung <alex.hung at amd.com>
> Sent: Wednesday, April 30, 2025 6:41 AM
> To: dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org
> Cc: wayland-devel at lists.freedesktop.org; harry.wentland at amd.com;
> alex.hung at amd.com; leo.liu at amd.com; ville.syrjala at linux.intel.com;
> pekka.paalanen at collabora.com; contact at emersion.fr; mwen at igalia.com;
> jadahl at redhat.com; sebastian.wick at redhat.com;
> shashank.sharma at amd.com; agoins at nvidia.com; joshua at froggi.es;
> mdaenzer at redhat.com; aleixpol at kde.org; xaver.hugl at gmail.com;
> victoria at system76.com; daniel at ffwll.ch; Shankar, Uma
> <uma.shankar at intel.com>; quic_naseer at quicinc.com;
> quic_cbraga at quicinc.com; quic_abhinavk at quicinc.com; marcan at marcan.st;
> Liviu.Dudau at arm.com; sashamcintosh at google.com; Borah, Chaitanya
> Kumar <chaitanya.kumar.borah at intel.com>; louis.chauvet at bootlin.com;
> Daniel Stone <daniels at collabora.com>
> Subject: [PATCH V9 16/43] drm/colorop: Add 3x4 CTM type
>
> 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.
Is there any value to adding pre-offsets [1] in the uapi?
|R/Cr| | c0 c1 c2 | ( |R/Cr| |preoff0| ) |postoff0|
|G/Y | = | c3 c4 c5 | x ( |G/Y | + |preoff1| ) + |postoff1|
|B/Cb| | c6 c7 c8 | ( |B/Cb| |preoff2| ) |postoff2|
Handling limited range values is one use case that I can think of.
[1] https://cgit.freedesktop.org/drm-tip/tree/drivers/gpu/drm/i915/display/intel_color.c#n112
Regards
Chaitanya
>
> Reviewed-by: Simon Ser <contact at emersion.fr>
> Reviewed-by: Louis Chauvet <louis.chauvet at bootlin.com>
> Signed-off-by: Alex Hung <alex.hung at amd.com>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> Reviewed-by: Daniel Stone <daniels at collabora.com>
> ---
> v9:
> - Merge cleanup code for colorop->state->data in drm_colorop_cleanup
> (Simon Ser)
> - Update function names by _plane_ (Chaitanya Kumar Borah)
>
> 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 | 3 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 30 ++++++++++++++++++++
> drivers/gpu/drm/drm_colorop.c | 47 +++++++++++++++++++++++++++++++
> include/drm/drm_colorop.h | 24 ++++++++++++++++
> include/uapi/drm/amdgpu_drm.h | 9 ------
> include/uapi/drm/drm_mode.h | 32 ++++++++++++++++++++-
> 6 files changed, 135 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7b042c38d50d..809bd856d378 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -793,6 +793,9 @@ static void drm_atomic_colorop_print_state(struct
> drm_printer *p,
> 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;
> }
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index c0bcaec249de..be73cb9f502e 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -702,6 +702,31 @@ 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,
> @@ -711,6 +736,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", @@ -733,6 +761,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 57a8c1063fdd..65351aaa7771 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[] = { @@ -148,6
> +149,11 @@ static void drm_colorop_cleanup(struct drm_colorop *colorop)
> list_del(&colorop->head);
> config->num_colorop--;
>
> + if (colorop->state && colorop->state->data) {
> + drm_property_blob_put(colorop->state->data);
> + colorop->state->data = NULL;
> + }
> +
> kfree(colorop->state);
> }
>
> @@ -238,11 +244,51 @@ int drm_plane_colorop_curve_1d_init(struct
> drm_device *dev, struct drm_colorop * }
> EXPORT_SYMBOL(drm_plane_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_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> + struct drm_plane *plane)
> +{
> + int ret;
> +
> + ret = drm_plane_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_plane_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;
> }
>
> @@ -324,6 +370,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
> 8e4d79ba1eff..5f0cddc3922f 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -99,6 +99,17 @@ 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.
> + *
> + * See the &drm_colorop_type documentation for how blob is laid
> + * out.
> + */
> + struct drm_property_blob *data;
> +
> /** @state: backpointer to global drm_atomic_state */
> struct drm_atomic_state *state;
> };
> @@ -208,6 +219,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:
> *
> @@ -243,6 +265,8 @@ void drm_colorop_pipeline_destroy(struct drm_plane
> *plane);
>
> int drm_plane_colorop_curve_1d_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> struct drm_plane *plane, u64
> supported_tfs);
> +int drm_plane_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 25d5c6e90a99..96fabf9c9827
> 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -1300,15 +1300,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 ea6d88f683cd..651bdf48b766 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
> @@ -874,7 +888,23 @@ 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:
> + *
> + * enum string "3x4 Matrix"
> + *
> + * A 3x4 matrix. Its values are specified via the
> + * &drm_color_ctm_3x4 struct provided via the DATA property.
> + *
> + * The DATA blob is a float[12]:
> + * out matrix in
> + * | R | | 0 1 2 3 | | R |
> + * | G | = | 4 5 6 7 | x | G |
> + * | B | | 8 9 10 12 | | B |
> + */
> + DRM_COLOROP_CTM_3X4,
> };
>
> /**
> --
> 2.43.0
More information about the amd-gfx
mailing list