Hi all,
Here is the next version of our work to enable AMD driver-specific color
management properties [1][2]. This series is a collection of
contributions from Joshua, Harry, and me to enhance the AMD KMS color
pipeline for Steam Deck/SteamOS by exposing additional pre-blending and
post-blending color capabilities from those available in the current DRM
KMS API[3].
The userspace case here is Gamescope which is the compositor for
SteamOS. Gamescope is already using these features to implement its
color management pipeline [4].
In this version, I try to address all concerns shared in the previous
one, i.e.:
- Replace DRM_ by AMDGPU_ prefix for transfer function enumeration;
- Explicitly define EOTFs and inverse EOTFs and set props accordingly;
- Document pre-defined transfer functions;
- Remove misleading comments;
- Remove post-blending/MPC shaper and 3D LUT support;
- Move driver-specific property operations from amdgpu_display.c to
amdgpu_dm_color.c;
- Reset planes if any color props change;
- Nits/small fixes;
Bearing in mind the complexity of color concepts, I believe there is a
high chance of some misunderstanding from my side when defining EOTFs
and documenting pre-defined TFs. So, reviews are very important and
welcome (thanks in advance). FWIW, I added Harry as a co-developer of
this TF documentation since I based on his description of EOTF/inv_EOTF
and previous documentation work [5]. Let me know if there is a better
way for credits.
Two DC patches were already applied and, therefore, removed from the
series. I added r-b according to previous feedback. We also add plane
CTM to driver-specific properties. As a result, this is the updated list
of all driver-specific color properties exposed by this series:
- plane degamma LUT and pre-defined TF;
- plane HDR multiplier;
- plane CTM 3x4;
- plane shaper LUT and pre-defined TF;
- plane 3D LUT;
- plane blend LUT and pre-defined TF;
- CRTC gamma pre-defined TF;
Remember you can find the AMD HW color capabilities documented here:
https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#color-management-properties
Worth mentioning that the pre-blending degamma block can use ROM curves
for some pre-defined TFs, but the other blocks use the AMD color module
to calculate this curve considering pre-defined coefficients.
We need changes on DC gamut remap matrix to support the plane and CRTC
CTM on drivers that support both. I've sent a previous patch to apply
these changes to all DCN3+ families [6]. Here I use the same changes but
limited to DCN301. Just let me know if you prefer the previous/expanded
version.
Finally, this is the Linux/AMD color management API before and after
blending with the driver-specific properties:
+----------------------+
| PLANE |
| |
| +----------------+ |
| | AMD Degamma | |
| | | |
| | EOTF | 1D LUT | |
| +--------+-------+ |
| | |
| +--------v-------+ |
| | AMD HDR | |
| | Multiply | |
| +--------+-------+ |
| | |
| +--------v-------+ |
| | AMD CTM (3x4) | |
| +--------+-------+ |
| | |
| +--------v-------+ |
| | AMD Shaper | |
| | | |
| | inv_EOTF | | |
| | Custom 1D LUT | |
| +--------+-------+ |
| | |
| +--------v-------+ |
| | AMD 3D LUT | |
| | 17^3/12-bit | |
| +--------+-------+ |
| | |
| +--------v-------+ |
| | AMD Blend | |
| | | |
| | EOTF | 1D LUT | |
| +--------+-------+ |
| | |
++----------v---------++
|| Blending ||
++----------+---------++
| CRTC | |
| | |
| +-------v-------+ |
| | DRM Degamma | |
| | | |
| | Custom 1D LUT | |
| +-------+-------+ |
| | |
| +-------v-------+ |
| | DRM CTM (3x3) | |
| +-------+-------+ |
| | |
| +-------v-------+ |
| | DRM Gamma | |
| | | |
| | Custom 1D LUT | |
| +---------------+ |
| | *AMD Gamma | |
| | inv_EOTF | |
| +---------------+ |
| |
+----------------------+
Let me know your thoughts.
Best Regards,
Melissa Wen
[1] https://lore.kernel.org/dri-devel/[email protected]
[2] https://lore.kernel.org/dri-devel/[email protected]
[3] https://github.com/ValveSoftware/gamescope/blob/master/src/docs/Steam%20Deck%20Display%20Pipeline.png
[4] https://github.com/ValveSoftware/gamescope
[5] https://lore.kernel.org/dri-devel/[email protected]
[6] https://lore.kernel.org/dri-devel/[email protected]
Harry Wentland (1):
drm/amd/display: fix segment distribution for linear LUTs
Joshua Ashton (14):
drm/amd/display: add plane degamma TF driver-specific property
drm/amd/display: add plane HDR multiplier driver-specific property
drm/amd/display: add plane blend LUT and TF driver-specific properties
drm/amd/display: add CRTC gamma TF support
drm/amd/display: set sdr_ref_white_level to 80 for out_transfer_func
drm/amd/display: mark plane as needing reset if color props change
drm/amd/display: add plane degamma TF and LUT support
drm/amd/display: add dc_fixpt_from_s3132 helper
drm/amd/display: add HDR multiplier support
drm/amd/display: handle empty LUTs in __set_input_tf
drm/amd/display: add plane blend LUT and TF support
drm/amd/display: allow newer DC hardware to use degamma ROM for PQ/HLG
drm/amd/display: copy 3D LUT settings from crtc state to stream_update
drm/amd/display: Use 3x4 CTM for plane CTM
Melissa Wen (19):
drm/drm_mode_object: increase max objects to accommodate new color
props
drm/drm_property: make replace_property_blob_from_id a DRM helper
drm/drm_plane: track color mgmt changes per plane
drm/amd/display: add driver-specific property for plane degamma LUT
drm/amd/display: explicitly define EOTF and inverse EOTF
drm/amd/display: document AMDGPU pre-defined transfer functions
drm/amd/display: add plane 3D LUT driver-specific properties
drm/amd/display: add plane shaper LUT and TF driver-specific
properties
drm/amd/display: add CRTC gamma TF driver-specific property
drm/amd/display: add comments to describe DM crtc color mgmt behavior
drm/amd/display: encapsulate atomic regamma operation
drm/amd/display: decouple steps for mapping CRTC degamma to DC plane
drm/amd/display: reject atomic commit if setting both plane and CRTC
degamma
drm/amd/display: add plane shaper LUT support
drm/amd/display: add plane shaper TF support
drm/amd/display: add plane 3D LUT support
drm/amd/display: set stream gamut remap matrix to MPC for DCN301
drm/amd/display: add plane CTM driver-specific property
drm/amd/display: add plane CTM support
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 71 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 101 +++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 805 ++++++++++++++++--
.../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 72 ++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 224 ++++-
.../amd/display/dc/dcn10/dcn10_cm_common.c | 93 +-
.../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 37 +
.../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
.../drm/amd/display/dc/dcn301/dcn301_init.c | 2 +-
.../gpu/drm/amd/display/include/fixed31_32.h | 12 +
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/drm_atomic.c | 1 +
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 43 +-
drivers/gpu/drm/drm_property.c | 49 ++
include/drm/drm_mode_object.h | 2 +-
include/drm/drm_plane.h | 7 +
include/drm/drm_property.h | 6 +
include/uapi/drm/drm_mode.h | 8 +
20 files changed, 1446 insertions(+), 127 deletions(-)
--
2.40.1
We will wire up MPC 3D LUT to DM CRTC color pipeline in the next patch,
but so far, only for atomic interface. By checking
set_output_transfer_func in DC drivers with MPC 3D LUT support, we can
verify that regamma is only programmed when 3D LUT programming fails. As
a groundwork to introduce 3D LUT programming and better understand each
step, detach atomic regamma programming from the crtc colocr updating
code.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 53 ++++++++++++-------
1 file changed, 34 insertions(+), 19 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 0a9aa162d4a0..c0bf55416b4d 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
@@ -503,6 +503,36 @@ static int __set_output_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
}
+static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
+ const struct drm_color_lut *regamma_lut,
+ uint32_t regamma_size, bool has_rom)
+{
+ struct dc_transfer_func *out_tf = stream->out_transfer_func;
+ int ret = 0;
+
+ if (regamma_size) {
+ /* CRTC RGM goes into RGM LUT.
+ *
+ * Note: there is no implicit sRGB regamma here. We are using
+ * degamma calculation from color module to calculate the curve
+ * from a linear base.
+ */
+ out_tf->type = TF_TYPE_DISTRIBUTED_POINTS;
+ out_tf->tf = TRANSFER_FUNCTION_LINEAR;
+
+ ret = __set_output_tf(out_tf, regamma_lut, regamma_size, has_rom);
+ } else {
+ /*
+ * No CRTC RGM means we can just put the block into bypass
+ * since we don't have any plane level adjustments using it.
+ */
+ out_tf->type = TF_TYPE_BYPASS;
+ out_tf->tf = TRANSFER_FUNCTION_LINEAR;
+ }
+
+ return ret;
+}
+
/**
* __set_input_tf - calculates the input transfer function based on expected
* input space.
@@ -650,27 +680,12 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
regamma_size, has_rom);
if (r)
return r;
- } else if (has_regamma) {
- /* CRTC RGM goes into RGM LUT.
- *
- * Note: there is no implicit sRGB regamma here. We are using
- * degamma calculation from color module to calculate the curve
- * from a linear base.
- */
- stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
- stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
-
- r = __set_output_tf(stream->out_transfer_func, regamma_lut,
- regamma_size, has_rom);
+ } else {
+ regamma_size = has_regamma ? regamma_size : 0;
+ r = amdgpu_dm_set_atomic_regamma(stream, regamma_lut,
+ regamma_size, has_rom);
if (r)
return r;
- } else {
- /*
- * No CRTC RGM means we can just put the block into bypass
- * since we don't have any plane level adjustments using it.
- */
- stream->out_transfer_func->type = TF_TYPE_BYPASS;
- stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
}
/*
--
2.40.1
DC only has pre-blending degamma caps (plane/DPP) that is currently in
use for CRTC/post-blending degamma, so that we don't have HW caps to
perform plane and CRTC degamma at the same time. Reject atomic updates
when serspace sets both plane and CRTC degamma properties.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
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 d019a091b08e..5659f88d1f2c 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
@@ -919,9 +919,20 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
ret = __set_dm_plane_degamma(plane_state, dc_plane_state);
- if (ret != -EINVAL)
+ if (ret == -ENOMEM)
return ret;
+ /* We only have one degamma block available (pre-blending) for the
+ * whole color correction pipeline, so that we can't actually perform
+ * plane and CRTC degamma at the same time. Explicitly reject atomic
+ * updates when userspace sets both plane and CRTC degamma properties.
+ */
+ if (has_crtc_cm_degamma && ret != -EINVAL){
+ drm_dbg_kms(crtc->base.crtc->dev,
+ "doesn't support plane and CRTC degamma at the same time\n");
+ return -EINVAL;
+ }
+
/* If we are here, it means we don't have plane degamma settings, check
* if we have CRTC degamma waiting for mapping to pre-blending degamma
* block
--
2.40.1
From: Joshua Ashton <[email protected]>
Detach value translation from CTM to reuse it for programming HDR
multiplier property.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 8 +-------
drivers/gpu/drm/amd/display/include/fixed31_32.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 7 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 5659f88d1f2c..db771c895720 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
@@ -383,7 +383,6 @@ static void __drm_lut_to_dc_gamma(const struct drm_color_lut *lut,
static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm,
struct fixed31_32 *matrix)
{
- int64_t val;
int i;
/*
@@ -402,12 +401,7 @@ static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm,
}
/* gamut_remap_matrix[i] = ctm[i - floor(i/4)] */
- val = ctm->matrix[i - (i / 4)];
- /* If negative, convert to 2's complement. */
- if (val & (1ULL << 63))
- val = -(val & ~(1ULL << 63));
-
- matrix[i].value = val;
+ matrix[i] = dc_fixpt_from_s3132(ctm->matrix[i - (i / 4)]);
}
}
diff --git a/drivers/gpu/drm/amd/display/include/fixed31_32.h b/drivers/gpu/drm/amd/display/include/fixed31_32.h
index d4cf7ead1d87..84da1dd34efd 100644
--- a/drivers/gpu/drm/amd/display/include/fixed31_32.h
+++ b/drivers/gpu/drm/amd/display/include/fixed31_32.h
@@ -69,6 +69,18 @@ static const struct fixed31_32 dc_fixpt_epsilon = { 1LL };
static const struct fixed31_32 dc_fixpt_half = { 0x80000000LL };
static const struct fixed31_32 dc_fixpt_one = { 0x100000000LL };
+static inline struct fixed31_32 dc_fixpt_from_s3132(__u64 x)
+{
+ struct fixed31_32 val;
+
+ /* If negative, convert to 2's complement. */
+ if (x & (1ULL << 63))
+ x = -(x & ~(1ULL << 63));
+
+ val.value = x;
+ return val;
+}
+
/*
* @brief
* Initialization routines
--
2.40.1
From: Joshua Ashton <[email protected]>
Map plane blend properties to DPP blend gamma. Plane blend is a
post-3D LUT curve that linearizes color space for blending. It may be
defined by a user-blob LUT and/or predefined transfer function. As
hardcoded curve (ROM) is not supported on blend gamma, we use AMD color
module to fill parameters when setting non-linear TF with empty LUT.
v2:
- rename DRM TFs to AMDGPU TFs
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 55 +++++++++++++++++--
2 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 025a7eb5c8aa..d37269a2179e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8063,6 +8063,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
bundle->surface_updates[planes_count].hdr_mult = dc_plane->hdr_mult;
bundle->surface_updates[planes_count].func_shaper = dc_plane->in_shaper_func;
bundle->surface_updates[planes_count].lut3d_func = dc_plane->lut3d_func;
+ bundle->surface_updates[planes_count].blend_tf = dc_plane->blend_tf;
}
amdgpu_dm_plane_fill_dc_scaling_info(dm->adev, new_plane_state,
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 2c96501d2fc0..f638e5b3a70b 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
@@ -706,6 +706,34 @@ static int amdgpu_dm_atomic_shaper_lut(const struct drm_color_lut *shaper_lut,
return ret;
}
+static int amdgpu_dm_atomic_blend_lut(const struct drm_color_lut *blend_lut,
+ bool has_rom,
+ enum dc_transfer_func_predefined tf,
+ uint32_t blend_size,
+ struct dc_transfer_func *func_blend)
+{
+ int ret = 0;
+
+ if (blend_size || tf != TRANSFER_FUNCTION_LINEAR) {
+ /* DRM plane gamma LUT or TF means we are linearizing color
+ * space before blending (similar to degamma programming). As
+ * we don't have hardcoded curve support, or we use AMD color
+ * module to fill the parameters that will be translated to HW
+ * points.
+ */
+ func_blend->type = TF_TYPE_DISTRIBUTED_POINTS;
+ func_blend->tf = tf;
+ func_blend->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
+
+ ret = __set_input_tf(func_blend, blend_lut, blend_size);
+ } else {
+ func_blend->type = TF_TYPE_BYPASS;
+ func_blend->tf = TRANSFER_FUNCTION_LINEAR;
+ }
+
+ return ret;
+}
+
/* amdgpu_dm_lut3d_size - get expected size according to hw color caps
* @adev: amdgpu device
* @lut_size: default size
@@ -1053,8 +1081,9 @@ amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
enum amdgpu_transfer_function shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
- const struct drm_color_lut *shaper_lut, *lut3d;
- uint32_t shaper_size, lut3d_size;
+ enum amdgpu_transfer_function blend_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
+ const struct drm_color_lut *shaper_lut, *lut3d, *blend_lut;
+ uint32_t shaper_size, lut3d_size, blend_size;
int ret;
/* We have nothing to do here, return */
@@ -1074,12 +1103,30 @@ amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
amdgpu_tf_to_dc_tf(shaper_tf),
shaper_size,
dc_plane_state->in_shaper_func);
- if (ret)
+ if (ret) {
drm_dbg_kms(plane_state->plane->dev,
"setting plane %d shaper LUT failed.\n",
plane_state->plane->index);
- return ret;
+ return ret;
+ }
+
+ blend_tf = dm_plane_state->blend_tf;
+ blend_lut = __extract_blob_lut(dm_plane_state->blend_lut, &blend_size);
+ blend_size = blend_lut != NULL ? blend_size : 0;
+
+ ret = amdgpu_dm_atomic_blend_lut(blend_lut, false,
+ amdgpu_tf_to_dc_tf(blend_tf),
+ blend_size, dc_plane_state->blend_tf);
+ if (ret) {
+ drm_dbg_kms(plane_state->plane->dev,
+ "setting plane %d gamma lut failed.\n",
+ plane_state->plane->index);
+
+ return ret;
+ }
+
+ return 0;
}
/**
--
2.40.1
From: Joshua Ashton <[email protected]>
Unlike degamma, blend gamma doesn't support hardcoded curve
(predefined/ROM), but we can use AMD color module to fill blend gamma
parameters when we have non-linear plane gamma TF without plane gamma
LUT. The regular degamma path doesn't hit this.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 20 +++++++++++--------
1 file changed, 12 insertions(+), 8 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 58c4797f506e..2c96501d2fc0 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
@@ -551,17 +551,21 @@ static int __set_input_tf(struct dc_transfer_func *func,
struct dc_gamma *gamma = NULL;
bool res;
- gamma = dc_create_gamma();
- if (!gamma)
- return -ENOMEM;
+ if (lut_size) {
+ gamma = dc_create_gamma();
+ if (!gamma)
+ return -ENOMEM;
- gamma->type = GAMMA_CUSTOM;
- gamma->num_entries = lut_size;
+ gamma->type = GAMMA_CUSTOM;
+ gamma->num_entries = lut_size;
- __drm_lut_to_dc_gamma(lut, gamma, false);
+ __drm_lut_to_dc_gamma(lut, gamma, false);
+ }
- res = mod_color_calculate_degamma_params(NULL, func, gamma, true);
- dc_gamma_release(&gamma);
+ res = mod_color_calculate_degamma_params(NULL, func, gamma, gamma != NULL);
+
+ if (gamma)
+ dc_gamma_release(&gamma);
return res ? 0 : -ENOMEM;
}
--
2.40.1
From: Joshua Ashton <[email protected]>
With `dc_fixpt_from_s3132()` translation, we can just use it to set
hdr_mult.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 3e5aa1e46662..9fcc169fb87b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8060,6 +8060,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
bundle->surface_updates[planes_count].gamma = dc_plane->gamma_correction;
bundle->surface_updates[planes_count].in_transfer_func = dc_plane->in_transfer_func;
bundle->surface_updates[planes_count].gamut_remap_matrix = &dc_plane->gamut_remap_matrix;
+ bundle->surface_updates[planes_count].hdr_mult = dc_plane->hdr_mult;
}
amdgpu_dm_plane_fill_dc_scaling_info(dm->adev, new_plane_state,
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 db771c895720..15f7304d8f33 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
@@ -902,6 +902,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state)
{
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
bool has_crtc_cm_degamma;
int ret;
@@ -912,6 +913,8 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
/* After, we start to update values according to color props */
has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
+ dc_plane_state->hdr_mult = dc_fixpt_from_s3132(dm_plane_state->hdr_mult);
+
ret = __set_dm_plane_degamma(plane_state, dc_plane_state);
if (ret == -ENOMEM)
return ret;
--
2.40.1
Place it in drm_property where drm_property_replace_blob and
drm_property_lookup_blob live. Then we can use the DRM helper for
driver-specific KMS properties too.
Reviewed-by: Harry Wentland <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/drm_atomic_uapi.c | 43 ++++-----------------------
drivers/gpu/drm/drm_property.c | 49 +++++++++++++++++++++++++++++++
include/drm/drm_property.h | 6 ++++
4 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index dc01c43f6193..d72c22dcf685 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -221,7 +221,7 @@ static int malidp_crtc_atomic_check_ctm(struct drm_crtc *crtc,
/*
* The size of the ctm is checked in
- * drm_atomic_replace_property_blob_from_id.
+ * drm_property_replace_blob_from_id.
*/
ctm = (struct drm_color_ctm *)state->ctm->data;
for (i = 0; i < ARRAY_SIZE(ctm->matrix); ++i) {
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index d867e7f9f2cd..a6a9ee5086dd 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -362,39 +362,6 @@ static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state,
return fence_ptr;
}
-static int
-drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
- struct drm_property_blob **blob,
- uint64_t blob_id,
- ssize_t expected_size,
- ssize_t expected_elem_size,
- bool *replaced)
-{
- struct drm_property_blob *new_blob = NULL;
-
- if (blob_id != 0) {
- new_blob = drm_property_lookup_blob(dev, blob_id);
- if (new_blob == NULL)
- return -EINVAL;
-
- if (expected_size > 0 &&
- new_blob->length != expected_size) {
- drm_property_blob_put(new_blob);
- return -EINVAL;
- }
- if (expected_elem_size > 0 &&
- new_blob->length % expected_elem_size != 0) {
- drm_property_blob_put(new_blob);
- return -EINVAL;
- }
- }
-
- *replaced |= drm_property_replace_blob(blob, new_blob);
- drm_property_blob_put(new_blob);
-
- return 0;
-}
-
static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
struct drm_crtc_state *state, struct drm_property *property,
uint64_t val)
@@ -415,7 +382,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
} else if (property == config->prop_vrr_enabled) {
state->vrr_enabled = val;
} else if (property == config->degamma_lut_property) {
- ret = drm_atomic_replace_property_blob_from_id(dev,
+ ret = drm_property_replace_blob_from_id(dev,
&state->degamma_lut,
val,
-1, sizeof(struct drm_color_lut),
@@ -423,7 +390,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
state->color_mgmt_changed |= replaced;
return ret;
} else if (property == config->ctm_property) {
- ret = drm_atomic_replace_property_blob_from_id(dev,
+ ret = drm_property_replace_blob_from_id(dev,
&state->ctm,
val,
sizeof(struct drm_color_ctm), -1,
@@ -431,7 +398,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
state->color_mgmt_changed |= replaced;
return ret;
} else if (property == config->gamma_lut_property) {
- ret = drm_atomic_replace_property_blob_from_id(dev,
+ ret = drm_property_replace_blob_from_id(dev,
&state->gamma_lut,
val,
-1, sizeof(struct drm_color_lut),
@@ -563,7 +530,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
} else if (property == plane->color_range_property) {
state->color_range = val;
} else if (property == config->prop_fb_damage_clips) {
- ret = drm_atomic_replace_property_blob_from_id(dev,
+ ret = drm_property_replace_blob_from_id(dev,
&state->fb_damage_clips,
val,
-1,
@@ -729,7 +696,7 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
} else if (property == config->hdr_output_metadata_property) {
- ret = drm_atomic_replace_property_blob_from_id(dev,
+ ret = drm_property_replace_blob_from_id(dev,
&state->hdr_output_metadata,
val,
sizeof(struct hdr_output_metadata), -1,
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index dfec479830e4..f72ef6493340 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -751,6 +751,55 @@ bool drm_property_replace_blob(struct drm_property_blob **blob,
}
EXPORT_SYMBOL(drm_property_replace_blob);
+/**
+ * drm_property_replace_blob_from_id - replace a blob property taking a reference
+ * @dev: DRM device
+ * @blob: a pointer to the member blob to be replaced
+ * @blob_id: the id of the new blob to replace with
+ * @expected_size: expected size of the blob property
+ * @expected_elem_size: expected size of an element in the blob property
+ * @replaced: if the blob was in fact replaced
+ *
+ * Look up the new blob from id, take its reference, check expected sizes of
+ * the blob and its element and replace the old blob by the new one. Advertise
+ * if the replacement operation was successful.
+ *
+ * Return: true if the blob was in fact replaced. -EINVAL if the new blob was
+ * not found or sizes don't match.
+ */
+int drm_property_replace_blob_from_id(struct drm_device *dev,
+ struct drm_property_blob **blob,
+ uint64_t blob_id,
+ ssize_t expected_size,
+ ssize_t expected_elem_size,
+ bool *replaced)
+{
+ struct drm_property_blob *new_blob = NULL;
+
+ if (blob_id != 0) {
+ new_blob = drm_property_lookup_blob(dev, blob_id);
+ if (new_blob == NULL)
+ return -EINVAL;
+
+ if (expected_size > 0 &&
+ new_blob->length != expected_size) {
+ drm_property_blob_put(new_blob);
+ return -EINVAL;
+ }
+ if (expected_elem_size > 0 &&
+ new_blob->length % expected_elem_size != 0) {
+ drm_property_blob_put(new_blob);
+ return -EINVAL;
+ }
+ }
+
+ *replaced |= drm_property_replace_blob(blob, new_blob);
+ drm_property_blob_put(new_blob);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_property_replace_blob_from_id);
+
int drm_mode_getblob_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 65bc9710a470..082f29156b3e 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -279,6 +279,12 @@ struct drm_property_blob *drm_property_create_blob(struct drm_device *dev,
const void *data);
struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev,
uint32_t id);
+int drm_property_replace_blob_from_id(struct drm_device *dev,
+ struct drm_property_blob **blob,
+ uint64_t blob_id,
+ ssize_t expected_size,
+ ssize_t expected_elem_size,
+ bool *replaced);
int drm_property_replace_global_blob(struct drm_device *dev,
struct drm_property_blob **replace,
size_t length,
--
2.40.1
From: Joshua Ashton <[email protected]>
When commiting planes, we copy color mgmt resources to the stream state.
Do the same for shaper and 3D LUTs.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Co-developed-by: Melissa Wen <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d37269a2179e..dfe61c5ed49e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8274,6 +8274,10 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
&acrtc_state->stream->csc_color_matrix;
bundle->stream_update.out_transfer_func =
acrtc_state->stream->out_transfer_func;
+ bundle->stream_update.lut3d_func =
+ (struct dc_3dlut *) acrtc_state->stream->lut3d_func;
+ bundle->stream_update.func_shaper =
+ (struct dc_transfer_func *) acrtc_state->stream->func_shaper;
}
acrtc_state->stream->abm_level = acrtc_state->abm_level;
--
2.40.1
Map the plane CTM driver-specific property to DC plane, instead of DC
stream. The remaining steps to program DPP block are already implemented
on DC shared-code.
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 25 +++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dfe61c5ed49e..f239410234b3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9578,6 +9578,7 @@ static bool should_reset_plane(struct drm_atomic_state *state,
if (dm_old_other_state->degamma_tf != dm_new_other_state->degamma_tf ||
dm_old_other_state->degamma_lut != dm_new_other_state->degamma_lut ||
dm_old_other_state->hdr_mult != dm_new_other_state->hdr_mult ||
+ dm_old_other_state->ctm != dm_new_other_state->ctm ||
dm_old_other_state->shaper_lut != dm_new_other_state->shaper_lut ||
dm_old_other_state->shaper_tf != dm_new_other_state->shaper_tf ||
dm_old_other_state->lut3d != dm_new_other_state->lut3d ||
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 86a918ab82be..7ff329101fd4 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
@@ -1158,6 +1158,8 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
struct dc_plane_state *dc_plane_state)
{
struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
+ struct drm_color_ctm *ctm = NULL;
struct dc_color_caps *color_caps = NULL;
bool has_crtc_cm_degamma;
int ret;
@@ -1209,6 +1211,29 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
return ret;
}
+ /* Setup CRTC CTM. */
+ if (dm_plane_state->ctm) {
+ ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
+
+ /*
+ * So far, if we have both plane and CRTC CTM, plane CTM takes
+ * the priority and we discard data for CRTC CTM, as
+ * implemented in dcn10_program_gamut_remap(). However, we
+ * have MPC gamut_remap_matrix from DCN3 family, therefore we
+ * can remap MPC programing of the matrix to MPC block and
+ * provide support for both DPP and MPC matrix at the same
+ * time.
+ */
+ __drm_ctm_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
+
+ dc_plane_state->gamut_remap_matrix.enable_remap = true;
+ dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
+ } else {
+ /* Bypass CTM. */
+ dc_plane_state->gamut_remap_matrix.enable_remap = false;
+ dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
+ }
+
return amdgpu_dm_plane_set_color_properties(plane_state,
dc_plane_state, color_caps);
}
--
2.40.1
From: Joshua Ashton <[email protected]>
Need to funnel the color caps through to these functions so it can check
that the hardware is capable.
v2:
- remove redundant color caps assignment on plane degamma map (Harry)
- pass color caps to degamma params
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 35 ++++++++++++-------
1 file changed, 22 insertions(+), 13 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 f638e5b3a70b..4356846a2bce 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
@@ -538,6 +538,7 @@ static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
/**
* __set_input_tf - calculates the input transfer function based on expected
* input space.
+ * @caps: dc color capabilities
* @func: transfer function
* @lut: lookup table that defines the color space
* @lut_size: size of respective lut.
@@ -545,7 +546,7 @@ static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
* Returns:
* 0 in case of success. -ENOMEM if fails.
*/
-static int __set_input_tf(struct dc_transfer_func *func,
+static int __set_input_tf(struct dc_color_caps *caps, struct dc_transfer_func *func,
const struct drm_color_lut *lut, uint32_t lut_size)
{
struct dc_gamma *gamma = NULL;
@@ -562,7 +563,7 @@ static int __set_input_tf(struct dc_transfer_func *func,
__drm_lut_to_dc_gamma(lut, gamma, false);
}
- res = mod_color_calculate_degamma_params(NULL, func, gamma, gamma != NULL);
+ res = mod_color_calculate_degamma_params(caps, func, gamma, gamma != NULL);
if (gamma)
dc_gamma_release(&gamma);
@@ -725,7 +726,7 @@ static int amdgpu_dm_atomic_blend_lut(const struct drm_color_lut *blend_lut,
func_blend->tf = tf;
func_blend->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
- ret = __set_input_tf(func_blend, blend_lut, blend_size);
+ ret = __set_input_tf(NULL, func_blend, blend_lut, blend_size);
} else {
func_blend->type = TF_TYPE_BYPASS;
func_blend->tf = TRANSFER_FUNCTION_LINEAR;
@@ -950,7 +951,8 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
static int
map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
- struct dc_plane_state *dc_plane_state)
+ struct dc_plane_state *dc_plane_state,
+ struct dc_color_caps *caps)
{
const struct drm_color_lut *degamma_lut;
enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_SRGB;
@@ -1005,7 +1007,7 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
dc_plane_state->in_transfer_func->tf =
TRANSFER_FUNCTION_LINEAR;
- r = __set_input_tf(dc_plane_state->in_transfer_func,
+ r = __set_input_tf(caps, dc_plane_state->in_transfer_func,
degamma_lut, degamma_size);
if (r)
return r;
@@ -1018,7 +1020,7 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
dc_plane_state->in_transfer_func->tf = tf;
if (tf != TRANSFER_FUNCTION_SRGB &&
- !mod_color_calculate_degamma_params(NULL,
+ !mod_color_calculate_degamma_params(caps,
dc_plane_state->in_transfer_func,
NULL, false))
return -ENOMEM;
@@ -1029,7 +1031,8 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
static int
__set_dm_plane_degamma(struct drm_plane_state *plane_state,
- struct dc_plane_state *dc_plane_state)
+ struct dc_plane_state *dc_plane_state,
+ struct dc_color_caps *color_caps)
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
const struct drm_color_lut *degamma_lut;
@@ -1060,7 +1063,7 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
dc_plane_state->in_transfer_func->type =
TF_TYPE_DISTRIBUTED_POINTS;
- ret = __set_input_tf(dc_plane_state->in_transfer_func,
+ ret = __set_input_tf(color_caps, dc_plane_state->in_transfer_func,
degamma_lut, degamma_size);
if (ret)
return ret;
@@ -1068,7 +1071,7 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
dc_plane_state->in_transfer_func->type =
TF_TYPE_PREDEFINED;
- if (!mod_color_calculate_degamma_params(NULL,
+ if (!mod_color_calculate_degamma_params(color_caps,
dc_plane_state->in_transfer_func, NULL, false))
return -ENOMEM;
}
@@ -1077,7 +1080,8 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
static int
amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
- struct dc_plane_state *dc_plane_state)
+ struct dc_plane_state *dc_plane_state,
+ struct dc_color_caps *color_caps)
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
enum amdgpu_transfer_function shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
@@ -1147,6 +1151,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
struct dc_plane_state *dc_plane_state)
{
struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
+ struct dc_color_caps *color_caps = NULL;
bool has_crtc_cm_degamma;
int ret;
@@ -1156,6 +1161,9 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
return ret;
}
+ if (dc_plane_state->ctx && dc_plane_state->ctx->dc)
+ color_caps = &dc_plane_state->ctx->dc->caps.color;
+
/* Initially, we can just bypass the DGM block. */
dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
dc_plane_state->in_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
@@ -1163,7 +1171,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
/* After, we start to update values according to color props */
has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
- ret = __set_dm_plane_degamma(plane_state, dc_plane_state);
+ ret = __set_dm_plane_degamma(plane_state, dc_plane_state, color_caps);
if (ret == -ENOMEM)
return ret;
@@ -1189,10 +1197,11 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
* linearize (implicit degamma) from sRGB/BT709 according to
* the input space.
*/
- ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state);
+ ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state, color_caps);
if (ret)
return ret;
}
- return amdgpu_dm_plane_set_color_properties(plane_state, dc_plane_state);
+ return amdgpu_dm_plane_set_color_properties(plane_state,
+ dc_plane_state, color_caps);
}
--
2.40.1
From: Joshua Ashton <[email protected]>
Set DC plane with user degamma LUT or predefined TF from driver-specific
plane color properties. If plane and CRTC degamma are set in the same
time, plane degamma has priority. That means, we only set CRTC degamma
if we don't have plane degamma LUT or TF to configure. We return -EINVAL
if we don't have plane degamma settings, so we can continue and check
CRTC degamma.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 70 +++++++++++++++++--
3 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 78fdd0b95ae8..3e5aa1e46662 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5009,7 +5009,9 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
* Always set input transfer function, since plane state is refreshed
* every time.
*/
- ret = amdgpu_dm_update_plane_color_mgmt(dm_crtc_state, dc_plane_state);
+ ret = amdgpu_dm_update_plane_color_mgmt(dm_crtc_state,
+ plane_state,
+ dc_plane_state);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 51471675c298..23e3984f17fb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -903,6 +903,7 @@ int amdgpu_dm_create_color_properties(struct amdgpu_device *adev);
int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
+ struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state);
void amdgpu_dm_update_connector_after_detect(
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 74eb02655d96..d019a091b08e 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
@@ -843,9 +843,58 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
return 0;
}
+static int
+__set_dm_plane_degamma(struct drm_plane_state *plane_state,
+ struct dc_plane_state *dc_plane_state)
+{
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
+ const struct drm_color_lut *degamma_lut;
+ enum amdgpu_transfer_function tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
+ uint32_t degamma_size;
+ bool has_degamma_lut;
+ int ret;
+
+ degamma_lut = __extract_blob_lut(dm_plane_state->degamma_lut,
+ °amma_size);
+
+ has_degamma_lut = degamma_lut &&
+ !__is_lut_linear(degamma_lut, degamma_size);
+
+ tf = dm_plane_state->degamma_tf;
+
+ /* If we don't have plane degamma LUT nor TF to set on DC, we have
+ * nothing to do here, return.
+ */
+ if (!has_degamma_lut && tf == AMDGPU_TRANSFER_FUNCTION_DEFAULT)
+ return -EINVAL;
+
+ dc_plane_state->in_transfer_func->tf = amdgpu_tf_to_dc_tf(tf);
+
+ if (has_degamma_lut) {
+ ASSERT(degamma_size == MAX_COLOR_LUT_ENTRIES);
+
+ dc_plane_state->in_transfer_func->type =
+ TF_TYPE_DISTRIBUTED_POINTS;
+
+ ret = __set_input_tf(dc_plane_state->in_transfer_func,
+ degamma_lut, degamma_size);
+ if (ret)
+ return ret;
+ } else {
+ dc_plane_state->in_transfer_func->type =
+ TF_TYPE_PREDEFINED;
+
+ if (!mod_color_calculate_degamma_params(NULL,
+ dc_plane_state->in_transfer_func, NULL, false))
+ return -ENOMEM;
+ }
+ return 0;
+}
+
/**
* amdgpu_dm_update_plane_color_mgmt: Maps DRM color management to DC plane.
* @crtc: amdgpu_dm crtc state
+ * @plane_state: DRM plane state
* @dc_plane_state: target DC surface
*
* Update the underlying dc_stream_state's input transfer function (ITF) in
@@ -856,13 +905,28 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
* 0 on success. -ENOMEM if mem allocation fails.
*/
int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
+ struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state)
{
bool has_crtc_cm_degamma;
int ret;
+ /* Initially, we can just bypass the DGM block. */
+ dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
+ dc_plane_state->in_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
+
+ /* After, we start to update values according to color props */
has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
- if (has_crtc_cm_degamma){
+
+ ret = __set_dm_plane_degamma(plane_state, dc_plane_state);
+ if (ret != -EINVAL)
+ return ret;
+
+ /* If we are here, it means we don't have plane degamma settings, check
+ * if we have CRTC degamma waiting for mapping to pre-blending degamma
+ * block
+ */
+ if (has_crtc_cm_degamma) {
/* AMD HW doesn't have post-blending degamma caps. When DRM
* CRTC atomic degamma is set, we maps it to DPP degamma block
* (pre-blending) or, on legacy gamma, we use DPP degamma to
@@ -872,10 +936,6 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state);
if (ret)
return ret;
- } else {
- /* ...Otherwise we can just bypass the DGM block. */
- dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
- dc_plane_state->in_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
}
return 0;
--
2.40.1
We will add color mgmt properties to DRM planes in the next patches and
we want to track when one of this properties change to define atomic
commit behaviors. Using a similar approach from CRTC color props, we set
a color_mgmt_changed boolean whenever a plane color prop changes.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 1 +
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
include/drm/drm_plane.h | 7 +++++++
3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 11f3a130f6f4..30aa3248bb0d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -724,6 +724,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
drm_get_color_encoding_name(state->color_encoding));
drm_printf(p, "\tcolor-range=%s\n",
drm_get_color_range_name(state->color_range));
+ drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
if (plane->funcs->atomic_print_state)
plane->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..25bb0859fda7 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -338,6 +338,7 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
state->fence = NULL;
state->commit = NULL;
state->fb_damage_clips = NULL;
+ state->color_mgmt_changed = false;
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 51291983ea44..52c3287da0da 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -237,6 +237,13 @@ struct drm_plane_state {
/** @state: backpointer to global drm_atomic_state */
struct drm_atomic_state *state;
+
+ /**
+ * @color_mgmt_changed: Color management properties have changed. Used
+ * by the atomic helpers and drivers to steer the atomic commit control
+ * flow.
+ */
+ bool color_mgmt_changed : 1;
};
static inline struct drm_rect
--
2.40.1
From: Joshua Ashton <[email protected]>
Add predefined transfer function programming. There is no pre-blending
out gamma ROM, but we can use AMD color modules to program LUT
parameters from a pre-defined TF and an empty regamma LUT (or bump up
LUT parameters with pre-defined TF setup).
v2:
- update crtc color mgmt if regamma TF differs between states (Joshua)
- map inverse EOTF to DC transfer function (Melissa)
Signed-off-by: Joshua Ashton <[email protected]>
Co-developed-by: Melissa Wen <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 73 +++++++++++++++----
2 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 77b4d671a9e0..272974b88cda 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9456,6 +9456,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
* when a modeset is needed, to ensure it gets reprogrammed.
*/
if (dm_new_crtc_state->base.color_mgmt_changed ||
+ dm_old_crtc_state->regamma_tf != dm_new_crtc_state->regamma_tf ||
drm_atomic_crtc_needs_modeset(new_crtc_state)) {
ret = amdgpu_dm_update_crtc_color_mgmt(dm_new_crtc_state);
if (ret)
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 c0bf55416b4d..0188e82d1fdd 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
@@ -468,16 +468,18 @@ static int __set_output_tf(struct dc_transfer_func *func,
struct calculate_buffer cal_buffer = {0};
bool res;
- ASSERT(lut && lut_size == MAX_COLOR_LUT_ENTRIES);
-
cal_buffer.buffer_index = -1;
- gamma = dc_create_gamma();
- if (!gamma)
- return -ENOMEM;
+ if (lut_size) {
+ ASSERT(lut && lut_size == MAX_COLOR_LUT_ENTRIES);
- gamma->num_entries = lut_size;
- __drm_lut_to_dc_gamma(lut, gamma, false);
+ gamma = dc_create_gamma();
+ if (!gamma)
+ return -ENOMEM;
+
+ gamma->num_entries = lut_size;
+ __drm_lut_to_dc_gamma(lut, gamma, false);
+ }
if (func->tf == TRANSFER_FUNCTION_LINEAR) {
/*
@@ -485,32 +487,36 @@ static int __set_output_tf(struct dc_transfer_func *func,
* on top of a linear input. But degamma params can be used
* instead to simulate this.
*/
- gamma->type = GAMMA_CUSTOM;
+ if (gamma)
+ gamma->type = GAMMA_CUSTOM;
res = mod_color_calculate_degamma_params(NULL, func,
- gamma, true);
+ gamma, gamma != NULL);
} else {
/*
* Assume sRGB. The actual mapping will depend on whether the
* input was legacy or not.
*/
- gamma->type = GAMMA_CS_TFM_1D;
- res = mod_color_calculate_regamma_params(func, gamma, false,
+ if (gamma)
+ gamma->type = GAMMA_CS_TFM_1D;
+ res = mod_color_calculate_regamma_params(func, gamma, gamma != NULL,
has_rom, NULL, &cal_buffer);
}
- dc_gamma_release(&gamma);
+ if (gamma)
+ dc_gamma_release(&gamma);
return res ? 0 : -ENOMEM;
}
static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
const struct drm_color_lut *regamma_lut,
- uint32_t regamma_size, bool has_rom)
+ uint32_t regamma_size, bool has_rom,
+ enum dc_transfer_func_predefined tf)
{
struct dc_transfer_func *out_tf = stream->out_transfer_func;
int ret = 0;
- if (regamma_size) {
+ if (regamma_size || tf != TRANSFER_FUNCTION_LINEAR) {
/* CRTC RGM goes into RGM LUT.
*
* Note: there is no implicit sRGB regamma here. We are using
@@ -518,7 +524,7 @@ static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
* from a linear base.
*/
out_tf->type = TF_TYPE_DISTRIBUTED_POINTS;
- out_tf->tf = TRANSFER_FUNCTION_LINEAR;
+ out_tf->tf = tf;
ret = __set_output_tf(out_tf, regamma_lut, regamma_size, has_rom);
} else {
@@ -564,6 +570,38 @@ static int __set_input_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
}
+static enum dc_transfer_func_predefined
+amdgpu_tf_to_dc_tf(enum amdgpu_transfer_function tf)
+{
+ switch (tf)
+ {
+ default:
+ case AMDGPU_TRANSFER_FUNCTION_DEFAULT:
+ case AMDGPU_TRANSFER_FUNCTION_LINEAR:
+ return TRANSFER_FUNCTION_LINEAR;
+ case AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF:
+ case AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF:
+ return TRANSFER_FUNCTION_SRGB;
+ case AMDGPU_TRANSFER_FUNCTION_BT709_EOTF:
+ case AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF:
+ return TRANSFER_FUNCTION_BT709;
+ case AMDGPU_TRANSFER_FUNCTION_PQ_EOTF:
+ case AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF:
+ return TRANSFER_FUNCTION_PQ;
+ case AMDGPU_TRANSFER_FUNCTION_UNITY:
+ return TRANSFER_FUNCTION_UNITY;
+ case AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF:
+ case AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF:
+ return TRANSFER_FUNCTION_GAMMA22;
+ case AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF:
+ case AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF:
+ return TRANSFER_FUNCTION_GAMMA24;
+ case AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF:
+ case AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF:
+ return TRANSFER_FUNCTION_GAMMA26;
+ }
+}
+
/**
* amdgpu_dm_verify_lut_sizes - verifies if DRM luts match the hw supported sizes
* @crtc_state: the DRM CRTC state
@@ -631,9 +669,12 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
const struct drm_color_lut *degamma_lut, *regamma_lut;
uint32_t degamma_size, regamma_size;
bool has_regamma, has_degamma;
+ enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_LINEAR;
bool is_legacy;
int r;
+ tf = amdgpu_tf_to_dc_tf(crtc->regamma_tf);
+
r = amdgpu_dm_verify_lut_sizes(&crtc->base);
if (r)
return r;
@@ -683,7 +724,7 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
} else {
regamma_size = has_regamma ? regamma_size : 0;
r = amdgpu_dm_set_atomic_regamma(stream, regamma_lut,
- regamma_size, has_rom);
+ regamma_size, has_rom, tf);
if (r)
return r;
}
--
2.40.1
On AMD HW, 3D LUT always assumes a preceding shaper 1D LUT used for
delinearizing and/or normalizing the color space before applying a 3D
LUT. Add pre-defined transfer function to enable delinearizing content
with or without shaper LUT, where AMD color module calculates the
resulted shaper curve. We apply an inverse EOTF to go from linear values
to encoded values. If we are already in a non-linear space and/or don't
need to normalize values, we can bypass shaper LUT with a linear
transfer function that is also the default TF value.
v2:
- squash commits for shaper LUT and shaper TF
- define inverse EOTF as supported shaper TFs
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 16 ++++++++++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +++++++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 29 +++++++++++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 32 +++++++++++++++++++
4 files changed, 88 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 730a88236501..4fb164204ee6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -363,6 +363,22 @@ struct amdgpu_mode_info {
* @plane_hdr_mult_property:
*/
struct drm_property *plane_hdr_mult_property;
+ /**
+ * @shaper_lut_property: Plane property to set pre-blending shaper LUT
+ * that converts color content before 3D LUT.
+ */
+ struct drm_property *plane_shaper_lut_property;
+ /**
+ * @shaper_lut_size_property: Plane property for the size of
+ * pre-blending shaper LUT as supported by the driver (read-only).
+ */
+ struct drm_property *plane_shaper_lut_size_property;
+ /**
+ * @plane_shaper_tf_property: Plane property to set a predefined
+ * transfer function for pre-blending shaper (before applying 3D LUT)
+ * with or without LUT.
+ */
+ struct drm_property *plane_shaper_tf_property;
/**
* @plane_lut3d_property: Plane property for gamma correction using a
* 3D LUT (pre-blending).
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index deea90212e31..6b6c2980f0af 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -769,6 +769,17 @@ struct dm_plane_state {
* S31.32 sign-magnitude.
*/
__u64 hdr_mult;
+ /**
+ * @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
+ * array of &struct drm_color_lut.
+ */
+ struct drm_property_blob *shaper_lut;
+ /**
+ * @shaper_tf:
+ *
+ * Predefined transfer function to delinearize color space.
+ */
+ enum amdgpu_transfer_function shaper_tf;
/**
* @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
* &struct drm_color_lut.
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 7e6d4df99a0c..fbcee717bf0a 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
@@ -151,6 +151,14 @@ static const u32 amdgpu_eotf =
BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF) |
BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF);
+static const u32 amdgpu_inv_eotf =
+ BIT(AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF);
+
static struct drm_property *
amdgpu_create_tf_property(struct drm_device *dev,
const char *name,
@@ -209,6 +217,27 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_hdr_mult_property = prop;
+ prop = drm_property_create(adev_to_drm(adev),
+ DRM_MODE_PROP_BLOB,
+ "AMD_PLANE_SHAPER_LUT", 0);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_shaper_lut_property = prop;
+
+ prop = drm_property_create_range(adev_to_drm(adev),
+ DRM_MODE_PROP_IMMUTABLE,
+ "AMD_PLANE_SHAPER_LUT_SIZE", 0, UINT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_shaper_lut_size_property = prop;
+
+ prop = amdgpu_create_tf_property(adev_to_drm(adev),
+ "AMD_PLANE_SHAPER_TF",
+ amdgpu_inv_eotf);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_shaper_tf_property = prop;
+
prop = drm_property_create(adev_to_drm(adev),
DRM_MODE_PROP_BLOB,
"AMD_PLANE_LUT3D", 0);
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 882391f7add6..8d6ddf19bb87 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
@@ -1332,6 +1332,7 @@ static void dm_drm_plane_reset(struct drm_plane *plane)
__drm_atomic_helper_plane_reset(plane, &amdgpu_state->base);
amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
amdgpu_state->hdr_mult = AMDGPU_HDR_MULT_DEFAULT;
+ amdgpu_state->shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
}
static struct drm_plane_state *
@@ -1353,11 +1354,14 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
if (dm_plane_state->degamma_lut)
drm_property_blob_get(dm_plane_state->degamma_lut);
+ if (dm_plane_state->shaper_lut)
+ drm_property_blob_get(dm_plane_state->shaper_lut);
if (dm_plane_state->lut3d)
drm_property_blob_get(dm_plane_state->lut3d);
dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
dm_plane_state->hdr_mult = old_dm_plane_state->hdr_mult;
+ dm_plane_state->shaper_tf = old_dm_plane_state->shaper_tf;
return &dm_plane_state->base;
}
@@ -1430,6 +1434,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
drm_property_blob_put(dm_plane_state->degamma_lut);
if (dm_plane_state->lut3d)
drm_property_blob_put(dm_plane_state->lut3d);
+ if (dm_plane_state->shaper_lut)
+ drm_property_blob_put(dm_plane_state->shaper_lut);
if (dm_plane_state->dc_state)
dc_plane_state_release(dm_plane_state->dc_state);
@@ -1462,6 +1468,14 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
AMDGPU_HDR_MULT_DEFAULT);
if (dpp_color_caps.hw_3d_lut) {
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_shaper_lut_property, 0);
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_shaper_lut_size_property,
+ MAX_COLOR_LUT_ENTRIES);
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_shaper_tf_property,
+ AMDGPU_TRANSFER_FUNCTION_DEFAULT);
drm_object_attach_property(&plane->base,
mode_info.plane_lut3d_property, 0);
drm_object_attach_property(&plane->base,
@@ -1499,6 +1513,19 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
dm_plane_state->hdr_mult = val;
dm_plane_state->base.color_mgmt_changed = 1;
}
+ } else if (property == adev->mode_info.plane_shaper_lut_property) {
+ ret = drm_property_replace_blob_from_id(plane->dev,
+ &dm_plane_state->shaper_lut,
+ val, -1,
+ sizeof(struct drm_color_lut),
+ &replaced);
+ dm_plane_state->base.color_mgmt_changed |= replaced;
+ return ret;
+ } else if (property == adev->mode_info.plane_shaper_tf_property) {
+ if (dm_plane_state->shaper_tf != val) {
+ dm_plane_state->shaper_tf = val;
+ dm_plane_state->base.color_mgmt_changed = 1;
+ }
} else if (property == adev->mode_info.plane_lut3d_property) {
ret = drm_property_replace_blob_from_id(plane->dev,
&dm_plane_state->lut3d,
@@ -1534,6 +1561,11 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
*val = dm_plane_state->degamma_tf;
} else if (property == adev->mode_info.plane_hdr_mult_property) {
*val = dm_plane_state->hdr_mult;
+ } else if (property == adev->mode_info.plane_shaper_lut_property) {
+ *val = (dm_plane_state->shaper_lut) ?
+ dm_plane_state->shaper_lut->base.id : 0;
+ } else if (property == adev->mode_info.plane_shaper_tf_property) {
+ *val = dm_plane_state->shaper_tf;
} else if (property == adev->mode_info.plane_lut3d_property) {
*val = (dm_plane_state->lut3d) ?
dm_plane_state->lut3d->base.id : 0;
--
2.40.1
From: Joshua Ashton <[email protected]>
Allow userspace to tell the kernel driver the input space and,
therefore, uses correct predefined transfer function (TF) to delinearize
content with or without LUT.
v2:
- rename TF enum prefix from DRM_ to AMDGPU_ (Harry)
- remove HLG TF
Signed-off-by: Joshua Ashton <[email protected]>
Co-developed-by: Melissa Wen <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 +++++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +++++++++++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 21 +++++++++++++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 19 +++++++++++++++--
4 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index ec4621deac8c..6ef958a14e16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -354,6 +354,11 @@ struct amdgpu_mode_info {
* size of degamma LUT as supported by the driver (read-only).
*/
struct drm_property *plane_degamma_lut_size_property;
+ /**
+ * @plane_degamma_tf_property: Plane pre-defined transfer function to
+ * to go from scanout/encoded values to linear values.
+ */
+ struct drm_property *plane_degamma_tf_property;
};
#define AMDGPU_MAX_BL_LEVEL 0xFF
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index f0343bbf0fe1..c749c9cb3d94 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -716,6 +716,18 @@ static inline void amdgpu_dm_set_mst_status(uint8_t *status,
extern const struct amdgpu_ip_block_version dm_ip_block;
+enum amdgpu_transfer_function {
+ AMDGPU_TRANSFER_FUNCTION_DEFAULT,
+ AMDGPU_TRANSFER_FUNCTION_SRGB,
+ AMDGPU_TRANSFER_FUNCTION_BT709,
+ AMDGPU_TRANSFER_FUNCTION_PQ,
+ AMDGPU_TRANSFER_FUNCTION_LINEAR,
+ AMDGPU_TRANSFER_FUNCTION_UNITY,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA22,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA24,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA26,
+};
+
struct dm_plane_state {
struct drm_plane_state base;
struct dc_plane_state *dc_state;
@@ -729,6 +741,13 @@ struct dm_plane_state {
* The blob (if not NULL) is an array of &struct drm_color_lut.
*/
struct drm_property_blob *degamma_lut;
+ /**
+ * @degamma_tf:
+ *
+ * Predefined transfer function to tell DC driver the input space to
+ * linearize.
+ */
+ enum amdgpu_transfer_function degamma_tf;
};
struct dm_crtc_state {
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 cf175b86ba80..56ce008b9095 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
@@ -85,6 +85,18 @@ void amdgpu_dm_init_color_mod(void)
}
#ifdef AMD_PRIVATE_COLOR
+static const struct drm_prop_enum_list amdgpu_transfer_function_enum_list[] = {
+ { AMDGPU_TRANSFER_FUNCTION_DEFAULT, "Default" },
+ { AMDGPU_TRANSFER_FUNCTION_SRGB, "sRGB" },
+ { AMDGPU_TRANSFER_FUNCTION_BT709, "BT.709" },
+ { AMDGPU_TRANSFER_FUNCTION_PQ, "PQ (Perceptual Quantizer)" },
+ { AMDGPU_TRANSFER_FUNCTION_LINEAR, "Linear" },
+ { AMDGPU_TRANSFER_FUNCTION_UNITY, "Unity" },
+ { AMDGPU_TRANSFER_FUNCTION_GAMMA22, "Gamma 2.2" },
+ { AMDGPU_TRANSFER_FUNCTION_GAMMA24, "Gamma 2.4" },
+ { AMDGPU_TRANSFER_FUNCTION_GAMMA26, "Gamma 2.6" },
+};
+
int
amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
{
@@ -104,6 +116,15 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_degamma_lut_size_property = prop;
+ prop = drm_property_create_enum(adev_to_drm(adev),
+ DRM_MODE_PROP_ENUM,
+ "AMD_PLANE_DEGAMMA_TF",
+ amdgpu_transfer_function_enum_list,
+ ARRAY_SIZE(amdgpu_transfer_function_enum_list));
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_degamma_tf_property = prop;
+
return 0;
}
#endif
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 488012d1558d..0a955abb1abf 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
@@ -1326,8 +1326,11 @@ static void dm_drm_plane_reset(struct drm_plane *plane)
amdgpu_state = kzalloc(sizeof(*amdgpu_state), GFP_KERNEL);
WARN_ON(amdgpu_state == NULL);
- if (amdgpu_state)
- __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base);
+ if (!amdgpu_state)
+ return;
+
+ __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base);
+ amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
}
static struct drm_plane_state *
@@ -1350,6 +1353,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
if (dm_plane_state->degamma_lut)
drm_property_blob_get(dm_plane_state->degamma_lut);
+ dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
+
return &dm_plane_state->base;
}
@@ -1441,6 +1446,9 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
drm_object_attach_property(&plane->base,
mode_info.plane_degamma_lut_size_property,
MAX_COLOR_LUT_ENTRIES);
+ drm_object_attach_property(&plane->base,
+ dm->adev->mode_info.plane_degamma_tf_property,
+ AMDGPU_TRANSFER_FUNCTION_DEFAULT);
}
}
@@ -1463,6 +1471,11 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
+ } else if (property == adev->mode_info.plane_degamma_tf_property) {
+ if (dm_plane_state->degamma_tf != val) {
+ dm_plane_state->degamma_tf = val;
+ dm_plane_state->base.color_mgmt_changed = 1;
+ }
} else {
drm_dbg_atomic(plane->dev,
"[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
@@ -1486,6 +1499,8 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
if (property == adev->mode_info.plane_degamma_lut_property) {
*val = (dm_plane_state->degamma_lut) ?
dm_plane_state->degamma_lut->base.id : 0;
+ } else if (property == adev->mode_info.plane_degamma_tf_property) {
+ *val = dm_plane_state->degamma_tf;
} else {
return -EINVAL;
}
--
2.40.1
From: Joshua Ashton <[email protected]>
Blend 1D LUT or a pre-defined transfer function can be set to linearize
content before blending, so that it's positioned just before blending
planes in the AMD color mgmt pipeline, and after 3D LUT (non-linear
space). Shaper and Blend LUTs are 1D LUTs that sandwich 3D LUT. Drivers
should advertize blend properties according to HW caps.
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 18 ++++++++++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 12 +++++++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 21 +++++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 36 +++++++++++++++++++
4 files changed, 87 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 4fb164204ee6..fd0b7047d56b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -389,6 +389,24 @@ struct amdgpu_mode_info {
* size of 3D LUT as supported by the driver (read-only).
*/
struct drm_property *plane_lut3d_size_property;
+ /**
+ * @plane_blend_lut_property: Plane property for output gamma before
+ * blending. Userspace set a blend LUT to convert colors after 3D LUT
+ * conversion. It works as a post-3D LUT 1D LUT, with shaper LUT, they
+ * are sandwiching 3D LUT with two 1D LUT.
+ */
+ struct drm_property *plane_blend_lut_property;
+ /**
+ * @plane_blend_lut_size_property: Plane property to define the max
+ * size of blend LUT as supported by the driver (read-only).
+ */
+ struct drm_property *plane_blend_lut_size_property;
+ /**
+ * @plane_blend_tf_property: Plane property to set a predefined
+ * transfer function for pre-blending blend (before applying 3D LUT)
+ * with or without LUT.
+ */
+ struct drm_property *plane_blend_tf_property;
};
#define AMDGPU_MAX_BL_LEVEL 0xFF
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 6b6c2980f0af..b6fa271ab0dd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -785,6 +785,18 @@ struct dm_plane_state {
* &struct drm_color_lut.
*/
struct drm_property_blob *lut3d;
+ /**
+ * @blend_lut: blend lut lookup table blob. The blob (if not NULL) is an
+ * array of &struct drm_color_lut.
+ */
+ struct drm_property_blob *blend_lut;
+ /**
+ * @blend_tf:
+ *
+ * Pre-defined transfer function for converting plane pixel data before
+ * applying blend LUT.
+ */
+ enum amdgpu_transfer_function blend_tf;
};
struct dm_crtc_state {
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 fbcee717bf0a..2d64332e6b80 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
@@ -252,6 +252,27 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_lut3d_size_property = prop;
+ prop = drm_property_create(adev_to_drm(adev),
+ DRM_MODE_PROP_BLOB,
+ "AMD_PLANE_BLEND_LUT", 0);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_blend_lut_property = prop;
+
+ prop = drm_property_create_range(adev_to_drm(adev),
+ DRM_MODE_PROP_IMMUTABLE,
+ "AMD_PLANE_BLEND_LUT_SIZE", 0, UINT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_blend_lut_size_property = prop;
+
+ prop = amdgpu_create_tf_property(adev_to_drm(adev),
+ "AMD_PLANE_BLEND_TF",
+ amdgpu_eotf);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_blend_tf_property = prop;
+
return 0;
}
#endif
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 8d6ddf19bb87..3fd57de7c5be 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
@@ -1333,6 +1333,7 @@ static void dm_drm_plane_reset(struct drm_plane *plane)
amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
amdgpu_state->hdr_mult = AMDGPU_HDR_MULT_DEFAULT;
amdgpu_state->shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
+ amdgpu_state->blend_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
}
static struct drm_plane_state *
@@ -1358,10 +1359,13 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
drm_property_blob_get(dm_plane_state->shaper_lut);
if (dm_plane_state->lut3d)
drm_property_blob_get(dm_plane_state->lut3d);
+ if (dm_plane_state->blend_lut)
+ drm_property_blob_get(dm_plane_state->blend_lut);
dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
dm_plane_state->hdr_mult = old_dm_plane_state->hdr_mult;
dm_plane_state->shaper_tf = old_dm_plane_state->shaper_tf;
+ dm_plane_state->blend_tf = old_dm_plane_state->blend_tf;
return &dm_plane_state->base;
}
@@ -1436,6 +1440,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
drm_property_blob_put(dm_plane_state->lut3d);
if (dm_plane_state->shaper_lut)
drm_property_blob_put(dm_plane_state->shaper_lut);
+ if (dm_plane_state->blend_lut)
+ drm_property_blob_put(dm_plane_state->blend_lut);
if (dm_plane_state->dc_state)
dc_plane_state_release(dm_plane_state->dc_state);
@@ -1482,6 +1488,17 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
mode_info.plane_lut3d_size_property,
MAX_COLOR_3DLUT_ENTRIES);
}
+
+ if (dpp_color_caps.ogam_ram) {
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_blend_lut_property, 0);
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_blend_lut_size_property,
+ MAX_COLOR_LUT_ENTRIES);
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_blend_tf_property,
+ AMDGPU_TRANSFER_FUNCTION_DEFAULT);
+ }
}
static int
@@ -1534,6 +1551,19 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
+ } else if (property == adev->mode_info.plane_blend_lut_property) {
+ ret = drm_property_replace_blob_from_id(plane->dev,
+ &dm_plane_state->blend_lut,
+ val, -1,
+ sizeof(struct drm_color_lut),
+ &replaced);
+ dm_plane_state->base.color_mgmt_changed |= replaced;
+ return ret;
+ } else if (property == adev->mode_info.plane_blend_tf_property) {
+ if (dm_plane_state->blend_tf != val) {
+ dm_plane_state->blend_tf = val;
+ dm_plane_state->base.color_mgmt_changed = 1;
+ }
} else {
drm_dbg_atomic(plane->dev,
"[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
@@ -1569,6 +1599,12 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
} else if (property == adev->mode_info.plane_lut3d_property) {
*val = (dm_plane_state->lut3d) ?
dm_plane_state->lut3d->base.id : 0;
+ } else if (property == adev->mode_info.plane_blend_lut_property) {
+ *val = (dm_plane_state->blend_lut) ?
+ dm_plane_state->blend_lut->base.id : 0;
+ } else if (property == adev->mode_info.plane_blend_tf_property) {
+ *val = dm_plane_state->blend_tf;
+
} else {
return -EINVAL;
}
--
2.40.1
DRM_OBJECT_MAX_PROPERTY limits the number of properties to be attached
and we are increasing that value all time we add a new property (generic
or driver-specific).
In this series, we are adding 13 new KMS driver-specific properties for
AMD color manage:
- CRTC Gamma enumerated Transfer Function
- Plane: Degamma LUT+size+TF, HDR multiplier, shaper LUT+size+TF, 3D
LUT+size, blend LUT+size+TF (12)
Therefore, just increase DRM_OBJECT_MAX_PROPERTY to a number (64) that
accomodates these new properties and gives some room for others,
avoiding change this number everytime we add a new KMS property.
Reviewed-by: Harry Wentland <[email protected]>
Reviewed-by: Simon Ser <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
include/drm/drm_mode_object.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index 912f1e415685..08d7a7f0188f 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -60,7 +60,7 @@ struct drm_mode_object {
void (*free_cb)(struct kref *kref);
};
-#define DRM_OBJECT_MAX_PROPERTY 24
+#define DRM_OBJECT_MAX_PROPERTY 64
/**
* struct drm_object_properties - property tracking for &drm_mode_object
*/
--
2.40.1
Instead of relying on color block names to get the transfer function
intention regarding encoding pixel's luminance, define supported
Electro-Optical Transfer Functions (EOTFs) and inverse EOTFs, that
includes pure gamma or standardized transfer functions.
Suggested-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +++--
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 69 +++++++++++++++----
2 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index c749c9cb3d94..f6251ed89684 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -718,14 +718,21 @@ extern const struct amdgpu_ip_block_version dm_ip_block;
enum amdgpu_transfer_function {
AMDGPU_TRANSFER_FUNCTION_DEFAULT,
- AMDGPU_TRANSFER_FUNCTION_SRGB,
- AMDGPU_TRANSFER_FUNCTION_BT709,
- AMDGPU_TRANSFER_FUNCTION_PQ,
+ AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_BT709_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_PQ_EOTF,
AMDGPU_TRANSFER_FUNCTION_LINEAR,
AMDGPU_TRANSFER_FUNCTION_UNITY,
- AMDGPU_TRANSFER_FUNCTION_GAMMA22,
- AMDGPU_TRANSFER_FUNCTION_GAMMA24,
- AMDGPU_TRANSFER_FUNCTION_GAMMA26,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF,
+ AMDGPU_TRANSFER_FUNCTION_COUNT
};
struct dm_plane_state {
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 56ce008b9095..cc2187c0879a 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
@@ -85,18 +85,59 @@ void amdgpu_dm_init_color_mod(void)
}
#ifdef AMD_PRIVATE_COLOR
-static const struct drm_prop_enum_list amdgpu_transfer_function_enum_list[] = {
- { AMDGPU_TRANSFER_FUNCTION_DEFAULT, "Default" },
- { AMDGPU_TRANSFER_FUNCTION_SRGB, "sRGB" },
- { AMDGPU_TRANSFER_FUNCTION_BT709, "BT.709" },
- { AMDGPU_TRANSFER_FUNCTION_PQ, "PQ (Perceptual Quantizer)" },
- { AMDGPU_TRANSFER_FUNCTION_LINEAR, "Linear" },
- { AMDGPU_TRANSFER_FUNCTION_UNITY, "Unity" },
- { AMDGPU_TRANSFER_FUNCTION_GAMMA22, "Gamma 2.2" },
- { AMDGPU_TRANSFER_FUNCTION_GAMMA24, "Gamma 2.4" },
- { AMDGPU_TRANSFER_FUNCTION_GAMMA26, "Gamma 2.6" },
+static const char * const
+amdgpu_transfer_function_names[] = {
+ [AMDGPU_TRANSFER_FUNCTION_DEFAULT] = "Default",
+ [AMDGPU_TRANSFER_FUNCTION_LINEAR] = "Linear",
+ [AMDGPU_TRANSFER_FUNCTION_UNITY] = "Unity",
+ [AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF] = "sRGB EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_BT709_EOTF] = "BT.709 EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_PQ_EOTF] = "PQ EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF] = "Gamma 2.2 EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF] = "Gamma 2.4 EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF] = "Gamma 2.6 EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF] = "sRGB inv_EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF] = "BT.709 inv_EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF] = "PQ inv_EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF] = "Gamma 2.2 inv_EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF] = "Gamma 2.4 inv_EOTF",
+ [AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF] = "Gamma 2.6 inv_EOTF",
};
+static const u32 amdgpu_eotf =
+ BIT(AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_BT709_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_PQ_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF);
+
+static struct drm_property *
+amdgpu_create_tf_property(struct drm_device *dev,
+ const char *name,
+ u32 supported_tf)
+{
+ u32 transfer_functions = supported_tf |
+ BIT(AMDGPU_TRANSFER_FUNCTION_DEFAULT) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_LINEAR) |
+ BIT(AMDGPU_TRANSFER_FUNCTION_UNITY);
+ struct drm_prop_enum_list enum_list[AMDGPU_TRANSFER_FUNCTION_COUNT];
+ int i, len;
+
+ len = 0;
+ for (i = 0; i < AMDGPU_TRANSFER_FUNCTION_COUNT; i++) {
+ if ((transfer_functions & BIT(i)) == 0)
+ continue;
+
+ enum_list[len].type = i;
+ enum_list[len].name = amdgpu_transfer_function_names[i];
+ len++;
+ }
+
+ return drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
+ name, enum_list, len);
+}
+
int
amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
{
@@ -116,11 +157,9 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_degamma_lut_size_property = prop;
- prop = drm_property_create_enum(adev_to_drm(adev),
- DRM_MODE_PROP_ENUM,
- "AMD_PLANE_DEGAMMA_TF",
- amdgpu_transfer_function_enum_list,
- ARRAY_SIZE(amdgpu_transfer_function_enum_list));
+ prop = amdgpu_create_tf_property(adev_to_drm(adev),
+ "AMD_PLANE_DEGAMMA_TF",
+ amdgpu_eotf);
if (!prop)
return -ENOMEM;
adev->mode_info.plane_degamma_tf_property = prop;
--
2.40.1
dc->caps.color.mpc.gamut_remap says there is a post-blending color block
for gamut remap matrix for DCN3 HW family and newer versions. However,
those drivers still follow DCN10 programming that remap stream
gamut_remap_matrix to DPP (pre-blending).
To enable pre-blending and post-blending gamut_remap matrix supports at
the same time, set stream gamut_remap to MPC and plane gamut_remap to
DPP for DCN301 that support both.
It was tested using IGT KMS color tests for DRM CRTC CTM property and it
preserves test results.
Signed-off-by: Melissa Wen <[email protected]>
---
.../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 37 +++++++++++++++++++
.../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 ++
.../drm/amd/display/dc/dcn301/dcn301_init.c | 2 +-
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
index 4cd4ae07d73d..4fb4e9ec03f1 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
@@ -186,6 +186,43 @@ bool dcn30_set_input_transfer_func(struct dc *dc,
return result;
}
+void dcn30_program_gamut_remap(struct pipe_ctx *pipe_ctx)
+{
+ int i = 0;
+ struct dpp_grph_csc_adjustment dpp_adjust;
+ struct mpc_grph_gamut_adjustment mpc_adjust;
+ int mpcc_id = pipe_ctx->plane_res.hubp->inst;
+ struct mpc *mpc = pipe_ctx->stream_res.opp->ctx->dc->res_pool->mpc;
+
+ memset(&dpp_adjust, 0, sizeof(dpp_adjust));
+ dpp_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_BYPASS;
+
+ if (pipe_ctx->plane_state &&
+ pipe_ctx->plane_state->gamut_remap_matrix.enable_remap == true) {
+ dpp_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_SW;
+ for (i = 0; i < CSC_TEMPERATURE_MATRIX_SIZE; i++)
+ dpp_adjust.temperature_matrix[i] =
+ pipe_ctx->plane_state->gamut_remap_matrix.matrix[i];
+ }
+
+ pipe_ctx->plane_res.dpp->funcs->dpp_set_gamut_remap(pipe_ctx->plane_res.dpp,
+ &dpp_adjust);
+
+ memset(&mpc_adjust, 0, sizeof(mpc_adjust));
+ mpc_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_BYPASS;
+
+ if (pipe_ctx->top_pipe == NULL) {
+ if (pipe_ctx->stream->gamut_remap_matrix.enable_remap == true) {
+ mpc_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_SW;
+ for (i = 0; i < CSC_TEMPERATURE_MATRIX_SIZE; i++)
+ mpc_adjust.temperature_matrix[i] =
+ pipe_ctx->stream->gamut_remap_matrix.matrix[i];
+ }
+ }
+
+ mpc->funcs->set_gamut_remap(mpc, mpcc_id, &mpc_adjust);
+}
+
bool dcn30_set_output_transfer_func(struct dc *dc,
struct pipe_ctx *pipe_ctx,
const struct dc_stream_state *stream)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
index a24a8e33a3d2..cb34ca932a5f 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
@@ -58,6 +58,9 @@ bool dcn30_set_blend_lut(struct pipe_ctx *pipe_ctx,
bool dcn30_set_input_transfer_func(struct dc *dc,
struct pipe_ctx *pipe_ctx,
const struct dc_plane_state *plane_state);
+
+void dcn30_program_gamut_remap(struct pipe_ctx *pipe_ctx);
+
bool dcn30_set_output_transfer_func(struct dc *dc,
struct pipe_ctx *pipe_ctx,
const struct dc_stream_state *stream);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
index 257df8660b4c..81fd50ee97c3 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
@@ -33,7 +33,7 @@
#include "dcn301_init.h"
static const struct hw_sequencer_funcs dcn301_funcs = {
- .program_gamut_remap = dcn10_program_gamut_remap,
+ .program_gamut_remap = dcn30_program_gamut_remap,
.init_hw = dcn10_init_hw,
.power_down_on_boot = dcn10_power_down_on_boot,
.apply_ctx_to_hw = dce110_apply_ctx_to_hw,
--
2.40.1
From: Harry Wentland <[email protected]>
The region and segment calculation was incapable of dealing
with regions of more than 16 segments. We first fix this.
Now that we can support regions up to 256 elements we can
define a better segment distribution for near-linear LUTs
for our maximum of 256 HW-supported points.
With these changes an "identity" LUT looks visually
indistinguishable from bypass and allows us to use
our 3DLUT.
Signed-off-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/dc/dcn10/dcn10_cm_common.c | 93 +++++++++++++++----
1 file changed, 75 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
index 3538973bd0c6..04b2e04b68f3 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
@@ -349,20 +349,37 @@ bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx,
* segment is from 2^-10 to 2^1
* There are less than 256 points, for optimization
*/
- seg_distr[0] = 3;
- seg_distr[1] = 4;
- seg_distr[2] = 4;
- seg_distr[3] = 4;
- seg_distr[4] = 4;
- seg_distr[5] = 4;
- seg_distr[6] = 4;
- seg_distr[7] = 4;
- seg_distr[8] = 4;
- seg_distr[9] = 4;
- seg_distr[10] = 1;
+ if (output_tf->tf == TRANSFER_FUNCTION_LINEAR) {
+ seg_distr[0] = 0; /* 2 */
+ seg_distr[1] = 1; /* 4 */
+ seg_distr[2] = 2; /* 4 */
+ seg_distr[3] = 3; /* 8 */
+ seg_distr[4] = 4; /* 16 */
+ seg_distr[5] = 5; /* 32 */
+ seg_distr[6] = 6; /* 64 */
+ seg_distr[7] = 7; /* 128 */
+
+ region_start = -8;
+ region_end = 1;
+ } else {
+ seg_distr[0] = 3; /* 8 */
+ seg_distr[1] = 4; /* 16 */
+ seg_distr[2] = 4;
+ seg_distr[3] = 4;
+ seg_distr[4] = 4;
+ seg_distr[5] = 4;
+ seg_distr[6] = 4;
+ seg_distr[7] = 4;
+ seg_distr[8] = 4;
+ seg_distr[9] = 4;
+ seg_distr[10] = 1; /* 2 */
+ /* total = 8*16 + 8 + 64 + 2 = */
+
+ region_start = -10;
+ region_end = 1;
+ }
+
- region_start = -10;
- region_end = 1;
}
for (i = region_end - region_start; i < MAX_REGIONS_NUMBER ; i++)
@@ -375,16 +392,56 @@ bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx,
j = 0;
for (k = 0; k < (region_end - region_start); k++) {
- increment = NUMBER_SW_SEGMENTS / (1 << seg_distr[k]);
+ /*
+ * We're using an ugly-ish hack here. Our HW allows for
+ * 256 segments per region but SW_SEGMENTS is 16.
+ * SW_SEGMENTS has some undocumented relationship to
+ * the number of points in the tf_pts struct, which
+ * is 512, unlike what's suggested TRANSFER_FUNC_POINTS.
+ *
+ * In order to work past this dilemma we'll scale our
+ * increment by (1 << 4) and then do the inverse (1 >> 4)
+ * when accessing the elements in tf_pts.
+ *
+ * TODO: find a better way using SW_SEGMENTS and
+ * TRANSFER_FUNC_POINTS definitions
+ */
+ increment = (NUMBER_SW_SEGMENTS << 4) / (1 << seg_distr[k]);
start_index = (region_start + k + MAX_LOW_POINT) *
NUMBER_SW_SEGMENTS;
- for (i = start_index; i < start_index + NUMBER_SW_SEGMENTS;
+ for (i = (start_index << 4); i < (start_index << 4) + (NUMBER_SW_SEGMENTS << 4);
i += increment) {
+ struct fixed31_32 in_plus_one, in;
+ struct fixed31_32 value, red_value, green_value, blue_value;
+ uint32_t t = i & 0xf;
+
if (j == hw_points - 1)
break;
- rgb_resulted[j].red = output_tf->tf_pts.red[i];
- rgb_resulted[j].green = output_tf->tf_pts.green[i];
- rgb_resulted[j].blue = output_tf->tf_pts.blue[i];
+
+ in_plus_one = output_tf->tf_pts.red[(i >> 4) + 1];
+ in = output_tf->tf_pts.red[i >> 4];
+ value = dc_fixpt_sub(in_plus_one, in);
+ value = dc_fixpt_shr(dc_fixpt_mul_int(value, t), 4);
+ value = dc_fixpt_add(in, value);
+ red_value = value;
+
+ in_plus_one = output_tf->tf_pts.green[(i >> 4) + 1];
+ in = output_tf->tf_pts.green[i >> 4];
+ value = dc_fixpt_sub(in_plus_one, in);
+ value = dc_fixpt_shr(dc_fixpt_mul_int(value, t), 4);
+ value = dc_fixpt_add(in, value);
+ green_value = value;
+
+ in_plus_one = output_tf->tf_pts.blue[(i >> 4) + 1];
+ in = output_tf->tf_pts.blue[i >> 4];
+ value = dc_fixpt_sub(in_plus_one, in);
+ value = dc_fixpt_shr(dc_fixpt_mul_int(value, t), 4);
+ value = dc_fixpt_add(in, value);
+ blue_value = value;
+
+ rgb_resulted[j].red = red_value;
+ rgb_resulted[j].green = green_value;
+ rgb_resulted[j].blue = blue_value;
j++;
}
}
--
2.40.1
Add AMD pre-defined transfer function property to default DRM CRTC
gamma to convert to wire encoding with or without a user gamma LUT.
v2:
- enable CRTC prop in the end of driver-specific prop sequence
- define inverse EOTFs as supported regamma TFs
- reword driver-specific function doc to remove shaper/3D LUT
Co-developed-by: Joshua Ashton <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 7 ++
.../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 72 +++++++++++++++++++
4 files changed, 92 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index fd0b7047d56b..abb871a912d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -407,6 +407,11 @@ struct amdgpu_mode_info {
* with or without LUT.
*/
struct drm_property *plane_blend_tf_property;
+ /* @regamma_tf_property: Transfer function for CRTC regamma
+ * (post-blending). Possible values are defined by `enum
+ * amdgpu_transfer_function`.
+ */
+ struct drm_property *regamma_tf_property;
};
#define AMDGPU_MAX_BL_LEVEL 0xFF
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index b6fa271ab0dd..51471675c298 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -821,6 +821,14 @@ struct dm_crtc_state {
struct dc_info_packet vrr_infopacket;
int abm_level;
+
+ /**
+ * @regamma_tf:
+ *
+ * Pre-defined transfer function for converting internal FB -> wire
+ * encoding.
+ */
+ enum amdgpu_transfer_function regamma_tf;
};
#define to_dm_crtc_state(x) container_of(x, struct dm_crtc_state, base)
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 2d64332e6b80..841e0391f7fb 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
@@ -273,6 +273,13 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_blend_tf_property = prop;
+ prop = amdgpu_create_tf_property(adev_to_drm(adev),
+ "AMD_CRTC_REGAMMA_TF",
+ amdgpu_inv_eotf);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.regamma_tf_property = prop;
+
return 0;
}
#endif
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 440fc0869a34..d746f0aa0f11 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -253,6 +253,7 @@ static struct drm_crtc_state *dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->freesync_config = cur->freesync_config;
state->cm_has_degamma = cur->cm_has_degamma;
state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
+ state->regamma_tf = cur->regamma_tf;
state->crc_skip_count = cur->crc_skip_count;
state->mpo_requested = cur->mpo_requested;
/* TODO Duplicate dc_stream after objects are stream object is flattened */
@@ -289,6 +290,70 @@ static int amdgpu_dm_crtc_late_register(struct drm_crtc *crtc)
}
#endif
+#ifdef AMD_PRIVATE_COLOR
+/**
+ * drm_crtc_additional_color_mgmt - enable additional color properties
+ * @crtc: DRM CRTC
+ *
+ * This function lets the driver enable post-blending CRTC regamma transfer
+ * function property in addition to DRM CRTC gamma LUT. Default value means
+ * linear transfer function, which is the default CRTC gamma LUT behaviour
+ * without this property.
+ */
+static void
+dm_crtc_additional_color_mgmt(struct drm_crtc *crtc)
+{
+ struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+
+ if(adev->dm.dc->caps.color.mpc.ogam_ram)
+ drm_object_attach_property(&crtc->base,
+ adev->mode_info.regamma_tf_property,
+ AMDGPU_TRANSFER_FUNCTION_DEFAULT);
+}
+
+static int
+amdgpu_dm_atomic_crtc_set_property(struct drm_crtc *crtc,
+ struct drm_crtc_state *state,
+ struct drm_property *property,
+ uint64_t val)
+{
+ struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+ struct dm_crtc_state *acrtc_state = to_dm_crtc_state(state);
+
+ if (property == adev->mode_info.regamma_tf_property) {
+ if (acrtc_state->regamma_tf != val) {
+ acrtc_state->regamma_tf = val;
+ acrtc_state->base.color_mgmt_changed |= 1;
+ }
+ } else {
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] unknown property [PROP:%d:%s]]\n",
+ crtc->base.id, crtc->name,
+ property->base.id, property->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int
+amdgpu_dm_atomic_crtc_get_property(struct drm_crtc *crtc,
+ const struct drm_crtc_state *state,
+ struct drm_property *property,
+ uint64_t *val)
+{
+ struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+ struct dm_crtc_state *acrtc_state = to_dm_crtc_state(state);
+
+ if (property == adev->mode_info.regamma_tf_property)
+ *val = acrtc_state->regamma_tf;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+#endif
+
/* Implemented only the options currently available for the driver */
static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
.reset = dm_crtc_reset_state,
@@ -307,6 +372,10 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
#if defined(CONFIG_DEBUG_FS)
.late_register = amdgpu_dm_crtc_late_register,
#endif
+#ifdef AMD_PRIVATE_COLOR
+ .atomic_set_property = amdgpu_dm_atomic_crtc_set_property,
+ .atomic_get_property = amdgpu_dm_atomic_crtc_get_property,
+#endif
};
static void dm_crtc_helper_disable(struct drm_crtc *crtc)
@@ -470,6 +539,9 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
drm_mode_crtc_set_gamma_size(&acrtc->base, MAX_COLOR_LEGACY_LUT_ENTRIES);
+#ifdef AMD_PRIVATE_COLOR
+ dm_crtc_additional_color_mgmt(&acrtc->base);
+#endif
return 0;
fail:
--
2.40.1
Add 3D LUT property for plane gamma correction using a 3D lookup table.
Since a 3D LUT has a limited number of entries in each dimension we want
to use them in an optimal fashion. This means using the 3D LUT in a
colorspace that is optimized for human vision, such as sRGB, PQ, or
another non-linear space. Therefore, userpace may need one 1D LUT
(shaper) before it to delinearize content and another 1D LUT after 3D
LUT (blend) to linearize content again for blending. The next patches
add these 1D LUTs to the plane color mgmt pipeline.
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 10 ++++++++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 9 ++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 14 +++++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 23 +++++++++++++++++++
4 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 66bae0eed80c..730a88236501 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -363,6 +363,16 @@ struct amdgpu_mode_info {
* @plane_hdr_mult_property:
*/
struct drm_property *plane_hdr_mult_property;
+ /**
+ * @plane_lut3d_property: Plane property for gamma correction using a
+ * 3D LUT (pre-blending).
+ */
+ struct drm_property *plane_lut3d_property;
+ /**
+ * @plane_degamma_lut_size_property: Plane property to define the max
+ * size of 3D LUT as supported by the driver (read-only).
+ */
+ struct drm_property *plane_lut3d_size_property;
};
#define AMDGPU_MAX_BL_LEVEL 0xFF
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 44f17ac11a5f..deea90212e31 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -769,6 +769,11 @@ struct dm_plane_state {
* S31.32 sign-magnitude.
*/
__u64 hdr_mult;
+ /**
+ * @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
+ * &struct drm_color_lut.
+ */
+ struct drm_property_blob *lut3d;
};
struct dm_crtc_state {
@@ -854,6 +859,10 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
+/* 3D LUT max size is 17x17x17 */
+#define MAX_COLOR_3DLUT_ENTRIES 4913
+#define MAX_COLOR_3DLUT_BITDEPTH 12
+/* 1D LUT size */
#define MAX_COLOR_LUT_ENTRIES 4096
/* Legacy gamm LUT users such as X doesn't like large LUT sizes */
#define MAX_COLOR_LEGACY_LUT_ENTRIES 256
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 b891aaf5f7c1..7e6d4df99a0c 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
@@ -209,6 +209,20 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_hdr_mult_property = prop;
+ prop = drm_property_create(adev_to_drm(adev),
+ DRM_MODE_PROP_BLOB,
+ "AMD_PLANE_LUT3D", 0);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_lut3d_property = prop;
+
+ prop = drm_property_create_range(adev_to_drm(adev),
+ DRM_MODE_PROP_IMMUTABLE,
+ "AMD_PLANE_LUT3D_SIZE", 0, UINT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_lut3d_size_property = prop;
+
return 0;
}
#endif
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 ab7f0332c431..882391f7add6 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
@@ -1353,6 +1353,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
if (dm_plane_state->degamma_lut)
drm_property_blob_get(dm_plane_state->degamma_lut);
+ if (dm_plane_state->lut3d)
+ drm_property_blob_get(dm_plane_state->lut3d);
dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
dm_plane_state->hdr_mult = old_dm_plane_state->hdr_mult;
@@ -1426,6 +1428,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
if (dm_plane_state->degamma_lut)
drm_property_blob_put(dm_plane_state->degamma_lut);
+ if (dm_plane_state->lut3d)
+ drm_property_blob_put(dm_plane_state->lut3d);
if (dm_plane_state->dc_state)
dc_plane_state_release(dm_plane_state->dc_state);
@@ -1456,6 +1460,14 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
drm_object_attach_property(&plane->base,
dm->adev->mode_info.plane_hdr_mult_property,
AMDGPU_HDR_MULT_DEFAULT);
+
+ if (dpp_color_caps.hw_3d_lut) {
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_lut3d_property, 0);
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_lut3d_size_property,
+ MAX_COLOR_3DLUT_ENTRIES);
+ }
}
static int
@@ -1487,6 +1499,14 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
dm_plane_state->hdr_mult = val;
dm_plane_state->base.color_mgmt_changed = 1;
}
+ } else if (property == adev->mode_info.plane_lut3d_property) {
+ ret = drm_property_replace_blob_from_id(plane->dev,
+ &dm_plane_state->lut3d,
+ val, -1,
+ sizeof(struct drm_color_lut),
+ &replaced);
+ dm_plane_state->base.color_mgmt_changed |= replaced;
+ return ret;
} else {
drm_dbg_atomic(plane->dev,
"[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
@@ -1514,6 +1534,9 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
*val = dm_plane_state->degamma_tf;
} else if (property == adev->mode_info.plane_hdr_mult_property) {
*val = dm_plane_state->hdr_mult;
+ } else if (property == adev->mode_info.plane_lut3d_property) {
+ *val = (dm_plane_state->lut3d) ?
+ dm_plane_state->lut3d->base.id : 0;
} else {
return -EINVAL;
}
--
2.40.1
Map DC shaper LUT to DM plane color management. Shaper LUT can be used
to delinearize and/or normalize the color space for computational
efficiency and achiving specific visual styles. If a plane degamma is
apply to linearize the color space, a custom shaper 1D LUT can be used
just before applying 3D LUT.
v2:
- use DPP color caps to verify plane 3D LUT support
- add debug message if shaper LUT programming fails
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 108 +++++++++++++++++-
3 files changed, 107 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9fcc169fb87b..347ecff2c920 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8061,6 +8061,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
bundle->surface_updates[planes_count].in_transfer_func = dc_plane->in_transfer_func;
bundle->surface_updates[planes_count].gamut_remap_matrix = &dc_plane->gamut_remap_matrix;
bundle->surface_updates[planes_count].hdr_mult = dc_plane->hdr_mult;
+ bundle->surface_updates[planes_count].func_shaper = dc_plane->in_shaper_func;
}
amdgpu_dm_plane_fill_dc_scaling_info(dm->adev, new_plane_state,
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 23e3984f17fb..095f39f04210 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -893,6 +893,8 @@ void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
/* 3D LUT max size is 17x17x17 */
#define MAX_COLOR_3DLUT_ENTRIES 4913
#define MAX_COLOR_3DLUT_BITDEPTH 12
+int amdgpu_dm_verify_lut3d_size(struct amdgpu_device *adev,
+ struct drm_plane_state *plane_state);
/* 1D LUT size */
#define MAX_COLOR_LUT_ENTRIES 4096
/* Legacy gamm LUT users such as X doesn't like large LUT sizes */
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 15f7304d8f33..958bb5a5644d 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
@@ -598,6 +598,74 @@ amdgpu_tf_to_dc_tf(enum amdgpu_transfer_function tf)
}
}
+static int amdgpu_dm_atomic_shaper_lut(const struct drm_color_lut *shaper_lut,
+ uint32_t shaper_size,
+ struct dc_transfer_func *func_shaper)
+{
+ int ret = 0;
+
+ if (shaper_size) {
+ /* If DRM shaper LUT is set, we assume a linear color space
+ * (linearized by DRM degamma 1D LUT or not)
+ */
+ func_shaper->type = TF_TYPE_DISTRIBUTED_POINTS;
+ func_shaper->tf = TRANSFER_FUNCTION_LINEAR;
+
+ ret = __set_output_tf(func_shaper, shaper_lut, shaper_size, false);
+ } else {
+ func_shaper->type = TF_TYPE_BYPASS;
+ func_shaper->tf = TRANSFER_FUNCTION_LINEAR;
+ }
+
+ return ret;
+}
+
+/* amdgpu_dm_lut3d_size - get expected size according to hw color caps
+ * @adev: amdgpu device
+ * @lut_size: default size
+ *
+ * Return:
+ * lut_size if DC 3D LUT is supported, zero otherwise.
+ */
+static uint32_t amdgpu_dm_get_lut3d_size(struct amdgpu_device *adev,
+ uint32_t lut_size)
+{
+ return adev->dm.dc->caps.color.dpp.hw_3d_lut ? lut_size : 0;
+}
+
+/**
+ * amdgpu_dm_verify_lut3d_size - verifies if 3D LUT is supported and if DRM 3D
+ * LUT matches the hw supported size
+ * @adev: amdgpu device
+ * @crtc_state: the DRM CRTC state
+ *
+ * Verifies if post-blending (MPC) 3D LUT is supported by the HW (DCN 3.0 or
+ * newer) and if the DRM 3D LUT matches the supported size.
+ *
+ * Returns:
+ * 0 on success. -EINVAL if lut size are invalid.
+ */
+int amdgpu_dm_verify_lut3d_size(struct amdgpu_device *adev,
+ struct drm_plane_state *plane_state)
+{
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
+ const struct drm_color_lut *shaper = NULL;
+ uint32_t exp_size, size;
+
+ /* shaper LUT is only available if 3D LUT color caps*/
+ exp_size = amdgpu_dm_get_lut3d_size(adev, MAX_COLOR_LUT_ENTRIES);
+ shaper = __extract_blob_lut(dm_plane_state->shaper_lut, &size);
+
+ if (shaper && size != exp_size) {
+ drm_dbg(&adev->ddev,
+ "Invalid Shaper LUT size. Should be %u but got %u.\n",
+ exp_size, size);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/**
* amdgpu_dm_verify_lut_sizes - verifies if DRM luts match the hw supported sizes
* @crtc_state: the DRM CRTC state
@@ -885,6 +953,34 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
return 0;
}
+static int
+amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
+ struct dc_plane_state *dc_plane_state)
+{
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
+ const struct drm_color_lut *shaper_lut;
+ uint32_t shaper_size;
+ int ret;
+
+ /* We have nothing to do here, return */
+ if (!plane_state->color_mgmt_changed)
+ return 0;
+
+ dc_plane_state->hdr_mult = dc_fixpt_from_s3132(dm_plane_state->hdr_mult);
+
+ shaper_lut = __extract_blob_lut(dm_plane_state->shaper_lut, &shaper_size);
+ shaper_size = shaper_lut != NULL ? shaper_size : 0;
+
+ ret = amdgpu_dm_atomic_shaper_lut(shaper_lut, shaper_size,
+ dc_plane_state->in_shaper_func);
+ if (ret)
+ drm_dbg_kms(plane_state->plane->dev,
+ "setting plane %d shaper LUT failed.\n",
+ plane_state->plane->index);
+
+ return ret;
+}
+
/**
* amdgpu_dm_update_plane_color_mgmt: Maps DRM color management to DC plane.
* @crtc: amdgpu_dm crtc state
@@ -902,10 +998,16 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state)
{
- struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
+ struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
bool has_crtc_cm_degamma;
int ret;
+ ret = amdgpu_dm_verify_lut3d_size(adev, plane_state);
+ if (ret) {
+ drm_dbg_driver(&adev->ddev, "amdgpu_dm_verify_lut3d_size() failed\n");
+ return ret;
+ }
+
/* Initially, we can just bypass the DGM block. */
dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
dc_plane_state->in_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
@@ -913,8 +1015,6 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
/* After, we start to update values according to color props */
has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
- dc_plane_state->hdr_mult = dc_fixpt_from_s3132(dm_plane_state->hdr_mult);
-
ret = __set_dm_plane_degamma(plane_state, dc_plane_state);
if (ret == -ENOMEM)
return ret;
@@ -946,5 +1046,5 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
return ret;
}
- return 0;
+ return amdgpu_dm_plane_set_color_properties(plane_state, dc_plane_state);
}
--
2.40.1
From: Joshua Ashton <[email protected]>
Otherwise this is just initialized to 0. This needs to actually have a
value so that compute_curve can work for PQ EOTF.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Co-developed-by: Melissa Wen <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 2 ++
1 file changed, 2 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 0188e82d1fdd..68e9f2c62f2e 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
@@ -72,6 +72,7 @@
*/
#define MAX_DRM_LUT_VALUE 0xFFFF
+#define SDR_WHITE_LEVEL_INIT_VALUE 80
/**
* amdgpu_dm_init_color_mod - Initialize the color module.
@@ -525,6 +526,7 @@ static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
*/
out_tf->type = TF_TYPE_DISTRIBUTED_POINTS;
out_tf->tf = tf;
+ out_tf->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
ret = __set_output_tf(out_tf, regamma_lut, regamma_size, has_rom);
} else {
--
2.40.1
From: Joshua Ashton <[email protected]>
We should reset a plane state if at least one of the color management
properties differs from old and new state.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Co-developed-by: Melissa Wen <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 272974b88cda..78fdd0b95ae8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9524,6 +9524,10 @@ static bool should_reset_plane(struct drm_atomic_state *state,
*/
for_each_oldnew_plane_in_state(state, other, old_other_state, new_other_state, i) {
struct amdgpu_framebuffer *old_afb, *new_afb;
+ struct dm_plane_state *dm_new_other_state, *dm_old_other_state;
+
+ dm_new_other_state = to_dm_plane_state(new_other_state);
+ dm_old_other_state = to_dm_plane_state(old_other_state);
if (other->type == DRM_PLANE_TYPE_CURSOR)
continue;
@@ -9560,6 +9564,17 @@ static bool should_reset_plane(struct drm_atomic_state *state,
old_other_state->color_encoding != new_other_state->color_encoding)
return true;
+ /* HDR/Transfer Function changes. */
+ if (dm_old_other_state->degamma_tf != dm_new_other_state->degamma_tf ||
+ dm_old_other_state->degamma_lut != dm_new_other_state->degamma_lut ||
+ dm_old_other_state->hdr_mult != dm_new_other_state->hdr_mult ||
+ dm_old_other_state->shaper_lut != dm_new_other_state->shaper_lut ||
+ dm_old_other_state->shaper_tf != dm_new_other_state->shaper_tf ||
+ dm_old_other_state->lut3d != dm_new_other_state->lut3d ||
+ dm_old_other_state->blend_lut != dm_new_other_state->blend_lut ||
+ dm_old_other_state->blend_tf != dm_new_other_state->blend_tf)
+ return true;
+
/* Framebuffer checks fall at the end. */
if (!old_other_state->fb || !new_other_state->fb)
continue;
--
2.40.1
Wire up DC 3D LUT to DM plane color management (pre-blending). On AMD
display HW, 3D LUT comes after a shaper curve and we always have to
program a shaper curve to delinearize or normalize the color space
before applying a 3D LUT (since we have a reduced number of LUT
entries).
In this version, the default values of 3D LUT for size and bit_depth are
17x17x17 and 12-bit, but we already provide here a more generic
mechanisms to program other supported values (9x9x9 size and 10-bit).
v2:
- started with plane 3D LUT instead of CRTC 3D LUT support
Reviewed-by: Harry Wentland <[email protected]> (v1)
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 96 ++++++++++++++++++-
2 files changed, 94 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 347ecff2c920..025a7eb5c8aa 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8062,6 +8062,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
bundle->surface_updates[planes_count].gamut_remap_matrix = &dc_plane->gamut_remap_matrix;
bundle->surface_updates[planes_count].hdr_mult = dc_plane->hdr_mult;
bundle->surface_updates[planes_count].func_shaper = dc_plane->in_shaper_func;
+ bundle->surface_updates[planes_count].lut3d_func = dc_plane->lut3d_func;
}
amdgpu_dm_plane_fill_dc_scaling_info(dm->adev, new_plane_state,
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 90ec09ca4118..58c4797f506e 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
@@ -598,6 +598,85 @@ amdgpu_tf_to_dc_tf(enum amdgpu_transfer_function tf)
}
}
+static void __to_dc_lut3d_color(struct dc_rgb *rgb,
+ const struct drm_color_lut lut,
+ int bit_precision)
+{
+ rgb->red = drm_color_lut_extract(lut.red, bit_precision);
+ rgb->green = drm_color_lut_extract(lut.green, bit_precision);
+ rgb->blue = drm_color_lut_extract(lut.blue, bit_precision);
+}
+
+static void __drm_3dlut_to_dc_3dlut(const struct drm_color_lut *lut,
+ uint32_t lut3d_size,
+ struct tetrahedral_params *params,
+ bool use_tetrahedral_9,
+ int bit_depth)
+{
+ struct dc_rgb *lut0;
+ struct dc_rgb *lut1;
+ struct dc_rgb *lut2;
+ struct dc_rgb *lut3;
+ int lut_i, i;
+
+
+ if (use_tetrahedral_9) {
+ lut0 = params->tetrahedral_9.lut0;
+ lut1 = params->tetrahedral_9.lut1;
+ lut2 = params->tetrahedral_9.lut2;
+ lut3 = params->tetrahedral_9.lut3;
+ } else {
+ lut0 = params->tetrahedral_17.lut0;
+ lut1 = params->tetrahedral_17.lut1;
+ lut2 = params->tetrahedral_17.lut2;
+ lut3 = params->tetrahedral_17.lut3;
+ }
+
+ for (lut_i = 0, i = 0; i < lut3d_size - 4; lut_i++, i += 4) {
+ /* We should consider the 3dlut RGB values are distributed
+ * along four arrays lut0-3 where the first sizes 1229 and the
+ * other 1228. The bit depth supported for 3dlut channel is
+ * 12-bit, but DC also supports 10-bit.
+ *
+ * TODO: improve color pipeline API to enable the userspace set
+ * bit depth and 3D LUT size/stride, as specified by VA-API.
+ */
+ __to_dc_lut3d_color(&lut0[lut_i], lut[i], bit_depth);
+ __to_dc_lut3d_color(&lut1[lut_i], lut[i + 1], bit_depth);
+ __to_dc_lut3d_color(&lut2[lut_i], lut[i + 2], bit_depth);
+ __to_dc_lut3d_color(&lut3[lut_i], lut[i + 3], bit_depth);
+ }
+ /* lut0 has 1229 points (lut_size/4 + 1) */
+ __to_dc_lut3d_color(&lut0[lut_i], lut[i], bit_depth);
+}
+
+/* amdgpu_dm_atomic_lut3d - set DRM 3D LUT to DC stream
+ * @drm_lut3d: DRM CRTC (user) 3D LUT
+ * @drm_lut3d_size: size of 3D LUT
+ * @lut3d: DC 3D LUT
+ *
+ * Map DRM CRTC 3D LUT to DC 3D LUT and all necessary bits to program it
+ * on DCN MPC accordingly.
+ */
+static void amdgpu_dm_atomic_lut3d(const struct drm_color_lut *drm_lut,
+ uint32_t drm_lut3d_size,
+ struct dc_3dlut *lut)
+{
+ if (!drm_lut3d_size) {
+ lut->state.bits.initialized = 0;
+ } else {
+ /* Stride and bit depth are not programmable by API yet.
+ * Therefore, only supports 17x17x17 3D LUT (12-bit).
+ */
+ lut->lut_3d.use_tetrahedral_9 = false;
+ lut->lut_3d.use_12bits = true;
+ lut->state.bits.initialized = 1;
+ __drm_3dlut_to_dc_3dlut(drm_lut, drm_lut3d_size, &lut->lut_3d,
+ lut->lut_3d.use_tetrahedral_9,
+ MAX_COLOR_3DLUT_BITDEPTH);
+ }
+}
+
static int amdgpu_dm_atomic_shaper_lut(const struct drm_color_lut *shaper_lut,
bool has_rom,
enum dc_transfer_func_predefined tf,
@@ -652,7 +731,7 @@ int amdgpu_dm_verify_lut3d_size(struct amdgpu_device *adev,
struct drm_plane_state *plane_state)
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
- const struct drm_color_lut *shaper = NULL;
+ const struct drm_color_lut *shaper = NULL, *lut3d = NULL;
uint32_t exp_size, size;
/* shaper LUT is only available if 3D LUT color caps*/
@@ -663,6 +742,14 @@ int amdgpu_dm_verify_lut3d_size(struct amdgpu_device *adev,
drm_dbg(&adev->ddev,
"Invalid Shaper LUT size. Should be %u but got %u.\n",
exp_size, size);
+ }
+
+ exp_size = amdgpu_dm_get_lut3d_size(adev, MAX_COLOR_3DLUT_ENTRIES);
+ lut3d = __extract_blob_lut(dm_plane_state->lut3d, &size);
+
+ if (lut3d && size != exp_size) {
+ drm_dbg(&adev->ddev, "Invalid 3D LUT size. Should be %u but got %u.\n",
+ exp_size, size);
return -EINVAL;
}
@@ -962,8 +1049,8 @@ amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
enum amdgpu_transfer_function shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
- const struct drm_color_lut *shaper_lut;
- uint32_t shaper_size;
+ const struct drm_color_lut *shaper_lut, *lut3d;
+ uint32_t shaper_size, lut3d_size;
int ret;
/* We have nothing to do here, return */
@@ -975,7 +1062,10 @@ amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
shaper_lut = __extract_blob_lut(dm_plane_state->shaper_lut, &shaper_size);
shaper_size = shaper_lut != NULL ? shaper_size : 0;
shaper_tf = dm_plane_state->shaper_tf;
+ lut3d = __extract_blob_lut(dm_plane_state->lut3d, &lut3d_size);
+ lut3d_size = lut3d != NULL ? lut3d_size : 0;
+ amdgpu_dm_atomic_lut3d(lut3d, lut3d_size, dc_plane_state->lut3d_func);
ret = amdgpu_dm_atomic_shaper_lut(shaper_lut, false,
amdgpu_tf_to_dc_tf(shaper_tf),
shaper_size,
--
2.40.1
Enable usage of predefined transfer func in addition to shaper 1D LUT.
That means we can save some complexity by just setting a predefined
curve, instead of programming a custom curve when preparing color space
for applying 3D LUT.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 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 958bb5a5644d..90ec09ca4118 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
@@ -599,19 +599,22 @@ amdgpu_tf_to_dc_tf(enum amdgpu_transfer_function tf)
}
static int amdgpu_dm_atomic_shaper_lut(const struct drm_color_lut *shaper_lut,
+ bool has_rom,
+ enum dc_transfer_func_predefined tf,
uint32_t shaper_size,
struct dc_transfer_func *func_shaper)
{
int ret = 0;
- if (shaper_size) {
+ if (shaper_size || tf != TRANSFER_FUNCTION_LINEAR) {
/* If DRM shaper LUT is set, we assume a linear color space
* (linearized by DRM degamma 1D LUT or not)
*/
func_shaper->type = TF_TYPE_DISTRIBUTED_POINTS;
- func_shaper->tf = TRANSFER_FUNCTION_LINEAR;
+ func_shaper->tf = tf;
+ func_shaper->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
- ret = __set_output_tf(func_shaper, shaper_lut, shaper_size, false);
+ ret = __set_output_tf(func_shaper, shaper_lut, shaper_size, has_rom);
} else {
func_shaper->type = TF_TYPE_BYPASS;
func_shaper->tf = TRANSFER_FUNCTION_LINEAR;
@@ -958,6 +961,7 @@ amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state)
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
+ enum amdgpu_transfer_function shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
const struct drm_color_lut *shaper_lut;
uint32_t shaper_size;
int ret;
@@ -970,8 +974,11 @@ amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
shaper_lut = __extract_blob_lut(dm_plane_state->shaper_lut, &shaper_size);
shaper_size = shaper_lut != NULL ? shaper_size : 0;
+ shaper_tf = dm_plane_state->shaper_tf;
- ret = amdgpu_dm_atomic_shaper_lut(shaper_lut, shaper_size,
+ ret = amdgpu_dm_atomic_shaper_lut(shaper_lut, false,
+ amdgpu_tf_to_dc_tf(shaper_tf),
+ shaper_size,
dc_plane_state->in_shaper_func);
if (ret)
drm_dbg_kms(plane_state->plane->dev,
--
2.40.1
The next patch adds pre-blending degamma to AMD color mgmt pipeline, but
pre-blending degamma caps (DPP) is currently in use to provide DRM CRTC
atomic degamma or implict degamma on legacy gamma. Detach degamma usage
regarging CRTC color properties to manage plane and CRTC color
correction combinations.
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 59 +++++++++++++------
1 file changed, 41 insertions(+), 18 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 68e9f2c62f2e..74eb02655d96 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
@@ -764,20 +764,9 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
return 0;
}
-/**
- * amdgpu_dm_update_plane_color_mgmt: Maps DRM color management to DC plane.
- * @crtc: amdgpu_dm crtc state
- * @dc_plane_state: target DC surface
- *
- * Update the underlying dc_stream_state's input transfer function (ITF) in
- * preparation for hardware commit. The transfer function used depends on
- * the preparation done on the stream for color management.
- *
- * Returns:
- * 0 on success. -ENOMEM if mem allocation fails.
- */
-int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
- struct dc_plane_state *dc_plane_state)
+static int
+map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
+ struct dc_plane_state *dc_plane_state)
{
const struct drm_color_lut *degamma_lut;
enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_SRGB;
@@ -800,8 +789,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
°amma_size);
ASSERT(degamma_size == MAX_COLOR_LUT_ENTRIES);
- dc_plane_state->in_transfer_func->type =
- TF_TYPE_DISTRIBUTED_POINTS;
+ dc_plane_state->in_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
/*
* This case isn't fully correct, but also fairly
@@ -837,7 +825,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
degamma_lut, degamma_size);
if (r)
return r;
- } else if (crtc->cm_is_degamma_srgb) {
+ } else {
/*
* For legacy gamma support we need the regamma input
* in linear space. Assume that the input is sRGB.
@@ -847,8 +835,43 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
if (tf != TRANSFER_FUNCTION_SRGB &&
!mod_color_calculate_degamma_params(NULL,
- dc_plane_state->in_transfer_func, NULL, false))
+ dc_plane_state->in_transfer_func,
+ NULL, false))
return -ENOMEM;
+ }
+
+ return 0;
+}
+
+/**
+ * amdgpu_dm_update_plane_color_mgmt: Maps DRM color management to DC plane.
+ * @crtc: amdgpu_dm crtc state
+ * @dc_plane_state: target DC surface
+ *
+ * Update the underlying dc_stream_state's input transfer function (ITF) in
+ * preparation for hardware commit. The transfer function used depends on
+ * the preparation done on the stream for color management.
+ *
+ * Returns:
+ * 0 on success. -ENOMEM if mem allocation fails.
+ */
+int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
+ struct dc_plane_state *dc_plane_state)
+{
+ bool has_crtc_cm_degamma;
+ int ret;
+
+ has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
+ if (has_crtc_cm_degamma){
+ /* AMD HW doesn't have post-blending degamma caps. When DRM
+ * CRTC atomic degamma is set, we maps it to DPP degamma block
+ * (pre-blending) or, on legacy gamma, we use DPP degamma to
+ * linearize (implicit degamma) from sRGB/BT709 according to
+ * the input space.
+ */
+ ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state);
+ if (ret)
+ return ret;
} else {
/* ...Otherwise we can just bypass the DGM block. */
dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
--
2.40.1
Hook up driver-specific atomic operations for managing AMD color
properties. Create AMD driver-specific color management properties
and attach them according to HW capabilities defined by `struct
dc_color_caps`.
First add plane degamma LUT properties that means user-blob and its
size. We will add more plane color properties in the next patches. In
addition, we define AMD_PRIVATE_COLOR to guard these driver-specific
plane properties.
Plane degamma can be used to linearize input space for arithmetical
operations that are more accurate when applied in linear color.
v2:
- update degamma LUT prop description
- move private color operations from amdgpu_display to amdgpu_dm_color
Co-developed-by: Joshua Ashton <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 11 +++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 24 ++++++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 81 +++++++++++++++++++
5 files changed, 132 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 32fe05c810c6..ec4621deac8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -343,6 +343,17 @@ struct amdgpu_mode_info {
int disp_priority;
const struct amdgpu_display_funcs *funcs;
const enum drm_plane_type *plane_type;
+
+ /* Driver-private color mgmt props */
+
+ /* @plane_degamma_lut_property: Plane property to set a degamma LUT to
+ * convert input space before blending.
+ */
+ struct drm_property *plane_degamma_lut_property;
+ /* @plane_degamma_lut_size_property: Plane property to define the max
+ * size of degamma LUT as supported by the driver (read-only).
+ */
+ struct drm_property *plane_degamma_lut_size_property;
};
#define AMDGPU_MAX_BL_LEVEL 0xFF
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dffa584410a5..77b4d671a9e0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3949,6 +3949,11 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
return r;
}
+#ifdef AMD_PRIVATE_COLOR
+ if (amdgpu_dm_create_color_properties(adev))
+ return -ENOMEM;
+#endif
+
r = amdgpu_dm_audio_init(adev);
if (r) {
dc_release_state(state->context);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index a2d34be82613..f0343bbf0fe1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -719,6 +719,16 @@ extern const struct amdgpu_ip_block_version dm_ip_block;
struct dm_plane_state {
struct drm_plane_state base;
struct dc_plane_state *dc_state;
+
+ /* Plane color mgmt */
+ /**
+ * @degamma_lut:
+ *
+ * 1D LUT for mapping framebuffer/plane pixel data before sampling or
+ * blending operations. It's usually applied to linearize input space.
+ * The blob (if not NULL) is an array of &struct drm_color_lut.
+ */
+ struct drm_property_blob *degamma_lut;
};
struct dm_crtc_state {
@@ -809,6 +819,7 @@ void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
#define MAX_COLOR_LEGACY_LUT_ENTRIES 256
void amdgpu_dm_init_color_mod(void);
+int amdgpu_dm_create_color_properties(struct amdgpu_device *adev);
int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
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 a4cb23d059bd..cf175b86ba80 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
@@ -84,6 +84,30 @@ void amdgpu_dm_init_color_mod(void)
setup_x_points_distribution();
}
+#ifdef AMD_PRIVATE_COLOR
+int
+amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create(adev_to_drm(adev),
+ DRM_MODE_PROP_BLOB,
+ "AMD_PLANE_DEGAMMA_LUT", 0);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_degamma_lut_property = prop;
+
+ prop = drm_property_create_range(adev_to_drm(adev),
+ DRM_MODE_PROP_IMMUTABLE,
+ "AMD_PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_degamma_lut_size_property = prop;
+
+ return 0;
+}
+#endif
+
/**
* __extract_blob_lut - Extracts the DRM lut and lut size from a blob.
* @blob: DRM color mgmt property blob
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 8eeca160d434..488012d1558d 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
@@ -1347,6 +1347,9 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
dc_plane_state_retain(dm_plane_state->dc_state);
}
+ if (dm_plane_state->degamma_lut)
+ drm_property_blob_get(dm_plane_state->degamma_lut);
+
return &dm_plane_state->base;
}
@@ -1414,12 +1417,83 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
{
struct dm_plane_state *dm_plane_state = to_dm_plane_state(state);
+ if (dm_plane_state->degamma_lut)
+ drm_property_blob_put(dm_plane_state->degamma_lut);
+
if (dm_plane_state->dc_state)
dc_plane_state_release(dm_plane_state->dc_state);
drm_atomic_helper_plane_destroy_state(plane, state);
}
+#ifdef AMD_PRIVATE_COLOR
+static void
+dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
+ struct drm_plane *plane)
+{
+ struct amdgpu_mode_info mode_info = dm->adev->mode_info;
+ struct dpp_color_caps dpp_color_caps = dm->dc->caps.color.dpp;
+
+ /* Check HW color pipeline capabilities for DPP (pre-blending) before expose*/
+ if (dpp_color_caps.dgam_ram || dpp_color_caps.gamma_corr) {
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_degamma_lut_property, 0);
+ drm_object_attach_property(&plane->base,
+ mode_info.plane_degamma_lut_size_property,
+ MAX_COLOR_LUT_ENTRIES);
+ }
+}
+
+static int
+dm_atomic_plane_set_property(struct drm_plane *plane,
+ struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t val)
+{
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(state);
+ struct amdgpu_device *adev = drm_to_adev(plane->dev);
+ bool replaced = false;
+ int ret;
+
+ if (property == adev->mode_info.plane_degamma_lut_property) {
+ ret = drm_property_replace_blob_from_id(plane->dev,
+ &dm_plane_state->degamma_lut,
+ val,
+ -1, sizeof(struct drm_color_lut),
+ &replaced);
+ dm_plane_state->base.color_mgmt_changed |= replaced;
+ return ret;
+ } else {
+ drm_dbg_atomic(plane->dev,
+ "[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
+ plane->base.id, plane->name,
+ property->base.id, property->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int
+dm_atomic_plane_get_property(struct drm_plane *plane,
+ const struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t *val)
+{
+ struct dm_plane_state *dm_plane_state = to_dm_plane_state(state);
+ struct amdgpu_device *adev = drm_to_adev(plane->dev);
+
+ if (property == adev->mode_info.plane_degamma_lut_property) {
+ *val = (dm_plane_state->degamma_lut) ?
+ dm_plane_state->degamma_lut->base.id : 0;
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+#endif
+
static const struct drm_plane_funcs dm_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -1428,6 +1502,10 @@ static const struct drm_plane_funcs dm_plane_funcs = {
.atomic_duplicate_state = dm_drm_plane_duplicate_state,
.atomic_destroy_state = dm_drm_plane_destroy_state,
.format_mod_supported = dm_plane_format_mod_supported,
+#ifdef AMD_PRIVATE_COLOR
+ .atomic_set_property = dm_atomic_plane_set_property,
+ .atomic_get_property = dm_atomic_plane_get_property,
+#endif
};
int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
@@ -1498,6 +1576,9 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
drm_plane_helper_add(plane, &dm_plane_helper_funcs);
+#ifdef AMD_PRIVATE_COLOR
+ dm_atomic_plane_attach_color_mgmt_properties(dm, plane);
+#endif
/* Create (reset) the plane state */
if (plane->funcs->reset)
plane->funcs->reset(plane);
--
2.40.1
Describe some expected behavior of the AMD DM color mgmt programming.
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
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 841e0391f7fb..0a9aa162d4a0 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
@@ -640,12 +640,23 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB;
+ /* Note: although we pass has_rom as parameter here, we never
+ * actually use ROM because the color module only takes the ROM
+ * path if transfer_func->type == PREDEFINED.
+ *
+ * See more in mod_color_calculate_regamma_params()
+ */
r = __set_legacy_tf(stream->out_transfer_func, regamma_lut,
regamma_size, has_rom);
if (r)
return r;
} else if (has_regamma) {
- /* If atomic regamma, CRTC RGM goes into RGM LUT. */
+ /* CRTC RGM goes into RGM LUT.
+ *
+ * Note: there is no implicit sRGB regamma here. We are using
+ * degamma calculation from color module to calculate the curve
+ * from a linear base.
+ */
stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
--
2.40.1
Brief documentation about pre-defined transfer function usage on AMD
display driver and standardized EOTFs and inverse EOTFs.
Co-developed-by: Harry Wentland <[email protected]>
Signed-off-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 39 +++++++++++++++++++
1 file changed, 39 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 cc2187c0879a..7f13bcdaf016 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
@@ -85,6 +85,45 @@ void amdgpu_dm_init_color_mod(void)
}
#ifdef AMD_PRIVATE_COLOR
+/* Pre-defined Transfer Functions (TF)
+ *
+ * AMD driver supports pre-defined mathematical functions for transferring
+ * between encoded values and optical/linear space. Depending on HW color caps,
+ * ROMs and curves built by the AMD color module support these transforms.
+ *
+ * The driver-specific color implementation exposes properties for pre-blending
+ * degamma TF, shaper TF (before 3D LUT), and blend(dpp.ogam) TF and
+ * post-blending regamma (mpc.ogam) TF. However, only pre-blending degamma
+ * supports ROM curves. AMD color module uses pre-defined coefficients to build
+ * curves for the other blocks. What can be done by each color block is
+ * described by struct dpp_color_capsand struct mpc_color_caps.
+ *
+ * AMD driver-specific color API exposes the following pre-defined transfer
+ * functions:
+ *
+ * - Linear/Unity: linear/identity relationship between pixel value and
+ * luminance value;
+ * - Gamma 2.2, Gamma 2.4, Gamma 2.6: pure gamma functions;
+ * - sRGB: 2.4 gamma with small initial linear section as standardized by IEC
+ * 61966-2-1:1999;
+ * - BT.709 (BT.1886): 2.4 gamma with differences in the dark end of the scale.
+ * Used in HD-TV and standardized by ITU-R BT.1886;
+ * - PQ (Perceptual Quantizer): used for HDR display, allows luminance range
+ * capability of 0 to 10,000 nits; standardized by SMPTE ST 2084.
+ *
+ * In the driver-specific API, color block names attached to TF properties
+ * suggest the intention regarding non-linear encoding pixel's luminance
+ * values. As some newer encodings don't use gamma curve, we make encoding and
+ * decoding explicit by defining an enum list of transfer functions supported
+ * in terms of EOTF and inverse EOTF, where:
+ *
+ * - EOTF (electro-optical transfer function): is the transfer function to go
+ * from the encoded value to an optical (linear) value. De-gamma functions
+ * traditionally do this.
+ * - Inverse EOTF (simply the inverse of the EOTF): is usually intended to go
+ * from an optical/linear space (which might have been used for blending)
+ * back to the encoded values. Gamma functions traditionally do this.
+ */
static const char * const
amdgpu_transfer_function_names[] = {
[AMDGPU_TRANSFER_FUNCTION_DEFAULT] = "Default",
--
2.40.1
From: Joshua Ashton <[email protected]>
Signed-off-by: Joshua Ashton <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 32 +++++++++++++++++--
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 2 +-
include/uapi/drm/drm_mode.h | 8 +++++
3 files changed, 38 insertions(+), 4 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 7ff329101fd4..0a51af44efd5 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
@@ -412,6 +412,32 @@ static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm,
}
}
+/**
+ * __drm_ctm2_to_dc_matrix - converts a DRM CTM2 to a DC CSC float matrix
+ * @ctm: DRM color transformation matrix
+ * @matrix: DC CSC float matrix
+ *
+ * The matrix needs to be a 3x4 (12 entry) matrix.
+ */
+static void __drm_ctm2_to_dc_matrix(const struct drm_color_ctm2 *ctm,
+ struct fixed31_32 *matrix)
+{
+ int i;
+
+ /*
+ * DRM gives a 3x3 matrix, but DC wants 3x4. Assuming we're operating
+ * with homogeneous coordinates, augment the matrix with 0's.
+ *
+ * The format provided is S31.32, using signed-magnitude representation.
+ * Our fixed31_32 is also S31.32, but is using 2's complement. We have
+ * to convert from signed-magnitude to 2's complement.
+ */
+ for (i = 0; i < 12; i++) {
+ /* gamut_remap_matrix[i] = ctm[i - floor(i/4)] */
+ matrix[i] = dc_fixpt_from_s3132(ctm->matrix[i]);
+ }
+}
+
/**
* __set_legacy_tf - Calculates the legacy transfer function
* @func: transfer function
@@ -1159,7 +1185,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
{
struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
- struct drm_color_ctm *ctm = NULL;
+ struct drm_color_ctm2 *ctm = NULL;
struct dc_color_caps *color_caps = NULL;
bool has_crtc_cm_degamma;
int ret;
@@ -1213,7 +1239,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
/* Setup CRTC CTM. */
if (dm_plane_state->ctm) {
- ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
+ ctm = (struct drm_color_ctm2 *)dm_plane_state->ctm->data;
/*
* So far, if we have both plane and CRTC CTM, plane CTM takes
@@ -1224,7 +1250,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
* provide support for both DPP and MPC matrix at the same
* time.
*/
- __drm_ctm_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
+ __drm_ctm2_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
dc_plane_state->gamut_remap_matrix.enable_remap = true;
dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
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 0b1081c690cb..27962a3d30f5 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
@@ -1543,7 +1543,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
ret = drm_property_replace_blob_from_id(plane->dev,
&dm_plane_state->ctm,
val,
- sizeof(struct drm_color_ctm), -1,
+ sizeof(struct drm_color_ctm2), -1,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 46becedf5b2f..402288133e4c 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -838,6 +838,14 @@ struct drm_color_ctm {
__u64 matrix[9];
};
+struct drm_color_ctm2 {
+ /*
+ * Conversion matrix in S31.32 sign-magnitude
+ * (not two's complement!) format.
+ */
+ __u64 matrix[12];
+};
+
struct drm_color_lut {
/*
* Values are mapped linearly to 0.0 - 1.0 range, with 0x0 == 0.0 and
--
2.40.1
Plane CTM for pre-blending color space conversion. Only enable
driver-specific plane CTM property on drivers that support both pre- and
post-blending gamut remap matrix, i.e., DCN3+ family. Otherwise it
conflits with DRM CRTC CTM property.
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 +++++++
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 7 +++++++
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 20 +++++++++++++++++++
4 files changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index abb871a912d7..84bf501b02f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -363,6 +363,8 @@ struct amdgpu_mode_info {
* @plane_hdr_mult_property:
*/
struct drm_property *plane_hdr_mult_property;
+
+ struct drm_property *plane_ctm_property;
/**
* @shaper_lut_property: Plane property to set pre-blending shaper LUT
* that converts color content before 3D LUT.
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 095f39f04210..6252ee912a63 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -769,6 +769,13 @@ struct dm_plane_state {
* S31.32 sign-magnitude.
*/
__u64 hdr_mult;
+ /**
+ * @ctm:
+ *
+ * Color transformation matrix. See drm_crtc_enable_color_mgmt(). The
+ * blob (if not NULL) is a &struct drm_color_ctm.
+ */
+ struct drm_property_blob *ctm;
/**
* @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
* array of &struct drm_color_lut.
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 4356846a2bce..86a918ab82be 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
@@ -218,6 +218,13 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
return -ENOMEM;
adev->mode_info.plane_hdr_mult_property = prop;
+ prop = drm_property_create(adev_to_drm(adev),
+ DRM_MODE_PROP_BLOB,
+ "AMD_PLANE_CTM", 0);
+ if (!prop)
+ return -ENOMEM;
+ adev->mode_info.plane_ctm_property = prop;
+
prop = drm_property_create(adev_to_drm(adev),
DRM_MODE_PROP_BLOB,
"AMD_PLANE_SHAPER_LUT", 0);
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 3fd57de7c5be..0b1081c690cb 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
@@ -1355,6 +1355,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
if (dm_plane_state->degamma_lut)
drm_property_blob_get(dm_plane_state->degamma_lut);
+ if (dm_plane_state->ctm)
+ drm_property_blob_get(dm_plane_state->ctm);
if (dm_plane_state->shaper_lut)
drm_property_blob_get(dm_plane_state->shaper_lut);
if (dm_plane_state->lut3d)
@@ -1436,6 +1438,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
if (dm_plane_state->degamma_lut)
drm_property_blob_put(dm_plane_state->degamma_lut);
+ if (dm_plane_state->ctm)
+ drm_property_blob_put(dm_plane_state->ctm);
if (dm_plane_state->lut3d)
drm_property_blob_put(dm_plane_state->lut3d);
if (dm_plane_state->shaper_lut)
@@ -1473,6 +1477,11 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
dm->adev->mode_info.plane_hdr_mult_property,
AMDGPU_HDR_MULT_DEFAULT);
+ /* Only enable plane CTM if both DPP and MPC gamut remap is available. */
+ if (dm->dc->caps.color.mpc.gamut_remap)
+ drm_object_attach_property(&plane->base,
+ dm->adev->mode_info.plane_ctm_property, 0);
+
if (dpp_color_caps.hw_3d_lut) {
drm_object_attach_property(&plane->base,
mode_info.plane_shaper_lut_property, 0);
@@ -1530,6 +1539,14 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
dm_plane_state->hdr_mult = val;
dm_plane_state->base.color_mgmt_changed = 1;
}
+ } else if (property == adev->mode_info.plane_ctm_property) {
+ ret = drm_property_replace_blob_from_id(plane->dev,
+ &dm_plane_state->ctm,
+ val,
+ sizeof(struct drm_color_ctm), -1,
+ &replaced);
+ dm_plane_state->base.color_mgmt_changed |= replaced;
+ return ret;
} else if (property == adev->mode_info.plane_shaper_lut_property) {
ret = drm_property_replace_blob_from_id(plane->dev,
&dm_plane_state->shaper_lut,
@@ -1591,6 +1608,9 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
*val = dm_plane_state->degamma_tf;
} else if (property == adev->mode_info.plane_hdr_mult_property) {
*val = dm_plane_state->hdr_mult;
+ } else if (property == adev->mode_info.plane_ctm_property) {
+ *val = (dm_plane_state->ctm) ?
+ dm_plane_state->ctm->base.id : 0;
} else if (property == adev->mode_info.plane_shaper_lut_property) {
*val = (dm_plane_state->shaper_lut) ?
dm_plane_state->shaper_lut->base.id : 0;
--
2.40.1
On 08/22, Pekka Paalanen wrote:
> On Thu, 10 Aug 2023 15:02:59 -0100
> Melissa Wen <[email protected]> wrote:
>
> > The next patch adds pre-blending degamma to AMD color mgmt pipeline, but
> > pre-blending degamma caps (DPP) is currently in use to provide DRM CRTC
> > atomic degamma or implict degamma on legacy gamma. Detach degamma usage
> > regarging CRTC color properties to manage plane and CRTC color
> > correction combinations.
> >
> > Reviewed-by: Harry Wentland <[email protected]>
> > Signed-off-by: Melissa Wen <[email protected]>
> > ---
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 59 +++++++++++++------
> > 1 file changed, 41 insertions(+), 18 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 68e9f2c62f2e..74eb02655d96 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
> > @@ -764,20 +764,9 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
> > return 0;
> > }
> >
> > -/**
> > - * amdgpu_dm_update_plane_color_mgmt: Maps DRM color management to DC plane.
> > - * @crtc: amdgpu_dm crtc state
> > - * @dc_plane_state: target DC surface
> > - *
> > - * Update the underlying dc_stream_state's input transfer function (ITF) in
> > - * preparation for hardware commit. The transfer function used depends on
> > - * the preparation done on the stream for color management.
> > - *
> > - * Returns:
> > - * 0 on success. -ENOMEM if mem allocation fails.
> > - */
> > -int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > - struct dc_plane_state *dc_plane_state)
> > +static int
> > +map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
> > + struct dc_plane_state *dc_plane_state)
> > {
> > const struct drm_color_lut *degamma_lut;
> > enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_SRGB;
> > @@ -800,8 +789,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > °amma_size);
> > ASSERT(degamma_size == MAX_COLOR_LUT_ENTRIES);
> >
> > - dc_plane_state->in_transfer_func->type =
> > - TF_TYPE_DISTRIBUTED_POINTS;
> > + dc_plane_state->in_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
> >
> > /*
> > * This case isn't fully correct, but also fairly
> > @@ -837,7 +825,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > degamma_lut, degamma_size);
> > if (r)
> > return r;
> > - } else if (crtc->cm_is_degamma_srgb) {
> > + } else {
> > /*
> > * For legacy gamma support we need the regamma input
> > * in linear space. Assume that the input is sRGB.
> > @@ -847,8 +835,43 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> >
> > if (tf != TRANSFER_FUNCTION_SRGB &&
> > !mod_color_calculate_degamma_params(NULL,
> > - dc_plane_state->in_transfer_func, NULL, false))
> > + dc_plane_state->in_transfer_func,
> > + NULL, false))
> > return -ENOMEM;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * amdgpu_dm_update_plane_color_mgmt: Maps DRM color management to DC plane.
> > + * @crtc: amdgpu_dm crtc state
> > + * @dc_plane_state: target DC surface
> > + *
> > + * Update the underlying dc_stream_state's input transfer function (ITF) in
> > + * preparation for hardware commit. The transfer function used depends on
> > + * the preparation done on the stream for color management.
> > + *
> > + * Returns:
> > + * 0 on success. -ENOMEM if mem allocation fails.
> > + */
> > +int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > + struct dc_plane_state *dc_plane_state)
> > +{
> > + bool has_crtc_cm_degamma;
> > + int ret;
> > +
> > + has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
> > + if (has_crtc_cm_degamma){
> > + /* AMD HW doesn't have post-blending degamma caps. When DRM
> > + * CRTC atomic degamma is set, we maps it to DPP degamma block
> > + * (pre-blending) or, on legacy gamma, we use DPP degamma to
> > + * linearize (implicit degamma) from sRGB/BT709 according to
> > + * the input space.
>
> Uhh, you can't just move degamma before blending if KMS userspace
> wants it after blending. That would be incorrect behaviour. If you
> can't implement it correctly, reject it.
>
> I hope that magical unexpected linearization is not done with atomic,
> either.
>
> Or maybe this is all a lost cause, and only the new color-op pipeline
> UAPI will actually work across drivers.
I agree that crtc degamma is an optional property and should be not
exposed if not available. I did something in this line for DCE that has
no degamma block[1]. Then, AMD DDX driver stopped to advertise atomic
API for DCE, that was not correct too[2].
But I see it as a lost cause that will only be fixed in a new generic
color API. I don't think we should change it using the current DRM CRTC
API with driver-specific props.
[1] https://lore.kernel.org/amd-gfx/[email protected]/
[2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/-/issues/67
>
>
> Thanks,
> pq
>
> > + */
> > + ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state);
> > + if (ret)
> > + return ret;
> > } else {
> > /* ...Otherwise we can just bypass the DGM block. */
> > dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
>
On 09/06, Harry Wentland wrote:
> On 2023-08-10 12:02, Melissa Wen wrote:
> > From: Harry Wentland <[email protected]>
> >
> > The region and segment calculation was incapable of dealing
> > with regions of more than 16 segments. We first fix this.
> >
> > Now that we can support regions up to 256 elements we can
> > define a better segment distribution for near-linear LUTs
> > for our maximum of 256 HW-supported points.
> >
> > With these changes an "identity" LUT looks visually
> > indistinguishable from bypass and allows us to use
> > our 3DLUT.
> >
>
> Have you had a chance to test whether this patch makes a
> difference? I haven't had the time yet.
Last time I tested there was a banding issue on plane shaper LUT PQ ->
Display Native, but it seems I don't have this use case on tester
anymore, so I wasn't able to double-check if the issue persist. Maybe
Joshua can provide some inputs here.
Something I noticed is that shaper LUTs are the only 1D LUT on DCN30
pipeline that uses cm_helper_translate_curve_to_hw_format(), all others
(dpp-degamma/dpp-blend/mpc-regamma) call cm3_helper_translate_curve_*.
We can drop it from this series until we get the steps to report the
issue properly.
Melissa
>
> Harry
>
> > Signed-off-by: Harry Wentland <[email protected]>
> > Signed-off-by: Melissa Wen <[email protected]>
> > ---
> > .../amd/display/dc/dcn10/dcn10_cm_common.c | 93 +++++++++++++++----
> > 1 file changed, 75 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> > index 3538973bd0c6..04b2e04b68f3 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> > @@ -349,20 +349,37 @@ bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx,
> > * segment is from 2^-10 to 2^1
> > * There are less than 256 points, for optimization
> > */
> > - seg_distr[0] = 3;
> > - seg_distr[1] = 4;
> > - seg_distr[2] = 4;
> > - seg_distr[3] = 4;
> > - seg_distr[4] = 4;
> > - seg_distr[5] = 4;
> > - seg_distr[6] = 4;
> > - seg_distr[7] = 4;
> > - seg_distr[8] = 4;
> > - seg_distr[9] = 4;
> > - seg_distr[10] = 1;
> > + if (output_tf->tf == TRANSFER_FUNCTION_LINEAR) {
> > + seg_distr[0] = 0; /* 2 */
> > + seg_distr[1] = 1; /* 4 */
> > + seg_distr[2] = 2; /* 4 */
> > + seg_distr[3] = 3; /* 8 */
> > + seg_distr[4] = 4; /* 16 */
> > + seg_distr[5] = 5; /* 32 */
> > + seg_distr[6] = 6; /* 64 */
> > + seg_distr[7] = 7; /* 128 */
> > +
> > + region_start = -8;
> > + region_end = 1;
> > + } else {
> > + seg_distr[0] = 3; /* 8 */
> > + seg_distr[1] = 4; /* 16 */
> > + seg_distr[2] = 4;
> > + seg_distr[3] = 4;
> > + seg_distr[4] = 4;
> > + seg_distr[5] = 4;
> > + seg_distr[6] = 4;
> > + seg_distr[7] = 4;
> > + seg_distr[8] = 4;
> > + seg_distr[9] = 4;
> > + seg_distr[10] = 1; /* 2 */
> > + /* total = 8*16 + 8 + 64 + 2 = */
> > +
> > + region_start = -10;
> > + region_end = 1;
> > + }
> > +
> >
> > - region_start = -10;
> > - region_end = 1;
> > }
> >
> > for (i = region_end - region_start; i < MAX_REGIONS_NUMBER ; i++)
> > @@ -375,16 +392,56 @@ bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx,
> >
> > j = 0;
> > for (k = 0; k < (region_end - region_start); k++) {
> > - increment = NUMBER_SW_SEGMENTS / (1 << seg_distr[k]);
> > + /*
> > + * We're using an ugly-ish hack here. Our HW allows for
> > + * 256 segments per region but SW_SEGMENTS is 16.
> > + * SW_SEGMENTS has some undocumented relationship to
> > + * the number of points in the tf_pts struct, which
> > + * is 512, unlike what's suggested TRANSFER_FUNC_POINTS.
> > + *
> > + * In order to work past this dilemma we'll scale our
> > + * increment by (1 << 4) and then do the inverse (1 >> 4)
> > + * when accessing the elements in tf_pts.
> > + *
> > + * TODO: find a better way using SW_SEGMENTS and
> > + * TRANSFER_FUNC_POINTS definitions
> > + */
> > + increment = (NUMBER_SW_SEGMENTS << 4) / (1 << seg_distr[k]);
> > start_index = (region_start + k + MAX_LOW_POINT) *
> > NUMBER_SW_SEGMENTS;
> > - for (i = start_index; i < start_index + NUMBER_SW_SEGMENTS;
> > + for (i = (start_index << 4); i < (start_index << 4) + (NUMBER_SW_SEGMENTS << 4);
> > i += increment) {
> > + struct fixed31_32 in_plus_one, in;
> > + struct fixed31_32 value, red_value, green_value, blue_value;
> > + uint32_t t = i & 0xf;
> > +
> > if (j == hw_points - 1)
> > break;
> > - rgb_resulted[j].red = output_tf->tf_pts.red[i];
> > - rgb_resulted[j].green = output_tf->tf_pts.green[i];
> > - rgb_resulted[j].blue = output_tf->tf_pts.blue[i];
> > +
> > + in_plus_one = output_tf->tf_pts.red[(i >> 4) + 1];
> > + in = output_tf->tf_pts.red[i >> 4];
> > + value = dc_fixpt_sub(in_plus_one, in);
> > + value = dc_fixpt_shr(dc_fixpt_mul_int(value, t), 4);
> > + value = dc_fixpt_add(in, value);
> > + red_value = value;
> > +
> > + in_plus_one = output_tf->tf_pts.green[(i >> 4) + 1];
> > + in = output_tf->tf_pts.green[i >> 4];
> > + value = dc_fixpt_sub(in_plus_one, in);
> > + value = dc_fixpt_shr(dc_fixpt_mul_int(value, t), 4);
> > + value = dc_fixpt_add(in, value);
> > + green_value = value;
> > +
> > + in_plus_one = output_tf->tf_pts.blue[(i >> 4) + 1];
> > + in = output_tf->tf_pts.blue[i >> 4];
> > + value = dc_fixpt_sub(in_plus_one, in);
> > + value = dc_fixpt_shr(dc_fixpt_mul_int(value, t), 4);
> > + value = dc_fixpt_add(in, value);
> > + blue_value = value;
> > +
> > + rgb_resulted[j].red = red_value;
> > + rgb_resulted[j].green = green_value;
> > + rgb_resulted[j].blue = blue_value;
> > j++;
> > }
> > }
>
On 09/06, Harry Wentland wrote:
> On 2023-08-10 12:02, Melissa Wen wrote:
> > On AMD HW, 3D LUT always assumes a preceding shaper 1D LUT used for
> > delinearizing and/or normalizing the color space before applying a 3D
> > LUT. Add pre-defined transfer function to enable delinearizing content
> > with or without shaper LUT, where AMD color module calculates the
> > resulted shaper curve. We apply an inverse EOTF to go from linear values
> > to encoded values. If we are already in a non-linear space and/or don't
> > need to normalize values, we can bypass shaper LUT with a linear
> > transfer function that is also the default TF value.
> >
>
> I think the color module will combine the TF and the custom 1D LUT
> into the LUT that's actually programmed. We should spell out this
> behavior in the comments below and in the patch description as it's
> important for a userspace application to know.
>
> The same applies to all other TF+LUT blocks.
yeah, you're right, I'll describe it better.
Thanks,
Melissa
>
> Harry
>
> > v2:
> > - squash commits for shaper LUT and shaper TF
> > - define inverse EOTF as supported shaper TFs
> >
> > Signed-off-by: Melissa Wen <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 16 ++++++++++
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +++++++
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 29 +++++++++++++++++
> > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 32 +++++++++++++++++++
> > 4 files changed, 88 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > index 730a88236501..4fb164204ee6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > @@ -363,6 +363,22 @@ struct amdgpu_mode_info {
> > * @plane_hdr_mult_property:
> > */
> > struct drm_property *plane_hdr_mult_property;
> > + /**
> > + * @shaper_lut_property: Plane property to set pre-blending shaper LUT
> > + * that converts color content before 3D LUT.
> > + */
> > + struct drm_property *plane_shaper_lut_property;
> > + /**
> > + * @shaper_lut_size_property: Plane property for the size of
> > + * pre-blending shaper LUT as supported by the driver (read-only).
> > + */
> > + struct drm_property *plane_shaper_lut_size_property;
> > + /**
> > + * @plane_shaper_tf_property: Plane property to set a predefined
> > + * transfer function for pre-blending shaper (before applying 3D LUT)
> > + * with or without LUT.
> > + */
> > + struct drm_property *plane_shaper_tf_property;
> > /**
> > * @plane_lut3d_property: Plane property for gamma correction using a
> > * 3D LUT (pre-blending).
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > index deea90212e31..6b6c2980f0af 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > @@ -769,6 +769,17 @@ struct dm_plane_state {
> > * S31.32 sign-magnitude.
> > */
> > __u64 hdr_mult;
> > + /**
> > + * @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
> > + * array of &struct drm_color_lut.
> > + */
> > + struct drm_property_blob *shaper_lut;
> > + /**
> > + * @shaper_tf:
> > + *
> > + * Predefined transfer function to delinearize color space.
> > + */
> > + enum amdgpu_transfer_function shaper_tf;
> > /**
> > * @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
> > * &struct drm_color_lut.
> > 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 7e6d4df99a0c..fbcee717bf0a 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
> > @@ -151,6 +151,14 @@ static const u32 amdgpu_eotf =
> > BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF) |
> > BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF);
> >
> > +static const u32 amdgpu_inv_eotf =
> > + BIT(AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF) |
> > + BIT(AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF) |
> > + BIT(AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF) |
> > + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF) |
> > + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF) |
> > + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF);
> > +
> > static struct drm_property *
> > amdgpu_create_tf_property(struct drm_device *dev,
> > const char *name,
> > @@ -209,6 +217,27 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
> > return -ENOMEM;
> > adev->mode_info.plane_hdr_mult_property = prop;
> >
> > + prop = drm_property_create(adev_to_drm(adev),
> > + DRM_MODE_PROP_BLOB,
> > + "AMD_PLANE_SHAPER_LUT", 0);
> > + if (!prop)
> > + return -ENOMEM;
> > + adev->mode_info.plane_shaper_lut_property = prop;
> > +
> > + prop = drm_property_create_range(adev_to_drm(adev),
> > + DRM_MODE_PROP_IMMUTABLE,
> > + "AMD_PLANE_SHAPER_LUT_SIZE", 0, UINT_MAX);
> > + if (!prop)
> > + return -ENOMEM;
> > + adev->mode_info.plane_shaper_lut_size_property = prop;
> > +
> > + prop = amdgpu_create_tf_property(adev_to_drm(adev),
> > + "AMD_PLANE_SHAPER_TF",
> > + amdgpu_inv_eotf);
> > + if (!prop)
> > + return -ENOMEM;
> > + adev->mode_info.plane_shaper_tf_property = prop;
> > +
> > prop = drm_property_create(adev_to_drm(adev),
> > DRM_MODE_PROP_BLOB,
> > "AMD_PLANE_LUT3D", 0);
> > 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 882391f7add6..8d6ddf19bb87 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
> > @@ -1332,6 +1332,7 @@ static void dm_drm_plane_reset(struct drm_plane *plane)
> > __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base);
> > amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
> > amdgpu_state->hdr_mult = AMDGPU_HDR_MULT_DEFAULT;
> > + amdgpu_state->shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
> > }
> >
> > static struct drm_plane_state *
> > @@ -1353,11 +1354,14 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
> >
> > if (dm_plane_state->degamma_lut)
> > drm_property_blob_get(dm_plane_state->degamma_lut);
> > + if (dm_plane_state->shaper_lut)
> > + drm_property_blob_get(dm_plane_state->shaper_lut);
> > if (dm_plane_state->lut3d)
> > drm_property_blob_get(dm_plane_state->lut3d);
> >
> > dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
> > dm_plane_state->hdr_mult = old_dm_plane_state->hdr_mult;
> > + dm_plane_state->shaper_tf = old_dm_plane_state->shaper_tf;
> >
> > return &dm_plane_state->base;
> > }
> > @@ -1430,6 +1434,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
> > drm_property_blob_put(dm_plane_state->degamma_lut);
> > if (dm_plane_state->lut3d)
> > drm_property_blob_put(dm_plane_state->lut3d);
> > + if (dm_plane_state->shaper_lut)
> > + drm_property_blob_put(dm_plane_state->shaper_lut);
> >
> > if (dm_plane_state->dc_state)
> > dc_plane_state_release(dm_plane_state->dc_state);
> > @@ -1462,6 +1468,14 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
> > AMDGPU_HDR_MULT_DEFAULT);
> >
> > if (dpp_color_caps.hw_3d_lut) {
> > + drm_object_attach_property(&plane->base,
> > + mode_info.plane_shaper_lut_property, 0);
> > + drm_object_attach_property(&plane->base,
> > + mode_info.plane_shaper_lut_size_property,
> > + MAX_COLOR_LUT_ENTRIES);
> > + drm_object_attach_property(&plane->base,
> > + mode_info.plane_shaper_tf_property,
> > + AMDGPU_TRANSFER_FUNCTION_DEFAULT);
> > drm_object_attach_property(&plane->base,
> > mode_info.plane_lut3d_property, 0);
> > drm_object_attach_property(&plane->base,
> > @@ -1499,6 +1513,19 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
> > dm_plane_state->hdr_mult = val;
> > dm_plane_state->base.color_mgmt_changed = 1;
> > }
> > + } else if (property == adev->mode_info.plane_shaper_lut_property) {
> > + ret = drm_property_replace_blob_from_id(plane->dev,
> > + &dm_plane_state->shaper_lut,
> > + val, -1,
> > + sizeof(struct drm_color_lut),
> > + &replaced);
> > + dm_plane_state->base.color_mgmt_changed |= replaced;
> > + return ret;
> > + } else if (property == adev->mode_info.plane_shaper_tf_property) {
> > + if (dm_plane_state->shaper_tf != val) {
> > + dm_plane_state->shaper_tf = val;
> > + dm_plane_state->base.color_mgmt_changed = 1;
> > + }
> > } else if (property == adev->mode_info.plane_lut3d_property) {
> > ret = drm_property_replace_blob_from_id(plane->dev,
> > &dm_plane_state->lut3d,
> > @@ -1534,6 +1561,11 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
> > *val = dm_plane_state->degamma_tf;
> > } else if (property == adev->mode_info.plane_hdr_mult_property) {
> > *val = dm_plane_state->hdr_mult;
> > + } else if (property == adev->mode_info.plane_shaper_lut_property) {
> > + *val = (dm_plane_state->shaper_lut) ?
> > + dm_plane_state->shaper_lut->base.id : 0;
> > + } else if (property == adev->mode_info.plane_shaper_tf_property) {
> > + *val = dm_plane_state->shaper_tf;
> > } else if (property == adev->mode_info.plane_lut3d_property) {
> > *val = (dm_plane_state->lut3d) ?
> > dm_plane_state->lut3d->base.id : 0;
>
On 09/07, Pekka Paalanen wrote:
> On Wed, 6 Sep 2023 15:30:04 -0400
> Harry Wentland <[email protected]> wrote:
>
> > On 2023-08-10 12:02, Melissa Wen wrote:
> > > Add 3D LUT property for plane gamma correction using a 3D lookup table.
> > > Since a 3D LUT has a limited number of entries in each dimension we want
> > > to use them in an optimal fashion. This means using the 3D LUT in a
> > > colorspace that is optimized for human vision, such as sRGB, PQ, or
> > > another non-linear space. Therefore, userpace may need one 1D LUT
> > > (shaper) before it to delinearize content and another 1D LUT after 3D
> > > LUT (blend) to linearize content again for blending. The next patches
> > > add these 1D LUTs to the plane color mgmt pipeline.
> > >
> > > Signed-off-by: Melissa Wen <[email protected]>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 10 ++++++++
> > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 9 ++++++++
> > > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 14 +++++++++++
> > > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 23 +++++++++++++++++++
> > > 4 files changed, 56 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > > index 66bae0eed80c..730a88236501 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > > @@ -363,6 +363,16 @@ struct amdgpu_mode_info {
> > > * @plane_hdr_mult_property:
> > > */
> > > struct drm_property *plane_hdr_mult_property;
> > > + /**
> > > + * @plane_lut3d_property: Plane property for gamma correction using a
> > > + * 3D LUT (pre-blending).
> > > + */
> >
> > I think we'll want to describe how the 3DLUT entries are laid out.
> > Something that describes how userspace should fill it, like
> > gamescope does for example:
> > https://github.com/ValveSoftware/gamescope/blob/7108880ed80b68c21750369e2ac9b7315fecf264/src/color_helpers.cpp#L302
> >
> > Something like: a three-dimensional array, with each dimension
> > having a size of the cubed root of lut3d_size, blue being the
> > outermost dimension, red the innermost.
> >
>
> Here is an example of how we defined a 3D LUT layout in Weston:
>
> https://gitlab.freedesktop.org/wayland/weston/-/blob/68e2a606c056c8453c770263f41f34cd68bdc9d0/libweston/color.h#L114-152
>
> I think that is the most clear definition it can be, without needing to
> understand specific terminology.
Thanks for sharing it, Pekka! I'll check Weston's definition for 3D LUT
and also other elements for improving docs.
Melissa
>
>
> Thanks,
> pq
>
> >
> > > + struct drm_property *plane_lut3d_property;
> > > + /**
> > > + * @plane_degamma_lut_size_property: Plane property to define the max
> > > + * size of 3D LUT as supported by the driver (read-only).
> > > + */
> >
> > We should probably document that the size of the 3DLUT should
> > be the size of one dimension cubed, or that the cubed root of
> > the LUT size gives the size per dimension.
> >
> > Harry
On 09/06, Harry Wentland wrote:
>
>
> On 2023-08-10 12:03, Melissa Wen wrote:
> > From: Joshua Ashton <[email protected]>
> >
> > Need to funnel the color caps through to these functions so it can check
> > that the hardware is capable.
> >
> > v2:
> > - remove redundant color caps assignment on plane degamma map (Harry)
> > - pass color caps to degamma params
> >
> > Signed-off-by: Joshua Ashton <[email protected]>
> > Signed-off-by: Melissa Wen <[email protected]>
> > ---
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 35 ++++++++++++-------
> > 1 file changed, 22 insertions(+), 13 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 f638e5b3a70b..4356846a2bce 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
> > @@ -538,6 +538,7 @@ static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
> > /**
> > * __set_input_tf - calculates the input transfer function based on expected
> > * input space.
> > + * @caps: dc color capabilities
> > * @func: transfer function
> > * @lut: lookup table that defines the color space
> > * @lut_size: size of respective lut.
> > @@ -545,7 +546,7 @@ static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
> > * Returns:
> > * 0 in case of success. -ENOMEM if fails.
> > */
> > -static int __set_input_tf(struct dc_transfer_func *func,
> > +static int __set_input_tf(struct dc_color_caps *caps, struct dc_transfer_func *func,
> > const struct drm_color_lut *lut, uint32_t lut_size)
> > {
> > struct dc_gamma *gamma = NULL;
> > @@ -562,7 +563,7 @@ static int __set_input_tf(struct dc_transfer_func *func,
> > __drm_lut_to_dc_gamma(lut, gamma, false);
> > }
> >
> > - res = mod_color_calculate_degamma_params(NULL, func, gamma, gamma != NULL);
> > + res = mod_color_calculate_degamma_params(caps, func, gamma, gamma != NULL);
> >
> > if (gamma)
> > dc_gamma_release(&gamma);
> > @@ -725,7 +726,7 @@ static int amdgpu_dm_atomic_blend_lut(const struct drm_color_lut *blend_lut,
> > func_blend->tf = tf;
> > func_blend->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
> >
> > - ret = __set_input_tf(func_blend, blend_lut, blend_size);
> > + ret = __set_input_tf(NULL, func_blend, blend_lut, blend_size);
> > } else {
> > func_blend->type = TF_TYPE_BYPASS;
> > func_blend->tf = TRANSFER_FUNCTION_LINEAR;
> > @@ -950,7 +951,8 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
> >
> > static int
> > map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
> > - struct dc_plane_state *dc_plane_state)
> > + struct dc_plane_state *dc_plane_state,
> > + struct dc_color_caps *caps)
> > {
> > const struct drm_color_lut *degamma_lut;
> > enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_SRGB;
> > @@ -1005,7 +1007,7 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
> > dc_plane_state->in_transfer_func->tf =
> > TRANSFER_FUNCTION_LINEAR;
> >
> > - r = __set_input_tf(dc_plane_state->in_transfer_func,
> > + r = __set_input_tf(caps, dc_plane_state->in_transfer_func,
> > degamma_lut, degamma_size);
> > if (r)
> > return r;
> > @@ -1018,7 +1020,7 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
> > dc_plane_state->in_transfer_func->tf = tf;
> >
> > if (tf != TRANSFER_FUNCTION_SRGB &&
> > - !mod_color_calculate_degamma_params(NULL,
> > + !mod_color_calculate_degamma_params(caps,
> > dc_plane_state->in_transfer_func,
> > NULL, false))
> > return -ENOMEM;
> > @@ -1029,7 +1031,8 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
> >
> > static int
> > __set_dm_plane_degamma(struct drm_plane_state *plane_state,
> > - struct dc_plane_state *dc_plane_state)
> > + struct dc_plane_state *dc_plane_state,
> > + struct dc_color_caps *color_caps)
> > {
> > struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> > const struct drm_color_lut *degamma_lut;
> > @@ -1060,7 +1063,7 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
> > dc_plane_state->in_transfer_func->type =
> > TF_TYPE_DISTRIBUTED_POINTS;
> >
> > - ret = __set_input_tf(dc_plane_state->in_transfer_func,
> > + ret = __set_input_tf(color_caps, dc_plane_state->in_transfer_func,
> > degamma_lut, degamma_size);
> > if (ret)
> > return ret;
> > @@ -1068,7 +1071,7 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
> > dc_plane_state->in_transfer_func->type =
> > TF_TYPE_PREDEFINED;
> >
> > - if (!mod_color_calculate_degamma_params(NULL,
> > + if (!mod_color_calculate_degamma_params(color_caps,
> > dc_plane_state->in_transfer_func, NULL, false))
> > return -ENOMEM;
> > }
> > @@ -1077,7 +1080,8 @@ __set_dm_plane_degamma(struct drm_plane_state *plane_state,
> >
> > static int
> > amdgpu_dm_plane_set_color_properties(struct drm_plane_state *plane_state,
> > - struct dc_plane_state *dc_plane_state)
> > + struct dc_plane_state *dc_plane_state,
> > + struct dc_color_caps *color_caps)
>
> This seems unused.
Nice catch, it isn't. I'll remove in the next version.
Melissa
>
> Harry
>
> > {
> > struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> > enum amdgpu_transfer_function shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
> > @@ -1147,6 +1151,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > struct dc_plane_state *dc_plane_state)
> > {
> > struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
> > + struct dc_color_caps *color_caps = NULL;
> > bool has_crtc_cm_degamma;
> > int ret;
> >
> > @@ -1156,6 +1161,9 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > return ret;
> > }
> >
> > + if (dc_plane_state->ctx && dc_plane_state->ctx->dc)
> > + color_caps = &dc_plane_state->ctx->dc->caps.color;
> > +
> > /* Initially, we can just bypass the DGM block. */
> > dc_plane_state->in_transfer_func->type = TF_TYPE_BYPASS;
> > dc_plane_state->in_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
> > @@ -1163,7 +1171,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > /* After, we start to update values according to color props */
> > has_crtc_cm_degamma = (crtc->cm_has_degamma || crtc->cm_is_degamma_srgb);
> >
> > - ret = __set_dm_plane_degamma(plane_state, dc_plane_state);
> > + ret = __set_dm_plane_degamma(plane_state, dc_plane_state, color_caps);
> > if (ret == -ENOMEM)
> > return ret;
> >
> > @@ -1189,10 +1197,11 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > * linearize (implicit degamma) from sRGB/BT709 according to
> > * the input space.
> > */
> > - ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state);
> > + ret = map_crtc_degamma_to_dc_plane(crtc, dc_plane_state, color_caps);
> > if (ret)
> > return ret;
> > }
> >
> > - return amdgpu_dm_plane_set_color_properties(plane_state, dc_plane_state);
> > + return amdgpu_dm_plane_set_color_properties(plane_state,
> > + dc_plane_state, color_caps);
> > }
>
On 09/06, Harry Wentland wrote:
>
>
> On 2023-08-10 12:03, Melissa Wen wrote:
> > Plane CTM for pre-blending color space conversion. Only enable
> > driver-specific plane CTM property on drivers that support both pre- and
> > post-blending gamut remap matrix, i.e., DCN3+ family. Otherwise it
> > conflits with DRM CRTC CTM property.
> >
> > Signed-off-by: Melissa Wen <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 ++
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 +++++++
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 7 +++++++
> > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 20 +++++++++++++++++++
> > 4 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > index abb871a912d7..84bf501b02f4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > @@ -363,6 +363,8 @@ struct amdgpu_mode_info {
> > * @plane_hdr_mult_property:
> > */
> > struct drm_property *plane_hdr_mult_property;
> > +
> > + struct drm_property *plane_ctm_property;
> > /**
> > * @shaper_lut_property: Plane property to set pre-blending shaper LUT
> > * that converts color content before 3D LUT.
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > index 095f39f04210..6252ee912a63 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > @@ -769,6 +769,13 @@ struct dm_plane_state {
> > * S31.32 sign-magnitude.
> > */
> > __u64 hdr_mult;
> > + /**
> > + * @ctm:
> > + *
> > + * Color transformation matrix. See drm_crtc_enable_color_mgmt(). The
> > + * blob (if not NULL) is a &struct drm_color_ctm.
> > + */
> > + struct drm_property_blob *ctm;
> > /**
> > * @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
> > * array of &struct drm_color_lut.
> > 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 4356846a2bce..86a918ab82be 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
> > @@ -218,6 +218,13 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
> > return -ENOMEM;
> > adev->mode_info.plane_hdr_mult_property = prop;
> >
> > + prop = drm_property_create(adev_to_drm(adev),
> > + DRM_MODE_PROP_BLOB,
> > + "AMD_PLANE_CTM", 0);
>
> We'll want to wrap the property creation/attachment with
> #ifdef AMD_PRIVATE_COLOR here as well.
yeah, it's already wrapped because it's created and attached together
with the other properties.
>
> Harry
>
> > + if (!prop)
> > + return -ENOMEM;
> > + adev->mode_info.plane_ctm_property = prop;
> > +
> > prop = drm_property_create(adev_to_drm(adev),
> > DRM_MODE_PROP_BLOB,
> > "AMD_PLANE_SHAPER_LUT", 0);
> > 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 3fd57de7c5be..0b1081c690cb 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
> > @@ -1355,6 +1355,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
> >
> > if (dm_plane_state->degamma_lut)
> > drm_property_blob_get(dm_plane_state->degamma_lut);
> > + if (dm_plane_state->ctm)
> > + drm_property_blob_get(dm_plane_state->ctm);
> > if (dm_plane_state->shaper_lut)
> > drm_property_blob_get(dm_plane_state->shaper_lut);
> > if (dm_plane_state->lut3d)
> > @@ -1436,6 +1438,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
> >
> > if (dm_plane_state->degamma_lut)
> > drm_property_blob_put(dm_plane_state->degamma_lut);
> > + if (dm_plane_state->ctm)
> > + drm_property_blob_put(dm_plane_state->ctm);
> > if (dm_plane_state->lut3d)
> > drm_property_blob_put(dm_plane_state->lut3d);
> > if (dm_plane_state->shaper_lut)
> > @@ -1473,6 +1477,11 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
> > dm->adev->mode_info.plane_hdr_mult_property,
> > AMDGPU_HDR_MULT_DEFAULT);
> >
> > + /* Only enable plane CTM if both DPP and MPC gamut remap is available. */
> > + if (dm->dc->caps.color.mpc.gamut_remap)
> > + drm_object_attach_property(&plane->base,
> > + dm->adev->mode_info.plane_ctm_property, 0);
> > +
> > if (dpp_color_caps.hw_3d_lut) {
> > drm_object_attach_property(&plane->base,
> > mode_info.plane_shaper_lut_property, 0);
> > @@ -1530,6 +1539,14 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
> > dm_plane_state->hdr_mult = val;
> > dm_plane_state->base.color_mgmt_changed = 1;
> > }
> > + } else if (property == adev->mode_info.plane_ctm_property) {
> > + ret = drm_property_replace_blob_from_id(plane->dev,
> > + &dm_plane_state->ctm,
> > + val,
> > + sizeof(struct drm_color_ctm), -1,
> > + &replaced);
> > + dm_plane_state->base.color_mgmt_changed |= replaced;
> > + return ret;
> > } else if (property == adev->mode_info.plane_shaper_lut_property) {
> > ret = drm_property_replace_blob_from_id(plane->dev,
> > &dm_plane_state->shaper_lut,
> > @@ -1591,6 +1608,9 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
> > *val = dm_plane_state->degamma_tf;
> > } else if (property == adev->mode_info.plane_hdr_mult_property) {
> > *val = dm_plane_state->hdr_mult;
> > + } else if (property == adev->mode_info.plane_ctm_property) {
> > + *val = (dm_plane_state->ctm) ?
> > + dm_plane_state->ctm->base.id : 0;
> > } else if (property == adev->mode_info.plane_shaper_lut_property) {
> > *val = (dm_plane_state->shaper_lut) ?
> > dm_plane_state->shaper_lut->base.id : 0;
>
On 09/06, Harry Wentland wrote:
>
>
> On 2023-08-25 10:18, Melissa Wen wrote:
> > On 08/22, Pekka Paalanen wrote:
> >> On Thu, 10 Aug 2023 15:02:47 -0100
> >> Melissa Wen <[email protected]> wrote:
> >>
> >>> Instead of relying on color block names to get the transfer function
> >>> intention regarding encoding pixel's luminance, define supported
> >>> Electro-Optical Transfer Functions (EOTFs) and inverse EOTFs, that
> >>> includes pure gamma or standardized transfer functions.
> >>>
> >>> Suggested-by: Harry Wentland <[email protected]>
> >>> Signed-off-by: Melissa Wen <[email protected]>
> >>> ---
> >>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +++--
> >>> .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 69 +++++++++++++++----
> >>> 2 files changed, 67 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> >>> index c749c9cb3d94..f6251ed89684 100644
> >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> >>> @@ -718,14 +718,21 @@ extern const struct amdgpu_ip_block_version dm_ip_block;
> >>>
> >>> enum amdgpu_transfer_function {
> >>> AMDGPU_TRANSFER_FUNCTION_DEFAULT,
> >>> - AMDGPU_TRANSFER_FUNCTION_SRGB,
> >>> - AMDGPU_TRANSFER_FUNCTION_BT709,
> >>> - AMDGPU_TRANSFER_FUNCTION_PQ,
> >>> + AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_BT709_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_PQ_EOTF,
> >>> AMDGPU_TRANSFER_FUNCTION_LINEAR,
> >>> AMDGPU_TRANSFER_FUNCTION_UNITY,
> >>> - AMDGPU_TRANSFER_FUNCTION_GAMMA22,
> >>> - AMDGPU_TRANSFER_FUNCTION_GAMMA24,
> >>> - AMDGPU_TRANSFER_FUNCTION_GAMMA26,
> >>> + AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF,
> >>> + AMDGPU_TRANSFER_FUNCTION_COUNT
> >>> };
> >>>
> >>> struct dm_plane_state {
> >>> 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 56ce008b9095..cc2187c0879a 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
> >>> @@ -85,18 +85,59 @@ void amdgpu_dm_init_color_mod(void)
> >>> }
> >>>
> >>> #ifdef AMD_PRIVATE_COLOR
> >>> -static const struct drm_prop_enum_list amdgpu_transfer_function_enum_list[] = {
> >>> - { AMDGPU_TRANSFER_FUNCTION_DEFAULT, "Default" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_SRGB, "sRGB" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_BT709, "BT.709" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_PQ, "PQ (Perceptual Quantizer)" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_LINEAR, "Linear" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_UNITY, "Unity" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_GAMMA22, "Gamma 2.2" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_GAMMA24, "Gamma 2.4" },
> >>> - { AMDGPU_TRANSFER_FUNCTION_GAMMA26, "Gamma 2.6" },
> >>> +static const char * const
> >>> +amdgpu_transfer_function_names[] = {
> >>> + [AMDGPU_TRANSFER_FUNCTION_DEFAULT] = "Default",
> >>> + [AMDGPU_TRANSFER_FUNCTION_LINEAR] = "Linear",
> >>
> >> Hi,
> >>
> >> if the below is identity, then what is linear? Is there a coefficient
> >> (multiplier) somewhere? Offset?
> >>
> >>> + [AMDGPU_TRANSFER_FUNCTION_UNITY] = "Unity",
> >>
> >> Should "Unity" be called "Identity"?
> >
> > AFAIU, AMD treats Linear and Unity as the same: Identity. So, IIUC,
> > indeed merging both as identity sounds the best approach.
>
> Agreed.
>
> >>
> >> Doesn't unity mean that the output is always 1.0 regardless of input?
> >>
> >>> + [AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF] = "sRGB EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_BT709_EOTF] = "BT.709 EOTF",
> >>
> >> BT.709 says about "Overall opto-electronic transfer characteristics at
> >> source":
> >>
> >> In typical production practice the encoding function of image
> >> sources is adjusted so that the final picture has the desired
> >> look, as viewed on a reference monitor having the reference
> >> decoding function of Recommendation ITU-R BT.1886, in the
> >> reference viewing environment defined in Recommendation ITU-R
> >> BT.2035.
> >>
> >> IOW, typically people tweak the encoding function instead of using
> >> BT.709 OETF as is, which means that inverting the BT.709 OETF produces
> >> something slightly unknown. The note about BT.1886 means that that
> >> something is also not quite how it's supposed to be turned into light.
> >>
> >> Should this enum item be "BT.709 inverse OETF" and respectively below a
> >> "BT.709 OETF"?
> >>
> >> What curve does the hardware actually implement?
> >
> > Hmmmm.. I think I got confused in using OETF here since it's done within
> > a camera. Looking at the coefficients used by AMD color module when not
> > using ROM but build encoding and decoding curves[1] on pre-defined TF
> > setup, I understand it's using OETF parameters for building both sRGB
> > and BT 709:
> >
> > ```
> > /*sRGB 709 2.2 2.4 P3*/
> > static const int32_t numerator01[] = { 31308, 180000, 0, 0, 0};
> > static const int32_t numerator02[] = { 12920, 4500, 0, 0, 0};
> > static const int32_t numerator03[] = { 55, 99, 0, 0, 0};
> > static const int32_t numerator04[] = { 55, 99, 0, 0, 0};
> > static const int32_t numerator05[] = { 2400, 2222, 2200, 2400, 2600};
> > ```
> >
>
> The first column here looks like the sRGB coefficients in Skia:
> https://skia.googlesource.com/skia/+/19936eb1b23fef5187b07fb2e0e67dcf605c0672/include/core/SkColorSpace.h#46
>
> The color module uses the same coefficients to calculate the transform
> to linear space and from linear space. So it would support a TF and its
> inverse.
>
> From what I understand for sRGB and PQ its the EOTF and its inverse.
>
> For BT.709 we should probably call it BT.709 inverse OETF (instead of
> EOTF) and BT.709 OETF (instead of inverse EOTF).
I see. I'll update the transfer function list and docs accordingly.
Thanks
Melissa
>
> While I'm okay to move ahead with these AMD driver-specific properties
> without IGT tests (since they're not enabled and not UABI) we really
> need IGT tests once they become UABI with the Color Pipeline API. And we
> need more than just CRC testing. We'll need to do pixel-by-pixel comparison
> so we can verify that the KMS driver behaves exactly how we expect for a
> large range of values.
>
> Harry
>
> > Then EOTF and inverse EOTF for PQ [2], and OETF and it seems an inverse
> > OETF but called EOTF for HLG[3]. But I'm an external dev, better if
> > Harry can confirm.
> >
> > Thank you for pointing it out.
> >
> > [1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/amd/display/modules/color/color_gamma.c#n55
> > [2] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/amd/display/modules/color/color_gamma.c#n106
> > [3] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/amd/display/modules/color/color_gamma.c#n174
> >
> >>
> >> The others seem fine to me.
> >>
> >>
> >> Thanks,
> >> pq
> >>
> >>> + [AMDGPU_TRANSFER_FUNCTION_PQ_EOTF] = "PQ EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF] = "Gamma 2.2 EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF] = "Gamma 2.4 EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF] = "Gamma 2.6 EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF] = "sRGB inv_EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF] = "BT.709 inv_EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF] = "PQ inv_EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF] = "Gamma 2.2 inv_EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF] = "Gamma 2.4 inv_EOTF",
> >>> + [AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF] = "Gamma 2.6 inv_EOTF",
> >>> };
> >>>
> >>> +static const u32 amdgpu_eotf =
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_BT709_EOTF) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_PQ_EOTF) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF);
> >>> +
> >>> +static struct drm_property *
> >>> +amdgpu_create_tf_property(struct drm_device *dev,
> >>> + const char *name,
> >>> + u32 supported_tf)
> >>> +{
> >>> + u32 transfer_functions = supported_tf |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_DEFAULT) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_LINEAR) |
> >>> + BIT(AMDGPU_TRANSFER_FUNCTION_UNITY);
> >>> + struct drm_prop_enum_list enum_list[AMDGPU_TRANSFER_FUNCTION_COUNT];
> >>> + int i, len;
> >>> +
> >>> + len = 0;
> >>> + for (i = 0; i < AMDGPU_TRANSFER_FUNCTION_COUNT; i++) {
> >>> + if ((transfer_functions & BIT(i)) == 0)
> >>> + continue;
> >>> +
> >>> + enum_list[len].type = i;
> >>> + enum_list[len].name = amdgpu_transfer_function_names[i];
> >>> + len++;
> >>> + }
> >>> +
> >>> + return drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> >>> + name, enum_list, len);
> >>> +}
> >>> +
> >>> int
> >>> amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
> >>> {
> >>> @@ -116,11 +157,9 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
> >>> return -ENOMEM;
> >>> adev->mode_info.plane_degamma_lut_size_property = prop;
> >>>
> >>> - prop = drm_property_create_enum(adev_to_drm(adev),
> >>> - DRM_MODE_PROP_ENUM,
> >>> - "AMD_PLANE_DEGAMMA_TF",
> >>> - amdgpu_transfer_function_enum_list,
> >>> - ARRAY_SIZE(amdgpu_transfer_function_enum_list));
> >>> + prop = amdgpu_create_tf_property(adev_to_drm(adev),
> >>> + "AMD_PLANE_DEGAMMA_TF",
> >>> + amdgpu_eotf);
> >>> if (!prop)
> >>> return -ENOMEM;
> >>> adev->mode_info.plane_degamma_tf_property = prop;
> >>
> >
> >
>
On 09/06, Harry Wentland wrote:
> On 2023-08-10 12:02, Melissa Wen wrote:
> > Hi all,
> >
> > Here is the next version of our work to enable AMD driver-specific color
> > management properties [1][2]. This series is a collection of
> > contributions from Joshua, Harry, and me to enhance the AMD KMS color
> > pipeline for Steam Deck/SteamOS by exposing additional pre-blending and
> > post-blending color capabilities from those available in the current DRM
> > KMS API[3].
> >
> > The userspace case here is Gamescope which is the compositor for
> > SteamOS. Gamescope is already using these features to implement its
> > color management pipeline [4].
> >
> > In this version, I try to address all concerns shared in the previous
> > one, i.e.:
> > - Replace DRM_ by AMDGPU_ prefix for transfer function enumeration;
> > - Explicitly define EOTFs and inverse EOTFs and set props accordingly;
> > - Document pre-defined transfer functions;
> > - Remove misleading comments;
> > - Remove post-blending/MPC shaper and 3D LUT support;
> > - Move driver-specific property operations from amdgpu_display.c to
> > amdgpu_dm_color.c;
> > - Reset planes if any color props change;
> > - Nits/small fixes;
> >
> > Bearing in mind the complexity of color concepts, I believe there is a
> > high chance of some misunderstanding from my side when defining EOTFs
> > and documenting pre-defined TFs. So, reviews are very important and
> > welcome (thanks in advance). FWIW, I added Harry as a co-developer of
> > this TF documentation since I based on his description of EOTF/inv_EOTF
> > and previous documentation work [5]. Let me know if there is a better
> > way for credits.
> >
> > Two DC patches were already applied and, therefore, removed from the
> > series. I added r-b according to previous feedback. We also add plane
> > CTM to driver-specific properties. As a result, this is the updated list
> > of all driver-specific color properties exposed by this series:
> >
> > - plane degamma LUT and pre-defined TF;
> > - plane HDR multiplier;
> > - plane CTM 3x4;
> > - plane shaper LUT and pre-defined TF;
> > - plane 3D LUT;
> > - plane blend LUT and pre-defined TF;
> > - CRTC gamma pre-defined TF;
> >
> > Remember you can find the AMD HW color capabilities documented here:
> > https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#color-management-properties
> >
> > Worth mentioning that the pre-blending degamma block can use ROM curves
> > for some pre-defined TFs, but the other blocks use the AMD color module
> > to calculate this curve considering pre-defined coefficients.
> >
> > We need changes on DC gamut remap matrix to support the plane and CRTC
> > CTM on drivers that support both. I've sent a previous patch to apply
> > these changes to all DCN3+ families [6]. Here I use the same changes but
> > limited to DCN301. Just let me know if you prefer the previous/expanded
> > version.
> >
> > Finally, this is the Linux/AMD color management API before and after
> > blending with the driver-specific properties:
> >
> > +----------------------+
> > | PLANE |
> > | |
> > | +----------------+ |
> > | | AMD Degamma | |
> > | | | |
> > | | EOTF | 1D LUT | |
> > | +--------+-------+ |
> > | | |
> > | +--------v-------+ |
> > | | AMD HDR | |
> > | | Multiply | |
> > | +--------+-------+ |
> > | | |
> > | +--------v-------+ |
> > | | AMD CTM (3x4) | |
> > | +--------+-------+ |
> > | | |
> > | +--------v-------+ |
> > | | AMD Shaper | |
> > | | | |
> > | | inv_EOTF | | |
> > | | Custom 1D LUT | |
> > | +--------+-------+ |
> > | | |
> > | +--------v-------+ |
> > | | AMD 3D LUT | |
> > | | 17^3/12-bit | |
> > | +--------+-------+ |
> > | | |
> > | +--------v-------+ |
> > | | AMD Blend | |
> > | | | |
> > | | EOTF | 1D LUT | |
> > | +--------+-------+ |
> > | | |
> > ++----------v---------++
> > || Blending ||
> > ++----------+---------++
> > | CRTC | |
> > | | |
> > | +-------v-------+ |
> > | | DRM Degamma | |
> > | | | |
> > | | Custom 1D LUT | |
> > | +-------+-------+ |
> > | | |
> > | +-------v-------+ |
> > | | DRM CTM (3x3) | |
> > | +-------+-------+ |
> > | | |
> > | +-------v-------+ |
> > | | DRM Gamma | |
> > | | | |
> > | | Custom 1D LUT | |
> > | +---------------+ |
> > | | *AMD Gamma | |
> > | | inv_EOTF | |
> > | +---------------+ |
> > | |
> > +----------------------+
> >
> > Let me know your thoughts.
> >
>
> Thanks again for your amazing work on this.
>
> Patches 5, 6, 14, 16, and 24 are
> Reviewed-by: Harry Wentland <[email protected]>
>
> I left comments on the remaining unreviewed patches.
Thank you for the detailed review. I'll address your comments and also
those from Pekka in the next version.
Melissa
>
> Harry
>
> > Best Regards,
> >
> > Melissa Wen
> >
> > [1] https://lore.kernel.org/dri-devel/[email protected]
> > [2] https://lore.kernel.org/dri-devel/[email protected]
> > [3] https://github.com/ValveSoftware/gamescope/blob/master/src/docs/Steam%20Deck%20Display%20Pipeline.png
> > [4] https://github.com/ValveSoftware/gamescope
> > [5] https://lore.kernel.org/dri-devel/[email protected]
> > [6] https://lore.kernel.org/dri-devel/[email protected]
> >
> >
> > Harry Wentland (1):
> > drm/amd/display: fix segment distribution for linear LUTs
> >
> > Joshua Ashton (14):
> > drm/amd/display: add plane degamma TF driver-specific property
> > drm/amd/display: add plane HDR multiplier driver-specific property
> > drm/amd/display: add plane blend LUT and TF driver-specific properties
> > drm/amd/display: add CRTC gamma TF support
> > drm/amd/display: set sdr_ref_white_level to 80 for out_transfer_func
> > drm/amd/display: mark plane as needing reset if color props change
> > drm/amd/display: add plane degamma TF and LUT support
> > drm/amd/display: add dc_fixpt_from_s3132 helper
> > drm/amd/display: add HDR multiplier support
> > drm/amd/display: handle empty LUTs in __set_input_tf
> > drm/amd/display: add plane blend LUT and TF support
> > drm/amd/display: allow newer DC hardware to use degamma ROM for PQ/HLG
> > drm/amd/display: copy 3D LUT settings from crtc state to stream_update
> > drm/amd/display: Use 3x4 CTM for plane CTM
> >
> > Melissa Wen (19):
> > drm/drm_mode_object: increase max objects to accommodate new color
> > props
> > drm/drm_property: make replace_property_blob_from_id a DRM helper
> > drm/drm_plane: track color mgmt changes per plane
> > drm/amd/display: add driver-specific property for plane degamma LUT
> > drm/amd/display: explicitly define EOTF and inverse EOTF
> > drm/amd/display: document AMDGPU pre-defined transfer functions
> > drm/amd/display: add plane 3D LUT driver-specific properties
> > drm/amd/display: add plane shaper LUT and TF driver-specific
> > properties
> > drm/amd/display: add CRTC gamma TF driver-specific property
> > drm/amd/display: add comments to describe DM crtc color mgmt behavior
> > drm/amd/display: encapsulate atomic regamma operation
> > drm/amd/display: decouple steps for mapping CRTC degamma to DC plane
> > drm/amd/display: reject atomic commit if setting both plane and CRTC
> > degamma
> > drm/amd/display: add plane shaper LUT support
> > drm/amd/display: add plane shaper TF support
> > drm/amd/display: add plane 3D LUT support
> > drm/amd/display: set stream gamut remap matrix to MPC for DCN301
> > drm/amd/display: add plane CTM driver-specific property
> > drm/amd/display: add plane CTM support
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 71 ++
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 +-
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 101 +++
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 805 ++++++++++++++++--
> > .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 72 ++
> > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 224 ++++-
> > .../amd/display/dc/dcn10/dcn10_cm_common.c | 93 +-
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 37 +
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 2 +-
> > .../gpu/drm/amd/display/include/fixed31_32.h | 12 +
> > drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
> > drivers/gpu/drm/drm_atomic.c | 1 +
> > drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
> > drivers/gpu/drm/drm_atomic_uapi.c | 43 +-
> > drivers/gpu/drm/drm_property.c | 49 ++
> > include/drm/drm_mode_object.h | 2 +-
> > include/drm/drm_plane.h | 7 +
> > include/drm/drm_property.h | 6 +
> > include/uapi/drm/drm_mode.h | 8 +
> > 20 files changed, 1446 insertions(+), 127 deletions(-)
> >
>
On 09/06, Harry Wentland wrote:
>
>
> On 2023-08-10 12:03, Melissa Wen wrote:
> > Map the plane CTM driver-specific property to DC plane, instead of DC
> > stream. The remaining steps to program DPP block are already implemented
> > on DC shared-code.
> >
> > Signed-off-by: Melissa Wen <[email protected]>
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 25 +++++++++++++++++++
> > 2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index dfe61c5ed49e..f239410234b3 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -9578,6 +9578,7 @@ static bool should_reset_plane(struct drm_atomic_state *state,
> > if (dm_old_other_state->degamma_tf != dm_new_other_state->degamma_tf ||
> > dm_old_other_state->degamma_lut != dm_new_other_state->degamma_lut ||
> > dm_old_other_state->hdr_mult != dm_new_other_state->hdr_mult ||
> > + dm_old_other_state->ctm != dm_new_other_state->ctm ||
> > dm_old_other_state->shaper_lut != dm_new_other_state->shaper_lut ||
> > dm_old_other_state->shaper_tf != dm_new_other_state->shaper_tf ||
> > dm_old_other_state->lut3d != dm_new_other_state->lut3d ||
> > 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 86a918ab82be..7ff329101fd4 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
> > @@ -1158,6 +1158,8 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > struct dc_plane_state *dc_plane_state)
> > {
> > struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
> > + struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> > + struct drm_color_ctm *ctm = NULL;
> > struct dc_color_caps *color_caps = NULL;
> > bool has_crtc_cm_degamma;
> > int ret;
> > @@ -1209,6 +1211,29 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> > return ret;
> > }
> >
> > + /* Setup CRTC CTM. */
> > + if (dm_plane_state->ctm) {
> > + ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
> > +
> > + /*
> > + * So far, if we have both plane and CRTC CTM, plane CTM takes
> > + * the priority and we discard data for CRTC CTM, as
> > + * implemented in dcn10_program_gamut_remap(). However, we
>
> Isn't it the opposite? If stream (crtc) has a CTM we program that, only if
> stream doesn't have a CTM we program the plane one?
yeah, you're right. It was an intermediate approach that was discarded.
Indeed, I think I need to rewrite it to better describe that CRTC CTM
priority regarding DPP gamut remap matrix.
Thanks for pointing it out.
Melissa
>
> Harry
>
> > + * have MPC gamut_remap_matrix from DCN3 family, therefore we
> > + * can remap MPC programing of the matrix to MPC block and
> > + * provide support for both DPP and MPC matrix at the same
> > + * time.
> > + */
> > + __drm_ctm_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
> > +
> > + dc_plane_state->gamut_remap_matrix.enable_remap = true;
> > + dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
> > + } else {
> > + /* Bypass CTM. */
> > + dc_plane_state->gamut_remap_matrix.enable_remap = false;
> > + dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
> > + }
> > +
> > return amdgpu_dm_plane_set_color_properties(plane_state,
> > dc_plane_state, color_caps);
> > }
>
