Re: [PATCH] drm/amd/display: Avoid -Wenum-float-conversion in add_margin_and_round_to_dfs_grainularity()
On 2024-04-24 12:14, Nathan Chancellor wrote:
> When building with clang 19 or newer (which strengthened some of the
> enum conversion warnings for C), there is a warning (or error with
> CONFIG_WERROR=y) around doing arithmetic with an enumerated type and a
> floating point expression.
>
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c:181:58:
> error: arithmetic between enumeration type 'enum dentist_divider_range' and
> floating-point type 'double' [-Werror,-Wenum-float-conversion]
> 181 | divider = (unsigned int)(DFS_DIVIDER_RANGE_SCALE_FACTOR *
> (vco_freq_khz / clock_khz));
> | ~~ ^
> ~~
> 1 error generated.
>
> This conversion is expected due to the nature of the enumerated value
> and definition, so silence the warning by casting the enumeration to an
> integer explicitly to make it clear to the compiler.
>
> Fixes: 3df48ddedee4 ("drm/amd/display: Add new DCN401 sources")
Thanks.
Reviewed-by: Harry Wentland
In the process of merging it into amd-staging-drm-next.
Harry
> Signed-off-by: Nathan Chancellor
> ---
> Alternatively, perhaps the potential truncation could happen before the
> multiplication?
>
> divider = DFS_DIVIDER_RANGE_SCALE_FACTOR * (unsigned int)(vco_freq_khz /
> clock_khz);
>
> I suspect the result of the division is probably not very large
> (certainly not within UINT_MAX / 4), so I would not expect the
> multiplication to overflow, but I was not sure so I decided to take the
> safer, NFC change. I am happy to respin as necessary.
> ---
> .../gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c| 2
> +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
> b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
> index e6698ee65843..65eb0187e965 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
> @@ -178,7 +178,7 @@ static bool
> add_margin_and_round_to_dfs_grainularity(double clock_khz, double ma
>
> clock_khz *= 1.0 + margin;
>
> - divider = (unsigned int)(DFS_DIVIDER_RANGE_SCALE_FACTOR * (vco_freq_khz
> / clock_khz));
> + divider = (unsigned int)((int)DFS_DIVIDER_RANGE_SCALE_FACTOR *
> (vco_freq_khz / clock_khz));
>
> /* we want to floor here to get higher clock than required rather than
> lower */
> if (divider < DFS_DIVIDER_RANGE_2_START) {
>
> ---
> base-commit: d60dc4dd72412d5d9566fdf391e4202b05f88912
> change-id: 20240424-amdgpu-display-dcn401-enum-float-conversion-c09cc1826ea2
>
> Best regards,
[PATCH] drm/amd/display: Avoid -Wenum-float-conversion in add_margin_and_round_to_dfs_grainularity()
When building with clang 19 or newer (which strengthened some of the
enum conversion warnings for C), there is a warning (or error with
CONFIG_WERROR=y) around doing arithmetic with an enumerated type and a
floating point expression.
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c:181:58:
error: arithmetic between enumeration type 'enum dentist_divider_range' and
floating-point type 'double' [-Werror,-Wenum-float-conversion]
181 | divider = (unsigned int)(DFS_DIVIDER_RANGE_SCALE_FACTOR *
(vco_freq_khz / clock_khz));
| ~~ ^
~~
1 error generated.
This conversion is expected due to the nature of the enumerated value
and definition, so silence the warning by casting the enumeration to an
integer explicitly to make it clear to the compiler.
Fixes: 3df48ddedee4 ("drm/amd/display: Add new DCN401 sources")
Signed-off-by: Nathan Chancellor
---
Alternatively, perhaps the potential truncation could happen before the
multiplication?
divider = DFS_DIVIDER_RANGE_SCALE_FACTOR * (unsigned int)(vco_freq_khz /
clock_khz);
I suspect the result of the division is probably not very large
(certainly not within UINT_MAX / 4), so I would not expect the
multiplication to overflow, but I was not sure so I decided to take the
safer, NFC change. I am happy to respin as necessary.
---
.../gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
index e6698ee65843..65eb0187e965 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c
@@ -178,7 +178,7 @@ static bool add_margin_and_round_to_dfs_grainularity(double
clock_khz, double ma
clock_khz *= 1.0 + margin;
- divider = (unsigned int)(DFS_DIVIDER_RANGE_SCALE_FACTOR * (vco_freq_khz
/ clock_khz));
+ divider = (unsigned int)((int)DFS_DIVIDER_RANGE_SCALE_FACTOR *
(vco_freq_khz / clock_khz));
/* we want to floor here to get higher clock than required rather than
lower */
if (divider < DFS_DIVIDER_RANGE_2_START) {
---
base-commit: d60dc4dd72412d5d9566fdf391e4202b05f88912
change-id: 20240424-amdgpu-display-dcn401-enum-float-conversion-c09cc1826ea2
Best regards,
--
Nathan Chancellor
