[AMD Official Use Only - General]
> I think the proper fix is to set it to:
> build_coefficients(&coeff, TRANSFER_FUNCTION_SRGB);
I agree, default arg should be TRANSFER_FUNCTION_SRGB.
Even though it's a change in behaviour, previous behaviour was wrong.
Ideally it would be based on input TF, but afaik on both Linux and Win there's no current case where this is not sRGB.
Thanks,
Kruno
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Monday, September 19, 2022 12:07 PM
To: Zeng Heng <zengheng4@xxxxxxxxxx>; Chung, Jaehyun <Jaehyun.Chung@xxxxxxx>; Kovac, Krunoslav <Krunoslav.Kovac@xxxxxxx>
Cc: Wentland, Harry <Harry.Wentland@xxxxxxx>; Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx>; Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; airlied@xxxxxxxx; daniel@xxxxxxxx; Kotarac, Pavle <Pavle.Kotarac@xxxxxxx>; Liu, HaoPing (Alan) <HaoPing.Liu@xxxxxxx>; Wang, Sherry <Yao.Wang1@xxxxxxx>; weiyongjun1@xxxxxxxxxx; liwei391@xxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] gpu: color: eliminate implicit conversion about enum type
Add the relevant AMD developers to comment.
On Mon, Sep 19, 2022 at 12:05 PM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>
> On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng <zengheng4@xxxxxxxxxx> wrote:
> >
> > Fix below compile warning when open enum-conversion option check:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:
> > In function ‘apply_degamma_for_user_regamma’:
> > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1695:29:
> > error: implicit conversion from ‘enum <anonymous>’ to ‘enum
> > dc_transfer_func_predefined’ [-Werror=enum-conversion]
> > 1695 | build_coefficients(&coeff, true);
> > | ^~~~
> >
> > As 'build_coefficients' definition, it needs enum
> > 'dc_transfer_func_predefined' type acts as the second argument,
> > instead of bool-type one.
> >
> > The numerical values of TRANSFER_FUNCTION_BT709 & true happen to be
> > the same, so there is no change in behavior.
>
> This looks like a regression from:
>
> commit 9b3d76527f6ea50270f7f7ac749493b41783e8bd
> Author: Jaehyun Chung <jaehyun.chung@xxxxxxx>
> Date: Mon Aug 30 16:46:42 2021 -0400
>
> drm/amd/display: Revert adding degamma coefficients
>
> [Why]
> Degamma coefficients are calculated in our degamma formula using
> the regamma coefficients. We do not need to add separate degamma
> coefficients.
>
> [How]
> Remove the change to add separate degamma coefficients.
>
> Reviewed-by: Krunoslav Kovac <Krunoslav.Kovac@xxxxxxx>
> Acked-by: Mikita Lipski <mikita.lipski@xxxxxxx>
> Signed-off-by: Jaehyun Chung <jaehyun.chung@xxxxxxx>
> Tested-by: Daniel Wheeler <daniel.wheeler@xxxxxxx>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
>
> Which seems to improperly revert:
> commit d020970959169627d59a711769f8c4b87bf5f90c
> Author: Jaehyun Chung <jaehyun.chung@xxxxxxx>
> Date: Tue Aug 24 14:05:48 2021 -0400
>
> drm/amd/display: Add regamma/degamma coefficients and set sRGB
> when TF is BT709
>
> [Why]
> In YUV case, need to set the input TF to sRGB instead of BT709,
> even though the input TF type is distributed. SRGB was not
> being used because pixel format was not being set in the
> surface update sequence.
> Also, we were using the same coefficients for degamma and
> regamma formula, causing the cutoff point of the linear
> section of the curve to be incorrect.
>
> [How]
> Set pixel format in the surface update sequence. Add separate
> coefficient arrays for regamma and degamma.
>
> Reviewed-by: Krunoslav Kovac <Krunoslav.Kovac@xxxxxxx>
> Acked-by: Mikita Lipski <mikita.lipski@xxxxxxx>
> Signed-off-by: Jaehyun Chung <jaehyun.chung@xxxxxxx>
> Tested-by: Daniel Wheeler <daniel.wheeler@xxxxxxx>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
>
> I think the proper fix is to set it to:
> build_coefficients(&coeff, TRANSFER_FUNCTION_SRGB);
>
> Alex
>
> >
> > Signed-off-by: Zeng Heng <zengheng4@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > index 04f7656906ca..2f807d787c77 100644
> > --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> > @@ -1692,7 +1692,7 @@ static void apply_degamma_for_user_regamma(struct pwl_float_data_ex *rgb_regamma
> > struct pwl_float_data_ex *rgb = rgb_regamma;
> > const struct hw_x_point *coord_x = coordinates_x;
> >
> > - build_coefficients(&coeff, true);
> > + build_coefficients(&coeff, TRANSFER_FUNCTION_BT709);
> >
> > i = 0;
> > while (i != hw_points_num + 1) {
> > --
> > 2.25.1
> >
