<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [Public] <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Fixed locally. Thanks! </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Lazar, Lijo <Lijo.Lazar@amd.com> Sent: Wednesday, September 22, 2021 3:32 AM To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Subject: Re: [PATCH 23/66] drm/amdgpu/amdgpu_smu: convert to IP version checking <div> </div> </div> <div class="BodyFragment"> <div class="PlainText"> On 9/21/2021 11:36 PM, Alex Deucher wrote: > Use IP versions rather than asic_type to differentiate > IP version specific features. > > v2: rebase > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 94 +++++++++++++---------- > 1 file changed, 55 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > index 04863a797115..5f372d353d9d 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > @@ -455,7 +455,8 @@ static int smu_get_power_num_states(void *handle, > > bool is_support_sw_smu(struct amdgpu_device *adev) > { > - if (adev->asic_type >= CHIP_ARCTURUS) > + if ((adev->asic_type >= CHIP_ARCTURUS) || > + (adev->ip_versions[MP1_HWIP] >= IP_VERSION(11, 0, 0))) > return true; > > return false; > @@ -575,43 +576,47 @@ static int smu_set_funcs(struct amdgpu_device *adev) > if (adev->pm.pp_feature & PP_OVERDRIVE_MASK) > smu->od_enabled = true; > > - switch (adev->asic_type) { > - case CHIP_NAVI10: > - case CHIP_NAVI14: > - case CHIP_NAVI12: > + switch (adev->ip_versions[MP1_HWIP]) { > + case IP_VERSION(11, 0, 0): > + case IP_VERSION(11, 0, 5): > + case IP_VERSION(11, 0, 9): > navi10_set_ppt_funcs(smu); > break; > - case CHIP_ARCTURUS: > - adev->pm.pp_feature &= ~PP_GFXOFF_MASK; > - arcturus_set_ppt_funcs(smu); > - /* OD is not supported on Arcturus */ > - smu->od_enabled =false; > - break; > - case CHIP_SIENNA_CICHLID: > - case CHIP_NAVY_FLOUNDER: > - case CHIP_DIMGREY_CAVEFISH: > - case CHIP_BEIGE_GOBY: > + case IP_VERSION(11, 0, 7): > + case IP_VERSION(11, 0, 11): > + case IP_VERSION(11, 0, 12): > + case IP_VERSION(11, 0, 13): > sienna_cichlid_set_ppt_funcs(smu); > break; > - case CHIP_ALDEBARAN: > - aldebaran_set_ppt_funcs(smu); > - /* Enable pp_od_clk_voltage node */ > - smu->od_enabled = true; > - break; > - case CHIP_RENOIR: > + case IP_VERSION(12, 0, 0): > renoir_set_ppt_funcs(smu); > break; > - case CHIP_VANGOGH: > + case IP_VERSION(11, 5, 0): > vangogh_set_ppt_funcs(smu); > break; > - case CHIP_YELLOW_CARP: > + case IP_VERSION(13, 0, 1): > yellow_carp_set_ppt_funcs(smu); > break; > - case CHIP_CYAN_SKILLFISH: > + case IP_VERSION(11, 0, 8): > cyan_skillfish_set_ppt_funcs(smu); > break; > default: > - return -EINVAL; > + switch (adev->asic_type) { > + case CHIP_ARCTURUS: > + adev->pm.pp_feature &= ~PP_GFXOFF_MASK; > + arcturus_set_ppt_funcs(smu); > + /* OD is not supported on Arcturus */ > + smu->od_enabled =false; > + break; > + case CHIP_ALDEBARAN: > + aldebaran_set_ppt_funcs(smu); > + /* Enable pp_od_clk_voltage node */ > + smu->od_enabled = true; > + break; > + default: > + return -EINVAL; > + } > + break; > } > > return 0; > @@ -694,7 +699,7 @@ static int smu_late_init(void *handle) > return ret; > } > > - if (adev->asic_type == CHIP_YELLOW_CARP) > + if (adev->ip_versions[MP1_HWIP] == IP_VERSION(13, 0, 1)) > return 0; > > if (!amdgpu_sriov_vf(adev) || smu->od_enabled) { > @@ -1140,8 +1145,10 @@ static int smu_smc_hw_setup(struct smu_context *smu) > if (adev->in_suspend && smu_is_dpm_running(smu)) { > dev_info(adev->dev, "dpm has been enabled\n"); > /* this is needed specifically */ > - if ((adev->asic_type >= CHIP_SIENNA_CICHLID) && > - (adev->asic_type <= CHIP_DIMGREY_CAVEFISH)) > + if ((adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 11)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 5, 0)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 12))) switch...case looks better here > ret = smu_system_features_control(smu, true); > return ret; > } > @@ -1284,7 +1291,7 @@ static int smu_start_smc_engine(struct smu_context *smu) > int ret = 0; > > if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) { > - if (adev->asic_type < CHIP_NAVI10) { > + if (adev->ip_versions[MP1_HWIP] < IP_VERSION(11, 0, 0)) { > if (smu->ppt_funcs->load_microcode) { > ret = smu->ppt_funcs->load_microcode(smu); > if (ret) > @@ -1403,8 +1410,14 @@ static int smu_disable_dpms(struct smu_context *smu) > * properly. > */ > if (smu->uploading_custom_pp_table && > - (adev->asic_type >= CHIP_NAVI10) && > - (adev->asic_type <= CHIP_BEIGE_GOBY)) > + ((adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 0)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 5)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 9)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 11)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 5, 0)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 12)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 13)))) Same - switch...case > return smu_disable_all_features_with_exception(smu, > true, > SMU_FEATURE_COUNT); > @@ -1413,9 +1426,11 @@ static int smu_disable_dpms(struct smu_context *smu) > * For Sienna_Cichlid, PMFW will handle the features disablement properly > * on BACO in. Driver involvement is unnecessary. > */ > - if (((adev->asic_type == CHIP_SIENNA_CICHLID) || > - ((adev->asic_type >= CHIP_NAVI10) && (adev->asic_type <= CHIP_NAVI12))) && > - use_baco) > + if (((adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 0)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 5)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 9))) && > + use_baco) Here as well. Thanks, Lijo > return smu_disable_all_features_with_exception(smu, > true, > SMU_FEATURE_BACO_BIT); > @@ -1436,7 +1451,7 @@ static int smu_disable_dpms(struct smu_context *smu) > dev_err(adev->dev, "Failed to disable smu features.\n"); > } > > - if (adev->asic_type >= CHIP_NAVI10 && > + if (adev->ip_versions[MP1_HWIP] >= IP_VERSION(11, 0, 0) && > adev->gfx.rlc.funcs->stop) > adev->gfx.rlc.funcs->stop(adev); > > @@ -2229,6 +2244,7 @@ int smu_get_power_limit(void *handle, > enum pp_power_type pp_power_type) > { > struct smu_context *smu = handle; > + struct amdgpu_device *adev = smu->adev; > enum smu_ppt_limit_level limit_level; > uint32_t limit_type; > int ret = 0; > @@ -2273,10 +2289,10 @@ int smu_get_power_limit(void *handle, > switch (limit_level) { > case SMU_PPT_LIMIT_CURRENT: > if ((smu->adev->asic_type == CHIP_ALDEBARAN) || > - (smu->adev->asic_type == CHIP_SIENNA_CICHLID) || > - (smu->adev->asic_type == CHIP_NAVY_FLOUNDER) || > - (smu->adev->asic_type == CHIP_DIMGREY_CAVEFISH) || > - (smu->adev->asic_type == CHIP_BEIGE_GOBY)) > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 11)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 12)) || > + (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 13))) > ret = smu_get_asic_power_limits(smu, > &smu->current_power_limit, > NULL, > </div> </div> </div> </body> </html>