Re: [PATCH V9 32/43] drm/amd/display: Add support for BT.709 and BT.2020 TFs

2025-05-12 Thread Melissa Wen 
On 04/29, Alex Hung wrote:
> From: Harry Wentland 
> 
> This adds support for the BT.709/BT.2020 transfer functions
> on all current 1D curve plane colorops, i.e., on DEGAM, SHAPER,
> and BLND blocks.
> 
> With this change the following IGT subtests pass:
> kms_colorop --run plane-XR30-XR30-bt2020_inv_oetf
> kms_colorop --run plane-XR30-XR30-bt2020_oetf
> 
> Signed-off-by: Alex Hung 
> Signed-off-by: Harry Wentland 
> Reviewed-by: Daniel Stone 
> ---
> V9:
>  - Move DRM_COLOROP_1D_CURVE_BT2020_* from middle to end
> 
> v8: 
>  - Move BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) in amdgpu_dm_supported_blnd_tfs
>to "drm/amd/display: Enable support for PQ 125 EOTF and Inverse" (Leo Li)
> 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c   | 11 ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c |  9 ++---
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 63044e0296cb..f645f9ded95f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -679,6 +679,9 @@ amdgpu_colorop_tf_to_dc_tf(enum drm_colorop_curve_1d_type 
> tf)
>   case DRM_COLOROP_1D_CURVE_PQ_125_EOTF:
>   case DRM_COLOROP_1D_CURVE_PQ_125_INV_EOTF:
>   return TRANSFER_FUNCTION_PQ;
> + case DRM_COLOROP_1D_CURVE_BT2020_INV_OETF:
> + case DRM_COLOROP_1D_CURVE_BT2020_OETF:
> + return TRANSFER_FUNCTION_BT709;
>   default:
>   return TRANSFER_FUNCTION_LINEAR;
>   }
> @@ -1284,8 +1287,10 @@ __set_colorop_1d_curve_blend_tf_lut(struct 
> dc_plane_state *dc_plane_state,
>   const struct drm_color_lut *blend_lut;
>   uint32_t blend_size = 0;
>  
> - if (colorop->type != DRM_COLOROP_1D_CURVE &&
> - colorop_state->curve_1d_type != DRM_COLOROP_1D_CURVE_SRGB_EOTF)
> + if (colorop->type != DRM_COLOROP_1D_CURVE)
> + return -EINVAL;
> +
> + if (!(BIT(colorop_state->curve_1d_type) & amdgpu_dm_supported_blnd_tfs))
>   return -EINVAL;
>  
>   if (colorop_state->bypass) {
> @@ -1321,7 +1326,7 @@ __set_dm_plane_colorop_blend(struct drm_plane_state 
> *plane_state,
>   /* 3nd op: 1d curve - blend */
>   for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
>   if (new_colorop_state->colorop == old_colorop &&
> - new_colorop_state->curve_1d_type == 
> DRM_COLOROP_1D_CURVE_SRGB_EOTF) {
> + (BIT(new_colorop_state->curve_1d_type) & 
> amdgpu_dm_supported_blnd_tfs)) {

nitpick: I think these changes to check amdgpu_dm_supported_*_tfs should
happen earlier, together with degamma and shaper curves in the patch 30.

Melissa

>   colorop_state = new_colorop_state;
>   break;
>   }
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index aa753b0d6f13..b989e1ca19e2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -33,15 +33,18 @@
>  
>  const u64 amdgpu_dm_supported_degam_tfs =
>   BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
> - BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF);
> + BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) |
> + BIT(DRM_COLOROP_1D_CURVE_BT2020_INV_OETF);
>  
>  const u64 amdgpu_dm_supported_shaper_tfs =
>   BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF) |
> - BIT(DRM_COLOROP_1D_CURVE_PQ_125_INV_EOTF);
> + BIT(DRM_COLOROP_1D_CURVE_PQ_125_INV_EOTF) |
> + BIT(DRM_COLOROP_1D_CURVE_BT2020_OETF);
>  
>  const u64 amdgpu_dm_supported_blnd_tfs =
>   BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
> - BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF);
> + BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) |
> + BIT(DRM_COLOROP_1D_CURVE_BT2020_INV_OETF);
>  
>  #define MAX_COLOR_PIPELINE_OPS 10
>  
> -- 
> 2.43.0
>

