[PATCH V8 32/43] drm/colorop: Add 1D Curve Custom LUT type
Shankar, Uma
uma.shankar at intel.com
Tue Apr 15 06:09:22 UTC 2025
> -----Original Message-----
> From: Alex Hung <alex.hung at amd.com>
> Sent: Thursday, March 27, 2025 5:17 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
> Subject: [PATCH V8 32/43] drm/colorop: Add 1D Curve Custom LUT type
>
> We've previously introduced DRM_COLOROP_1D_CURVE for pre-defined 1D
> curves. But we also have HW that supports custom curves and userspace needs
> the ability to pass custom curves, aka LUTs.
>
> This patch introduces a new colorop type, called DRM_COLOROP_1D_LUT that
> provides a SIZE property which is used by a driver to advertise the supported SIZE
> of the LUT, as well as a DATA property which userspace uses to set the LUT.
>
> DATA and size function in the same way as current drm_crtc GAMMA and
> DEGAMMA LUTs.
Thanks Alex and Harry for driving this forward.
We want to have just one change in the way we expose the hardware capabilities else
all looks good in general.
To expose the capabilities of 1D Lut (of any kind), we can use a generic
implementation which covers the distribution of LUT samples along with size and
precision in a common color op. Something like below:
struct drm_color_lut_range {
__u32 flags;
__u16 count;
__s32 start, end;
__u32 norm_factor;
struct {
__u16 intp;
__u16 fracp;
} precision;
};
The idea is to expose the lut distribution covering the entire color range from 0 to 1.0.
The underlying implementation in hardware can have just 1 segment covering the full range
OR a complex LUT distribution in Hardware like PWL, multi segmented etc.
To explain the usage with some real life example
------------------------------------------------
1. Conventional 1D LUT with just one segment
|---|---|------------------------------------|
0 1 2 1024
- Hardware Description: A color block with a LUT linearly interpolating and
covering range from 0 to 1.0
- Number of segments - 1
- Number of samples in LUT 1024
- Precision of LUT samples in HW 0.10
- Normalization Factor - Max value to represent 1.0
in terms of smallest step size which is 1024.
In this case, it will be represented by the following structure.
struct drm_color_lut_range lut_1024[] = {
.start = 0 .end = (1 << 10);
.normalization_factor = 1024;
.count = 1024;
.precision {
.int_comp = 0;
.fractional_comp = 10;
}
}
2. Piece Wise Linear 1D LUT
|---|---|------------------------------------|
0 1 2 32
| \
| \
| \
| \
0 \
|---|---|--...---|
0 1 2 8
- Hardware Description: A color block with a LUT linearly interpolating and
covering range from 0 to 1.0
- Number of segments 2
- Number of samples
- segment 1 - 9 (covers range from 0 to 1/32)
- segment 2 - 30 (covers range from 2/32 to 1.0)
- Precision of LUT samples in HW 0.24
- Normalization Factor - Max value to represent 1.0
in terms of smallest step size which is 8*32.
struct drm_color_lut_range lut_pwl[] = {
/* segment 1 */
{
.count = 9,
.start = 0, .end = 8,
.norm_factor = 8*32,
.precision = {
.intp = 0,
.fracp = 24,
},
},
/* segment 2 */
{
.count = 30,
.start = 8*2, .end = 8*32,
.norm_factor = 8*32,
.precision = {
.intp = 0,
.fracp = 24,
},
},
}
We can add and modify the relevant fields to be exposed in the capability structure as per
the community feedback, but this should be able to cover all the 1d LUT aspects and generically
expose the entire hardware block to userspace.
With this info, userspace will be able to compute the LUT samples for any specific usecase and we
may not need any exclusive "SIZE" or separate 1D lut property and just have this common property
which exposes hardware lut capabilities through "struct drm_color_lut_range"
Please refer to below links explaining the implementation:
[1] https://patchwork.freedesktop.org/patch/642652/?series=129812&rev=4
[2] https://patchwork.freedesktop.org/patch/642591/?series=129812&rev=4
[3] https://patchwork.freedesktop.org/patch/642597/?series=129812&rev=4
Regards,
Uma Shankar
> Signed-off-by: Alex Hung <alex.hung at amd.com>
> Co-developed-by: Harry Wentland <harry.wentland at amd.com>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> ---
> v8:
> - Add DRM_MODE_PROP_ATOMIC to drm_property_create_range (Simon Ser)
> - Change "1D Curve Custom LUT" to "1D LUT" (Simon Ser)
>
> v7:
> - Change "size" to "lut_size" (this affects multiple following commits)
> - Move "lut_size" from drm_colorop_state to drm_colorop
> - Modify other files accordingly (i.e. from drm_colorop_state->size
> to drm_colorop->lut_size)
>
> v5:
> - Add kernel doc
> - Define SIZE in similar manner to GAMMA_SIZE on drm_crtc (Melissa)
>
> drivers/gpu/drm/drm_atomic.c | 4 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 5 ++++
> drivers/gpu/drm/drm_colorop.c | 43 +++++++++++++++++++++++++++++++
> include/drm/drm_colorop.h | 16 ++++++++++++
> include/uapi/drm/drm_mode.h | 14 ++++++++++
> 5 files changed, 82 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5223cf363692..f713d177241d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -793,6 +793,10 @@ 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_1D_LUT:
> + drm_printf(p, "\tsize=%d\n", colorop->lut_size);
> + drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data-
> >base.id : 0);
> + break;
> case DRM_COLOROP_CTM_3X4:
> drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data-
> >base.id : 0);
> break;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index dcd12fc0bd8f..dfd88a227da7 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -713,6 +713,9 @@ static int drm_atomic_color_set_data_property(struct
> drm_colorop *colorop,
> bool replaced = false;
>
> switch (colorop->type) {
> + case DRM_COLOROP_1D_LUT:
> + size = colorop->lut_size * sizeof(struct drm_color_lut);
> + break;
> case DRM_COLOROP_CTM_3X4:
> size = sizeof(struct drm_color_ctm_3x4);
> break;
> @@ -762,6 +765,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->lut_size_property) {
> + *val = colorop->lut_size;
> } else if (property == colorop->data_property) {
> *val = (state->data) ? state->data->base.id : 0;
> } else {
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index fcb4a8d0e38d..15ffbba60b3d 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_1D_LUT, "1D LUT" },
> { DRM_COLOROP_CTM_3X4, "3x4 Matrix"},
> };
>
> @@ -225,6 +226,47 @@ static int drm_colorop_create_data_prop(struct
> drm_device *dev, struct drm_color
> return 0;
> }
>
> +/**
> + * drm_colorop_curve_1d_lut_init - Initialize a DRM_COLOROP_1D_LUT
> + *
> + * @dev: DRM device
> + * @colorop: The drm_colorop object to initialize
> + * @plane: The associated drm_plane
> + * @lut_size: LUT size supported by driver
> + * @return zero on success, -E value on failure */ int
> +drm_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> + struct drm_plane *plane, uint32_t lut_size) {
> + struct drm_property *prop;
> + int ret;
> +
> + ret = drm_colorop_init(dev, colorop, plane, DRM_COLOROP_1D_LUT);
> + if (ret)
> + return ret;
> +
> + /* initialize 1D LUT only attribute */
> + /* LUT size */
> + prop = drm_property_create_range(dev,
> DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_ATOMIC,
> + "SIZE", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> +
> + colorop->lut_size_property = prop;
> + drm_object_attach_property(&colorop->base, colorop-
> >lut_size_property, lut_size);
> + colorop->lut_size = lut_size;
> +
> + /* data */
> + ret = drm_colorop_create_data_prop(dev, colorop);
> + if (ret)
> + return ret;
> +
> + drm_colorop_reset(colorop);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_colorop_curve_1d_lut_init);
> +
> int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> struct drm_plane *plane)
> {
> @@ -333,6 +375,7 @@ void drm_colorop_reset(struct drm_colorop *colorop)
>
> static const char * const colorop_type_name[] = {
> [DRM_COLOROP_1D_CURVE] = "1D Curve",
> + [DRM_COLOROP_1D_LUT] = "1D LUT",
> [DRM_COLOROP_CTM_3X4] = "3x4 Matrix",
> };
>
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index
> d3a60c1beed1..d66c76033343 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -259,6 +259,13 @@ struct drm_colorop {
> */
> struct drm_property *bypass_property;
>
> + /**
> + * @lut_size:
> + *
> + * Number of entries of the custom LUT. This should be read-only.
> + */
> + uint32_t lut_size;
> +
> /**
> * @curve_1d_type_property:
> *
> @@ -266,6 +273,13 @@ struct drm_colorop {
> */
> struct drm_property *curve_1d_type_property;
>
> + /**
> + * @lut_size_property:
> + *
> + * Size property for custom LUT from userspace.
> + */
> + struct drm_property *lut_size_property;
> +
> /**
> * @data_property:
> *
> @@ -310,6 +324,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_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> + struct drm_plane *plane, uint32_t lut_size);
> int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> struct drm_plane *plane);
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 651bdf48b766..dde250dd7a51 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -890,6 +890,20 @@ enum drm_colorop_type {
> */
> DRM_COLOROP_1D_CURVE,
>
> + /**
> + * @DRM_COLOROP_1D_LUT:
> + *
> + * enum string "1D LUT"
> + *
> + * A simple 1D LUT of uniformly spaced &drm_color_lut entries,
> + * packed into a blob via the DATA property. The driver's
> + * expected LUT size is advertised via the SIZE property.
> + *
> + * The DATA blob is an array of struct drm_color_lut with size
> + * of "lut_size".
> + */
> + DRM_COLOROP_1D_LUT,
> +
> /**
> * @DRM_COLOROP_CTM_3X4:
> *
> --
> 2.43.0
More information about the Intel-gfx
mailing list