Re: [PATCH v3 3/5] drm/amd/display: use a separate helper to translate degamma curves

On 7/23/26 07:29, Melissa Wen wrote:
> 
> 
> 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?

I tested above on
  - DCN35 - issue fiexed
  - DCN401 - no issue and no regression.

Does it pass DCN321 that you mentioned abouve?

DCN301 doesn't use cm3_helper_translate_curve_to_degamma_hw_format() so 
it wasn't affected.

The changes look good to me, but let's ask @Wentland, Harry for review too.

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