Re: [PATCH V9 36/43] drm/colorop: Add mutliplier type

2025-05-12 Thread Melissa Wen 




On 29/04/2025 22:11, Alex Hung wrote:

Typo in the commit msg: mutliplier -> multiplier

This introduces a new drm_colorop_type: DRM_COLOROP_MULTIPLIER.

It's a simple multiplier to all pixel values. The value is
specified via a S31.32 fixed point provided via the
"MULTIPLIER" property.

Reviewed-by: Simon Ser 
Signed-off-by: Alex Hung 
Reviewed-by: Daniel Stone 
---
V9:
  - Update function names by _plane_ (Chaitanya Kumar Borah)

v7:
  - Modify size_property to lut_size_property

  drivers/gpu/drm/drm_atomic.c  |  3 +++
  drivers/gpu/drm/drm_atomic_uapi.c |  4 
  drivers/gpu/drm/drm_colorop.c | 33 +++
  include/drm/drm_colorop.h | 16 +++
  include/uapi/drm/drm_mode.h   | 11 +++
  5 files changed, 67 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1dda90554e46..907ca790689f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -800,6 +800,9 @@ static void drm_atomic_colorop_print_state(struct 
drm_printer *p,
case DRM_COLOROP_CTM_3X4:
drm_printf(p, "\tdata blob id=%d\n", state->data ? 
state->data->base.id : 0);
break;
+   case DRM_COLOROP_MULTIPLIER:
+   drm_printf(p, "\tmultiplier=%llu\n", state->multiplier);
+   break;
default:
break;
}
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 0cd250db3981..a7f1d75bb4ea 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -739,6 +739,8 @@ 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->multiplier_property) {
+   state->multiplier = val;
} else if (property == colorop->data_property) {
return drm_atomic_color_set_data_property(colorop, state,
  property, val);
@@ -764,6 +766,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->multiplier_property) {
+   *val = state->multiplier;
} else if (property == colorop->lut_size_property) {
*val = colorop->lut_size;
} else if (property == colorop->data_property) {
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 6c87f5b9f7f9..a3cf62c5e263 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -66,6 +66,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"},
+   { DRM_COLOROP_MULTIPLIER, "Multiplier"},
  };
  
  static const char * const colorop_curve_1d_type_names[] = {

@@ -327,6 +328,37 @@ int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, 
struct drm_colorop *c
  }
  EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
  
+/**

+ * drm_plane_colorop_mult_init - Initialize a DRM_COLOROP_MULTIPLIER
+ *
+ * @dev: DRM device
+ * @colorop: The drm_colorop object to initialize
+ * @plane: The associated drm_plane
+ * @return zero on success, -E value on failure
+ */
+int drm_plane_colorop_mult_init(struct drm_device *dev, struct drm_colorop 
*colorop,
+   struct drm_plane *plane)
+{
+   struct drm_property *prop;
+   int ret;
+
+   ret = drm_plane_colorop_init(dev, colorop, plane, 
DRM_COLOROP_MULTIPLIER);
+   if (ret)
+   return ret;
+
+   prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC, 
"MULTIPLIER", 0, U64_MAX);
+   if (!prop)
+   return -ENOMEM;
+
+   colorop->multiplier_property = prop;
+   drm_object_attach_property(&colorop->base, 
colorop->multiplier_property, 0);
+
+   drm_colorop_reset(colorop);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_plane_colorop_mult_init);
+
  static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop 
*colorop,
struct 
drm_colorop_state *state)
  {
@@ -418,6 +450,7 @@ 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",
+   [DRM_COLOROP_MULTIPLIER] = "Multiplier",
  };
  
  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 52e965577bd8..888184aef7a0 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -146,6

Re: [PATCH V9 22/43] drm/colorop: define a new macro for_each_new_colorop_in_state

2025-05-12 Thread Melissa Wen 
On 04/29, Alex Hung wrote:
> Create a new macro for_each_new_colorop_in_state to access new
> drm_colorop_state updated from uapi.
> 
> Reviewed-by: Simon Ser 
> Signed-off-by: Alex Hung 
> Reviewed-by: Daniel Stone 
> ---
>  include/drm/drm_atomic.h | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 4a30ed8ab1a8..4b3bd459a1eb 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -1069,6 +1069,26 @@ void drm_state_dump(struct drm_device *dev, struct 
> drm_printer *p);
> (new_colorop_state) = 
> (__state)->colorops[__i].new_state, 1))
>  
>  
> +/**
> + * for_each_new_colorop_in_state - iterate over all colorops in an atomic 
> update
> + * @__state: &struct drm_atomic_state pointer
> + * @colorop: &struct drm_colorop iteration cursor
> + * @new_colorop_state: &struct drm_colorop_state iteration cursor for the 
> new state
> + * @__i: int iteration cursor, for macro-internal use
> + *
> + * This iterates over all colorops in an atomic update, tracking new state. 
> This is
> + * useful is useful in places where the state delta needs to be considered, 
> for

nit: "is useful" duplication

> + * example in atomic check functions.
> + */
> +#define for_each_new_colorop_in_state(__state, colorop, new_colorop_state, 
> __i) \
> + for ((__i) = 0; \
> +  (__i) < (__state)->dev->mode_config.num_colorop;   \
> +  (__i)++)   \
> + for_each_if ((__state)->colorops[__i].ptr &&\
> +  ((colorop) = (__state)->colorops[__i].ptr, \
> +   (void)(colorop) /* Only to avoid 
> unused-but-set-variable warning */, \
> +   (new_colorop_state) = 
> (__state)->colorops[__i].new_state, 1))
> +
>  /**
>   * for_each_oldnew_plane_in_state - iterate over all planes in an atomic 
> update
>   * @__state: &struct drm_atomic_state pointer
> -- 
> 2.43.0
>

Re: [PATCH V9 24/43] drm/amd/display: Add bypass COLOR PIPELINE

2025-05-12 Thread Melissa Wen 
On 04/29, Alex Hung wrote:
> From: Harry Wentland 
> 
> Add the default Bypass pipeline and ensure it passes the
> kms_colorop test plane-XR30-XR30-bypass.
> 
> Signed-off-by: Alex Hung 
> Signed-off-by: Harry Wentland 
> Reviewed-by: Daniel Stone 
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 3e0f45f1711c..fb6f07603050 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1784,6 +1784,20 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
>  }
>  #endif
> 
> +#define MAX_COLOR_PIPELINES 5
> +
> +static int
> +dm_plane_init_colorops(struct drm_plane *plane)
> +{
> + struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
> + int len = 0;
> +
> + /* Create COLOR_PIPELINE property and attach */
> + drm_plane_create_color_pipeline_property(plane, pipelines, len);
> +
> + return 0;
> +}
> +

