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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> >




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux