[PATCH v2 19/34] drm/amd/display: decouple steps for mapping CRTC degamma to DC plane

[Joshua Ashton]

Degamma has always been on the plane on AMD. CRTC DEGAMMA_LUT has actually
just been applying it to every plane pre-blend.

Degamma makes no sense after blending anyway.
The entire point is for it to happen before blending to blend in linear
space. Otherwise DEGAMMA_LUT and REGAMMA_LUT are the exact same thing...

- Joshie 🐸

On Tuesday, 22 August 2023, Pekka Paalanen <pekka.paalanen at collabora.com>
wrote:
> On Thu, 10 Aug 2023 15:02:59 -0100
> Melissa Wen <mwen at igalia.com> 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 <harry.wentland at amd.com>
>> Signed-off-by: Melissa Wen <mwen at igalia.com>
>> ---
>>  .../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,
>>                                                &degamma_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.
>
>
> 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;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230828/374fdd16/attachment-0001.htm>