All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpu: color: eliminate implicit conversion about enum type
@ 2022-09-19  1:44 Zeng Heng
  2022-09-19  2:31 ` Wei Yongjun
  2022-09-19 16:05   ` Alex Deucher
  0 siblings, 2 replies; 8+ messages in thread
From: Zeng Heng @ 2022-09-19  1:44 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, Rodrigo.Siqueira, alexander.deucher,
	christian.koenig, Xinhui.Pan, airlied, daniel, Pavle.Kotarac,
	HaoPing.Liu, Krunoslav.Kovac, Yao.Wang1
  Cc: zengheng4, weiyongjun1, liwei391, dri-devel, amd-gfx

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.

Signed-off-by: Zeng Heng <zengheng4@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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] gpu: color: eliminate implicit conversion about enum type
  2022-09-19  1:44 [PATCH] gpu: color: eliminate implicit conversion about enum type Zeng Heng
@ 2022-09-19  2:31 ` Wei Yongjun
  2022-09-19 16:05   ` Alex Deucher
  1 sibling, 0 replies; 8+ messages in thread
From: Wei Yongjun @ 2022-09-19  2:31 UTC (permalink / raw)
  To: Zeng Heng, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
	alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	Pavle.Kotarac, HaoPing.Liu, Krunoslav.Kovac, Yao.Wang1
  Cc: liwei391, dri-devel, amd-gfx

gpu: color: fix enum-conversion compile warning

标题我记得不跟你所过让你改的


On 2022/9/19 9:44, Zeng Heng 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.
> 
> Signed-off-by: Zeng Heng <zengheng4@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) {
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gpu: color: eliminate implicit conversion about enum type
  2022-09-19  1:44 [PATCH] gpu: color: eliminate implicit conversion about enum type Zeng Heng
@ 2022-09-19 16:05   ` Alex Deucher
  2022-09-19 16:05   ` Alex Deucher
  1 sibling, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2022-09-19 16:05 UTC (permalink / raw)
  To: Zeng Heng
  Cc: HaoPing.Liu, Krunoslav.Kovac, sunpeng.li, liwei391, Xinhui.Pan,
	Rodrigo.Siqueira, dri-devel, airlied, Yao.Wang1, weiyongjun1,
	amd-gfx, alexander.deucher, christian.koenig, Pavle.Kotarac

On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng <zengheng4@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@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@amd.com>
    Acked-by: Mikita Lipski <mikita.lipski@amd.com>
    Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
    Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Which seems to improperly revert:
commit d020970959169627d59a711769f8c4b87bf5f90c
Author: Jaehyun Chung <jaehyun.chung@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@amd.com>
    Acked-by: Mikita Lipski <mikita.lipski@amd.com>
    Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
    Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@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@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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gpu: color: eliminate implicit conversion about enum type
@ 2022-09-19 16:05   ` Alex Deucher
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2022-09-19 16:05 UTC (permalink / raw)
  To: Zeng Heng
  Cc: HaoPing.Liu, Krunoslav.Kovac, sunpeng.li, liwei391, Xinhui.Pan,
	Rodrigo.Siqueira, dri-devel, airlied, Yao.Wang1, weiyongjun1,
	amd-gfx, daniel, alexander.deucher, harry.wentland,
	christian.koenig, Pavle.Kotarac

On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng <zengheng4@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@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@amd.com>
    Acked-by: Mikita Lipski <mikita.lipski@amd.com>
    Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
    Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Which seems to improperly revert:
commit d020970959169627d59a711769f8c4b87bf5f90c
Author: Jaehyun Chung <jaehyun.chung@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@amd.com>
    Acked-by: Mikita Lipski <mikita.lipski@amd.com>
    Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
    Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@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@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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gpu: color: eliminate implicit conversion about enum type
  2022-09-19 16:05   ` Alex Deucher
@ 2022-09-19 16:06     ` Alex Deucher
  -1 siblings, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2022-09-19 16:06 UTC (permalink / raw)
  To: Zeng Heng, jaehyun.chung, Krunoslav Kovac
  Cc: HaoPing.Liu, sunpeng.li, liwei391, Xinhui.Pan, Rodrigo.Siqueira,
	dri-devel, airlied, Yao.Wang1, weiyongjun1, amd-gfx,
	alexander.deucher, christian.koenig, Pavle.Kotarac

Add the relevant AMD developers to comment.

On Mon, Sep 19, 2022 at 12:05 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng <zengheng4@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@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@amd.com>
>     Acked-by: Mikita Lipski <mikita.lipski@amd.com>
>     Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
>     Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> Which seems to improperly revert:
> commit d020970959169627d59a711769f8c4b87bf5f90c
> Author: Jaehyun Chung <jaehyun.chung@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@amd.com>
>     Acked-by: Mikita Lipski <mikita.lipski@amd.com>
>     Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
>     Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher@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@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
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gpu: color: eliminate implicit conversion about enum type
@ 2022-09-19 16:06     ` Alex Deucher
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2022-09-19 16:06 UTC (permalink / raw)
  To: Zeng Heng, jaehyun.chung, Krunoslav Kovac
  Cc: HaoPing.Liu, sunpeng.li, liwei391, Xinhui.Pan, Rodrigo.Siqueira,
	dri-devel, airlied, Yao.Wang1, weiyongjun1, amd-gfx, daniel,
	alexander.deucher, harry.wentland, christian.koenig,
	Pavle.Kotarac

Add the relevant AMD developers to comment.

On Mon, Sep 19, 2022 at 12:05 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Mon, Sep 19, 2022 at 3:19 AM Zeng Heng <zengheng4@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@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@amd.com>
>     Acked-by: Mikita Lipski <mikita.lipski@amd.com>
>     Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
>     Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> Which seems to improperly revert:
> commit d020970959169627d59a711769f8c4b87bf5f90c
> Author: Jaehyun Chung <jaehyun.chung@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@amd.com>
>     Acked-by: Mikita Lipski <mikita.lipski@amd.com>
>     Signed-off-by: Jaehyun Chung <jaehyun.chung@amd.com>
>     Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher@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@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
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] gpu: color: eliminate implicit conversion about enum type
  2022-09-19 16:06     ` Alex Deucher
@ 2022-09-19 16:27       ` Kovac, Krunoslav
  -1 siblings, 0 replies; 8+ messages in thread
