On 22/07/2026 20:27, Alex Hung wrote:
On 6/23/26 09:58, Melissa Wen wrote:
In newer DCN families, there is no hw predefined curves for shaper,
blend and regamma. When userspace sets pre-defined curves for these
blocks, the driver uses AMD color module to program predefined curve as
LUT. However, it was using the same LUT segmentation for EOTF and
inverse EOTF by using the same color management helper. This is causing
banding on blend when PQ predefined curve is set. Besides that, degamma
predefined HW curve cannot be used with subsampled 4:2:0/4:2:2 formats
as it affects the linearity of color space in which HW scaler operates.
To mitigate banding when using the blend block and better support
subsampled format on degamma, use different translation helpers when
linearizing and delinearizing.
Tested-by: Matthew Schwartz <[email protected]>
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---
.../gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
index 1340f673ec3b..c2ea25927765 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
@@ -493,11 +493,9 @@ bool dcn32_set_mcm_luts(
if (plane_state->cm.blend_func.type == TF_TYPE_HWPWL)
lut_params = &plane_state->cm.blend_func.pwl;
else if (plane_state->cm.blend_func.type ==
TF_TYPE_DISTRIBUTED_POINTS) {
- result = cm3_helper_translate_curve_to_hw_format(
- plane_state->ctx,
+ result = cm3_helper_translate_curve_to_degamma_hw_format(
&plane_state->cm.blend_func,
- &dpp_base->regamma_params,
- false);
+ &dpp_base->regamma_params);
if (!result)
return result;
The above change broke two IGT's kms_colorop subtests (tested on
DCN3.5) as below
Hi Alex,
Thanks for pointing it out.
I tested with DCN301 (all tests pass) and DCN321 (same failures).
I'm a bit curious about this difference, probably some differences in
precision, etc. I'll leave that out.
=====================================
Starting subtest: plane-XR30-XR30-srgb_inv_eotf_lut-srgb_eotf_lut
i 2569 j 0 shift 10 mask 3ff comp1 3bf comp2 3ff, pixel1 3bfeffbf
pixel2 3fffffff
[79.216137] (kms_colorop:10824) CRITICAL: Test assertion failure
function colorop_plane_test, file ../tests/kms_colorop.c:281:
[79.216143] (kms_colorop:10824) CRITICAL: Failed assertion:
compare_with_bracket(&sw_transform_fb, output_fb)
[79.216165] (kms_colorop:10824) CRITICAL: Last errno: 38, Function not
implemented
Stack trace: not implemented
Subtest plane-XR30-XR30-srgb_inv_eotf_lut-srgb_eotf_lut failed.
Starting subtest: plane-XR30-XR30-gamma_2_2-gamma_2_2_inv-gamma_2_2
i 15390 j 0 shift 10 mask 3ff comp1 398 comp2 3b8, pixel1 398e6398
pixel2 3b8ee3b8
[81.747233] (kms_colorop:10824) CRITICAL: Test assertion failure
function colorop_plane_test, file ../tests/kms_colorop.c:281:
[81.747239] (kms_colorop:10824) CRITICAL: Failed assertion:
compare_with_bracket(&sw_transform_fb, output_fb)
[81.747256] (kms_colorop:10824) CRITICAL: Last errno: 38, Function not
implemented
Stack trace: not implemented
Subtest plane-XR30-XR30-gamma_2_2-gamma_2_2_inv-gamma_2_2 failed.
=====================================
The root cause is from color delta increases, as also confirmed by the
following IGT POC change:
--- tests/kms_colorop.c
+++ tests/kms_colorop.c
@@ -182,7 +182,7 @@ static bool compare_with_bracket(igt_fb_t *in,
igt_fb_t *out)
return igt_cmp_fb_pixels(in, out, 1, 1);
if (is_amdgpu_device(in->fd))
- return igt_cmp_fb_pixels(in, out, 13, 13);
+ return igt_cmp_fb_pixels(in, out, 70, 13);
Is there any concerns on reverting above changes (or this patch)?
Note: I am looking for early feedback because I have not yet tested on
other DCN hardware.
Reverting this patch is not ideal because it addresses an issue on HDR
Ori game on gamescope with external HDR monitor for DCN301, DCN321 and
also DCN35 (IIRC Matt has verified the issue and this series in this hw
version).
I think the possible solution is making
cm3_helper_translate_curve_to_degamma_hw_format() take the same segment
distribution for PQ/SRGB to any pre-defined TF or LUT.
Something like removing the if/else clause in dcn30_cm_common and
applying PQ/SRGB distribution to all cases, as below:
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_cm_common.c
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_cm_common.c
index 66fe7f313ea3..1b4ce77fdd91 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_cm_common.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_cm_common.c
@@ -332,6 +332,7 @@ bool cm3_helper_translate_curve_to_degamma_hw_format(
int32_t region_start, region_end;
int32_t i;
uint32_t j, k, seg_distr[MAX_REGIONS_NUMBER], increment,
start_index, hw_points;
+ const uint8_t SEG_COUNT = 9; // 9 segments, segments are from
2^-9 to 0
if (output_tf == NULL || lut_params == NULL || output_tf->type
== TF_TYPE_BYPASS)
return false;
@@ -343,31 +344,12 @@ bool cm3_helper_translate_curve_to_degamma_hw_format(
memset(lut_params, 0, sizeof(struct pwl_params));
memset(seg_distr, 0, sizeof(seg_distr));
- if (output_tf->tf == TRANSFER_FUNCTION_PQ ||
- output_tf->tf == TRANSFER_FUNCTION_SRGB) {
- /* 9 segments
- * segments are from 2^-9 to 0
- */
- const uint8_t SEG_COUNT = 9;
- seg_distr[0] = 0; // Since we only have one point in
darkest region
- for (k = 1; k < SEG_COUNT; k++)
- seg_distr[k] = k - 1; // 2^(k-1) points per
region; halves as k decreases
+ seg_distr[0] = 0; // Since we only have one point in darkest region
+ for (k = 1; k < SEG_COUNT; k++)
+ seg_distr[k] = k - 1; // 2^(k-1) points per region;
halves as k decreases
- region_start = -SEG_COUNT;
- region_end = 0;
- } else {
- /* 12 segments
- * segments are from 2^-12 to 2^0
- * There are less than 256 points, for optimization
- */
- const uint8_t SEG_COUNT = 12;
-
- for (i = 0; i < SEG_COUNT; i++)
- seg_distr[i] = 4;
-
- region_start = -SEG_COUNT;
- region_end = 0;
- }
+ region_start = -SEG_COUNT;
+ region_end = 0;
for (i = region_end - region_start; i < MAX_REGIONS_NUMBER ; i++)
seg_distr[i] = -1;
I'll check if there is any side effect on gamescope with this change
ASAP, but I don't think so.
WDYT?
Melissa
@@ -554,9 +552,8 @@ bool dcn32_set_input_transfer_func(struct dc *dc,
if (plane_state->in_transfer_func.type == TF_TYPE_HWPWL)
params = &plane_state->in_transfer_func.pwl;
else if (plane_state->in_transfer_func.type ==
TF_TYPE_DISTRIBUTED_POINTS &&
- cm3_helper_translate_curve_to_hw_format(plane_state->ctx,
- &plane_state->in_transfer_func,
- &dpp_base->degamma_params, false))
+
cm3_helper_translate_curve_to_degamma_hw_format(&plane_state->in_transfer_func,
+ &dpp_base->degamma_params))
params = &dpp_base->degamma_params;
dpp_base->funcs->dpp_program_gamcor_lut(dpp_base, params);
