[AMD Official Use Only - General] Hi Tim
-----Original Message----- From: Huang, Tim <tim.hu...@amd.com> Sent: Sunday, April 28, 2024 12:34 PM To: Zhang, Jesse(Jie) <jesse.zh...@amd.com>; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Koenig, Christian <christian.koe...@amd.com>; Zhang, Jesse(Jie) <jesse.zh...@amd.com>; Zhang, Jesse(Jie) <jesse.zh...@amd.com> Subject: RE: [PATCH 2/2] drm/amd/pm: fix uninitialized variable warning [AMD Official Use Only - General] > -----Original Message----- > From: Jesse Zhang <jesse.zh...@amd.com> > Sent: Friday, April 26, 2024 5:53 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Koenig, Christian > <christian.koe...@amd.com>; Huang, Tim <tim.hu...@amd.com>; Zhang, > Jesse(Jie) <jesse.zh...@amd.com>; Zhang, Jesse(Jie) > <jesse.zh...@amd.com> > Subject: [PATCH 2/2] drm/amd/pm: fix uninitialized variable warning > > Check the return of function smum_send_msg_to_smc as it may fail to > initialize the variable. > > Signed-off-by: Jesse Zhang <jesse.zh...@amd.com> > --- > .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c | 8 +++++-- > .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 21 ++++++++++++----- > -- > .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 19 +++++++++++------ > .../amd/pm/powerplay/smumgr/smu10_smumgr.c | 5 ++++- > 4 files changed, 37 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c > index 02ba68d7c654..f9f016cb60ce 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c > @@ -1310,13 +1310,17 @@ static int smu10_read_sensor(struct pp_hwmgr > *hwmgr, int idx, > > switch (idx) { > case AMDGPU_PP_SENSOR_GFX_SCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetGfxclkFrequency, &sclk); > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetGfxclkFrequency, &sclk); > + if (ret) > + break; > /* in units of 10KHZ */ > *((uint32_t *)value) = sclk * 100; > *size = 4; > break; > case AMDGPU_PP_SENSOR_GFX_MCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetFclkFrequency, &mclk); > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetFclkFrequency, &mclk); > + if (ret) > + break; > /* in units of 10KHZ */ > *((uint32_t *)value) = mclk * 100; > *size = 4; > diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > index 1fcd4451001f..5c95eda6cbd2 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > @@ -4000,6 +4000,7 @@ static int smu7_read_sensor(struct pp_hwmgr > *hwmgr, int idx, > uint32_t offset, val_vid; > struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr- > >backend); > struct amdgpu_device *adev = hwmgr->adev; > + int ret = 0; > > /* size must be at least 4 bytes for all sensors */ > if (*size < 4) > @@ -4007,12 +4008,16 @@ static int smu7_read_sensor(struct pp_hwmgr > *hwmgr, int idx, > > switch (idx) { > case AMDGPU_PP_SENSOR_GFX_SCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetSclkFrequency, &sclk); > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetSclkFrequency, &sclk); > + if (ret) > + return ret; > *((uint32_t *)value) = sclk; > *size = 4; > return 0; > case AMDGPU_PP_SENSOR_GFX_MCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetMclkFrequency, &mclk); > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetMclkFrequency, &mclk); > + if (ret) > + return ret; > *((uint32_t *)value) = mclk; > *size = 4; > return 0; > @@ -4965,13 +4970,14 @@ static int smu7_print_clock_levels(struct > pp_hwmgr *hwmgr, > struct smu7_odn_dpm_table *odn_table = &(data->odn_dpm_table); > struct phm_odn_clock_levels *odn_sclk_table = &(odn_table- > >odn_core_clock_dpm_levels); > struct phm_odn_clock_levels *odn_mclk_table = &(odn_table- > >odn_memory_clock_dpm_levels); > - int size = 0; > + int size = 0, ret = 0; > uint32_t i, now, clock, pcie_speed; > > switch (type) { > case PP_SCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetSclkFrequency, &clock); > - > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetSclkFrequency, &clock); > + if (ret) > + return ret; > for (i = 0; i < sclk_table->count; i++) { > if (clock > sclk_table->dpm_levels[i].value) > continue; @@ -4985,8 +4991,9 @@ static > int smu7_print_clock_levels(struct pp_hwmgr *hwmgr, > (i == now) ? "*" : ""); > break; > case PP_MCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetMclkFrequency, &clock); > - > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_API_GetMclkFrequency, &clock); > + if (ret) > + return ret; > for (i = 0; i < mclk_table->count; i++) { > if (clock > mclk_table->dpm_levels[i].value) > continue; diff --git > a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c > index 9f5bd998c6bf..b47e1ab12430 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c > @@ -2481,10 +2481,12 @@ static int > vega10_populate_and_upload_avfs_fuse_override(struct pp_hwmgr > *hwmgr) > struct vega10_hwmgr *data = hwmgr->backend; > AvfsFuseOverride_t *avfs_fuse_table = &(data- > >smc_state_table.avfs_fuse_override_table); > > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_ReadSerialNumTop32, &top32); > - > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_ReadSerialNumBottom32, &bottom32); > - > + result = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_ReadSerialNumTop32, &top32); > + if (result) > + return result; > + result = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_ReadSerialNumBottom32, &bottom32); > + if (result) > + result; Hi jesse, Not have a return, Is this a typo? [Zhang, Jesse(Jie)] Return check added just to fix warning about uninitialized variables top32 and botmon32. > serial_number = ((uint64_t)bottom32 << 32) | top32; > > if (pp_override_get_default_fuse_value(serial_number, &fuse) == > 0) { @@ -3924,11 +3926,16 @@ static int vega10_read_sensor(struct > pp_hwmgr *hwmgr, int idx, > > switch (idx) { > case AMDGPU_PP_SENSOR_GFX_SCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetAverageGfxclkActualFrequency, &sclk_mhz); > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetAverageGfxclkActualFrequency, &sclk_mhz); > + if (ret) > + break; > + > *((uint32_t *)value) = sclk_mhz * 100; > break; > case AMDGPU_PP_SENSOR_GFX_MCLK: > - smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetCurrentUclkIndex, &mclk_idx); > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetCurrentUclkIndex, &mclk_idx); > + if (ret) > + break; > if (mclk_idx < dpm_table->mem_table.count) { > *((uint32_t *)value) = dpm_table- > >mem_table.dpm_levels[mclk_idx].value; > *size = 4; > diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu10_smumgr.c > b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu10_smumgr.c > index 7eeab84d421a..ac9ec8257f82 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu10_smumgr.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu10_smumgr.c > @@ -185,10 +185,13 @@ static int smu10_copy_table_to_smc(struct > pp_hwmgr *hwmgr, static int smu10_verify_smc_interface(struct > pp_hwmgr > *hwmgr) { > uint32_t smc_driver_if_version; > + int ret = 0; > > - smum_send_msg_to_smc(hwmgr, > + ret = smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetDriverIfVersion, > &smc_driver_if_version); > + if (ret) > + return ret; > > if ((smc_driver_if_version != SMU10_DRIVER_IF_VERSION) && > (smc_driver_if_version != SMU10_DRIVER_IF_VERSION + 1)) { > -- > 2.25.1