Re: [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan wrote: > > Postpone some operations which are not must for hw setup to > late_init. Thus, code sharing is possible between hw_init/fini and > suspend/resume. Also this makes code more clean and readable. > > Change-Id: Id3996fd9e2dbf2ff59d8a6032cc5f6730db1295c > Signed-off-by: Evan Quan I'm having trouble parsing all of the changes in this patch. I get the general idea, but Is there any way to break this up into more logical patches or provide a more detailed description? Alex > --- > drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 327 ++--- > 1 file changed, 157 insertions(+), 170 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > index 9b81b6519a96..e55c6458b212 100644 > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > @@ -789,10 +789,36 @@ static int smu_late_init(void *handle) > { > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > struct smu_context *smu = >smu; > + int ret = 0; > > if (!smu->pm_enabled) > return 0; > > + ret = smu_set_default_od_settings(smu); > + if (ret) > + return ret; > + > + /* > +* Set initialized values (get from vbios) to dpm tables context such > as > +* gfxclk, memclk, dcefclk, and etc. And enable the DPM feature for > each > +* type of clks. > +*/ > + ret = smu_populate_smc_tables(smu); > + if (ret) > + return ret; > + > + ret = smu_init_max_sustainable_clocks(smu); > + if (ret) > + return ret; > + > + ret = smu_populate_umd_state_clk(smu); > + if (ret) > + return ret; > + > + ret = smu_get_power_limit(smu, >default_power_limit, false, > false); > + if (ret) > + return ret; > + > smu_handle_task(>smu, > smu->smu_dpm.dpm_level, > AMD_PP_TASK_COMPLETE_INIT, > @@ -1107,8 +1133,7 @@ static int smu_sw_fini(void *handle) > return 0; > } > > -static int smu_smc_table_hw_init(struct smu_context *smu, > -bool initialize) > +static int smu_internal_hw_setup(struct smu_context *smu) > { > struct amdgpu_device *adev = smu->adev; > int ret; > @@ -1122,26 +1147,22 @@ static int smu_smc_table_hw_init(struct smu_context > *smu, > if (ret) > return ret; > > - if (initialize) { > - /* get boot_values from vbios to set revision, gfxclk, and > etc. */ > - ret = smu_get_vbios_bootup_values(smu); > - if (ret) > - return ret; > - > - ret = smu_setup_pptable(smu); > - if (ret) > - return ret; > + ret = smu_set_driver_table_location(smu); > + if (ret) > + return ret; > > - /* > -* Send msg GetDriverIfVersion to check if the return value > is equal > -* with DRIVER_IF_VERSION of smc header. > -*/ > - ret = smu_check_fw_version(smu); > - if (ret) > - return ret; > - } > + /* > +* Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for > tools. > +*/ > + ret = smu_set_tool_table_location(smu); > + if (ret) > + return ret; > > - ret = smu_set_driver_table_location(smu); > + /* > +* Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify > +* pool location. > +*/ > + ret = smu_notify_memory_pool_location(smu); > if (ret) > return ret; > > @@ -1158,6 +1179,11 @@ static int smu_smc_table_hw_init(struct smu_context > *smu, > ret = smu_run_btc(smu); > if (ret) > return ret; > + > + ret = smu_feature_init_dpm(smu); > + if (ret) > + return ret; > + > ret = smu_feature_set_allowed_mask(smu); > if (ret) > return ret; > @@ -1166,12 +1192,19 @@ static int smu_smc_table_hw_init(struct smu_context > *smu, > if (ret) > return ret; > > + if (!smu_is_dpm_running(smu)) > + pr_info("dpm has been disabled\n"); > + > ret = smu_disable_umc_cdr_12gbps_workaround(smu); > if (ret) { > pr_err("Workaround failed to disable UMC CDR feature on > 12Gbps SKU!\n"); > return ret; > } > > + ret = smu_override_pcie_parameters(smu); > + if (ret) > + return ret; > + > /* > * For Navi1X, manually switch it to AC mode as PMFW > * may boot it with DC mode. > @@ -1184,6 +1217,14 @@ static int smu_smc_table_hw_init(struct smu_context > *smu, >
[PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations
Postpone some operations which are not must for hw setup to late_init. Thus, code sharing is possible between hw_init/fini and suspend/resume. Also this makes code more clean and readable. Change-Id: Id3996fd9e2dbf2ff59d8a6032cc5f6730db1295c Signed-off-by: Evan Quan --- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 327 ++--- 1 file changed, 157 insertions(+), 170 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c index 9b81b6519a96..e55c6458b212 100644 --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c @@ -789,10 +789,36 @@ static int smu_late_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = >smu; + int ret = 0; if (!smu->pm_enabled) return 0; + ret = smu_set_default_od_settings(smu); + if (ret) + return ret; + + /* +* Set initialized values (get from vbios) to dpm tables context such as +* gfxclk, memclk, dcefclk, and etc. And enable the DPM feature for each +* type of clks. +*/ + ret = smu_populate_smc_tables(smu); + if (ret) + return ret; + + ret = smu_init_max_sustainable_clocks(smu); + if (ret) + return ret; + + ret = smu_populate_umd_state_clk(smu); + if (ret) + return ret; + + ret = smu_get_power_limit(smu, >default_power_limit, false, false); + if (ret) + return ret; + smu_handle_task(>smu, smu->smu_dpm.dpm_level, AMD_PP_TASK_COMPLETE_INIT, @@ -1107,8 +1133,7 @@ static int smu_sw_fini(void *handle) return 0; } -static int smu_smc_table_hw_init(struct smu_context *smu, -bool initialize) +static int smu_internal_hw_setup(struct smu_context *smu) { struct amdgpu_device *adev = smu->adev; int ret; @@ -1122,26 +1147,22 @@ static int smu_smc_table_hw_init(struct smu_context *smu, if (ret) return ret; - if (initialize) { - /* get boot_values from vbios to set revision, gfxclk, and etc. */ - ret = smu_get_vbios_bootup_values(smu); - if (ret) - return ret; - - ret = smu_setup_pptable(smu); - if (ret) - return ret; + ret = smu_set_driver_table_location(smu); + if (ret) + return ret; - /* -* Send msg GetDriverIfVersion to check if the return value is equal -* with DRIVER_IF_VERSION of smc header. -*/ - ret = smu_check_fw_version(smu); - if (ret) - return ret; - } + /* +* Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools. +*/ + ret = smu_set_tool_table_location(smu); + if (ret) + return ret; - ret = smu_set_driver_table_location(smu); + /* +* Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify +* pool location. +*/ + ret = smu_notify_memory_pool_location(smu); if (ret) return ret; @@ -1158,6 +1179,11 @@ static int smu_smc_table_hw_init(struct smu_context *smu, ret = smu_run_btc(smu); if (ret) return ret; + + ret = smu_feature_init_dpm(smu); + if (ret) + return ret; + ret = smu_feature_set_allowed_mask(smu); if (ret) return ret; @@ -1166,12 +1192,19 @@ static int smu_smc_table_hw_init(struct smu_context *smu, if (ret) return ret; + if (!smu_is_dpm_running(smu)) + pr_info("dpm has been disabled\n"); + ret = smu_disable_umc_cdr_12gbps_workaround(smu); if (ret) { pr_err("Workaround failed to disable UMC CDR feature on 12Gbps SKU!\n"); return ret; } + ret = smu_override_pcie_parameters(smu); + if (ret) + return ret; + /* * For Navi1X, manually switch it to AC mode as PMFW * may boot it with DC mode. @@ -1184,6 +1217,14 @@ static int smu_smc_table_hw_init(struct smu_context *smu, return ret; } + ret = smu_enable_thermal_alert(smu); + if (ret) + return ret; + + ret = smu_i2c_eeprom_init(smu, >pm.smu_i2c); + if (ret) + return ret; + ret = smu_notify_display_change(smu); if (ret) return ret; @@ -1193,51 +1234,89 @@ static int smu_smc_table_hw_init(struct smu_context *smu, * SetMinDeepSleepDcefclk MSG. */ ret = smu_set_min_dcef_deep_sleep(smu); - if (ret) - return ret;