Re: [RFC PATCH v4 31/42] drm/amd/display: Add support for sRGB EOTF in DEGAM block
On 05/21, Melissa Wen wrote:
> On 02/26, Harry Wentland wrote:
> > From: Alex Hung
> >
> > 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
> > Signed-off-by: Harry Wentland
> > Co-developed-by: Harry Wentland
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/Makefile| 3 +-
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 88 +++
> > .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 58
> > .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h | 34 +++
> > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 10 +++
> > 5 files changed, 192 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 9b527bffe11a..3ec759934669 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,19 @@ 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;;
Nitpick: double `;`
> > + }
> > +}
> > +
> > static void __to_dc_lut3d_color(struct dc_rgb *rgb,
> > const struct drm_color_lut lut,
> > int bit_precision)
> > @@ -1137,6 +1150,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;
>
> For this patch and the next two, it ^ should be:
> `_plane_state->in_transfer_func` (same for shape and blend), right?
>
> > + 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_state, colorop_state);
> > +}
> > +
> > static int
> > amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
> > struct
Re: [RFC PATCH v4 31/42] drm/amd/display: Add support for sRGB EOTF in DEGAM block
On 02/26, Harry Wentland wrote:
> From: Alex Hung
>
> 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
> Signed-off-by: Harry Wentland
> Co-developed-by: Harry Wentland
> ---
> .../gpu/drm/amd/display/amdgpu_dm/Makefile| 3 +-
> .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 88 +++
> .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 58
> .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h | 34 +++
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 10 +++
> 5 files changed, 192 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 9b527bffe11a..3ec759934669 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,19 @@ 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 +1150,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;
For this patch and the next two, it ^ should be:
`_plane_state->in_transfer_func` (same for shape and blend), right?
> + 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_state, colorop_state);
> +}
> +
> static int
> amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
>struct dc_plane_state *dc_plane_state)
> @@ -1187,6 +1253,25 @@ amdgpu_dm_plane_set_color_properties(struct
> drm_plane_state *plane_state,
> return 0;
> }
>
> +static int
> +amdgpu_dm_plane_set_colorop_properties(struct
[RFC PATCH v4 31/42] drm/amd/display: Add support for sRGB EOTF in DEGAM block
From: Alex Hung
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
Signed-off-by: Harry Wentland
Co-developed-by: Harry Wentland
---
.../gpu/drm/amd/display/amdgpu_dm/Makefile| 3 +-
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 88 +++
.../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 58
.../amd/display/amdgpu_dm/amdgpu_dm_colorop.h | 34 +++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 10 +++
5 files changed, 192 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 9b527bffe11a..3ec759934669 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,19 @@ 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 +1150,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_state, colorop_state);
+}
+
static int
amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state)
@@ -1187,6 +1253,25 @@ amdgpu_dm_plane_set_color_properties(struct
drm_plane_state *plane_state,
return 0;
}
+static int
+amdgpu_dm_plane_set_colorop_properties(struct drm_plane_state *plane_state,
+ struct dc_plane_state *dc_plane_state)
+{
+ struct drm_colorop *colorop = plane_state->color_pipeline;
+ int ret;
+
+ /* 1D Curve - DEGAM TF */
+ if (!colorop) {
+ return -EINVAL;
+ }
+
+ ret =
