| Message ID | 20230810160314.48225-20-mwen@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: add AMD driver-specific properties for color mgmt | expand |
On Thu, 10 Aug 2023 15:02:59 -0100 Melissa Wen <mwen@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@amd.com> > Signed-off-by: Melissa Wen <mwen@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, > °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;
On 08/22, Pekka Paalanen wrote: > On Thu, 10 Aug 2023 15:02:59 -0100 > Melissa Wen <mwen@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@amd.com> > > Signed-off-by: Melissa Wen <mwen@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, > > °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. I agree that crtc degamma is an optional property and should be not exposed if not available. I did something in this line for DCE that has no degamma block[1]. Then, AMD DDX driver stopped to advertise atomic API for DCE, that was not correct too[2]. But I see it as a lost cause that will only be fixed in a new generic color API. I don't think we should change it using the current DRM CRTC API with driver-specific props. [1] https://lore.kernel.org/amd-gfx/20221103184500.14450-1-mwen@igalia.com/ [2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/-/issues/67 > > > 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; >
On Fri, 25 Aug 2023 13:29:44 -0100 Melissa Wen <mwen@igalia.com> wrote: > On 08/22, Pekka Paalanen wrote: > > On Thu, 10 Aug 2023 15:02:59 -0100 > > Melissa Wen <mwen@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@amd.com> > > > Signed-off-by: Melissa Wen <mwen@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, > > > °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. > > I agree that crtc degamma is an optional property and should be not > exposed if not available. I did something in this line for DCE that has > no degamma block[1]. Then, AMD DDX driver stopped to advertise atomic > API for DCE, that was not correct too[2]. Did AMD go through all the trouble of making their Xorg DDX use KMS atomic, even after the kernel took it away from X due to modesetting DDX screwing it up? I'm surprised, what did that achieve? I saw that between 5.15 and 6.1 amdgpu stopped advertising CRTC DEGAMMA on my card, which seems like the right thing to do if there is no hardware for it. > But I see it as a lost cause that will only be fixed in a new generic > color API. I don't think we should change it using the current DRM CRTC > API with driver-specific props. > > [1] https://lore.kernel.org/amd-gfx/20221103184500.14450-1-mwen@igalia.com/ > [2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/-/issues/67 Oh well. Is the old CRTC GAMMA property still "safe" to use in that it is definitely always after blending? 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; > >
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 Mon, 28 Aug 2023 09:45:44 +0100 Joshua Ashton <joshua@froggi.es> wrote: > Degamma has always been on the plane on AMD. CRTC DEGAMMA_LUT has actually > just been applying it to every plane pre-blend. I've never seen that documented anywhere. It has seemed obvious, that since we have KMS objects for planes and CRTCs, stuff on the CRTC does not do plane stuff before blending. That also has not been documented in the past, but it seemed the most logical choice. Even today https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties make no mention of whether they apply before or after blending. > Degamma makes no sense after blending anyway. If the goal is to allow blending in optical or other space, you are correct. However, APIs do not need to make sense to exist, like most of the options of "Colorspace" connector property. I have always thought the CRTC DEGAMMA only exists to allow the CRTC CTM to work in linear or other space. I have at times been puzzled by what the DEGAMMA and CTM are actually good for. > 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... The CRTC CTM is between CRTC DEGAMMA and CRTC GAMMA, meaning they are not interchangeable. I have literally believed that DRM KMS UAPI simply does not support blending in optical space, unless your framebuffers are in optical which no-one does, until the color management properties are added to KMS planes. This never even seemed weird, because non-linear blending is so common. So I have been misunderstanding the CRTC DEGAMMA property forever. Am I the only one? Do all drivers agree today at what point does CRTC DEGAMMA apply, before blending on all planes or after blending? Does anyone know of any doc about that? If drivers do not agree on the behaviour of a KMS property, then that property is useless for generic userspace. Thanks, pq > On Tuesday, 22 August 2023, Pekka Paalanen <pekka.paalanen@collabora.com> > wrote: > > On Thu, 10 Aug 2023 15:02:59 -0100 > > Melissa Wen <mwen@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@amd.com> > >> Signed-off-by: Melissa Wen <mwen@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, > >> °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; > > > >
On 08/28, Pekka Paalanen wrote: > On Mon, 28 Aug 2023 09:45:44 +0100 > Joshua Ashton <joshua@froggi.es> wrote: > > > Degamma has always been on the plane on AMD. CRTC DEGAMMA_LUT has actually > > just been applying it to every plane pre-blend. > > I've never seen that documented anywhere. > > It has seemed obvious, that since we have KMS objects for planes and > CRTCs, stuff on the CRTC does not do plane stuff before blending. That > also has not been documented in the past, but it seemed the most > logical choice. > > Even today > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties > make no mention of whether they apply before or after blending. It's mentioned in the next section: https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#dc-color-capabilities-between-dcn-generations In hindsight, maybe it isn't the best place... > > > Degamma makes no sense after blending anyway. > > If the goal is to allow blending in optical or other space, you are > correct. However, APIs do not need to make sense to exist, like most of > the options of "Colorspace" connector property. > > I have always thought the CRTC DEGAMMA only exists to allow the CRTC > CTM to work in linear or other space. > > I have at times been puzzled by what the DEGAMMA and CTM are actually > good for. > > > 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... > > The CRTC CTM is between CRTC DEGAMMA and CRTC GAMMA, meaning they are > not interchangeable. > > I have literally believed that DRM KMS UAPI simply does not support > blending in optical space, unless your framebuffers are in optical > which no-one does, until the color management properties are added to > KMS planes. This never even seemed weird, because non-linear blending > is so common. > > So I have been misunderstanding the CRTC DEGAMMA property forever. Am I > the only one? Do all drivers agree today at what point does CRTC > DEGAMMA apply, before blending on all planes or after blending? > I'd like to know current userspace cases on Linux of this CRTC DEGAMMA LUT. > Does anyone know of any doc about that? From what I retrieved about the introduction of CRTC color props[1], it seems the main concern at that point was getting a linear space for CTM[2] and CRTC degamma property seems to have followed intel requirements, but didn't find anything about the blending space. AFAIU, we have just interpreted that all CRTC color properties for DRM interface are after blending[3]. Can this be seen in another way? [1] https://patchwork.freedesktop.org/series/2720/ [2] https://codereview.chromium.org/1182063002 [3] https://dri.freedesktop.org/docs/drm/_images/dcn3_cm_drm_current.svg > > If drivers do not agree on the behaviour of a KMS property, then that > property is useless for generic userspace. > > > Thanks, > pq > > > > On Tuesday, 22 August 2023, Pekka Paalanen <pekka.paalanen@collabora.com> > > wrote: > > > On Thu, 10 Aug 2023 15:02:59 -0100 > > > Melissa Wen <mwen@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@amd.com> > > >> Signed-off-by: Melissa Wen <mwen@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, > > >> °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; > > > > > > >
On Mon, 28 Aug 2023 12:56:04 -0100 Melissa Wen <mwen@igalia.com> wrote: > On 08/28, Pekka Paalanen wrote: > > On Mon, 28 Aug 2023 09:45:44 +0100 > > Joshua Ashton <joshua@froggi.es> wrote: > > > > > Degamma has always been on the plane on AMD. CRTC DEGAMMA_LUT has actually > > > just been applying it to every plane pre-blend. > > > > I've never seen that documented anywhere. > > > > It has seemed obvious, that since we have KMS objects for planes and > > CRTCs, stuff on the CRTC does not do plane stuff before blending. That > > also has not been documented in the past, but it seemed the most > > logical choice. > > > > Even today > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties > > make no mention of whether they apply before or after blending. > > It's mentioned in the next section: > https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#dc-color-capabilities-between-dcn-generations > In hindsight, maybe it isn't the best place... That is driver-specific documentation. As a userspace dev, I'd never look at driver-specific documentation, because I'm interested in the KMS UAPI which is supposed to be generic, and therefore documented with the DRM "core". Maybe kernel reviewers also never look at driver-specific docs to find attempts at redefining common KMS properties? (I still don't know which definition is prevalent.) > > > > > Degamma makes no sense after blending anyway. > > > > If the goal is to allow blending in optical or other space, you are > > correct. However, APIs do not need to make sense to exist, like most of > > the options of "Colorspace" connector property. > > > > I have always thought the CRTC DEGAMMA only exists to allow the CRTC > > CTM to work in linear or other space. > > > > I have at times been puzzled by what the DEGAMMA and CTM are actually > > good for. > > > > > 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... > > > > The CRTC CTM is between CRTC DEGAMMA and CRTC GAMMA, meaning they are > > not interchangeable. > > > > I have literally believed that DRM KMS UAPI simply does not support > > blending in optical space, unless your framebuffers are in optical > > which no-one does, until the color management properties are added to > > KMS planes. This never even seemed weird, because non-linear blending > > is so common. > > > > So I have been misunderstanding the CRTC DEGAMMA property forever. Am I > > the only one? Do all drivers agree today at what point does CRTC > > DEGAMMA apply, before blending on all planes or after blending? > > > > I'd like to know current userspace cases on Linux of this CRTC DEGAMMA > LUT. I don't know of any, but that doesn't mean anything. > > Does anyone know of any doc about that? > > From what I retrieved about the introduction of CRTC color props[1], it > seems the main concern at that point was getting a linear space for > CTM[2] and CRTC degamma property seems to have followed intel > requirements, but didn't find anything about the blending space. Right. I've always thought CRTC props apply after blending. > AFAIU, we have just interpreted that all CRTC color properties for DRM > interface are after blending[3]. Can this be seen in another way? Joshua did, and he has a logical point. I guess if we really want to know, someone would need review all drivers exposing these props, and even check if they changed in the past. FWIW, the usefulness of (RE)GAMMA (not DEGAMMA) LUT is limited by the fact that attempting to represent 1/2.2 power function as a uniformly distributed LUT is infeasible due to the approximation errors near zero. Thanks, pq > [1] https://patchwork.freedesktop.org/series/2720/ > [2] https://codereview.chromium.org/1182063002 > [3] https://dri.freedesktop.org/docs/drm/_images/dcn3_cm_drm_current.svg > > > > > If drivers do not agree on the behaviour of a KMS property, then that > > property is useless for generic userspace. > > > > > > Thanks, > > pq > > > > > > > On Tuesday, 22 August 2023, Pekka Paalanen <pekka.paalanen@collabora.com> > > > wrote: > > > > On Thu, 10 Aug 2023 15:02:59 -0100 > > > > Melissa Wen <mwen@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@amd.com> > > > >> Signed-off-by: Melissa Wen <mwen@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, > > > >> °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; > > > > > > > > > >
On 8/28/23 10:17, Pekka Paalanen wrote: > On Fri, 25 Aug 2023 13:29:44 -0100 > Melissa Wen <mwen@igalia.com> wrote: > >> On 08/22, Pekka Paalanen wrote: >>> On Thu, 10 Aug 2023 15:02:59 -0100 >>> Melissa Wen <mwen@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@amd.com> >>>> Signed-off-by: Melissa Wen <mwen@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, >>>> °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. >> >> I agree that crtc degamma is an optional property and should be not >> exposed if not available. I did something in this line for DCE that has >> no degamma block[1]. Then, AMD DDX driver stopped to advertise atomic >> API for DCE, that was not correct too[2]. > > Did AMD go through all the trouble of making their Xorg DDX use KMS > atomic, even after the kernel took it away from X due to modesetting > DDX screwing it up? No, I think Melissa meant the KMS properties for advanced colour transforms, which xf86-video-amdgpu uses, not with atomic KMS though.
On 2023-08-28 04:17, Pekka Paalanen wrote: > On Fri, 25 Aug 2023 13:29:44 -0100 > Melissa Wen <mwen@igalia.com> wrote: > >> On 08/22, Pekka Paalanen wrote: >>> On Thu, 10 Aug 2023 15:02:59 -0100 >>> Melissa Wen <mwen@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@amd.com> >>>> Signed-off-by: Melissa Wen <mwen@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, >>>> °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. >> >> I agree that crtc degamma is an optional property and should be not >> exposed if not available. I did something in this line for DCE that has >> no degamma block[1]. Then, AMD DDX driver stopped to advertise atomic >> API for DCE, that was not correct too[2]. > > Did AMD go through all the trouble of making their Xorg DDX use KMS > atomic, even after the kernel took it away from X due to modesetting > DDX screwing it up? I'm surprised, what did that achieve? > > I saw that between 5.15 and 6.1 amdgpu stopped advertising CRTC DEGAMMA > on my card, which seems like the right thing to do if there is no > hardware for it. > >> But I see it as a lost cause that will only be fixed in a new generic >> color API. I don't think we should change it using the current DRM CRTC >> API with driver-specific props. >> >> [1] https://lore.kernel.org/amd-gfx/20221103184500.14450-1-mwen@igalia.com/ >> [2] https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/-/issues/67 > > Oh well. > > Is the old CRTC GAMMA property still "safe" to use in that it is > definitely always after blending? > CRTC GAMMA is always post-blending. DEGAMMA and CTM are not always post-blending and need to be fixed (or removed). Harry > > 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; >>> >
On 2023-08-29 04:51, Pekka Paalanen wrote: > On Mon, 28 Aug 2023 12:56:04 -0100 > Melissa Wen <mwen@igalia.com> wrote: > >> On 08/28, Pekka Paalanen wrote: >>> On Mon, 28 Aug 2023 09:45:44 +0100 >>> Joshua Ashton <joshua@froggi.es> wrote: >>> >>>> Degamma has always been on the plane on AMD. CRTC DEGAMMA_LUT has actually >>>> just been applying it to every plane pre-blend. >>> >>> I've never seen that documented anywhere. >>> >>> It has seemed obvious, that since we have KMS objects for planes and >>> CRTCs, stuff on the CRTC does not do plane stuff before blending. That >>> also has not been documented in the past, but it seemed the most >>> logical choice. >>> >>> Even today >>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties >>> make no mention of whether they apply before or after blending. >> >> It's mentioned in the next section: >> https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#dc-color-capabilities-between-dcn-generations >> In hindsight, maybe it isn't the best place... > > That is driver-specific documentation. As a userspace dev, I'd never > look at driver-specific documentation, because I'm interested in the > KMS UAPI which is supposed to be generic, and therefore documented with > the DRM "core". > > Maybe kernel reviewers also never look at driver-specific docs to find > attempts at redefining common KMS properties? > > (I still don't know which definition is prevalent.) > >>> >>>> Degamma makes no sense after blending anyway. >>> >>> If the goal is to allow blending in optical or other space, you are >>> correct. However, APIs do not need to make sense to exist, like most of >>> the options of "Colorspace" connector property. >>> >>> I have always thought the CRTC DEGAMMA only exists to allow the CRTC >>> CTM to work in linear or other space. >>> >>> I have at times been puzzled by what the DEGAMMA and CTM are actually >>> good for. >>> >>>> 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... >>> >>> The CRTC CTM is between CRTC DEGAMMA and CRTC GAMMA, meaning they are >>> not interchangeable. >>> >>> I have literally believed that DRM KMS UAPI simply does not support >>> blending in optical space, unless your framebuffers are in optical >>> which no-one does, until the color management properties are added to I think Mario Kleiner had a use-case that made use of that and introduced FP16 format support in amdgpu. >>> KMS planes. This never even seemed weird, because non-linear blending >>> is so common. >>> >>> So I have been misunderstanding the CRTC DEGAMMA property forever. Am I >>> the only one? Do all drivers agree today at what point does CRTC >>> DEGAMMA apply, before blending on all planes or after blending? >>> >> >> I'd like to know current userspace cases on Linux of this CRTC DEGAMMA >> LUT. > > I don't know of any, but that doesn't mean anything. > >>> Does anyone know of any doc about that? >> >> From what I retrieved about the introduction of CRTC color props[1], it >> seems the main concern at that point was getting a linear space for >> CTM[2] and CRTC degamma property seems to have followed intel >> requirements, but didn't find anything about the blending space. > > Right. I've always thought CRTC props apply after blending. > >> AFAIU, we have just interpreted that all CRTC color properties for DRM >> interface are after blending[3]. Can this be seen in another way? > > Joshua did, and he has a logical point. > > I guess if we really want to know, someone would need review all > drivers exposing these props, and even check if they changed in the > past. > > FWIW, the usefulness of (RE)GAMMA (not DEGAMMA) LUT is limited by the > fact that attempting to represent 1/2.2 power function as a uniformly > distributed LUT is infeasible due to the approximation errors near zero. > IMO, CRTC should be post-blending. Blending is at the plane/crtc boundary by design, therefore CRTC properties apply post-blending. Though I can understand why DEGAMMA can be interpreted to be applied pre-blending. Though, I think that's wrong for the DRM/KMS model and should be fixed in amdgpu. Harry > > Thanks, > pq > >> [1] https://patchwork.freedesktop.org/series/2720/ >> [2] https://codereview.chromium.org/1182063002 >> [3] https://dri.freedesktop.org/docs/drm/_images/dcn3_cm_drm_current.svg >> >>> >>> If drivers do not agree on the behaviour of a KMS property, then that >>> property is useless for generic userspace. >>> >>> >>> Thanks, >>> pq >>> >>> >>>> On Tuesday, 22 August 2023, Pekka Paalanen <pekka.paalanen@collabora.com> >>>> wrote: >>>>> On Thu, 10 Aug 2023 15:02:59 -0100 >>>>> Melissa Wen <mwen@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@amd.com> >>>>>> Signed-off-by: Melissa Wen <mwen@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, >>>>>> °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; >>>>> >>>>> >>> >
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. + */ + 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;