Skip to content

Commit

Permalink
drm/amd/powerplay: Zero initialize some variables
Browse files Browse the repository at this point in the history
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: ClangBuiltLinux/linux#627
Reviewed-by: Evan Quan <evan.quan@amd.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
  • Loading branch information
nathanchance authored and evadot committed Aug 4, 2020
1 parent d847ef4 commit ad1c9b2
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/amd/powerplay/navi10_ppt.c
Expand Up @@ -1555,7 +1555,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;

Expand Down

0 comments on commit ad1c9b2

Please sign in to comment.