[AMD Official Use Only - General]
Series is
Reviewed-by: Yang Wang <kevinyang.wang@xxxxxxx>
Best Regards,
Kevin
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Huang, Tim
Sent: Tuesday, April 30, 2024 11:28 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>
Subject: RE: [PATCH 1/2] drm/amd/pm: fix uninitialized variable warnings for vega10_hwmgr
[AMD Official Use Only - General]
[AMD Official Use Only - General]
Ping ...
> -----Original Message-----
> From: Huang, Tim <Tim.Huang@xxxxxxx>
> Sent: Sunday, April 28, 2024 4:45 PM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx>
> Subject: [PATCH 1/2] drm/amd/pm: fix uninitialized variable warnings
> for vega10_hwmgr
>
> Clear warnings that using uninitialized variable when fails to get the
> valid value from SMU.
>
> Signed-off-by: Tim Huang <Tim.Huang@xxxxxxx>
> ---
> .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 46 ++++++++++++++-----
> .../amd/pm/powerplay/smumgr/vega10_smumgr.c | 6 ++-
> 2 files changed, 39 insertions(+), 13 deletions(-)
>
> 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..488ad9de4694 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> @@ -354,13 +354,13 @@ static int
> vega10_odn_initial_default_setting(struct
> pp_hwmgr *hwmgr)
> return 0;
> }
>
> -static void vega10_init_dpm_defaults(struct pp_hwmgr *hwmgr)
> +static int vega10_init_dpm_defaults(struct pp_hwmgr *hwmgr)
> {
> struct vega10_hwmgr *data = hwmgr->backend;
> - int i;
> uint32_t sub_vendor_id, hw_revision;
> uint32_t top32, bottom32;
> struct amdgpu_device *adev = hwmgr->adev;
> + int ret, i;
>
> vega10_initialize_power_tune_defaults(hwmgr);
>
> @@ -485,9 +485,12 @@ static void vega10_init_dpm_defaults(struct
> pp_hwmgr
> *hwmgr)
> if (data->registry_data.vr0hot_enabled)
> data->smu_features[GNLD_VR0HOT].supported = true;
>
> - smum_send_msg_to_smc(hwmgr,
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetSmuVersion,
> &hwmgr->smu_version);
> + if (ret)
> + return ret;
> +
> /* ACG firmware has major version 5 */
> if ((hwmgr->smu_version & 0xff000000) == 0x5000000)
> data->smu_features[GNLD_ACG].supported = true; @@
> -505,10
> +508,16 @@ static void vega10_init_dpm_defaults(struct pp_hwmgr
> +*hwmgr)
> data->smu_features[GNLD_PCC_LIMIT].supported = true;
>
> /* Get the SN to turn into a Unique ID */
> - smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32,
> &top32);
> - smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_ReadSerialNumTop32, &top32);
> + if (ret)
> + return ret;
> +
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
> + if (ret)
> + return ret;
>
> adev->unique_id = ((uint64_t)bottom32 << 32) | top32;
> + return 0;
> }
>
> #ifdef PPLIB_VEGA10_EVV_SUPPORT
> @@ -882,7 +891,9 @@ static int vega10_hwmgr_backend_init(struct
> pp_hwmgr
> *hwmgr)
>
> vega10_set_features_platform_caps(hwmgr);
>
> - vega10_init_dpm_defaults(hwmgr);
> + result = vega10_init_dpm_defaults(hwmgr);
> + if (result)
> + return result;
>
> #ifdef PPLIB_VEGA10_EVV_SUPPORT
> /* Get leakage voltage based on leakage ID. */ @@ -3900,11
> +3911,14 @@ static int vega10_get_gpu_power(struct pp_hwmgr *hwmgr,
> uint32_t *query)
> {
> uint32_t value;
> + int ret;
>
> if (!query)
> return -EINVAL;
>
> - smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr,
> &value);
> + ret = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr,
> &value);
> + if (ret)
> + return ret;
>
> /* SMC returning actual watts, keep consistent with legacy
> asics, low 8 bit as 8 fractional bits */
> *query = value << 8;
> @@ -4800,14 +4814,16 @@ static int vega10_print_clock_levels(struct
> pp_hwmgr *hwmgr,
> uint32_t gen_speed, lane_width, current_gen_speed,
> current_lane_width;
> PPTable_t *pptable = &(data->smc_state_table.pp_table);
>
> - int i, now, size = 0, count = 0;
> + int i, ret, now, size = 0, count = 0;
>
> switch (type) {
> case PP_SCLK:
> if (data->registry_data.sclk_dpm_key_disabled)
> break;
>
> - smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetCurrentGfxclkIndex, &now);
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetCurrentGfxclkIndex, &now);
> + if (ret)
> + break;
>
> if (hwmgr->pp_one_vf &&
> (hwmgr->dpm_level ==
> AMD_DPM_FORCED_LEVEL_PROFILE_PEAK))
> @@ -4823,7 +4839,9 @@ static int vega10_print_clock_levels(struct
> pp_hwmgr *hwmgr,
> if (data->registry_data.mclk_dpm_key_disabled)
> break;
>
> - smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetCurrentUclkIndex, &now);
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetCurrentUclkIndex, &now);
> + if (ret)
> + break;
>
> for (i = 0; i < mclk_table->count; i++)
> size += sprintf(buf + size, "%d: %uMhz %s\n", @@
> -
> 4834,7 +4852,9 @@ static int vega10_print_clock_levels(struct pp_hwmgr
> *hwmgr,
> if (data->registry_data.socclk_dpm_key_disabled)
> break;
>
> - smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetCurrentSocclkIndex, &now);
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetCurrentSocclkIndex, &now);
> + if (ret)
> + break;
>
> for (i = 0; i < soc_table->count; i++)
> size += sprintf(buf + size, "%d: %uMhz %s\n", @@
> -
> 4845,8 +4865,10 @@ static int vega10_print_clock_levels(struct
> pp_hwmgr *hwmgr,
> if (data->registry_data.dcefclk_dpm_key_disabled)
> break;
>
> - smum_send_msg_to_smc_with_parameter(hwmgr,
> + ret = smum_send_msg_to_smc_with_parameter(hwmgr,
> PPSMC_MSG_GetClockFreqMHz, CLK_DCEFCLK,
> &now);
> + if (ret)
> + break;
>
> for (i = 0; i < dcef_table->count; i++)
> size += sprintf(buf + size, "%d: %uMhz %s\n",
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> index a70d73896649..f9c0f117725d 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> @@ -130,13 +130,17 @@ int vega10_get_enabled_smc_features(struct
> pp_hwmgr *hwmgr,
> uint64_t *features_enabled) {
> uint32_t enabled_features;
> + int ret;
>
> if (features_enabled == NULL)
> return -EINVAL;
>
> - smum_send_msg_to_smc(hwmgr,
> + ret = smum_send_msg_to_smc(hwmgr,
> PPSMC_MSG_GetEnabledSmuFeatures,
> &enabled_features);
> + if (ret)
> + return ret;
> +
> *features_enabled = enabled_features;
>
> return 0;
> --
> 2.39.2
