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 <<a href="mailto:pekka.paalanen@collabora.com">pekka.paalanen@collabora.com</a>> wrote: > On Thu, 10 Aug 2023 15:02:59 -0100 > Melissa Wen <<a href="mailto:mwen@igalia.com">mwen@igalia.com</a>> 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 <<a href="mailto:harry.wentland@amd.com">harry.wentland@amd.com</a>> >> Signed-off-by: Melissa Wen <<a href="mailto:mwen@igalia.com">mwen@igalia.com</a>> >> --- >> .../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; > >