From: Kovac, Krunoslav @ 2022-09-19 16:27 UTC (permalink / raw)
  To: Alex Deucher, Zeng Heng, Chung, Jaehyun
  Cc: Liu, HaoPing (Alan), Li, Sun peng (Leo),
	liwei391, Pan, Xinhui, Siqueira, Rodrigo, dri-devel, airlied,
	Wang, Sherry, weiyongjun1, amd-gfx, Deucher, Alexander, Koenig,
	Christian, Kotarac, Pavle

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] gpu: color: eliminate implicit conversion about enum type
@ 2022-09-19 16:27       ` Kovac, Krunoslav
  0 siblings, 0 replies; 8+ messages in thread
From: Kovac, Krunoslav @ 2022-09-19 16:27 UTC (permalink / raw)
  To: Alex Deucher, Zeng Heng, Chung, Jaehyun
  Cc: Liu, HaoPing (Alan), Li, Sun peng (Leo),
	liwei391, Pan, Xinhui, Siqueira, Rodrigo, dri-devel, airlied,
	Wang, Sherry, weiyongjun1, amd-gfx, daniel, Deucher, Alexander,
	Wentland, Harry, Koenig, Christian, Kotarac, Pavle

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-09-19 16:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19  1:44 [PATCH] gpu: color: eliminate implicit conversion about enum type Zeng Heng
2022-09-19  2:31 ` Wei Yongjun
2022-09-19 16:05 ` Alex Deucher
2022-09-19 16:05   ` Alex Deucher
2022-09-19 16:06   ` Alex Deucher
2022-09-19 16:06     ` Alex Deucher
2022-09-19 16:27     ` Kovac, Krunoslav
2022-09-19 16:27       ` Kovac, Krunoslav

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.