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

2025-05-13 Thread Pekka Paalanen 
On Mon, 12 May 2025 15:50:17 -0300
Melissa Wen  wrote:

> 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) {
>

Re: [PATCH V9 00/43] Color Pipeline API w/ VKMS

2025-05-13 Thread Melissa Wen 
On 04/29, Alex Hung wrote:
> This is an RFC set for a color pipeline API, along with implementations
> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
> allowable delta variation as the goal for these transformations is
> perceptual correctness, not complete pixel accuracy.

Hi,

Thanks for this work. I found it very solid and well documented.
The drm colorop implementation looks good to me, but I still have some
questions about AMD specific bits.

Patches 1, 3-13, 16, 21-23, 25, 35-41 and 43 are:
Reviewed-by: Melissa Wen 

And I dropped some comments and questions to some AMD-specific patches.

> 
> v5 of this patchset fleshed out documentation for colorops and the
> various defines that are being introduced.
> 
> v6 addresses a few comments from various reviewers.
> 
> v7 simplifies 3D LUT and addresses more comments from various reviewers.
> 
> v8 fixes typo and errors and address comments from reviewers.
> 
> v9 refactors cleanup functions, fixes typo and errors, and addresses
>comments from reviewers.
> 
> VKMS supports two named transfer function colorops and two matrix
> colorops.
> 
> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
> 
> 1. 1D Curve EOTF
> 2. 3x4 CTM
> 3. Multiplier
> 4. 1D Curve Inverse EOTF
> 5. 1D LUT
> 6. 3D LUT
> 7. 1D Curve EOTF
> 8. 1D LUT
> 
> The supported curves for the 1D Curve type are:
> - sRGB EOTF and its inverse
> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
> - BT.2020/BT.709 OETF and its inverse
> 
> Note that the 1st and 5th colorops take the EOTF or Inverse
> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
> 
> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
> exists for other drivers to describe their own 3D LUT capability.
> 
> This mirrors the color pipeline used by gamescope and presented by
> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
> used. See [1]
> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
> 
> At this point we're hoping to see gamescope and kwin implementations
> take shape. The existing pipeline should be enough to satisfy the
> gamescope use-cases on the drm_plane.
> 
> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
> support to the color pipeline. I have sketched these out already but
> don't have it all hooked up yet. This should not hinder adoption of this
> API for gaming use-cases.
> 
> We'll also want to advertise IN_FORMATS on a color pipeline as some
> color pipelines won't be able to work for all IN_FORMATS on a plane.
> Again, I have a sketch but no full implementation yet. This is not
> currently required by the AMD color pipeline and could be added after
> the merge of this set.
> 
> VKMS patches could still be improved in a few ways, though the
> payoff might be limited and I would rather focus on other work
> at the moment. The most obvious thing to improve would be to
> eliminate the hard-coded LUTs for identity, and sRGB, and replace
> them with fixed-point math instead.
> 
> There are plenty of things that I would like to see, but they could
> be added after the merge of this patchset:
>  - COLOR_ENCODING and COLOR_RANGE
>  - IN_FORMATS for a color pipeline
>  - Is it possible to support HW which can't bypass entire pipeline?
>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
>  - read-only scaling colorop which defines scaling taps and position
>  - named matrices, for things like converting YUV to RGB
>  - Add custom LUT colorops to VKMS
> 
> IGT tests can be found at [1] or on the igt-dev mailing list. There
> have been no updates since v5 and rebase on latest main is straight-
> forward.
> 
> A kernel branch can be found at [2].
> 
> [1] 
> https://gitlab.freedesktop.org/alex.hung/igt-gpu-tools/-/tree/amd-color-pipeline-v8
> [2] 
> https://gitlab.freedesktop.org/alex.hung/linux/-/tree/amd-color-pipeline-v9
> 
> v9:
>  - Update RFC documents for 3DLUT and fallback behaviours (Simon Ser)
>  - Specify colorop function names by _plane_ (Chaitanya Kumar Borah)
>  - Remove redundant comments (Simon Ser)
>  - Fix typo in commit description (Shengyu Qu)
>  - Move destroy and cleanup functions earlier (Simon Ser)
>  - Move DRM_COLOROP_1D_CURVE_BT2020_* from middle to end (Simon Ser)
>  - Chagne "bool allow_bypass" to "uint32_t flags" for better extensibility 
> (Simon Ser)
>  - Return a value in __set_dm_plane_colorop_3dlut
> 
> v8:
>  - Change VKMS config names (Louis Chauvet)
>  - Remove deprecated function "drm_atomic_get_existing_colorop_state" (Louis 
> Chauvet)
>  - Remove null check in drm_colorop_set_next_property (Simon Ser)
>  - Remove MAX_COLOR_PIPELINES in drm (Simon Ser)
>  - Update kernel docs and documents for DRM_COLOROP_3D_LUT (Simon Ser)
>  - Add comments for dmr_color_lut (Louis Chauvet)
>  - Fix typos and replace D

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

2025-05-13 Thread Melissa Wen 
On 05/12, Alex Hung wrote:
> 
> 
> 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

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

2025-05-13 Thread Melissa Wen 
On 05/13, Pekka Paalanen wrote:
> On Mon, 12 May 2025 15:50:17 -0300
> Melissa Wen  wrote:
> 
> > 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_coloro

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

2025-05-13 Thread Harry Wentland 



On 2025-05-13 11:36, Melissa Wen wrote:
> On 05/13, Pekka Paalanen wrote:
>> On Mon, 12 May 2025 15:50:17 -0300
>> Melissa Wen  wrote:
>>
>>> 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