[PATCH] gpu: color: eliminate implicit conversion about enum type

[Kovac, Krunoslav]

[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 at gmail.com>
Sent: Monday, September 19, 2022 12:07 PM
To: Zeng Heng <zengheng4 at huawei.com>; Chung, Jaehyun <Jaehyun.Chung at amd.com>; Kovac, Krunoslav <Krunoslav.Kovac at amd.com>
Cc: Wentland, Harry <Harry.Wentland at amd.com>; Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Pan, Xinhui <Xinhui.Pan at amd.com>; airlied at linux.ie; daniel at ffwll.ch; Kotarac, Pavle <Pavle.Kotarac at amd.com>; Liu, HaoPing (Alan) <HaoPing.Liu at amd.com>; Wang, Sherry <Yao.Wang1 at amd.com>; weiyongjun1 at huawei.com; liwei391 at huawei.com; dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org
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 at gmail.com> wrote:
>
> On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng <zengheng4 at huawei.com> 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 at amd.com>
> 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 at amd.com>
>     Acked-by: Mikita Lipski <mikita.lipski at amd.com>
>     Signed-off-by: Jaehyun Chung <jaehyun.chung at amd.com>
>     Tested-by: Daniel Wheeler <daniel.wheeler at amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>
> Which seems to improperly revert:
> commit d020970959169627d59a711769f8c4b87bf5f90c
> Author: Jaehyun Chung <jaehyun.chung at amd.com>
> 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 at amd.com>
>     Acked-by: Mikita Lipski <mikita.lipski at amd.com>
>     Signed-off-by: Jaehyun Chung <jaehyun.chung at amd.com>
>     Tested-by: Daniel Wheeler <daniel.wheeler at amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>
> I think the proper fix is to set it to:
> build_coefficients(&coeff, TRANSFER_FUNCTION_SRGB);
>
> Alex
>
> >
> > Signed-off-by: Zeng Heng <zengheng4 at huawei.com>
> > ---
> >  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
> >