Thanks Nathan. The patch is reviewed-by: Evan Quan <evan.quan@xxxxxxx>
> -----Original Message-----
> From: Nathan Chancellor <natechancellor@xxxxxxxxx>
> Sent: Monday, August 05, 2019 4:37 AM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Zhou, David(ChunMing)
> <David1.Zhou@xxxxxxx>
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; clang-built-linux@xxxxxxxxxxxxxxxx; Nathan
> Chancellor <natechancellor@xxxxxxxxx>
> Subject: [PATCH] drm/amd/powerplay: Zero initialize some variables
>
> Clang warns (only Navi warning shown but Arcturus warns as well):
>
> drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1534:4: warning:
> variable 'asic_default_power_limit' is used uninitialized whenever '?:'
> condition is false [-Wsometimes-uninitialized]
> smu_read_smc_arg(smu, &asic_default_power_limit);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:588:3:
> note:
> expanded from macro 'smu_read_smc_arg'
> ((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu),
> (arg)) : 0)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1550:30: note:
> uninitialized use occurs here
> smu->default_power_limit = asic_default_power_limit;
> ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1534:4: note:
> remove the '?:' if its condition is always true
> smu_read_smc_arg(smu, &asic_default_power_limit);
> ^
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:588:3:
> note:
> expanded from macro 'smu_read_smc_arg'
> ((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu),
> (arg)) : 0)
> ^
> drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1517:35: note:
> initialize the variable 'asic_default_power_limit' to silence this warning
> uint32_t asic_default_power_limit;
> ^
> = 0
> 1 warning generated.
>
> As the code is currently written, if read_smc_arg were ever NULL, arg would
> fail to be initialized but the code would continue executing as normal
> because the return value would just be zero.
>
> There are a few different possible solutions to resolve this class of warnings
> which have appeared in these drivers before:
>
> 1. Assume the function pointer will never be NULL and eliminate the
> wrapper macros.
>
> 2. Have the wrapper macros initialize arg when the function pointer is
> NULL.
>
> 3. Have the wrapper macros return an error code instead of 0 when the
> function pointer is NULL so that the callsites can properly bail out
> before arg can be used.
>
> 4. Initialize arg at the top of its function.
>
> Number four is the path of least resistance right now as every other change
> will be driver wide so do that here. I only make the comment now as food for
> thought.
>
> Fixes: b4af964e75c4 ("drm/amd/powerplay: make power limit retrieval as
> asic specific")
> Link: https://github.com/ClangBuiltLinux/linux/issues/627
> Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
> ---
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 2 +-
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 215f7173fca8..b92eded7374f 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -1326,7 +1326,7 @@ static int arcturus_get_power_limit(struct
> smu_context *smu,
> bool asic_default)
> {
> PPTable_t *pptable = smu->smu_table.driver_pptable;
> - uint32_t asic_default_power_limit;
> + uint32_t asic_default_power_limit = 0;
> int ret = 0;
> int power_src;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 106352a4fb82..d844bc8411aa 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -1514,7 +1514,7 @@ static int navi10_get_power_limit(struct
> smu_context *smu,
> bool asic_default)
> {
> PPTable_t *pptable = smu->smu_table.driver_pptable;
> - uint32_t asic_default_power_limit;
> + uint32_t asic_default_power_limit = 0;
> int ret = 0;
> int power_src;
>
> --
> 2.23.0.rc1
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