This function is unused if AMD_PRIVATE_COLOR (warning).
I think you can add an #else just before the define MAX_COLOR_PIPELINES
(replacing the above #endif there, and move the #endif to here to solve
it.

Melissa

>  static const struct drm_plane_funcs dm_plane_funcs = {
>   .update_plane   = drm_atomic_helper_update_plane,
>   .disable_plane  = drm_atomic_helper_disable_plane,
> @@ -1890,7 +1904,12 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
> *dm,
>  
>  #ifdef AMD_PRIVATE_COLOR
>   dm_atomic_plane_attach_color_mgmt_properties(dm, plane);
> +#else
> + res = dm_plane_init_colorops(plane);
> + if (res)
> + return res;
>  #endif
> +
>   /* Create (reset) the plane state */
>   if (plane->funcs->reset)
>   plane->funcs->reset(plane);
> -- 
> 2.43.0
>

Re: [PATCH V9 26/43] drm/amd/display: Add support for sRGB EOTF in DEGAM block

2025-05-12 Thread Melissa Wen 
On 04/29, Alex Hung wrote:
> Expose one 1D curve colorop with support for
> DRM_COLOROP_1D_CURVE_SRGB_EOTF and program HW to perform
> the sRGB transform when the colorop is not in bypass.
> 
> With this change the following IGT test passes:
> kms_colorop --run plane-XR30-XR30-srgb_eotf
> 
> The color pipeline now consists of a single colorop:
> 1. 1D curve colorop w/ sRGB EOTF
> 
> Signed-off-by: Alex Hung 
> Co-developed-by: Harry Wentland 
> Signed-off-by: Harry Wentland 
> Reviewed-by: Daniel Stone 
> ---
> V9:
>  - Update function names by _plane_ (Chaitanya Kumar Borah)
>  - Update replace cleanup code by drm_colorop_pipeline_destroy (Simon Ser)
> 
> v8:
>  - Fix incorrect && by || in __set_colorop_in_tf_1d_curve (Leo Li)
> 
> v7:
>  - Fix checkpatch warnings
>   - Change switch "{ }" position
>   - Delete double ";"
>   - Delete "{ }" for single-line if-statement
>   - Add a new line at EOF
>   - Change SPDX-License-Identifier: GPL-2.0+ from // to /* */
> 
> v6:
>  - cleanup if colorop alloc or init fails
> 
>  .../gpu/drm/amd/display/amdgpu_dm/Makefile|  3 +-
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 86 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 69 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h | 34 
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 10 +++
>  5 files changed, 201 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
> index ab2a97e354da..46158d67ab12 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
> @@ -38,7 +38,8 @@ AMDGPUDM = \
>   amdgpu_dm_pp_smu.o \
>   amdgpu_dm_psr.o \
>   amdgpu_dm_replay.o \
> - amdgpu_dm_wb.o
> + amdgpu_dm_wb.o \
> + amdgpu_dm_colorop.o
>  
>  ifdef CONFIG_DRM_AMD_DC_FP
>  AMDGPUDM += dc_fpu.o
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index ebabfe3a512f..0b513ab5050f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -668,6 +668,18 @@ amdgpu_tf_to_dc_tf(enum amdgpu_transfer_function tf)
>   }
>  }
>  
> +static enum dc_transfer_func_predefined
> +amdgpu_colorop_tf_to_dc_tf(enum drm_colorop_curve_1d_type tf)
> +{
> + switch (tf) {
> + case DRM_COLOROP_1D_CURVE_SRGB_EOTF:
> + case DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF:
> + return TRANSFER_FUNCTION_SRGB;
> + default:
> + return TRANSFER_FUNCTION_LINEAR;
> + }
> +}
> +
>  static void __to_dc_lut3d_color(struct dc_rgb *rgb,
>   const struct drm_color_lut lut,
>   int bit_precision)
> @@ -1137,6 +1149,59 @@ __set_dm_plane_degamma(struct drm_plane_state 
> *plane_state,
>   return 0;
>  }
>  
> +static int
> +__set_colorop_in_tf_1d_curve(struct dc_plane_state *dc_plane_state,
> +struct drm_colorop_state *colorop_state)
> +{
> + struct dc_transfer_func *tf = &dc_plane_state->in_transfer_func;
> + struct drm_colorop *colorop = colorop_state->colorop;
> + struct drm_device *drm = colorop->dev;
> +
> + if (colorop->type != DRM_COLOROP_1D_CURVE ||
> + colorop_state->curve_1d_type != DRM_COLOROP_1D_CURVE_SRGB_EOTF)
> + return -EINVAL;
> +
> + if (colorop_state->bypass) {
> + tf->type = TF_TYPE_BYPASS;
> + tf->tf = TRANSFER_FUNCTION_LINEAR;
> + return 0;
> + }
> +
> + drm_dbg(drm, "Degamma colorop with ID: %d\n", colorop->base.id);
> +
> + tf->type = TF_TYPE_PREDEFINED;
> + tf->tf = amdgpu_colorop_tf_to_dc_tf(colorop_state->curve_1d_type);
> +
> + return 0;
> +}
> +
> +static int
> +__set_dm_plane_colorop_degamma(struct drm_plane_state *plane_state,
> +struct dc_plane_state *dc_plane_state,
> +struct drm_colorop *colorop)
> +{
> + struct drm_colorop *old_colorop;
> + struct drm_colorop_state *colorop_state = NULL, *new_colorop_state;
> + struct drm_atomic_state *state = plane_state->state;
> + int i = 0;
> +
> + old_colorop = colorop;
> +
> + /* 1st op: 1d curve - degamma */
> + for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
> + if (new_colorop_state->colorop == old_colorop &&
> + new_colorop_state->curve_1d_type == 
> DRM_COLOROP_1D_CURVE_SRGB_EOTF) {
> + colorop_state = new_colorop_state;
> + break;
> + }
> + }
> +
> + if (!colorop_state)
> + return -EINVAL;
> +
> + return __set_colorop_in_tf_1d_curve(dc_plane_

Re: [PATCH V9 42/43] drm/amd/display: add 3D LUT colorop

2025-05-12 Thread Melissa Wen 
On 04/29, Alex Hung wrote:
> This adds support for a 3D LUT.
> 
> The color pipeline now consists of the following colorops:
> 1. 1D curve colorop
> 2. Multiplier
> 3. 3x4 CTM
> 4. 1D curve colorop
> 5. 1D LUT
> 6. 3D LUT
> 7. 1D curve colorop
> 8. 1D LUT
> 
> Signed-off-by: Alex Hung 
> Reviewed-by: Daniel Stone 
> ---
> V9:
>  - Return a value in __set_dm_plane_colorop_3dlut
> 
> v8:
>  - Set initialized to 0 and return when drm_lut3d_size is 0 (Harry Wentland)
>  - Rework tf->type = TF_TYPE_BYPASS for shaper (Harry Wentland & Leo Li)
> 
> v7:
>  - Simplify 3D LUT according to drm_colorop changes (Simon Ser)
> 
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 94 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 20 
>  2 files changed, 114 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 313716f2003f..dfdd3f557570 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -1293,6 +1293,7 @@ __set_dm_plane_colorop_shaper(struct drm_plane_state 
> *plane_state,
>   struct dc_transfer_func *tf = &dc_plane_state->in_shaper_func;
>   const struct drm_color_lut *shaper_lut;
>   struct drm_device *dev = colorop->dev;
> + bool enabled = false;
>   uint32_t shaper_size;
>   int i = 0, ret = 0;
>  
> @@ -1314,6 +1315,7 @@ __set_dm_plane_colorop_shaper(struct drm_plane_state 
> *plane_state,
>   ret = __set_output_tf(tf, 0, 0, false);
>   if (ret)
>   return ret;
> + enabled = true;
>   }
>  
>   /* 1D LUT - SHAPER LUT */
> @@ -1345,12 +1347,93 @@ __set_dm_plane_colorop_shaper(struct drm_plane_state 
> *plane_state,
>   ret = __set_output_tf(tf, shaper_lut, shaper_size, 
> false);
>   if (ret)
>   return ret;
> + enabled = true;
>   }
>   }
>  
> + if (!enabled)
> + tf->type = TF_TYPE_BYPASS;
> +
>   return 0;
>  }
>  
> +/* __set_colorop_3dlut - set DRM 3D LUT to DC stream
> + * @drm_lut3d: user 3D LUT
> + * @drm_lut3d_size: size of 3D LUT
> + * @lut3d: DC 3D LUT
> + *
> + * Map user 3D LUT data to DC 3D LUT and all necessary bits to program it
> + * on DCN accordingly.
> + */
> +static void __set_colorop_3dlut(const struct drm_color_lut *drm_lut3d,
> + uint32_t drm_lut3d_size,
> + struct dc_3dlut *lut)
> +{
> + if (!drm_lut3d_size) {
> + lut->state.bits.initialized = 0;
> + return;
> + }
> +
> + /* Only supports 17x17x17 3D LUT (12-bit) now */
> + lut->lut_3d.use_12bits = true;
> + lut->lut_3d.use_tetrahedral_9 = false;
> +
> + lut->state.bits.initialized = 1;
> + __drm_3dlut_to_dc_3dlut(drm_lut3d, drm_lut3d_size, &lut->lut_3d,
> + lut->lut_3d.use_tetrahedral_9, 12);
> +
> +}
> +
> +static int
> +__set_dm_plane_colorop_3dlut(struct drm_plane_state *plane_state,
> +  struct dc_plane_state *dc_plane_state,
> +  struct drm_colorop *colorop)
> +{
> + struct drm_colorop *old_colorop;
> + struct drm_colorop_state *colorop_state = NULL, *new_colorop_state;
> + struct dc_transfer_func *tf = &dc_plane_state->in_shaper_func;
> + struct drm_atomic_state *state = plane_state->state;
> + const struct amdgpu_device *adev = drm_to_adev(colorop->dev);
> + const struct drm_device *dev = colorop->dev;
> + const struct drm_color_lut *lut3d;
> + uint32_t lut3d_size;
> + int i = 0, ret = 0;
> +
> + /* 3D LUT */
> + old_colorop = colorop;
> + for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
> + if (new_colorop_state->colorop == old_colorop &&
> + new_colorop_state->colorop->type == DRM_COLOROP_3D_LUT) {
> + colorop_state = new_colorop_state;
> + break;
> + }
> + }
> +
> + if (colorop_state && !colorop_state->bypass && colorop->type == 
> DRM_COLOROP_3D_LUT) {
> + if (!adev->dm.dc->caps.color.dpp.hw_3d_lut) {

I wonder if this check is no longer accurate in DCN versions with MCM
(MPC only) 3D LUT caps, such as DCN 3.2 and DCN 4.01.

Also, looking back those patches that introduced shaper and blnd tf and
luts, I don't see similar validation, but IIRC shaper caps directly
depends on 3d lut, for example. IIRC something around blnd func caps
also changed in the above-mentioned DCN versions.

Melissa

> + drm_dbg(dev, "3D LUT is not supported by hardware\n");
> + return -EINVAL;
> + }
> +
> + drm_dbg(dev, "3D LUT colorop with ID: %d\n", colorop->base.id);
> + lut3d = __extract_blob_lut(colorop_state->

Re: [PATCH V8 40/43] drm/colorop: Add 3D LUT support to color pipeline

2025-05-12 Thread Alex Hung 




On 4/25/25 07:50, Leandro Ribeiro wrote:



On 3/26/25 20:47, Alex Hung wrote:

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 
---
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",
+   dr

Re: [PATCH V9 42/43] drm/amd/display: add 3D LUT colorop

2025-05-12 Thread Alex Hung 




On 5/12/25 18:52, Melissa Wen wrote:

On 04/29, Alex Hung wrote:

This adds support for a 3D LUT.

The color pipeline now consists of the following colorops:
1. 1D curve colorop
2. Multiplier
3. 3x4 CTM
4. 1D curve colorop
5. 1D LUT
6. 3D LUT
7. 1D curve colorop
8. 1D LUT

Signed-off-by: Alex Hung 
Reviewed-by: Daniel Stone 
---
V9:
  - Return a value in __set_dm_plane_colorop_3dlut

v8:
  - Set initialized to 0 and return when drm_lut3d_size is 0 (Harry Wentland)
  - Rework tf->type = TF_TYPE_BYPASS for shaper (Harry Wentland & Leo Li)

v7:
  - Simplify 3D LUT according to drm_colorop changes (Simon Ser)

  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 94 +++
  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 20 
  2 files changed, 114 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index 313716f2003f..dfdd3f557570 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -1293,6 +1293,7 @@ __set_dm_plane_colorop_shaper(struct drm_plane_state 
*plane_state,
struct dc_transfer_func *tf = &dc_plane_state->in_shaper_func;
const struct drm_color_lut *shaper_lut;
struct drm_device *dev = colorop->dev;
+   bool enabled = false;
uint32_t shaper_size;
int i = 0, ret = 0;
  
@@ -1314,6 +1315,7 @@ __set_dm_plane_colorop_shaper(struct drm_plane_state *plane_state,

ret = __set_output_tf(tf, 0, 0, false);
if (ret)
return ret;
+   enabled = true;
}
  
  	/* 1D LUT - SHAPER LUT */

@@ -1345,12 +1347,93 @@ __set_dm_plane_colorop_shaper(struct drm_plane_state 
*plane_state,
ret = __set_output_tf(tf, shaper_lut, shaper_size, 
false);
if (ret)
return ret;
+   enabled = true;
}
}
  
+	if (!enabled)

+   tf->type = TF_TYPE_BYPASS;
+
return 0;
  }
  
+/* __set_colorop_3dlut - set DRM 3D LUT to DC stream

+ * @drm_lut3d: user 3D LUT
+ * @drm_lut3d_size: size of 3D LUT
+ * @lut3d: DC 3D LUT
+ *
+ * Map user 3D LUT data to DC 3D LUT and all necessary bits to program it
+ * on DCN accordingly.
+ */
+static void __set_colorop_3dlut(const struct drm_color_lut *drm_lut3d,
+   uint32_t drm_lut3d_size,
+   struct dc_3dlut *lut)
+{
+   if (!drm_lut3d_size) {
+   lut->state.bits.initialized = 0;
+   return;
+   }
+
+   /* Only supports 17x17x17 3D LUT (12-bit) now */
+   lut->lut_3d.use_12bits = true;
+   lut->lut_3d.use_tetrahedral_9 = false;
+
+   lut->state.bits.initialized = 1;
+   __drm_3dlut_to_dc_3dlut(drm_lut3d, drm_lut3d_size, &lut->lut_3d,
+   lut->lut_3d.use_tetrahedral_9, 12);
+
+}
+
+static int
+__set_dm_plane_colorop_3dlut(struct drm_plane_state *plane_state,
+struct dc_plane_state *dc_plane_state,
+struct drm_colorop *colorop)
+{
+   struct drm_colorop *old_colorop;
+   struct drm_colorop_state *colorop_state = NULL, *new_colorop_state;
+   struct dc_transfer_func *tf = &dc_plane_state->in_shaper_func;
+   struct drm_atomic_state *state = plane_state->state;
+   const struct amdgpu_device *adev = drm_to_adev(colorop->dev);
+   const struct drm_device *dev = colorop->dev;
+   const struct drm_color_lut *lut3d;
+   uint32_t lut3d_size;
+   int i = 0, ret = 0;
+
+   /* 3D LUT */
+   old_colorop = colorop;
+   for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+   if (new_colorop_state->colorop == old_colorop &&
+   new_colorop_state->colorop->type == DRM_COLOROP_3D_LUT) {
+   colorop_state = new_colorop_state;
+   break;
+   }
+   }
+
+   if (colorop_state && !colorop_state->bypass && colorop->type == 
DRM_COLOROP_3D_LUT) {
+   if (!adev->dm.dc->caps.color.dpp.hw_3d_lut) {


I wonder if this check is no longer accurate in DCN versions with MCM
(MPC only) 3D LUT caps, such as DCN 3.2 and DCN 4.01.


The current goal is to validate on specific fixed platforms. We will add 
more platform-specific implemenation when we enable this on new DCN 
hardware.


In some case, new hardware may have a different color pipeline. For 
example, there can be no DRM_COLOROP_3D_LUT colorop in a color pipeline, 
and it is not necessary to check DCN versions. In other cases, we will 
need to check different DCN versions or different flags for sure.




Also, looking back those patches that introduced shaper and blnd tf and
luts, I don't see similar validation, but IIRC shaper caps directly
depends on 3d lut, for example. IIRC something around b