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@xxxxxxxxxxxxx> wrote:
> On Thu, 10 Aug 2023 15:02:59 -0100
> Melissa Wen <mwen@xxxxxxxxxx> 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@xxxxxxx>
>> Signed-off-by: Melissa Wen <mwen@xxxxxxxxxx>
>> ---
>> .../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.
>
>
> 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;
>
>