Am Do., 27. März 2025 um 00:58 Uhr schrieb Alex Hung <alex.h...@amd.com>:
>
> It is to be used to enable HDR by allowing userpace to create and pass
> 3D LUTs to kernel and hardware.
>
> new drm_colorop_type: DRM_COLOROP_3D_LUT.
>
> Signed-off-by: Alex Hung <alex.h...@amd.com>
> ---
> v8:
> - Fix typo in subject (Simon Ser)
> - Update documentation for DRM_COLOROP_3D_LUT (Simon Ser)
> - Delete empty lines (Simon Ser)
>
> v7:
> - Simplify 3D LUT by removing lut_3d_modes and related functions (Simon Ser)
>
> drivers/gpu/drm/drm_atomic.c | 6 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 6 +++
> drivers/gpu/drm/drm_colorop.c | 72 +++++++++++++++++++++++++++++++
> include/drm/drm_colorop.h | 21 +++++++++
> include/uapi/drm/drm_mode.h | 33 ++++++++++++++
> 5 files changed, 138 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 0efb0ead204a..ef47a06344f3 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -806,6 +806,12 @@ static void drm_atomic_colorop_print_state(struct
> drm_printer *p,
> case DRM_COLOROP_MULTIPLIER:
> drm_printf(p, "\tmultiplier=%llu\n", state->multiplier);
> break;
> + case DRM_COLOROP_3D_LUT:
> + drm_printf(p, "\tsize=%d\n", colorop->lut_size);
> + drm_printf(p, "\tinterpolation=%s\n",
> +
> drm_get_colorop_lut3d_interpolation_name(colorop->lut3d_interpolation));
> + 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 947c18e8bf9b..d5d464b4d0f6 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -719,6 +719,10 @@ static int drm_atomic_color_set_data_property(struct
> drm_colorop *colorop,
> case DRM_COLOROP_CTM_3X4:
> size = sizeof(struct drm_color_ctm_3x4);
> break;
> + case DRM_COLOROP_3D_LUT:
> + size = colorop->lut_size * colorop->lut_size *
> colorop->lut_size *
> + sizeof(struct drm_color_lut);
> + break;
> default:
> /* should never get here */
> return -EINVAL;
> @@ -771,6 +775,8 @@ drm_atomic_colorop_get_property(struct drm_colorop
> *colorop,
> *val = state->multiplier;
> } else if (property == colorop->lut_size_property) {
> *val = colorop->lut_size;
> + } else if (property == colorop->lut3d_interpolation_property) {
> + *val = colorop->lut3d_interpolation;
> } 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 e03706e7179b..224c6be237d2 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -67,6 +67,7 @@ static const struct drm_prop_enum_list
> drm_colorop_type_enum_list[] = {
> { DRM_COLOROP_1D_LUT, "1D LUT" },
> { DRM_COLOROP_CTM_3X4, "3x4 Matrix"},
> { DRM_COLOROP_MULTIPLIER, "Multiplier"},
> + { DRM_COLOROP_3D_LUT, "3D LUT"},
> };
>
> static const char * const colorop_curve_1d_type_names[] = {
> @@ -82,6 +83,11 @@ static const struct drm_prop_enum_list
> drm_colorop_lut1d_interpolation_list[] =
> { DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, "Linear" },
> };
>
> +
> +static const struct drm_prop_enum_list
> drm_colorop_lut3d_interpolation_list[] = {
> + { DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL, "Tetrahedral" },
> +};
> +
> /* Init Helpers */
>
> static int drm_colorop_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> @@ -349,6 +355,51 @@ int drm_colorop_mult_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> }
> EXPORT_SYMBOL(drm_colorop_mult_init);
>
> +int drm_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> + struct drm_plane *plane,
> + uint32_t lut_size,
> + enum drm_colorop_lut3d_interpolation_type
> interpolation,
> + bool allow_bypass)
> +{
> + struct drm_property *prop;
> + int ret;
> +
> + ret = drm_colorop_init(dev, colorop, plane, DRM_COLOROP_3D_LUT,
> allow_bypass);
> + if (ret)
> + return ret;
> +
> + /* 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;
> +
> + /* interpolation */
> + prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> "LUT3D_INTERPOLATION",
Do we ever expect this to be something with multiple options that
userspace could select? I think it would be good to keep our options
open and make this property not immutable (properties that are
sometimes but not always immutable are more annoying to deal with in
userspace).
Same applies to 1D LUTs as well.
