* [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard
@ 2020-06-01 7:29 Evan Quan
2020-06-01 7:29 ` [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes Evan Quan
` (8 more replies)
0 siblings, 9 replies; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:29 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
These APIs internally guard they will not break ARCTURUS.
Change-Id: Ib6775c1c8c5211ea45db6c3fb604a8279411ab37
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 38 +++++++++-----------
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 8 ++---
2 files changed, 20 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 5294aa7cdde1..4998ea942760 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1049,11 +1049,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
return 0;
}
- if (adev->asic_type != CHIP_ARCTURUS) {
- ret = smu_init_display_count(smu, 0);
- if (ret)
- return ret;
- }
+ ret = smu_init_display_count(smu, 0);
+ if (ret)
+ return ret;
if (initialize) {
/* get boot_values from vbios to set revision, gfxclk, and etc. */
@@ -1159,19 +1157,17 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
}
}
- if (adev->asic_type != CHIP_ARCTURUS) {
- ret = smu_notify_display_change(smu);
- if (ret)
- return ret;
+ ret = smu_notify_display_change(smu);
+ if (ret)
+ return ret;
- /*
- * Set min deep sleep dce fclk with bootup value from vbios via
- * SetMinDeepSleepDcefclk MSG.
- */
- ret = smu_set_min_dcef_deep_sleep(smu);
- if (ret)
- return ret;
- }
+ /*
+ * Set min deep sleep dce fclk with bootup value from vbios via
+ * SetMinDeepSleepDcefclk MSG.
+ */
+ ret = smu_set_min_dcef_deep_sleep(smu);
+ if (ret)
+ return ret;
/*
* Set initialized values (get from vbios) to dpm tables context such as
@@ -1188,11 +1184,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
return ret;
}
- if (adev->asic_type != CHIP_ARCTURUS) {
- ret = smu_override_pcie_parameters(smu);
- if (ret)
- return ret;
- }
+ ret = smu_override_pcie_parameters(smu);
+ if (ret)
+ return ret;
ret = smu_set_default_od_settings(smu, initialize);
if (ret)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 302b7e9cb5ba..e856ad36ab01 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2429,16 +2429,16 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.populate_smc_tables = smu_v11_0_populate_smc_pptable,
.check_fw_version = smu_v11_0_check_fw_version,
.write_pptable = smu_v11_0_write_pptable,
- .set_min_dcef_deep_sleep = smu_v11_0_set_min_dcef_deep_sleep,
+ .set_min_dcef_deep_sleep = NULL,
.set_driver_table_location = smu_v11_0_set_driver_table_location,
.set_tool_table_location = smu_v11_0_set_tool_table_location,
.notify_memory_pool_location = smu_v11_0_notify_memory_pool_location,
.system_features_control = smu_v11_0_system_features_control,
.send_smc_msg_with_param = smu_v11_0_send_msg_with_param,
- .init_display_count = smu_v11_0_init_display_count,
+ .init_display_count = NULL,
.set_allowed_mask = smu_v11_0_set_allowed_mask,
.get_enabled_mask = smu_v11_0_get_enabled_mask,
- .notify_display_change = smu_v11_0_notify_display_change,
+ .notify_display_change = NULL,
.set_power_limit = smu_v11_0_set_power_limit,
.get_current_clk_freq = smu_v11_0_get_current_clk_freq,
.init_max_sustainable_clocks = smu_v11_0_init_max_sustainable_clocks,
@@ -2462,7 +2462,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.baco_exit = smu_v11_0_baco_exit,
.get_dpm_ultimate_freq = smu_v11_0_get_dpm_ultimate_freq,
.set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
- .override_pcie_parameters = smu_v11_0_override_pcie_parameters,
+ .override_pcie_parameters = NULL,
.get_pptable_power_limit = arcturus_get_pptable_power_limit,
.set_df_cstate = arcturus_set_df_cstate,
.allow_xgmi_power_down = arcturus_allow_xgmi_power_down,
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
@ 2020-06-01 7:29 ` Evan Quan
2020-06-02 14:54 ` Alex Deucher
2020-06-01 7:29 ` [PATCH 3/9] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:29 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
To make code more clean and readable by moving ASIC
specific code to its own file, more code sharing and
dropping unused code.
Change-Id: I6b299f9e98c7678b48281cbed9beb17b644bb4cc
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 213 ++++++++-------------
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 19 ++
2 files changed, 102 insertions(+), 130 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 4998ea942760..b4f108cb52fa 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -817,22 +817,10 @@ int smu_get_atom_data_table(struct smu_context *smu, uint32_t table,
return 0;
}
-static int smu_initialize_pptable(struct smu_context *smu)
-{
- /* TODO */
- return 0;
-}
-
static int smu_smc_table_sw_init(struct smu_context *smu)
{
int ret;
- ret = smu_initialize_pptable(smu);
- if (ret) {
- pr_err("Failed to init smu_initialize_pptable!\n");
- return ret;
- }
-
/**
* Create smu_table structure, and init smc tables such as
* TABLE_PPTABLE, TABLE_WATERMARKS, TABLE_SMU_METRICS, and etc.
@@ -860,6 +848,12 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
{
int ret;
+ ret = smu_fini_power(smu);
+ if (ret) {
+ pr_err("Failed to init smu_fini_power!\n");
+ return ret;
+ }
+
ret = smu_fini_smc_tables(smu);
if (ret) {
pr_err("Failed to smu_fini_smc_tables!\n");
@@ -950,12 +944,6 @@ static int smu_sw_fini(void *handle)
return ret;
}
- ret = smu_fini_power(smu);
- if (ret) {
- pr_err("Failed to init smu_fini_power!\n");
- return ret;
- }
-
return 0;
}
@@ -1125,36 +1113,22 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
if (ret)
return ret;
- if (adev->asic_type == CHIP_NAVI10) {
- if ((adev->pdev->device == 0x731f && (adev->pdev->revision == 0xc2 ||
- adev->pdev->revision == 0xc3 ||
- adev->pdev->revision == 0xca ||
- adev->pdev->revision == 0xcb)) ||
- (adev->pdev->device == 0x66af && (adev->pdev->revision == 0xf3 ||
- adev->pdev->revision == 0xf4 ||
- adev->pdev->revision == 0xf5 ||
- adev->pdev->revision == 0xf6))) {
- 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_disable_umc_cdr_12gbps_workaround(smu);
+ if (ret) {
+ pr_err("Workaround failed to disable UMC CDR feature on 12Gbps SKU!\n");
+ return ret;
}
- if (smu->ppt_funcs->set_power_source) {
- /*
- * For Navi1X, manually switch it to AC mode as PMFW
- * may boot it with DC mode.
- */
- if (adev->pm.ac_power)
- ret = smu_set_power_source(smu, SMU_POWER_SOURCE_AC);
- else
- ret = smu_set_power_source(smu, SMU_POWER_SOURCE_DC);
- if (ret) {
- pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ? "AC" : "DC");
- return ret;
- }
+ /*
+ * For Navi1X, manually switch it to AC mode as PMFW
+ * may boot it with DC mode.
+ */
+ ret = smu_set_power_source(smu,
+ adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
+ SMU_POWER_SOURCE_DC);
+ if (ret) {
+ pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ? "AC" : "DC");
+ return ret;
}
ret = smu_notify_display_change(smu);
@@ -1362,9 +1336,65 @@ static int smu_hw_init(void *handle)
return ret;
}
-static int smu_stop_dpms(struct smu_context *smu)
+static int smu_disable_dpms(struct smu_context *smu)
{
- return smu_system_features_control(smu, false);
+ struct amdgpu_device *adev = smu->adev;
+ int ret = 0;
+ bool use_baco = !smu->is_apu &&
+ ((adev->in_gpu_reset &&
+ (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
+ ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
+
+ /*
+ * For custom pptable uploading, skip the DPM features
+ * disable process on Navi1x ASICs.
+ * - As the gfx related features are under control of
+ * RLC on those ASICs. RLC reinitialization will be
+ * needed to reenable them. That will cost much more
+ * efforts.
+ *
+ * - SMU firmware can handle the DPM reenablement
+ * properly.
+ */
+ if (smu->uploading_custom_pp_table &&
+ (adev->asic_type >= CHIP_NAVI10) &&
+ (adev->asic_type <= CHIP_NAVI12))
+ return 0;
+
+ /*
+ * Disable all enabled SMU features.
+ * This should be handled in SMU FW, as a backup
+ * driver can issue call to SMU FW until sequence
+ * in SMU FW is operational.
+ */
+ ret = smu_system_features_control(smu, false);
+ if (ret) {
+ pr_err("Failed to disable smu features.\n");
+ return ret;
+ }
+
+ /*
+ * For baco, need to leave BACO feature enabled
+ *
+ * Correct the way for checking whether SMU_FEATURE_BACO_BIT
+ * is supported.
+ *
+ * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
+ * always return false as the 'smu_system_features_control(smu, false)'
+ * was just issued above which disabled all SMU features.
+ *
+ * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
+ * now for the checking.
+ */
+ if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
+ ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
+ if (ret) {
+ pr_warn("set BACO feature enabled failed, return %d\n", ret);
+ return ret;
+ }
+ }
+
+ return ret;
}
static int smu_hw_fini(void *handle)
@@ -1396,25 +1426,10 @@ static int smu_hw_fini(void *handle)
return ret;
}
- /*
- * For custom pptable uploading, skip the DPM features
- * disable process on Navi1x ASICs.
- * - As the gfx related features are under control of
- * RLC on those ASICs. RLC reinitialization will be
- * needed to reenable them. That will cost much more
- * efforts.
- *
- * - SMU firmware can handle the DPM reenablement
- * properly.
- */
- if (!smu->uploading_custom_pp_table ||
- !((adev->asic_type >= CHIP_NAVI10) &&
- (adev->asic_type <= CHIP_NAVI12))) {
- ret = smu_stop_dpms(smu);
- if (ret) {
- pr_warn("Fail to stop Dpms!\n");
- return ret;
- }
+ ret = smu_disable_dpms(smu);
+ if (ret) {
+ pr_warn("Fail to stop Dpms!\n");
+ return ret;
}
kfree(table_context->driver_pptable);
@@ -1453,68 +1468,6 @@ int smu_reset(struct smu_context *smu)
return ret;
}
-static int smu_disable_dpm(struct smu_context *smu)
-{
- struct amdgpu_device *adev = smu->adev;
- uint32_t smu_version;
- int ret = 0;
- bool use_baco = !smu->is_apu &&
- ((adev->in_gpu_reset &&
- (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
- ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
-
- ret = smu_get_smc_version(smu, NULL, &smu_version);
- if (ret) {
- pr_err("Failed to get smu version.\n");
- return ret;
- }
-
- /*
- * Disable all enabled SMU features.
- * This should be handled in SMU FW, as a backup
- * driver can issue call to SMU FW until sequence
- * in SMU FW is operational.
- */
- ret = smu_system_features_control(smu, false);
- if (ret) {
- pr_err("Failed to disable smu features.\n");
- return ret;
- }
-
- /*
- * Arcturus does not have BACO bit in disable feature mask.
- * Enablement of BACO bit on Arcturus should be skipped.
- */
- if (adev->asic_type == CHIP_ARCTURUS) {
- if (use_baco && (smu_version > 0x360e00))
- return 0;
- }
-
- /* For baco, need to leave BACO feature enabled */
- if (use_baco) {
- /*
- * Correct the way for checking whether SMU_FEATURE_BACO_BIT
- * is supported.
- *
- * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
- * always return false as the 'smu_system_features_control(smu, false)'
- * was just issued above which disabled all SMU features.
- *
- * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
- * now for the checking.
- */
- if (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0) {
- ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
- if (ret) {
- pr_warn("set BACO feature enabled failed, return %d\n", ret);
- return ret;
- }
- }
- }
-
- return ret;
-}
-
static int smu_suspend(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -1537,7 +1490,7 @@ static int smu_suspend(void *handle)
return ret;
}
- ret = smu_disable_dpm(smu);
+ ret = smu_disable_dpms(smu);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 68142f6798c6..652728f18271 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -2209,12 +2209,31 @@ static int navi10_dummy_pstate_control(struct smu_context *smu, bool enable)
return result;
}
+static inline bool navi10_need_umc_cdr_12gbps_workaround(struct amdgpu_device *adev)
+{
+ if (adev->asic_type != CHIP_NAVI10)
+ return false;
+
+ if ((adev->pdev->device == 0x731f && (adev->pdev->revision == 0xc2 ||
+ adev->pdev->revision == 0xc3 || adev->pdev->revision == 0xca ||
+ adev->pdev->revision == 0xcb)) ||
+ (adev->pdev->device == 0x66af && (adev->pdev->revision == 0xf3 ||
+ adev->pdev->revision == 0xf4 || adev->pdev->revision == 0xf5 ||
+ adev->pdev->revision == 0xf6)))
+ return true;
+ else
+ return false;
+}
+
static int navi10_disable_umc_cdr_12gbps_workaround(struct smu_context *smu)
{
uint32_t uclk_count, uclk_min, uclk_max;
uint32_t smu_version;
int ret = 0;
+ if (!navi10_need_umc_cdr_12gbps_workaround(smu->adev))
+ return 0;
+
ret = smu_get_smc_version(smu, NULL, &smu_version);
if (ret)
return ret;
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/9] drm/amd/powerplay: centralize all buffer allocation in sw_init phase
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
2020-06-01 7:29 ` [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes Evan Quan
@ 2020-06-01 7:29 ` Evan Quan
2020-06-02 15:00 ` Alex Deucher
2020-06-01 7:29 ` [PATCH 4/9] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:29 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
To fit common design. And this can simplify the buffer deallocation.
Change-Id: Iee682e76aadb5f34861d69d5794ced44f0a78789
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 330 ++++++++++-----------
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 105 ++++---
2 files changed, 223 insertions(+), 212 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index b4f108cb52fa..70c7b3fdee79 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -817,6 +817,147 @@ int smu_get_atom_data_table(struct smu_context *smu, uint32_t table,
return 0;
}
+static int smu_init_fb_allocations(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+ struct smu_table_context *smu_table = &smu->smu_table;
+ struct smu_table *tables = smu_table->tables;
+ struct smu_table *driver_table = &(smu_table->driver_table);
+ uint32_t max_table_size = 0;
+ int ret, i;
+
+ /* VRAM allocation for tool table */
+ if (tables[SMU_TABLE_PMSTATUSLOG].size) {
+ ret = amdgpu_bo_create_kernel(adev,
+ tables[SMU_TABLE_PMSTATUSLOG].size,
+ tables[SMU_TABLE_PMSTATUSLOG].align,
+ tables[SMU_TABLE_PMSTATUSLOG].domain,
+ &tables[SMU_TABLE_PMSTATUSLOG].bo,
+ &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
+ &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
+ if (ret) {
+ pr_err("VRAM allocation for tool table failed!\n");
+ return ret;
+ }
+ }
+
+ /* VRAM allocation for driver table */
+ for (i = 0; i < SMU_TABLE_COUNT; i++) {
+ if (tables[i].size == 0)
+ continue;
+
+ if (i == SMU_TABLE_PMSTATUSLOG)
+ continue;
+
+ if (max_table_size < tables[i].size)
+ max_table_size = tables[i].size;
+ }
+
+ driver_table->size = max_table_size;
+ driver_table->align = PAGE_SIZE;
+ driver_table->domain = AMDGPU_GEM_DOMAIN_VRAM;
+
+ ret = amdgpu_bo_create_kernel(adev,
+ driver_table->size,
+ driver_table->align,
+ driver_table->domain,
+ &driver_table->bo,
+ &driver_table->mc_address,
+ &driver_table->cpu_addr);
+ if (ret) {
+ pr_err("VRAM allocation for driver table failed!\n");
+ if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
+ amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
+ &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
+ &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
+ }
+
+ return ret;
+}
+
+static int smu_fini_fb_allocations(struct smu_context *smu)
+{
+ struct smu_table_context *smu_table = &smu->smu_table;
+ struct smu_table *tables = smu_table->tables;
+ struct smu_table *driver_table = &(smu_table->driver_table);
+
+ if (!tables)
+ return 0;
+
+ if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
+ amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
+ &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
+ &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
+
+ amdgpu_bo_free_kernel(&driver_table->bo,
+ &driver_table->mc_address,
+ &driver_table->cpu_addr);
+
+ return 0;
+}
+
+/**
+ * smu_alloc_memory_pool - allocate memory pool in the system memory
+ *
+ * @smu: amdgpu_device pointer
+ *
+ * This memory pool will be used for SMC use and msg SetSystemVirtualDramAddr
+ * and DramLogSetDramAddr can notify it changed.
+ *
+ * Returns 0 on success, error on failure.
+ */
+static int smu_alloc_memory_pool(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+ struct smu_table_context *smu_table = &smu->smu_table;
+ struct smu_table *memory_pool = &smu_table->memory_pool;
+ uint64_t pool_size = smu->pool_size;
+ int ret = 0;
+
+ if (pool_size == SMU_MEMORY_POOL_SIZE_ZERO)
+ return ret;
+
+ memory_pool->size = pool_size;
+ memory_pool->align = PAGE_SIZE;
+ memory_pool->domain = AMDGPU_GEM_DOMAIN_GTT;
+
+ switch (pool_size) {
+ case SMU_MEMORY_POOL_SIZE_256_MB:
+ case SMU_MEMORY_POOL_SIZE_512_MB:
+ case SMU_MEMORY_POOL_SIZE_1_GB:
+ case SMU_MEMORY_POOL_SIZE_2_GB:
+ ret = amdgpu_bo_create_kernel(adev,
+ memory_pool->size,
+ memory_pool->align,
+ memory_pool->domain,
+ &memory_pool->bo,
+ &memory_pool->mc_address,
+ &memory_pool->cpu_addr);
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static int smu_free_memory_pool(struct smu_context *smu)
+{
+ struct smu_table_context *smu_table = &smu->smu_table;
+ struct smu_table *memory_pool = &smu_table->memory_pool;
+
+ if (memory_pool->size == SMU_MEMORY_POOL_SIZE_ZERO)
+ return 0;
+
+ amdgpu_bo_free_kernel(&memory_pool->bo,
+ &memory_pool->mc_address,
+ &memory_pool->cpu_addr);
+
+ memset(memory_pool, 0, sizeof(struct smu_table));
+
+ return 0;
+}
+
static int smu_smc_table_sw_init(struct smu_context *smu)
{
int ret;
@@ -841,6 +982,17 @@ static int smu_smc_table_sw_init(struct smu_context *smu)
return ret;
}
+ /*
+ * allocate vram bos to store smc table contents.
+ */
+ ret = smu_init_fb_allocations(smu);
+ if (ret)
+ return ret;
+
+ ret = smu_alloc_memory_pool(smu);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -848,6 +1000,14 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
{
int ret;
+ ret = smu_free_memory_pool(smu);
+ if (ret)
+ return ret;
+
+ ret = smu_fini_fb_allocations(smu);
+ if (ret)
+ return ret;
+
ret = smu_fini_power(smu);
if (ret) {
pr_err("Failed to init smu_fini_power!\n");
@@ -947,85 +1107,6 @@ static int smu_sw_fini(void *handle)
return 0;
}
-static int smu_init_fb_allocations(struct smu_context *smu)
-{
- struct amdgpu_device *adev = smu->adev;
- struct smu_table_context *smu_table = &smu->smu_table;
- struct smu_table *tables = smu_table->tables;
- struct smu_table *driver_table = &(smu_table->driver_table);
- uint32_t max_table_size = 0;
- int ret, i;
-
- /* VRAM allocation for tool table */
- if (tables[SMU_TABLE_PMSTATUSLOG].size) {
- ret = amdgpu_bo_create_kernel(adev,
- tables[SMU_TABLE_PMSTATUSLOG].size,
- tables[SMU_TABLE_PMSTATUSLOG].align,
- tables[SMU_TABLE_PMSTATUSLOG].domain,
- &tables[SMU_TABLE_PMSTATUSLOG].bo,
- &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
- &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
- if (ret) {
- pr_err("VRAM allocation for tool table failed!\n");
- return ret;
- }
- }
-
- /* VRAM allocation for driver table */
- for (i = 0; i < SMU_TABLE_COUNT; i++) {
- if (tables[i].size == 0)
- continue;
-
- if (i == SMU_TABLE_PMSTATUSLOG)
- continue;
-
- if (max_table_size < tables[i].size)
- max_table_size = tables[i].size;
- }
-
- driver_table->size = max_table_size;
- driver_table->align = PAGE_SIZE;
- driver_table->domain = AMDGPU_GEM_DOMAIN_VRAM;
-
- ret = amdgpu_bo_create_kernel(adev,
- driver_table->size,
- driver_table->align,
- driver_table->domain,
- &driver_table->bo,
- &driver_table->mc_address,
- &driver_table->cpu_addr);
- if (ret) {
- pr_err("VRAM allocation for driver table failed!\n");
- if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
- amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
- &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
- &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
- }
-
- return ret;
-}
-
-static int smu_fini_fb_allocations(struct smu_context *smu)
-{
- struct smu_table_context *smu_table = &smu->smu_table;
- struct smu_table *tables = smu_table->tables;
- struct smu_table *driver_table = &(smu_table->driver_table);
-
- if (!tables)
- return 0;
-
- if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
- amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
- &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
- &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
-
- amdgpu_bo_free_kernel(&driver_table->bo,
- &driver_table->mc_address,
- &driver_table->cpu_addr);
-
- return 0;
-}
-
static int smu_smc_table_hw_init(struct smu_context *smu,
bool initialize)
{
@@ -1063,13 +1144,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
if (ret)
return ret;
- /*
- * allocate vram bos to store smc table contents.
- */
- ret = smu_init_fb_allocations(smu);
- if (ret)
- return ret;
-
/*
* Parse pptable format and fill PPTable_t smc_pptable to
* smu_table_context structure. And read the smc_dpm_table from vbios,
@@ -1187,68 +1261,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
return ret;
}
-/**
- * smu_alloc_memory_pool - allocate memory pool in the system memory
- *
- * @smu: amdgpu_device pointer
- *
- * This memory pool will be used for SMC use and msg SetSystemVirtualDramAddr
- * and DramLogSetDramAddr can notify it changed.
- *
- * Returns 0 on success, error on failure.
- */
-static int smu_alloc_memory_pool(struct smu_context *smu)
-{
- struct amdgpu_device *adev = smu->adev;
- struct smu_table_context *smu_table = &smu->smu_table;
- struct smu_table *memory_pool = &smu_table->memory_pool;
- uint64_t pool_size = smu->pool_size;
- int ret = 0;
-
- if (pool_size == SMU_MEMORY_POOL_SIZE_ZERO)
- return ret;
-
- memory_pool->size = pool_size;
- memory_pool->align = PAGE_SIZE;
- memory_pool->domain = AMDGPU_GEM_DOMAIN_GTT;
-
- switch (pool_size) {
- case SMU_MEMORY_POOL_SIZE_256_MB:
- case SMU_MEMORY_POOL_SIZE_512_MB:
- case SMU_MEMORY_POOL_SIZE_1_GB:
- case SMU_MEMORY_POOL_SIZE_2_GB:
- ret = amdgpu_bo_create_kernel(adev,
- memory_pool->size,
- memory_pool->align,
- memory_pool->domain,
- &memory_pool->bo,
- &memory_pool->mc_address,
- &memory_pool->cpu_addr);
- break;
- default:
- break;
- }
-
- return ret;
-}
-
-static int smu_free_memory_pool(struct smu_context *smu)
-{
- struct smu_table_context *smu_table = &smu->smu_table;
- struct smu_table *memory_pool = &smu_table->memory_pool;
-
- if (memory_pool->size == SMU_MEMORY_POOL_SIZE_ZERO)
- return 0;
-
- amdgpu_bo_free_kernel(&memory_pool->bo,
- &memory_pool->mc_address,
- &memory_pool->cpu_addr);
-
- memset(memory_pool, 0, sizeof(struct smu_table));
-
- return 0;
-}
-
static int smu_start_smc_engine(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
@@ -1306,10 +1318,6 @@ static int smu_hw_init(void *handle)
if (ret)
goto failed;
- ret = smu_alloc_memory_pool(smu);
- if (ret)
- goto failed;
-
/*
* Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
* pool location.
@@ -1401,7 +1409,6 @@ static int smu_hw_fini(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;
- struct smu_table_context *table_context = &smu->smu_table;
int ret = 0;
if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
@@ -1432,23 +1439,6 @@ static int smu_hw_fini(void *handle)
return ret;
}
- kfree(table_context->driver_pptable);
- table_context->driver_pptable = NULL;
-
- kfree(table_context->max_sustainable_clocks);
- table_context->max_sustainable_clocks = NULL;
-
- kfree(table_context->overdrive_table);
- table_context->overdrive_table = NULL;
-
- ret = smu_fini_fb_allocations(smu);
- if (ret)
- return ret;
-
- ret = smu_free_memory_pool(smu);
- if (ret)
- return ret;
-
return 0;
}
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index d6bdd2126f72..3b22f66e3fbc 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -432,25 +432,67 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
struct smu_table *tables = NULL;
int ret = 0;
- if (smu_table->tables)
- return -EINVAL;
-
tables = kcalloc(SMU_TABLE_COUNT, sizeof(struct smu_table),
GFP_KERNEL);
- if (!tables)
- return -ENOMEM;
-
+ if (!tables) {
+ ret = -ENOMEM;
+ goto err0_out;
+ }
smu_table->tables = tables;
ret = smu_tables_init(smu, tables);
if (ret)
- return ret;
+ goto err1_out;
ret = smu_v11_0_init_dpm_context(smu);
if (ret)
- return ret;
+ goto err1_out;
+
+ smu_table->driver_pptable =
+ kzalloc(tables[SMU_TABLE_PPTABLE].size, GFP_KERNEL);
+ if (!smu_table->driver_pptable) {
+ ret = -ENOMEM;
+ goto err2_out;
+ }
+
+ smu_table->max_sustainable_clocks =
+ kzalloc(sizeof(struct smu_11_0_max_sustainable_clocks), GFP_KERNEL);
+ if (!smu_table->max_sustainable_clocks) {
+ ret = -ENOMEM;
+ goto err3_out;
+ }
+
+ /* Arcturus does not support OVERDRIVE */
+ if (tables[SMU_TABLE_OVERDRIVE].size) {
+ smu_table->overdrive_table =
+ kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
+ if (!smu_table->overdrive_table) {
+ ret = -ENOMEM;
+ goto err4_out;
+ }
+
+ smu_table->boot_overdrive_table =
+ kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
+ if (!smu_table->boot_overdrive_table) {
+ ret = -ENOMEM;
+ goto err5_out;
+ }
+ }
return 0;
+
+err5_out:
+ kfree(smu_table->overdrive_table);
+err4_out:
+ kfree(smu_table->max_sustainable_clocks);
+err3_out:
+ kfree(smu_table->driver_pptable);
+err2_out:
+ smu_v11_0_fini_dpm_context(smu);
+err1_out:
+ kfree(tables);
+err0_out:
+ return ret;
}
int smu_v11_0_fini_smc_tables(struct smu_context *smu)
@@ -461,6 +503,17 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
if (!smu_table->tables)
return -EINVAL;
+ kfree(smu_table->boot_overdrive_table);
+ kfree(smu_table->overdrive_table);
+ kfree(smu_table->max_sustainable_clocks);
+ kfree(smu_table->driver_pptable);
+ smu_table->boot_overdrive_table = NULL;
+ smu_table->overdrive_table = NULL;
+ smu_table->max_sustainable_clocks = NULL;
+ smu_table->driver_pptable = NULL;
+ kfree(smu_table->hardcode_pptable);
+ smu_table->hardcode_pptable = NULL;
+
kfree(smu_table->tables);
kfree(smu_table->metrics_table);
kfree(smu_table->watermarks_table);
@@ -723,18 +776,6 @@ int smu_v11_0_parse_pptable(struct smu_context *smu)
{
int ret;
- struct smu_table_context *table_context = &smu->smu_table;
- struct smu_table *table = &table_context->tables[SMU_TABLE_PPTABLE];
-
- /* during TDR we need to free and alloc the pptable */
- if (table_context->driver_pptable)
- kfree(table_context->driver_pptable);
-
- table_context->driver_pptable = kzalloc(table->size, GFP_KERNEL);
-
- if (!table_context->driver_pptable)
- return -ENOMEM;
-
ret = smu_store_powerplay_table(smu);
if (ret)
return -EINVAL;
@@ -975,17 +1016,10 @@ smu_v11_0_get_max_sustainable_clock(struct smu_context *smu, uint32_t *clock,
int smu_v11_0_init_max_sustainable_clocks(struct smu_context *smu)
{
- struct smu_11_0_max_sustainable_clocks *max_sustainable_clocks;
+ struct smu_11_0_max_sustainable_clocks *max_sustainable_clocks =
+ smu->smu_table.max_sustainable_clocks;
int ret = 0;
- if (!smu->smu_table.max_sustainable_clocks)
- max_sustainable_clocks = kzalloc(sizeof(struct smu_11_0_max_sustainable_clocks),
- GFP_KERNEL);
- else
- max_sustainable_clocks = smu->smu_table.max_sustainable_clocks;
-
- smu->smu_table.max_sustainable_clocks = (void *)max_sustainable_clocks;
-
max_sustainable_clocks->uclock = smu->smu_table.boot_values.uclk / 100;
max_sustainable_clocks->soc_clock = smu->smu_table.boot_values.socclk / 100;
max_sustainable_clocks->dcef_clock = smu->smu_table.boot_values.dcefclk / 100;
@@ -1930,24 +1964,11 @@ int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool initialize,
int ret = 0;
if (initialize) {
- if (table_context->overdrive_table) {
- return -EINVAL;
- }
- table_context->overdrive_table = kzalloc(overdrive_table_size, GFP_KERNEL);
- if (!table_context->overdrive_table) {
- return -ENOMEM;
- }
ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, false);
if (ret) {
pr_err("Failed to export overdrive table!\n");
return ret;
}
- if (!table_context->boot_overdrive_table) {
- table_context->boot_overdrive_table = kmemdup(table_context->overdrive_table, overdrive_table_size, GFP_KERNEL);
- if (!table_context->boot_overdrive_table) {
- return -ENOMEM;
- }
- }
}
ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, true);
if (ret) {
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/9] drm/amd/powerplay: clean up the APIs for bootup clocks
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
2020-06-01 7:29 ` [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes Evan Quan
2020-06-01 7:29 ` [PATCH 3/9] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
@ 2020-06-01 7:29 ` Evan Quan
2020-06-02 15:01 ` Alex Deucher
2020-06-01 7:29 ` [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:29 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
Combine and simplify the logics for retrieving bootup
clocks.
Change-Id: Ifca28c454f3769dece0cc705ba054ff34db0ab60
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 -
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 1 -
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 -
drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 2 -
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 1 -
drivers/gpu/drm/amd/powerplay/smu_internal.h | 2 -
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 141 +++++++-----------
7 files changed, 51 insertions(+), 101 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 70c7b3fdee79..9bafa6b3e123 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1132,10 +1132,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
if (ret)
return ret;
- ret = smu_get_clk_info_from_vbios(smu);
- if (ret)
- return ret;
-
/*
* check if the format_revision in vbios is up to pptable header
* version, and the structure size is not 0.
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index e856ad36ab01..902c8cfa4a3b 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2423,7 +2423,6 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.check_fw_status = smu_v11_0_check_fw_status,
.setup_pptable = smu_v11_0_setup_pptable,
.get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
- .get_clk_info_from_vbios = smu_v11_0_get_clk_info_from_vbios,
.check_pptable = smu_v11_0_check_pptable,
.parse_pptable = smu_v11_0_parse_pptable,
.populate_smc_tables = smu_v11_0_populate_smc_pptable,
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 5bb1ac821aeb..223678e329a5 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -505,7 +505,6 @@ struct pptable_funcs {
int (*check_fw_status)(struct smu_context *smu);
int (*setup_pptable)(struct smu_context *smu);
int (*get_vbios_bootup_values)(struct smu_context *smu);
- int (*get_clk_info_from_vbios)(struct smu_context *smu);
int (*check_pptable)(struct smu_context *smu);
int (*parse_pptable)(struct smu_context *smu);
int (*populate_smc_tables)(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
index 71f829ab306e..5b785816aa64 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -161,8 +161,6 @@ int smu_v11_0_setup_pptable(struct smu_context *smu);
int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu);
-int smu_v11_0_get_clk_info_from_vbios(struct smu_context *smu);
-
int smu_v11_0_check_pptable(struct smu_context *smu);
int smu_v11_0_parse_pptable(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 652728f18271..bea6a96b5afb 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -2320,7 +2320,6 @@ static const struct pptable_funcs navi10_ppt_funcs = {
.check_fw_status = smu_v11_0_check_fw_status,
.setup_pptable = smu_v11_0_setup_pptable,
.get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
- .get_clk_info_from_vbios = smu_v11_0_get_clk_info_from_vbios,
.check_pptable = smu_v11_0_check_pptable,
.parse_pptable = smu_v11_0_parse_pptable,
.populate_smc_tables = smu_v11_0_populate_smc_pptable,
diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
index 6c59eeef2590..a31df7f4e91a 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
+++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
@@ -47,8 +47,6 @@
#define smu_get_vbios_bootup_values(smu) \
((smu)->ppt_funcs->get_vbios_bootup_values ? (smu)->ppt_funcs->get_vbios_bootup_values((smu)) : 0)
-#define smu_get_clk_info_from_vbios(smu) \
- ((smu)->ppt_funcs->get_clk_info_from_vbios ? (smu)->ppt_funcs->get_clk_info_from_vbios((smu)) : 0)
#define smu_check_pptable(smu) \
((smu)->ppt_funcs->check_pptable ? (smu)->ppt_funcs->check_pptable((smu)) : 0)
#define smu_parse_pptable(smu) \
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 3b22f66e3fbc..be6dca8c6014 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -558,6 +558,32 @@ int smu_v11_0_fini_power(struct smu_context *smu)
return 0;
}
+static int smu_v11_0_atom_get_smu_clockinfo(struct amdgpu_device *adev,
+ uint8_t clk_id,
+ uint8_t syspll_id,
+ uint32_t *clk_freq)
+{
+ struct atom_get_smu_clock_info_parameters_v3_1 input = {0};
+ struct atom_get_smu_clock_info_output_parameters_v3_1 *output;
+ int ret, index;
+
+ input.clk_id = clk_id;
+ input.syspll_id = syspll_id;
+ input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
+ index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
+ getsmuclockinfo);
+
+ ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
+ (uint32_t *)&input);
+ if (ret)
+ return -EINVAL;
+
+ output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
+ *clk_freq = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
+
+ return 0;
+}
+
int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu)
{
int ret, index;
@@ -616,102 +642,37 @@ int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu)
smu->smu_table.boot_values.format_revision = header->format_revision;
smu->smu_table.boot_values.content_revision = header->content_revision;
- return 0;
-}
+ smu_v11_0_atom_get_smu_clockinfo(smu->adev,
+ (uint8_t)SMU11_SYSPLL0_SOCCLK_ID,
+ (uint8_t)0,
+ &smu->smu_table.boot_values.socclk);
-int smu_v11_0_get_clk_info_from_vbios(struct smu_context *smu)
-{
- int ret, index;
- struct amdgpu_device *adev = smu->adev;
- struct atom_get_smu_clock_info_parameters_v3_1 input = {0};
- struct atom_get_smu_clock_info_output_parameters_v3_1 *output;
+ smu_v11_0_atom_get_smu_clockinfo(smu->adev,
+ (uint8_t)SMU11_SYSPLL0_DCEFCLK_ID,
+ (uint8_t)0,
+ &smu->smu_table.boot_values.dcefclk);
- input.clk_id = SMU11_SYSPLL0_SOCCLK_ID;
- input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
- index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
- getsmuclockinfo);
-
- ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
- (uint32_t *)&input);
- if (ret)
- return -EINVAL;
+ smu_v11_0_atom_get_smu_clockinfo(smu->adev,
+ (uint8_t)SMU11_SYSPLL0_ECLK_ID,
+ (uint8_t)0,
+ &smu->smu_table.boot_values.eclk);
- output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
- smu->smu_table.boot_values.socclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
+ smu_v11_0_atom_get_smu_clockinfo(smu->adev,
+ (uint8_t)SMU11_SYSPLL0_VCLK_ID,
+ (uint8_t)0,
+ &smu->smu_table.boot_values.vclk);
- memset(&input, 0, sizeof(input));
- input.clk_id = SMU11_SYSPLL0_DCEFCLK_ID;
- input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
- index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
- getsmuclockinfo);
-
- ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
- (uint32_t *)&input);
- if (ret)
- return -EINVAL;
-
- output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
- smu->smu_table.boot_values.dcefclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
-
- memset(&input, 0, sizeof(input));
- input.clk_id = SMU11_SYSPLL0_ECLK_ID;
- input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
- index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
- getsmuclockinfo);
-
- ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
- (uint32_t *)&input);
- if (ret)
- return -EINVAL;
-
- output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
- smu->smu_table.boot_values.eclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
-
- memset(&input, 0, sizeof(input));
- input.clk_id = SMU11_SYSPLL0_VCLK_ID;
- input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
- index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
- getsmuclockinfo);
-
- ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
- (uint32_t *)&input);
- if (ret)
- return -EINVAL;
-
- output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
- smu->smu_table.boot_values.vclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
-
- memset(&input, 0, sizeof(input));
- input.clk_id = SMU11_SYSPLL0_DCLK_ID;
- input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
- index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
- getsmuclockinfo);
-
- ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
- (uint32_t *)&input);
- if (ret)
- return -EINVAL;
-
- output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
- smu->smu_table.boot_values.dclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
+ smu_v11_0_atom_get_smu_clockinfo(smu->adev,
+ (uint8_t)SMU11_SYSPLL0_DCLK_ID,
+ (uint8_t)0,
+ &smu->smu_table.boot_values.dclk);
if ((smu->smu_table.boot_values.format_revision == 3) &&
- (smu->smu_table.boot_values.content_revision >= 2)) {
- memset(&input, 0, sizeof(input));
- input.clk_id = SMU11_SYSPLL1_0_FCLK_ID;
- input.syspll_id = SMU11_SYSPLL1_2_ID;
- input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
- index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
- getsmuclockinfo);
-
- ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
- (uint32_t *)&input);
- if (ret)
- return -EINVAL;
-
- output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
- smu->smu_table.boot_values.fclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
- }
+ (smu->smu_table.boot_values.content_revision >= 2))
+ smu_v11_0_atom_get_smu_clockinfo(smu->adev,
+ (uint8_t)SMU11_SYSPLL1_0_FCLK_ID,
+ (uint8_t)SMU11_SYSPLL1_2_ID,
+ &smu->smu_table.boot_values.fclk);
return 0;
}
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
` (2 preceding siblings ...)
2020-06-01 7:29 ` [PATCH 4/9] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
@ 2020-06-01 7:29 ` Evan Quan
2020-06-02 15:04 ` Alex Deucher
2020-06-01 7:30 ` [PATCH 6/9] drm/amd/powerplay: clean up the overdrive settings Evan Quan
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:29 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
Combine and simplify the logics for setup pptable.
Change-Id: I062f15eab586050593afd960432c4c70fbdd5d41
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 17 ----
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 66 ++++++++-----
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 5 -
drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 4 -
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 92 ++++++++++---------
drivers/gpu/drm/amd/powerplay/smu_internal.h | 10 --
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 21 -----
7 files changed, 89 insertions(+), 126 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 9bafa6b3e123..b079ac6325d0 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1132,23 +1132,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
if (ret)
return ret;
- /*
- * check if the format_revision in vbios is up to pptable header
- * version, and the structure size is not 0.
- */
- ret = smu_check_pptable(smu);
- if (ret)
- return ret;
-
- /*
- * Parse pptable format and fill PPTable_t smc_pptable to
- * smu_table_context structure. And read the smc_dpm_table from vbios,
- * then fill it into smc_pptable.
- */
- ret = smu_parse_pptable(smu);
- if (ret)
- return ret;
-
/*
* Send msg GetDriverIfVersion to check if the return value is equal
* with DRIVER_IF_VERSION of smc header.
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 902c8cfa4a3b..c5c23126ec2d 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -487,33 +487,33 @@ static int arcturus_set_default_dpm_table(struct smu_context *smu)
static int arcturus_check_powerplay_table(struct smu_context *smu)
{
+ struct smu_table_context *table_context = &smu->smu_table;
+ struct smu_11_0_powerplay_table *powerplay_table =
+ table_context->power_play_table;
+ struct smu_baco_context *smu_baco = &smu->smu_baco;
+
+ mutex_lock(&smu_baco->mutex);
+ if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
+ powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
+ smu_baco->platform_support = true;
+ mutex_unlock(&smu_baco->mutex);
+
+ table_context->thermal_controller_type =
+ powerplay_table->thermal_controller_type;
+
return 0;
}
static int arcturus_store_powerplay_table(struct smu_context *smu)
{
- struct smu_11_0_powerplay_table *powerplay_table = NULL;
struct smu_table_context *table_context = &smu->smu_table;
- struct smu_baco_context *smu_baco = &smu->smu_baco;
- int ret = 0;
-
- if (!table_context->power_play_table)
- return -EINVAL;
-
- powerplay_table = table_context->power_play_table;
+ struct smu_11_0_powerplay_table *powerplay_table =
+ table_context->power_play_table;
memcpy(table_context->driver_pptable, &powerplay_table->smc_pptable,
sizeof(PPTable_t));
- table_context->thermal_controller_type = powerplay_table->thermal_controller_type;
-
- mutex_lock(&smu_baco->mutex);
- if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
- powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
- smu_baco->platform_support = true;
- mutex_unlock(&smu_baco->mutex);
-
- return ret;
+ return 0;
}
static int arcturus_append_powerplay_table(struct smu_context *smu)
@@ -544,6 +544,29 @@ static int arcturus_append_powerplay_table(struct smu_context *smu)
return 0;
}
+static int arcturus_setup_pptable(struct smu_context *smu)
+{
+ int ret = 0;
+
+ ret = smu_v11_0_setup_pptable(smu);
+ if (ret)
+ return ret;
+
+ ret = arcturus_store_powerplay_table(smu);
+ if (ret)
+ return ret;
+
+ ret = arcturus_append_powerplay_table(smu);
+ if (ret)
+ return ret;
+
+ ret = arcturus_check_powerplay_table(smu);
+ if (ret)
+ return ret;
+
+ return ret;
+}
+
static int arcturus_run_btc(struct smu_context *smu)
{
int ret = 0;
@@ -2383,10 +2406,6 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
/* internal structurs allocations */
.tables_init = arcturus_tables_init,
.alloc_dpm_context = arcturus_allocate_dpm_context,
- /* pptable related */
- .check_powerplay_table = arcturus_check_powerplay_table,
- .store_powerplay_table = arcturus_store_powerplay_table,
- .append_powerplay_table = arcturus_append_powerplay_table,
/* init dpm */
.get_allowed_feature_mask = arcturus_get_allowed_feature_mask,
/* btc */
@@ -2421,10 +2440,9 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.init_power = smu_v11_0_init_power,
.fini_power = smu_v11_0_fini_power,
.check_fw_status = smu_v11_0_check_fw_status,
- .setup_pptable = smu_v11_0_setup_pptable,
+ /* pptable related */
+ .setup_pptable = arcturus_setup_pptable,
.get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
- .check_pptable = smu_v11_0_check_pptable,
- .parse_pptable = smu_v11_0_parse_pptable,
.populate_smc_tables = smu_v11_0_populate_smc_pptable,
.check_fw_version = smu_v11_0_check_fw_version,
.write_pptable = smu_v11_0_write_pptable,
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 223678e329a5..14f4a850b553 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -417,9 +417,6 @@ struct i2c_adapter;
struct pptable_funcs {
int (*alloc_dpm_context)(struct smu_context *smu);
- int (*store_powerplay_table)(struct smu_context *smu);
- int (*check_powerplay_table)(struct smu_context *smu);
- int (*append_powerplay_table)(struct smu_context *smu);
int (*get_smu_msg_index)(struct smu_context *smu, uint32_t index);
int (*get_smu_clk_index)(struct smu_context *smu, uint32_t index);
int (*get_smu_feature_index)(struct smu_context *smu, uint32_t index);
@@ -505,8 +502,6 @@ struct pptable_funcs {
int (*check_fw_status)(struct smu_context *smu);
int (*setup_pptable)(struct smu_context *smu);
int (*get_vbios_bootup_values)(struct smu_context *smu);
- int (*check_pptable)(struct smu_context *smu);
- int (*parse_pptable)(struct smu_context *smu);
int (*populate_smc_tables)(struct smu_context *smu);
int (*check_fw_version)(struct smu_context *smu);
int (*powergate_sdma)(struct smu_context *smu, bool gate);
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
index 5b785816aa64..51868dc33238 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -161,10 +161,6 @@ int smu_v11_0_setup_pptable(struct smu_context *smu);
int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu);
-int smu_v11_0_check_pptable(struct smu_context *smu);
-
-int smu_v11_0_parse_pptable(struct smu_context *smu);
-
int smu_v11_0_populate_smc_pptable(struct smu_context *smu);
int smu_v11_0_check_fw_version(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index bea6a96b5afb..db38fb10524d 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -417,6 +417,29 @@ navi10_get_allowed_feature_mask(struct smu_context *smu,
static int navi10_check_powerplay_table(struct smu_context *smu)
{
+ struct smu_table_context *table_context = &smu->smu_table;
+ struct smu_11_0_powerplay_table *powerplay_table =
+ table_context->power_play_table;
+ struct smu_baco_context *smu_baco = &smu->smu_baco;
+
+ if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
+ smu->dc_controlled_by_gpio = true;
+
+ mutex_lock(&smu_baco->mutex);
+ if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
+ powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
+ smu_baco->platform_support = true;
+ mutex_unlock(&smu_baco->mutex);
+
+ table_context->thermal_controller_type =
+ powerplay_table->thermal_controller_type;
+
+ /*
+ * Instead of having its own buffer space and get overdrive_table copied,
+ * smu->od_settings just points to the actual overdrive_table
+ */
+ smu->od_settings = &powerplay_table->overdrive_table;
+
return 0;
}
@@ -475,30 +498,37 @@ static int navi10_append_powerplay_table(struct smu_context *smu)
static int navi10_store_powerplay_table(struct smu_context *smu)
{
- struct smu_11_0_powerplay_table *powerplay_table = NULL;
struct smu_table_context *table_context = &smu->smu_table;
- struct smu_baco_context *smu_baco = &smu->smu_baco;
-
- if (!table_context->power_play_table)
- return -EINVAL;
-
- powerplay_table = table_context->power_play_table;
+ struct smu_11_0_powerplay_table *powerplay_table =
+ table_context->power_play_table;
memcpy(table_context->driver_pptable, &powerplay_table->smc_pptable,
sizeof(PPTable_t));
- table_context->thermal_controller_type = powerplay_table->thermal_controller_type;
+ return 0;
+}
- if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
- smu->dc_controlled_by_gpio = true;
+static int navi10_setup_pptable(struct smu_context *smu)
+{
+ int ret = 0;
- mutex_lock(&smu_baco->mutex);
- if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
- powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
- smu_baco->platform_support = true;
- mutex_unlock(&smu_baco->mutex);
+ ret = smu_v11_0_setup_pptable(smu);
+ if (ret)
+ return ret;
- return 0;
+ ret = navi10_store_powerplay_table(smu);
+ if (ret)
+ return ret;
+
+ ret = navi10_append_powerplay_table(smu);
+ if (ret)
+ return ret;
+
+ ret = navi10_check_powerplay_table(smu);
+ if (ret)
+ return ret;
+
+ return ret;
}
static int navi10_tables_init(struct smu_context *smu, struct smu_table *tables)
@@ -1927,24 +1957,6 @@ static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
return 0;
}
-static int navi10_setup_od_limits(struct smu_context *smu) {
- struct smu_11_0_overdrive_table *overdrive_table = NULL;
- struct smu_11_0_powerplay_table *powerplay_table = NULL;
-
- if (!smu->smu_table.power_play_table) {
- pr_err("powerplay table uninitialized!\n");
- return -ENOENT;
- }
- powerplay_table = (struct smu_11_0_powerplay_table *)smu->smu_table.power_play_table;
- overdrive_table = &powerplay_table->overdrive_table;
- if (!smu->od_settings) {
- smu->od_settings = kmemdup(overdrive_table, sizeof(struct smu_11_0_overdrive_table), GFP_KERNEL);
- } else {
- memcpy(smu->od_settings, overdrive_table, sizeof(struct smu_11_0_overdrive_table));
- }
- return 0;
-}
-
static bool navi10_is_baco_supported(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
@@ -1968,11 +1980,6 @@ static int navi10_set_default_od_settings(struct smu_context *smu, bool initiali
od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
boot_od_table = (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
if (initialize) {
- ret = navi10_setup_od_limits(smu);
- if (ret) {
- pr_err("Failed to retrieve board OD limits\n");
- return ret;
- }
if (od_table) {
if (!od_table->GfxclkVolt1) {
ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
@@ -2274,9 +2281,6 @@ static int navi10_disable_umc_cdr_12gbps_workaround(struct smu_context *smu)
static const struct pptable_funcs navi10_ppt_funcs = {
.tables_init = navi10_tables_init,
.alloc_dpm_context = navi10_allocate_dpm_context,
- .store_powerplay_table = navi10_store_powerplay_table,
- .check_powerplay_table = navi10_check_powerplay_table,
- .append_powerplay_table = navi10_append_powerplay_table,
.get_smu_msg_index = navi10_get_smu_msg_index,
.get_smu_clk_index = navi10_get_smu_clk_index,
.get_smu_feature_index = navi10_get_smu_feature_index,
@@ -2318,10 +2322,8 @@ static const struct pptable_funcs navi10_ppt_funcs = {
.init_power = smu_v11_0_init_power,
.fini_power = smu_v11_0_fini_power,
.check_fw_status = smu_v11_0_check_fw_status,
- .setup_pptable = smu_v11_0_setup_pptable,
+ .setup_pptable = navi10_setup_pptable,
.get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
- .check_pptable = smu_v11_0_check_pptable,
- .parse_pptable = smu_v11_0_parse_pptable,
.populate_smc_tables = smu_v11_0_populate_smc_pptable,
.check_fw_version = smu_v11_0_check_fw_version,
.write_pptable = smu_v11_0_write_pptable,
diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
index a31df7f4e91a..33086f94267a 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
+++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
@@ -47,10 +47,6 @@
#define smu_get_vbios_bootup_values(smu) \
((smu)->ppt_funcs->get_vbios_bootup_values ? (smu)->ppt_funcs->get_vbios_bootup_values((smu)) : 0)
-#define smu_check_pptable(smu) \
- ((smu)->ppt_funcs->check_pptable ? (smu)->ppt_funcs->check_pptable((smu)) : 0)
-#define smu_parse_pptable(smu) \
- ((smu)->ppt_funcs->parse_pptable ? (smu)->ppt_funcs->parse_pptable((smu)) : 0)
#define smu_populate_smc_tables(smu) \
((smu)->ppt_funcs->populate_smc_tables ? (smu)->ppt_funcs->populate_smc_tables((smu)) : 0)
#define smu_check_fw_version(smu) \
@@ -96,12 +92,6 @@ static inline int smu_send_smc_msg(struct smu_context *smu, enum smu_message_typ
((smu)->ppt_funcs->is_dpm_running ? (smu)->ppt_funcs->is_dpm_running((smu)) : 0)
#define smu_notify_display_change(smu) \
((smu)->ppt_funcs->notify_display_change? (smu)->ppt_funcs->notify_display_change((smu)) : 0)
-#define smu_store_powerplay_table(smu) \
- ((smu)->ppt_funcs->store_powerplay_table ? (smu)->ppt_funcs->store_powerplay_table((smu)) : 0)
-#define smu_check_powerplay_table(smu) \
- ((smu)->ppt_funcs->check_powerplay_table ? (smu)->ppt_funcs->check_powerplay_table((smu)) : 0)
-#define smu_append_powerplay_table(smu) \
- ((smu)->ppt_funcs->append_powerplay_table ? (smu)->ppt_funcs->append_powerplay_table((smu)) : 0)
#define smu_set_default_dpm_table(smu) \
((smu)->ppt_funcs->set_default_dpm_table ? (smu)->ppt_funcs->set_default_dpm_table((smu)) : 0)
#define smu_populate_umd_state_clk(smu) \
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index be6dca8c6014..7a97a4510c6d 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -725,27 +725,6 @@ int smu_v11_0_notify_memory_pool_location(struct smu_context *smu)
return ret;
}
-int smu_v11_0_check_pptable(struct smu_context *smu)
-{
- int ret;
-
- ret = smu_check_powerplay_table(smu);
- return ret;
-}
-
-int smu_v11_0_parse_pptable(struct smu_context *smu)
-{
- int ret;
-
- ret = smu_store_powerplay_table(smu);
- if (ret)
- return -EINVAL;
-
- ret = smu_append_powerplay_table(smu);
-
- return ret;
-}
-
int smu_v11_0_populate_smc_pptable(struct smu_context *smu)
{
int ret;
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/9] drm/amd/powerplay: clean up the overdrive settings
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
` (3 preceding siblings ...)
2020-06-01 7:29 ` [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
@ 2020-06-01 7:30 ` Evan Quan
2020-06-02 15:06 ` Alex Deucher
2020-06-01 7:30 ` [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations Evan Quan
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:30 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
Eliminate the buffer allocation and drop the unnecessary
overdrive table uploading.
Change-Id: I8ba5383a330e6d5355cea219147500c1b4a43f47
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 +-
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 2 -
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 74 +++++++++----------
drivers/gpu/drm/amd/powerplay/smu_internal.h | 4 +-
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 20 -----
6 files changed, 38 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index b079ac6325d0..9b81b6519a96 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1215,7 +1215,7 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
if (ret)
return ret;
- ret = smu_set_default_od_settings(smu, initialize);
+ ret = smu_set_default_od_settings(smu);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 14f4a850b553..4aa63dc79124 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -480,7 +480,7 @@ struct pptable_funcs {
uint32_t *value);
int (*get_thermal_temperature_range)(struct smu_context *smu, struct smu_temperature_range *range);
int (*get_uclk_dpm_states)(struct smu_context *smu, uint32_t *clocks_in_khz, uint32_t *num_states);
- int (*set_default_od_settings)(struct smu_context *smu, bool initialize);
+ int (*set_default_od_settings)(struct smu_context *smu);
int (*set_performance_level)(struct smu_context *smu, enum amd_dpm_forced_level level);
int (*display_disable_memory_clock_switch)(struct smu_context *smu, bool disable_memory_clock_switch);
void (*dump_pptable)(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
index 51868dc33238..8d317e05f65b 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -258,8 +258,6 @@ int smu_v11_0_set_soft_freq_limited_range(struct smu_context *smu, enum smu_clk_
int smu_v11_0_override_pcie_parameters(struct smu_context *smu);
-int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool initialize, size_t overdrive_table_size);
-
uint32_t smu_v11_0_get_max_power_limit(struct smu_context *smu);
int smu_v11_0_set_performance_level(struct smu_context *smu,
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index db38fb10524d..caa4355b601e 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -1969,55 +1969,49 @@ static bool navi10_is_baco_supported(struct smu_context *smu)
return (val & RCC_BIF_STRAP0__STRAP_PX_CAPABLE_MASK) ? true : false;
}
-static int navi10_set_default_od_settings(struct smu_context *smu, bool initialize) {
- OverDriveTable_t *od_table, *boot_od_table;
+static int navi10_set_default_od_settings(struct smu_context *smu)
+{
+ OverDriveTable_t *od_table =
+ (OverDriveTable_t *)smu->smu_table.overdrive_table;
+ OverDriveTable_t *boot_od_table =
+ (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
int ret = 0;
- ret = smu_v11_0_set_default_od_settings(smu, initialize, sizeof(OverDriveTable_t));
- if (ret)
+ ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
+ if (ret) {
+ pr_err("Failed to get overdrive table!\n");
return ret;
+ }
- od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
- boot_od_table = (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
- if (initialize) {
- if (od_table) {
- if (!od_table->GfxclkVolt1) {
- ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
- &od_table->GfxclkVolt1,
- od_table->GfxclkFreq1);
- if (ret)
- od_table->GfxclkVolt1 = 0;
- if (boot_od_table)
- boot_od_table->GfxclkVolt1 = od_table->GfxclkVolt1;
- }
-
- if (!od_table->GfxclkVolt2) {
- ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
- &od_table->GfxclkVolt2,
- od_table->GfxclkFreq2);
- if (ret)
- od_table->GfxclkVolt2 = 0;
- if (boot_od_table)
- boot_od_table->GfxclkVolt2 = od_table->GfxclkVolt2;
- }
+ if (!od_table->GfxclkVolt1) {
+ ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
+ &od_table->GfxclkVolt1,
+ od_table->GfxclkFreq1);
+ if (ret)
+ return ret;
+ }
- if (!od_table->GfxclkVolt3) {
- ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
- &od_table->GfxclkVolt3,
- od_table->GfxclkFreq3);
- if (ret)
- od_table->GfxclkVolt3 = 0;
- if (boot_od_table)
- boot_od_table->GfxclkVolt3 = od_table->GfxclkVolt3;
- }
- }
+ if (!od_table->GfxclkVolt2) {
+ ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
+ &od_table->GfxclkVolt2,
+ od_table->GfxclkFreq2);
+ if (ret)
+ return ret;
}
- if (od_table) {
- navi10_dump_od_table(od_table);
+ if (!od_table->GfxclkVolt3) {
+ ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
+ &od_table->GfxclkVolt3,
+ od_table->GfxclkFreq3);
+ if (ret)
+ return ret;
}
- return ret;
+ memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
+
+ navi10_dump_od_table(od_table);
+
+ return 0;
}
static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABLE_COMMAND type, long input[], uint32_t size) {
diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
index 33086f94267a..0c7d5f0b1cd1 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
+++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
@@ -70,8 +70,8 @@
((smu)->ppt_funcs->system_features_control ? (smu)->ppt_funcs->system_features_control((smu), (en)) : 0)
#define smu_init_max_sustainable_clocks(smu) \
((smu)->ppt_funcs->init_max_sustainable_clocks ? (smu)->ppt_funcs->init_max_sustainable_clocks((smu)) : 0)
-#define smu_set_default_od_settings(smu, initialize) \
- ((smu)->ppt_funcs->set_default_od_settings ? (smu)->ppt_funcs->set_default_od_settings((smu), (initialize)) : 0)
+#define smu_set_default_od_settings(smu) \
+ ((smu)->ppt_funcs->set_default_od_settings ? (smu)->ppt_funcs->set_default_od_settings((smu)) : 0)
#define smu_send_smc_msg_with_param(smu, msg, param, read_arg) \
((smu)->ppt_funcs->send_smc_msg_with_param? (smu)->ppt_funcs->send_smc_msg_with_param((smu), (msg), (param), (read_arg)) : 0)
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 7a97a4510c6d..891781a5c0d4 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1898,26 +1898,6 @@ int smu_v11_0_override_pcie_parameters(struct smu_context *smu)
}
-int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool initialize, size_t overdrive_table_size)
-{
- struct smu_table_context *table_context = &smu->smu_table;
- int ret = 0;
-
- if (initialize) {
- ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, false);
- if (ret) {
- pr_err("Failed to export overdrive table!\n");
- return ret;
- }
- }
- ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, true);
- if (ret) {
- pr_err("Failed to import overdrive table!\n");
- return ret;
- }
- return ret;
-}
-
int smu_v11_0_set_performance_level(struct smu_context *smu,
enum amd_dpm_forced_level level)
{
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
` (4 preceding siblings ...)
2020-06-01 7:30 ` [PATCH 6/9] drm/amd/powerplay: clean up the overdrive settings Evan Quan
@ 2020-06-01 7:30 ` Evan Quan
2020-06-02 15:11 ` Alex Deucher
2020-06-01 7:30 ` [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation Evan Quan
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:30 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
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 <evan.quan@amd.com>
---
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 = &adev->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, &smu->default_power_limit, false, false);
+ if (ret)
+ return ret;
+
smu_handle_task(&adev->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, &adev->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;
+
+ return ret;
+}
+
+static int smu_disable_dpms(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+ int ret = 0;
+ bool use_baco = !smu->is_apu &&
+ ((adev->in_gpu_reset &&
+ (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
+ ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
/*
- * 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.
+ * For custom pptable uploading, skip the DPM features
+ * disable process on Navi1x ASICs.
+ * - As the gfx related features are under control of
+ * RLC on those ASICs. RLC reinitialization will be
+ * needed to reenable them. That will cost much more
+ * efforts.
+ *
+ * - SMU firmware can handle the DPM reenablement
+ * properly.
*/
- if (initialize) {
- ret = smu_populate_smc_tables(smu);
- if (ret)
- return ret;
+ if (smu->uploading_custom_pp_table &&
+ (adev->asic_type >= CHIP_NAVI10) &&
+ (adev->asic_type <= CHIP_NAVI12))
+ return 0;
- ret = smu_init_max_sustainable_clocks(smu);
- if (ret)
+ /*
+ * Disable all enabled SMU features.
+ * This should be handled in SMU FW, as a backup
+ * driver can issue call to SMU FW until sequence
+ * in SMU FW is operational.
+ */
+ ret = smu_system_features_control(smu, false);
+ if (ret) {
+ pr_err("Failed to disable smu features.\n");
+ return ret;
+ }
+
+ /*
+ * For baco, need to leave BACO feature enabled
+ *
+ * Correct the way for checking whether SMU_FEATURE_BACO_BIT
+ * is supported.
+ *
+ * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
+ * always return false as the 'smu_system_features_control(smu, false)'
+ * was just issued above which disabled all SMU features.
+ *
+ * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
+ * now for the checking.
+ */
+ if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
+ ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
+ if (ret) {
+ pr_warn("set BACO feature enabled failed, return %d\n", ret);
return ret;
+ }
}
- ret = smu_override_pcie_parameters(smu);
- if (ret)
- return ret;
+ return ret;
+}
- ret = smu_set_default_od_settings(smu);
- if (ret)
- return ret;
+static int smu_internal_hw_cleanup(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+ int ret = 0;
- if (initialize) {
- ret = smu_populate_umd_state_clk(smu);
- if (ret)
- return ret;
+ smu_i2c_eeprom_fini(smu, &adev->pm.smu_i2c);
- ret = smu_get_power_limit(smu, &smu->default_power_limit, false, false);
- if (ret)
- return ret;
+ ret = smu_disable_thermal_alert(smu);
+ if (ret) {
+ pr_warn("Fail to stop thermal control!\n");
+ return ret;
}
- /*
- * Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools.
- */
- ret = smu_set_tool_table_location(smu);
-
- if (!smu_is_dpm_running(smu))
- pr_info("dpm has been disabled\n");
+ ret = smu_disable_dpms(smu);
+ if (ret)
+ return ret;
- return ret;
+ return 0;
}
static int smu_start_smc_engine(struct smu_context *smu)
@@ -1257,10 +1336,20 @@ static int smu_start_smc_engine(struct smu_context *smu)
if (smu->ppt_funcs->check_fw_status) {
ret = smu->ppt_funcs->check_fw_status(smu);
- if (ret)
+ if (ret) {
pr_err("SMC is not ready\n");
+ 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;
+
return ret;
}
@@ -1289,99 +1378,24 @@ static int smu_hw_init(void *handle)
if (!smu->pm_enabled)
return 0;
- ret = smu_feature_init_dpm(smu);
- if (ret)
- goto failed;
-
- ret = smu_smc_table_hw_init(smu, true);
- if (ret)
- goto failed;
-
- /*
- * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
- * pool location.
- */
- ret = smu_notify_memory_pool_location(smu);
+ /* get boot_values from vbios to set revision, gfxclk, and etc. */
+ ret = smu_get_vbios_bootup_values(smu);
if (ret)
- goto failed;
+ return ret;
- ret = smu_enable_thermal_alert(smu);
+ ret = smu_setup_pptable(smu);
if (ret)
- goto failed;
+ return ret;
- ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
+ ret = smu_internal_hw_setup(smu);
if (ret)
- goto failed;
+ return ret;
adev->pm.dpm_enabled = true;
pr_info("SMU is initialized successfully!\n");
return 0;
-
-failed:
- return ret;
-}
-
-static int smu_disable_dpms(struct smu_context *smu)
-{
- struct amdgpu_device *adev = smu->adev;
- int ret = 0;
- bool use_baco = !smu->is_apu &&
- ((adev->in_gpu_reset &&
- (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
- ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
-
- /*
- * For custom pptable uploading, skip the DPM features
- * disable process on Navi1x ASICs.
- * - As the gfx related features are under control of
- * RLC on those ASICs. RLC reinitialization will be
- * needed to reenable them. That will cost much more
- * efforts.
- *
- * - SMU firmware can handle the DPM reenablement
- * properly.
- */
- if (smu->uploading_custom_pp_table &&
- (adev->asic_type >= CHIP_NAVI10) &&
- (adev->asic_type <= CHIP_NAVI12))
- return 0;
-
- /*
- * Disable all enabled SMU features.
- * This should be handled in SMU FW, as a backup
- * driver can issue call to SMU FW until sequence
- * in SMU FW is operational.
- */
- ret = smu_system_features_control(smu, false);
- if (ret) {
- pr_err("Failed to disable smu features.\n");
- return ret;
- }
-
- /*
- * For baco, need to leave BACO feature enabled
- *
- * Correct the way for checking whether SMU_FEATURE_BACO_BIT
- * is supported.
- *
- * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
- * always return false as the 'smu_system_features_control(smu, false)'
- * was just issued above which disabled all SMU features.
- *
- * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
- * now for the checking.
- */
- if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
- ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
- if (ret) {
- pr_warn("set BACO feature enabled failed, return %d\n", ret);
- return ret;
- }
- }
-
- return ret;
}
static int smu_hw_fini(void *handle)
@@ -1404,19 +1418,9 @@ static int smu_hw_fini(void *handle)
adev->pm.dpm_enabled = false;
- smu_i2c_eeprom_fini(smu, &adev->pm.smu_i2c);
-
- ret = smu_disable_thermal_alert(smu);
- if (ret) {
- pr_warn("Fail to stop thermal control!\n");
- return ret;
- }
-
- ret = smu_disable_dpms(smu);
- if (ret) {
- pr_warn("Fail to stop Dpms!\n");
+ ret = smu_internal_hw_cleanup(smu);
+ if (ret)
return ret;
- }
return 0;
}
@@ -1434,6 +1438,8 @@ int smu_reset(struct smu_context *smu)
if (ret)
return ret;
+ ret = smu_late_init(adev);
+
return ret;
}
@@ -1451,15 +1457,7 @@ static int smu_suspend(void *handle)
adev->pm.dpm_enabled = false;
- smu_i2c_eeprom_fini(smu, &adev->pm.smu_i2c);
-
- ret = smu_disable_thermal_alert(smu);
- if (ret) {
- pr_warn("Fail to stop thermal control!\n");
- return ret;
- }
-
- ret = smu_disable_dpms(smu);
+ ret = smu_internal_hw_cleanup(smu);
if (ret)
return ret;
@@ -1491,20 +1489,12 @@ static int smu_resume(void *handle)
ret = smu_start_smc_engine(smu);
if (ret) {
pr_err("SMU is not ready yet!\n");
- goto failed;
+ return ret;
}
- ret = smu_smc_table_hw_init(smu, false);
- if (ret)
- goto failed;
-
- ret = smu_enable_thermal_alert(smu);
- if (ret)
- goto failed;
-
- ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
+ ret = smu_internal_hw_setup(smu);
if (ret)
- goto failed;
+ return ret;
if (smu->is_apu)
smu_set_gfx_cgpg(&adev->smu, true);
@@ -1516,9 +1506,6 @@ static int smu_resume(void *handle)
pr_info("SMU is resumed successfully!\n");
return 0;
-
-failed:
- return ret;
}
int smu_display_configuration_change(struct smu_context *smu,
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
` (5 preceding siblings ...)
2020-06-01 7:30 ` [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations Evan Quan
@ 2020-06-01 7:30 ` Evan Quan
2020-06-02 15:12 ` Alex Deucher
2020-06-02 21:18 ` Luben Tuikov
2020-06-01 7:30 ` [PATCH 9/9] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
2020-06-02 14:49 ` [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Alex Deucher
8 siblings, 2 replies; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:30 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
Since the structure comes with only several bytes.
Change-Id: Ie9df0db543fdd4cf5b963a286ef40dee03c436bf
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 3 ---
drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +++------------
3 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index e55c6458b212..b353ac1b0f07 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1121,9 +1121,6 @@ static int smu_sw_fini(void *handle)
struct smu_context *smu = &adev->smu;
int ret;
- kfree(smu->irq_source);
- smu->irq_source = NULL;
-
ret = smu_smc_table_sw_fini(smu);
if (ret) {
pr_err("Failed to sw fini smc table!\n");
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 4aa63dc79124..7fed2556213f 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -356,7 +356,7 @@ struct smu_baco_context
struct smu_context
{
struct amdgpu_device *adev;
- struct amdgpu_irq_src *irq_source;
+ struct amdgpu_irq_src irq_source;
const struct pptable_funcs *ppt_funcs;
struct mutex mutex;
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 891781a5c0d4..e2b1c619151f 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1167,7 +1167,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context *smu)
if (ret)
return ret;
- ret = amdgpu_irq_get(adev, smu->irq_source, 0);
+ ret = amdgpu_irq_get(adev, &smu->irq_source, 0);
if (ret)
return ret;
@@ -1191,7 +1191,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context *smu)
int smu_v11_0_disable_thermal_alert(struct smu_context *smu)
{
- return amdgpu_irq_put(smu->adev, smu->irq_source, 0);
+ return amdgpu_irq_put(smu->adev, &smu->irq_source, 0);
}
static uint16_t convert_to_vddc(uint8_t vid)
@@ -1607,18 +1607,9 @@ static const struct amdgpu_irq_src_funcs smu_v11_0_irq_funcs =
int smu_v11_0_register_irq_handler(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
- struct amdgpu_irq_src *irq_src = smu->irq_source;
+ struct amdgpu_irq_src *irq_src = &smu->irq_source;
int ret = 0;
- /* already register */
- if (irq_src)
- return 0;
-
- irq_src = kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
- if (!irq_src)
- return -ENOMEM;
- smu->irq_source = irq_src;
-
irq_src->num_types = 1;
irq_src->funcs = &smu_v11_0_irq_funcs;
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 9/9] drm/amd/powerplay: add firmware cleanup on sw_fini
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
` (6 preceding siblings ...)
2020-06-01 7:30 ` [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation Evan Quan
@ 2020-06-01 7:30 ` Evan Quan
2020-06-02 15:12 ` Alex Deucher
2020-06-02 14:49 ` [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Alex Deucher
8 siblings, 1 reply; 20+ messages in thread
From: Evan Quan @ 2020-06-01 7:30 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, Evan Quan
To avoid possible memory leak.
Change-Id: I4740eac7fc2c6e934ec8f503e5a98057f0902f4a
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 ++
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 1 +
drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 +
drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 2 ++
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 1 +
drivers/gpu/drm/amd/powerplay/smu_internal.h | 2 ++
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 9 +++++++++
7 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index b353ac1b0f07..197fef6f59a8 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1127,6 +1127,8 @@ static int smu_sw_fini(void *handle)
return ret;
}
+ smu_fini_microcode(smu);
+
return 0;
}
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index c5c23126ec2d..db2d86e3953b 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2435,6 +2435,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.i2c_eeprom_fini = arcturus_i2c_eeprom_control_fini,
.init_microcode = smu_v11_0_init_microcode,
.load_microcode = smu_v11_0_load_microcode,
+ .fini_microcode = smu_v11_0_fini_microcode,
.init_smc_tables = smu_v11_0_init_smc_tables,
.fini_smc_tables = smu_v11_0_fini_smc_tables,
.init_power = smu_v11_0_init_power,
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 7fed2556213f..718aecde88c0 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -495,6 +495,7 @@ struct pptable_funcs {
int (*get_dpm_clock_table)(struct smu_context *smu, struct dpm_clocks *clock_table);
int (*init_microcode)(struct smu_context *smu);
int (*load_microcode)(struct smu_context *smu);
+ void (*fini_microcode)(struct smu_context *smu);
int (*init_smc_tables)(struct smu_context *smu);
int (*fini_smc_tables)(struct smu_context *smu);
int (*init_power)(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
index 8d317e05f65b..4da5f5e87c81 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -145,6 +145,8 @@ enum smu_v11_0_baco_seq {
int smu_v11_0_init_microcode(struct smu_context *smu);
+void smu_v11_0_fini_microcode(struct smu_context *smu);
+
int smu_v11_0_load_microcode(struct smu_context *smu);
int smu_v11_0_init_smc_tables(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index caa4355b601e..ebbbe38dfb63 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -2311,6 +2311,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
.update_pcie_parameters = navi10_update_pcie_parameters,
.init_microcode = smu_v11_0_init_microcode,
.load_microcode = smu_v11_0_load_microcode,
+ .fini_microcode = smu_v11_0_fini_microcode,
.init_smc_tables = smu_v11_0_init_smc_tables,
.fini_smc_tables = smu_v11_0_fini_smc_tables,
.init_power = smu_v11_0_init_power,
diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
index 0c7d5f0b1cd1..dbdb870011d3 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
+++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
@@ -27,6 +27,8 @@
#define smu_init_microcode(smu) \
((smu)->ppt_funcs->init_microcode ? (smu)->ppt_funcs->init_microcode((smu)) : 0)
+#define smu_fini_microcode(smu) \
+ ((smu)->ppt_funcs->fini_microcode ? (smu)->ppt_funcs->fini_microcode((smu)) : 0)
#define smu_init_smc_tables(smu) \
((smu)->ppt_funcs->init_smc_tables ? (smu)->ppt_funcs->init_smc_tables((smu)) : 0)
#define smu_fini_smc_tables(smu) \
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index e2b1c619151f..10ae4575ccb2 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -195,6 +195,15 @@ int smu_v11_0_init_microcode(struct smu_context *smu)
return err;
}
+void smu_v11_0_fini_microcode(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+
+ release_firmware(adev->pm.fw);
+ adev->pm.fw = NULL;
+ adev->pm.fw_version = 0;
+}
+
int smu_v11_0_load_microcode(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
` (7 preceding siblings ...)
2020-06-01 7:30 ` [PATCH 9/9] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
@ 2020-06-02 14:49 ` Alex Deucher
8 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 14:49 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan <evan.quan@amd.com> wrote:
>
> These APIs internally guard they will not break ARCTURUS.
>
> Change-Id: Ib6775c1c8c5211ea45db6c3fb604a8279411ab37
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 38 +++++++++-----------
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 8 ++---
> 2 files changed, 20 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 5294aa7cdde1..4998ea942760 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1049,11 +1049,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> return 0;
> }
>
> - if (adev->asic_type != CHIP_ARCTURUS) {
> - ret = smu_init_display_count(smu, 0);
> - if (ret)
> - return ret;
> - }
> + ret = smu_init_display_count(smu, 0);
> + if (ret)
> + return ret;
>
> if (initialize) {
> /* get boot_values from vbios to set revision, gfxclk, and etc. */
> @@ -1159,19 +1157,17 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> }
> }
>
> - if (adev->asic_type != CHIP_ARCTURUS) {
> - ret = smu_notify_display_change(smu);
> - if (ret)
> - return ret;
> + ret = smu_notify_display_change(smu);
> + if (ret)
> + return ret;
>
> - /*
> - * Set min deep sleep dce fclk with bootup value from vbios via
> - * SetMinDeepSleepDcefclk MSG.
> - */
> - ret = smu_set_min_dcef_deep_sleep(smu);
> - if (ret)
> - return ret;
> - }
> + /*
> + * Set min deep sleep dce fclk with bootup value from vbios via
> + * SetMinDeepSleepDcefclk MSG.
> + */
> + ret = smu_set_min_dcef_deep_sleep(smu);
> + if (ret)
> + return ret;
>
> /*
> * Set initialized values (get from vbios) to dpm tables context such as
> @@ -1188,11 +1184,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> return ret;
> }
>
> - if (adev->asic_type != CHIP_ARCTURUS) {
> - ret = smu_override_pcie_parameters(smu);
> - if (ret)
> - return ret;
> - }
> + ret = smu_override_pcie_parameters(smu);
> + if (ret)
> + return ret;
>
> ret = smu_set_default_od_settings(smu, initialize);
> if (ret)
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 302b7e9cb5ba..e856ad36ab01 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -2429,16 +2429,16 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> .populate_smc_tables = smu_v11_0_populate_smc_pptable,
> .check_fw_version = smu_v11_0_check_fw_version,
> .write_pptable = smu_v11_0_write_pptable,
> - .set_min_dcef_deep_sleep = smu_v11_0_set_min_dcef_deep_sleep,
> + .set_min_dcef_deep_sleep = NULL,
> .set_driver_table_location = smu_v11_0_set_driver_table_location,
> .set_tool_table_location = smu_v11_0_set_tool_table_location,
> .notify_memory_pool_location = smu_v11_0_notify_memory_pool_location,
> .system_features_control = smu_v11_0_system_features_control,
> .send_smc_msg_with_param = smu_v11_0_send_msg_with_param,
> - .init_display_count = smu_v11_0_init_display_count,
> + .init_display_count = NULL,
> .set_allowed_mask = smu_v11_0_set_allowed_mask,
> .get_enabled_mask = smu_v11_0_get_enabled_mask,
> - .notify_display_change = smu_v11_0_notify_display_change,
> + .notify_display_change = NULL,
> .set_power_limit = smu_v11_0_set_power_limit,
> .get_current_clk_freq = smu_v11_0_get_current_clk_freq,
> .init_max_sustainable_clocks = smu_v11_0_init_max_sustainable_clocks,
> @@ -2462,7 +2462,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> .baco_exit = smu_v11_0_baco_exit,
> .get_dpm_ultimate_freq = smu_v11_0_get_dpm_ultimate_freq,
> .set_soft_freq_limited_range = smu_v11_0_set_soft_freq_limited_range,
> - .override_pcie_parameters = smu_v11_0_override_pcie_parameters,
> + .override_pcie_parameters = NULL,
> .get_pptable_power_limit = arcturus_get_pptable_power_limit,
> .set_df_cstate = arcturus_set_df_cstate,
> .allow_xgmi_power_down = arcturus_allow_xgmi_power_down,
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
2020-06-01 7:29 ` [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes Evan Quan
@ 2020-06-02 14:54 ` Alex Deucher
2020-06-04 4:51 ` Quan, Evan
0 siblings, 1 reply; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 14:54 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan <evan.quan@amd.com> wrote:
>
> To make code more clean and readable by moving ASIC
> specific code to its own file, more code sharing and
> dropping unused code.
There seem to be multiple things going on here. It's kind of hard to
follow all of the changes. Maybe split this patch up? One additional
comment below.
Alex
>
> Change-Id: I6b299f9e98c7678b48281cbed9beb17b644bb4cc
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 213 ++++++++-------------
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 19 ++
> 2 files changed, 102 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 4998ea942760..b4f108cb52fa 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -817,22 +817,10 @@ int smu_get_atom_data_table(struct smu_context *smu, uint32_t table,
> return 0;
> }
>
> -static int smu_initialize_pptable(struct smu_context *smu)
> -{
> - /* TODO */
> - return 0;
> -}
> -
> static int smu_smc_table_sw_init(struct smu_context *smu)
> {
> int ret;
>
> - ret = smu_initialize_pptable(smu);
> - if (ret) {
> - pr_err("Failed to init smu_initialize_pptable!\n");
> - return ret;
> - }
> -
> /**
> * Create smu_table structure, and init smc tables such as
> * TABLE_PPTABLE, TABLE_WATERMARKS, TABLE_SMU_METRICS, and etc.
> @@ -860,6 +848,12 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
> {
> int ret;
>
> + ret = smu_fini_power(smu);
> + if (ret) {
> + pr_err("Failed to init smu_fini_power!\n");
> + return ret;
> + }
> +
> ret = smu_fini_smc_tables(smu);
> if (ret) {
> pr_err("Failed to smu_fini_smc_tables!\n");
> @@ -950,12 +944,6 @@ static int smu_sw_fini(void *handle)
> return ret;
> }
>
> - ret = smu_fini_power(smu);
> - if (ret) {
> - pr_err("Failed to init smu_fini_power!\n");
> - return ret;
> - }
> -
> return 0;
> }
>
> @@ -1125,36 +1113,22 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> if (ret)
> return ret;
>
> - if (adev->asic_type == CHIP_NAVI10) {
> - if ((adev->pdev->device == 0x731f && (adev->pdev->revision == 0xc2 ||
> - adev->pdev->revision == 0xc3 ||
> - adev->pdev->revision == 0xca ||
> - adev->pdev->revision == 0xcb)) ||
> - (adev->pdev->device == 0x66af && (adev->pdev->revision == 0xf3 ||
> - adev->pdev->revision == 0xf4 ||
> - adev->pdev->revision == 0xf5 ||
> - adev->pdev->revision == 0xf6))) {
> - 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_disable_umc_cdr_12gbps_workaround(smu);
> + if (ret) {
> + pr_err("Workaround failed to disable UMC CDR feature on 12Gbps SKU!\n");
> + return ret;
> }
>
> - if (smu->ppt_funcs->set_power_source) {
> - /*
> - * For Navi1X, manually switch it to AC mode as PMFW
> - * may boot it with DC mode.
> - */
> - if (adev->pm.ac_power)
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_AC);
> - else
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_DC);
> - if (ret) {
> - pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ? "AC" : "DC");
> - return ret;
> - }
> + /*
> + * For Navi1X, manually switch it to AC mode as PMFW
> + * may boot it with DC mode.
> + */
> + ret = smu_set_power_source(smu,
> + adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
> + SMU_POWER_SOURCE_DC);
> + if (ret) {
> + pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ? "AC" : "DC");
> + return ret;
> }
>
> ret = smu_notify_display_change(smu);
> @@ -1362,9 +1336,65 @@ static int smu_hw_init(void *handle)
> return ret;
> }
>
> -static int smu_stop_dpms(struct smu_context *smu)
> +static int smu_disable_dpms(struct smu_context *smu)
> {
> - return smu_system_features_control(smu, false);
> + struct amdgpu_device *adev = smu->adev;
> + int ret = 0;
> + bool use_baco = !smu->is_apu &&
> + ((adev->in_gpu_reset &&
> + (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> + ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
> +
> + /*
> + * For custom pptable uploading, skip the DPM features
> + * disable process on Navi1x ASICs.
> + * - As the gfx related features are under control of
> + * RLC on those ASICs. RLC reinitialization will be
> + * needed to reenable them. That will cost much more
> + * efforts.
> + *
> + * - SMU firmware can handle the DPM reenablement
> + * properly.
> + */
> + if (smu->uploading_custom_pp_table &&
> + (adev->asic_type >= CHIP_NAVI10) &&
> + (adev->asic_type <= CHIP_NAVI12))
> + return 0;
> +
> + /*
> + * Disable all enabled SMU features.
> + * This should be handled in SMU FW, as a backup
> + * driver can issue call to SMU FW until sequence
> + * in SMU FW is operational.
> + */
> + ret = smu_system_features_control(smu, false);
> + if (ret) {
> + pr_err("Failed to disable smu features.\n");
> + return ret;
> + }
> +
> + /*
> + * For baco, need to leave BACO feature enabled
> + *
> + * Correct the way for checking whether SMU_FEATURE_BACO_BIT
> + * is supported.
> + *
> + * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
> + * always return false as the 'smu_system_features_control(smu, false)'
> + * was just issued above which disabled all SMU features.
> + *
> + * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
> + * now for the checking.
> + */
> + if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
> + ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
> + if (ret) {
> + pr_warn("set BACO feature enabled failed, return %d\n", ret);
> + return ret;
> + }
> + }
> +
> + return ret;
> }
>
> static int smu_hw_fini(void *handle)
> @@ -1396,25 +1426,10 @@ static int smu_hw_fini(void *handle)
> return ret;
> }
>
> - /*
> - * For custom pptable uploading, skip the DPM features
> - * disable process on Navi1x ASICs.
> - * - As the gfx related features are under control of
> - * RLC on those ASICs. RLC reinitialization will be
> - * needed to reenable them. That will cost much more
> - * efforts.
> - *
> - * - SMU firmware can handle the DPM reenablement
> - * properly.
> - */
> - if (!smu->uploading_custom_pp_table ||
> - !((adev->asic_type >= CHIP_NAVI10) &&
> - (adev->asic_type <= CHIP_NAVI12))) {
> - ret = smu_stop_dpms(smu);
> - if (ret) {
> - pr_warn("Fail to stop Dpms!\n");
> - return ret;
> - }
> + ret = smu_disable_dpms(smu);
> + if (ret) {
> + pr_warn("Fail to stop Dpms!\n");
> + return ret;
> }
>
> kfree(table_context->driver_pptable);
> @@ -1453,68 +1468,6 @@ int smu_reset(struct smu_context *smu)
> return ret;
> }
>
> -static int smu_disable_dpm(struct smu_context *smu)
> -{
> - struct amdgpu_device *adev = smu->adev;
> - uint32_t smu_version;
> - int ret = 0;
> - bool use_baco = !smu->is_apu &&
> - ((adev->in_gpu_reset &&
> - (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> - ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
> -
> - ret = smu_get_smc_version(smu, NULL, &smu_version);
> - if (ret) {
> - pr_err("Failed to get smu version.\n");
> - return ret;
> - }
> -
> - /*
> - * Disable all enabled SMU features.
> - * This should be handled in SMU FW, as a backup
> - * driver can issue call to SMU FW until sequence
> - * in SMU FW is operational.
> - */
> - ret = smu_system_features_control(smu, false);
> - if (ret) {
> - pr_err("Failed to disable smu features.\n");
> - return ret;
> - }
> -
> - /*
> - * Arcturus does not have BACO bit in disable feature mask.
> - * Enablement of BACO bit on Arcturus should be skipped.
> - */
> - if (adev->asic_type == CHIP_ARCTURUS) {
> - if (use_baco && (smu_version > 0x360e00))
> - return 0;
> - }
> -
> - /* For baco, need to leave BACO feature enabled */
> - if (use_baco) {
> - /*
> - * Correct the way for checking whether SMU_FEATURE_BACO_BIT
> - * is supported.
> - *
> - * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
> - * always return false as the 'smu_system_features_control(smu, false)'
> - * was just issued above which disabled all SMU features.
> - *
> - * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
> - * now for the checking.
> - */
> - if (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0) {
> - ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
> - if (ret) {
> - pr_warn("set BACO feature enabled failed, return %d\n", ret);
> - return ret;
> - }
> - }
> - }
> -
> - return ret;
> -}
> -
> static int smu_suspend(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> @@ -1537,7 +1490,7 @@ static int smu_suspend(void *handle)
> return ret;
> }
>
> - ret = smu_disable_dpm(smu);
> + ret = smu_disable_dpms(smu);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 68142f6798c6..652728f18271 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -2209,12 +2209,31 @@ static int navi10_dummy_pstate_control(struct smu_context *smu, bool enable)
> return result;
> }
>
> +static inline bool navi10_need_umc_cdr_12gbps_workaround(struct amdgpu_device *adev)
> +{
> + if (adev->asic_type != CHIP_NAVI10)
> + return false;
> +
> + if ((adev->pdev->device == 0x731f && (adev->pdev->revision == 0xc2 ||
> + adev->pdev->revision == 0xc3 || adev->pdev->revision == 0xca ||
> + adev->pdev->revision == 0xcb)) ||
> + (adev->pdev->device == 0x66af && (adev->pdev->revision == 0xf3 ||
> + adev->pdev->revision == 0xf4 || adev->pdev->revision == 0xf5 ||
> + adev->pdev->revision == 0xf6)))
> + return true;
> + else
> + return false;
> +}
Do we need a separate function for this or can we just inline this
code in the function below?
> +
> static int navi10_disable_umc_cdr_12gbps_workaround(struct smu_context *smu)
> {
> uint32_t uclk_count, uclk_min, uclk_max;
> uint32_t smu_version;
> int ret = 0;
>
> + if (!navi10_need_umc_cdr_12gbps_workaround(smu->adev))
> + return 0;
> +
> ret = smu_get_smc_version(smu, NULL, &smu_version);
> if (ret)
> return ret;
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/9] drm/amd/powerplay: centralize all buffer allocation in sw_init phase
2020-06-01 7:29 ` [PATCH 3/9] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
@ 2020-06-02 15:00 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:00 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan <evan.quan@amd.com> wrote:
>
> To fit common design. And this can simplify the buffer deallocation.
>
> Change-Id: Iee682e76aadb5f34861d69d5794ced44f0a78789
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Took me a little while to sort out the functional changes from the
non-functional moves. Might be clearer to split those up. Either
way:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 330 ++++++++++-----------
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 105 ++++---
> 2 files changed, 223 insertions(+), 212 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index b4f108cb52fa..70c7b3fdee79 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -817,6 +817,147 @@ int smu_get_atom_data_table(struct smu_context *smu, uint32_t table,
> return 0;
> }
>
> +static int smu_init_fb_allocations(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> + struct smu_table_context *smu_table = &smu->smu_table;
> + struct smu_table *tables = smu_table->tables;
> + struct smu_table *driver_table = &(smu_table->driver_table);
> + uint32_t max_table_size = 0;
> + int ret, i;
> +
> + /* VRAM allocation for tool table */
> + if (tables[SMU_TABLE_PMSTATUSLOG].size) {
> + ret = amdgpu_bo_create_kernel(adev,
> + tables[SMU_TABLE_PMSTATUSLOG].size,
> + tables[SMU_TABLE_PMSTATUSLOG].align,
> + tables[SMU_TABLE_PMSTATUSLOG].domain,
> + &tables[SMU_TABLE_PMSTATUSLOG].bo,
> + &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
> + &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
> + if (ret) {
> + pr_err("VRAM allocation for tool table failed!\n");
> + return ret;
> + }
> + }
> +
> + /* VRAM allocation for driver table */
> + for (i = 0; i < SMU_TABLE_COUNT; i++) {
> + if (tables[i].size == 0)
> + continue;
> +
> + if (i == SMU_TABLE_PMSTATUSLOG)
> + continue;
> +
> + if (max_table_size < tables[i].size)
> + max_table_size = tables[i].size;
> + }
> +
> + driver_table->size = max_table_size;
> + driver_table->align = PAGE_SIZE;
> + driver_table->domain = AMDGPU_GEM_DOMAIN_VRAM;
> +
> + ret = amdgpu_bo_create_kernel(adev,
> + driver_table->size,
> + driver_table->align,
> + driver_table->domain,
> + &driver_table->bo,
> + &driver_table->mc_address,
> + &driver_table->cpu_addr);
> + if (ret) {
> + pr_err("VRAM allocation for driver table failed!\n");
> + if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
> + amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
> + &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
> + &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
> + }
> +
> + return ret;
> +}
> +
> +static int smu_fini_fb_allocations(struct smu_context *smu)
> +{
> + struct smu_table_context *smu_table = &smu->smu_table;
> + struct smu_table *tables = smu_table->tables;
> + struct smu_table *driver_table = &(smu_table->driver_table);
> +
> + if (!tables)
> + return 0;
> +
> + if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
> + amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
> + &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
> + &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
> +
> + amdgpu_bo_free_kernel(&driver_table->bo,
> + &driver_table->mc_address,
> + &driver_table->cpu_addr);
> +
> + return 0;
> +}
> +
> +/**
> + * smu_alloc_memory_pool - allocate memory pool in the system memory
> + *
> + * @smu: amdgpu_device pointer
> + *
> + * This memory pool will be used for SMC use and msg SetSystemVirtualDramAddr
> + * and DramLogSetDramAddr can notify it changed.
> + *
> + * Returns 0 on success, error on failure.
> + */
> +static int smu_alloc_memory_pool(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> + struct smu_table_context *smu_table = &smu->smu_table;
> + struct smu_table *memory_pool = &smu_table->memory_pool;
> + uint64_t pool_size = smu->pool_size;
> + int ret = 0;
> +
> + if (pool_size == SMU_MEMORY_POOL_SIZE_ZERO)
> + return ret;
> +
> + memory_pool->size = pool_size;
> + memory_pool->align = PAGE_SIZE;
> + memory_pool->domain = AMDGPU_GEM_DOMAIN_GTT;
> +
> + switch (pool_size) {
> + case SMU_MEMORY_POOL_SIZE_256_MB:
> + case SMU_MEMORY_POOL_SIZE_512_MB:
> + case SMU_MEMORY_POOL_SIZE_1_GB:
> + case SMU_MEMORY_POOL_SIZE_2_GB:
> + ret = amdgpu_bo_create_kernel(adev,
> + memory_pool->size,
> + memory_pool->align,
> + memory_pool->domain,
> + &memory_pool->bo,
> + &memory_pool->mc_address,
> + &memory_pool->cpu_addr);
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int smu_free_memory_pool(struct smu_context *smu)
> +{
> + struct smu_table_context *smu_table = &smu->smu_table;
> + struct smu_table *memory_pool = &smu_table->memory_pool;
> +
> + if (memory_pool->size == SMU_MEMORY_POOL_SIZE_ZERO)
> + return 0;
> +
> + amdgpu_bo_free_kernel(&memory_pool->bo,
> + &memory_pool->mc_address,
> + &memory_pool->cpu_addr);
> +
> + memset(memory_pool, 0, sizeof(struct smu_table));
> +
> + return 0;
> +}
> +
> static int smu_smc_table_sw_init(struct smu_context *smu)
> {
> int ret;
> @@ -841,6 +982,17 @@ static int smu_smc_table_sw_init(struct smu_context *smu)
> return ret;
> }
>
> + /*
> + * allocate vram bos to store smc table contents.
> + */
> + ret = smu_init_fb_allocations(smu);
> + if (ret)
> + return ret;
> +
> + ret = smu_alloc_memory_pool(smu);
> + if (ret)
> + return ret;
> +
> return 0;
> }
>
> @@ -848,6 +1000,14 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
> {
> int ret;
>
> + ret = smu_free_memory_pool(smu);
> + if (ret)
> + return ret;
> +
> + ret = smu_fini_fb_allocations(smu);
> + if (ret)
> + return ret;
> +
> ret = smu_fini_power(smu);
> if (ret) {
> pr_err("Failed to init smu_fini_power!\n");
> @@ -947,85 +1107,6 @@ static int smu_sw_fini(void *handle)
> return 0;
> }
>
> -static int smu_init_fb_allocations(struct smu_context *smu)
> -{
> - struct amdgpu_device *adev = smu->adev;
> - struct smu_table_context *smu_table = &smu->smu_table;
> - struct smu_table *tables = smu_table->tables;
> - struct smu_table *driver_table = &(smu_table->driver_table);
> - uint32_t max_table_size = 0;
> - int ret, i;
> -
> - /* VRAM allocation for tool table */
> - if (tables[SMU_TABLE_PMSTATUSLOG].size) {
> - ret = amdgpu_bo_create_kernel(adev,
> - tables[SMU_TABLE_PMSTATUSLOG].size,
> - tables[SMU_TABLE_PMSTATUSLOG].align,
> - tables[SMU_TABLE_PMSTATUSLOG].domain,
> - &tables[SMU_TABLE_PMSTATUSLOG].bo,
> - &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
> - &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
> - if (ret) {
> - pr_err("VRAM allocation for tool table failed!\n");
> - return ret;
> - }
> - }
> -
> - /* VRAM allocation for driver table */
> - for (i = 0; i < SMU_TABLE_COUNT; i++) {
> - if (tables[i].size == 0)
> - continue;
> -
> - if (i == SMU_TABLE_PMSTATUSLOG)
> - continue;
> -
> - if (max_table_size < tables[i].size)
> - max_table_size = tables[i].size;
> - }
> -
> - driver_table->size = max_table_size;
> - driver_table->align = PAGE_SIZE;
> - driver_table->domain = AMDGPU_GEM_DOMAIN_VRAM;
> -
> - ret = amdgpu_bo_create_kernel(adev,
> - driver_table->size,
> - driver_table->align,
> - driver_table->domain,
> - &driver_table->bo,
> - &driver_table->mc_address,
> - &driver_table->cpu_addr);
> - if (ret) {
> - pr_err("VRAM allocation for driver table failed!\n");
> - if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
> - amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
> - &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
> - &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
> - }
> -
> - return ret;
> -}
> -
> -static int smu_fini_fb_allocations(struct smu_context *smu)
> -{
> - struct smu_table_context *smu_table = &smu->smu_table;
> - struct smu_table *tables = smu_table->tables;
> - struct smu_table *driver_table = &(smu_table->driver_table);
> -
> - if (!tables)
> - return 0;
> -
> - if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
> - amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
> - &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
> - &tables[SMU_TABLE_PMSTATUSLOG].cpu_addr);
> -
> - amdgpu_bo_free_kernel(&driver_table->bo,
> - &driver_table->mc_address,
> - &driver_table->cpu_addr);
> -
> - return 0;
> -}
> -
> static int smu_smc_table_hw_init(struct smu_context *smu,
> bool initialize)
> {
> @@ -1063,13 +1144,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> if (ret)
> return ret;
>
> - /*
> - * allocate vram bos to store smc table contents.
> - */
> - ret = smu_init_fb_allocations(smu);
> - if (ret)
> - return ret;
> -
> /*
> * Parse pptable format and fill PPTable_t smc_pptable to
> * smu_table_context structure. And read the smc_dpm_table from vbios,
> @@ -1187,68 +1261,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> return ret;
> }
>
> -/**
> - * smu_alloc_memory_pool - allocate memory pool in the system memory
> - *
> - * @smu: amdgpu_device pointer
> - *
> - * This memory pool will be used for SMC use and msg SetSystemVirtualDramAddr
> - * and DramLogSetDramAddr can notify it changed.
> - *
> - * Returns 0 on success, error on failure.
> - */
> -static int smu_alloc_memory_pool(struct smu_context *smu)
> -{
> - struct amdgpu_device *adev = smu->adev;
> - struct smu_table_context *smu_table = &smu->smu_table;
> - struct smu_table *memory_pool = &smu_table->memory_pool;
> - uint64_t pool_size = smu->pool_size;
> - int ret = 0;
> -
> - if (pool_size == SMU_MEMORY_POOL_SIZE_ZERO)
> - return ret;
> -
> - memory_pool->size = pool_size;
> - memory_pool->align = PAGE_SIZE;
> - memory_pool->domain = AMDGPU_GEM_DOMAIN_GTT;
> -
> - switch (pool_size) {
> - case SMU_MEMORY_POOL_SIZE_256_MB:
> - case SMU_MEMORY_POOL_SIZE_512_MB:
> - case SMU_MEMORY_POOL_SIZE_1_GB:
> - case SMU_MEMORY_POOL_SIZE_2_GB:
> - ret = amdgpu_bo_create_kernel(adev,
> - memory_pool->size,
> - memory_pool->align,
> - memory_pool->domain,
> - &memory_pool->bo,
> - &memory_pool->mc_address,
> - &memory_pool->cpu_addr);
> - break;
> - default:
> - break;
> - }
> -
> - return ret;
> -}
> -
> -static int smu_free_memory_pool(struct smu_context *smu)
> -{
> - struct smu_table_context *smu_table = &smu->smu_table;
> - struct smu_table *memory_pool = &smu_table->memory_pool;
> -
> - if (memory_pool->size == SMU_MEMORY_POOL_SIZE_ZERO)
> - return 0;
> -
> - amdgpu_bo_free_kernel(&memory_pool->bo,
> - &memory_pool->mc_address,
> - &memory_pool->cpu_addr);
> -
> - memset(memory_pool, 0, sizeof(struct smu_table));
> -
> - return 0;
> -}
> -
> static int smu_start_smc_engine(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> @@ -1306,10 +1318,6 @@ static int smu_hw_init(void *handle)
> if (ret)
> goto failed;
>
> - ret = smu_alloc_memory_pool(smu);
> - if (ret)
> - goto failed;
> -
> /*
> * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
> * pool location.
> @@ -1401,7 +1409,6 @@ static int smu_hw_fini(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> struct smu_context *smu = &adev->smu;
> - struct smu_table_context *table_context = &smu->smu_table;
> int ret = 0;
>
> if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
> @@ -1432,23 +1439,6 @@ static int smu_hw_fini(void *handle)
> return ret;
> }
>
> - kfree(table_context->driver_pptable);
> - table_context->driver_pptable = NULL;
> -
> - kfree(table_context->max_sustainable_clocks);
> - table_context->max_sustainable_clocks = NULL;
> -
> - kfree(table_context->overdrive_table);
> - table_context->overdrive_table = NULL;
> -
> - ret = smu_fini_fb_allocations(smu);
> - if (ret)
> - return ret;
> -
> - ret = smu_free_memory_pool(smu);
> - if (ret)
> - return ret;
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index d6bdd2126f72..3b22f66e3fbc 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -432,25 +432,67 @@ int smu_v11_0_init_smc_tables(struct smu_context *smu)
> struct smu_table *tables = NULL;
> int ret = 0;
>
> - if (smu_table->tables)
> - return -EINVAL;
> -
> tables = kcalloc(SMU_TABLE_COUNT, sizeof(struct smu_table),
> GFP_KERNEL);
> - if (!tables)
> - return -ENOMEM;
> -
> + if (!tables) {
> + ret = -ENOMEM;
> + goto err0_out;
> + }
> smu_table->tables = tables;
>
> ret = smu_tables_init(smu, tables);
> if (ret)
> - return ret;
> + goto err1_out;
>
> ret = smu_v11_0_init_dpm_context(smu);
> if (ret)
> - return ret;
> + goto err1_out;
> +
> + smu_table->driver_pptable =
> + kzalloc(tables[SMU_TABLE_PPTABLE].size, GFP_KERNEL);
> + if (!smu_table->driver_pptable) {
> + ret = -ENOMEM;
> + goto err2_out;
> + }
> +
> + smu_table->max_sustainable_clocks =
> + kzalloc(sizeof(struct smu_11_0_max_sustainable_clocks), GFP_KERNEL);
> + if (!smu_table->max_sustainable_clocks) {
> + ret = -ENOMEM;
> + goto err3_out;
> + }
> +
> + /* Arcturus does not support OVERDRIVE */
> + if (tables[SMU_TABLE_OVERDRIVE].size) {
> + smu_table->overdrive_table =
> + kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> + if (!smu_table->overdrive_table) {
> + ret = -ENOMEM;
> + goto err4_out;
> + }
> +
> + smu_table->boot_overdrive_table =
> + kzalloc(tables[SMU_TABLE_OVERDRIVE].size, GFP_KERNEL);
> + if (!smu_table->boot_overdrive_table) {
> + ret = -ENOMEM;
> + goto err5_out;
> + }
> + }
>
> return 0;
> +
> +err5_out:
> + kfree(smu_table->overdrive_table);
> +err4_out:
> + kfree(smu_table->max_sustainable_clocks);
> +err3_out:
> + kfree(smu_table->driver_pptable);
> +err2_out:
> + smu_v11_0_fini_dpm_context(smu);
> +err1_out:
> + kfree(tables);
> +err0_out:
> + return ret;
> }
>
> int smu_v11_0_fini_smc_tables(struct smu_context *smu)
> @@ -461,6 +503,17 @@ int smu_v11_0_fini_smc_tables(struct smu_context *smu)
> if (!smu_table->tables)
> return -EINVAL;
>
> + kfree(smu_table->boot_overdrive_table);
> + kfree(smu_table->overdrive_table);
> + kfree(smu_table->max_sustainable_clocks);
> + kfree(smu_table->driver_pptable);
> + smu_table->boot_overdrive_table = NULL;
> + smu_table->overdrive_table = NULL;
> + smu_table->max_sustainable_clocks = NULL;
> + smu_table->driver_pptable = NULL;
> + kfree(smu_table->hardcode_pptable);
> + smu_table->hardcode_pptable = NULL;
> +
> kfree(smu_table->tables);
> kfree(smu_table->metrics_table);
> kfree(smu_table->watermarks_table);
> @@ -723,18 +776,6 @@ int smu_v11_0_parse_pptable(struct smu_context *smu)
> {
> int ret;
>
> - struct smu_table_context *table_context = &smu->smu_table;
> - struct smu_table *table = &table_context->tables[SMU_TABLE_PPTABLE];
> -
> - /* during TDR we need to free and alloc the pptable */
> - if (table_context->driver_pptable)
> - kfree(table_context->driver_pptable);
> -
> - table_context->driver_pptable = kzalloc(table->size, GFP_KERNEL);
> -
> - if (!table_context->driver_pptable)
> - return -ENOMEM;
> -
> ret = smu_store_powerplay_table(smu);
> if (ret)
> return -EINVAL;
> @@ -975,17 +1016,10 @@ smu_v11_0_get_max_sustainable_clock(struct smu_context *smu, uint32_t *clock,
>
> int smu_v11_0_init_max_sustainable_clocks(struct smu_context *smu)
> {
> - struct smu_11_0_max_sustainable_clocks *max_sustainable_clocks;
> + struct smu_11_0_max_sustainable_clocks *max_sustainable_clocks =
> + smu->smu_table.max_sustainable_clocks;
> int ret = 0;
>
> - if (!smu->smu_table.max_sustainable_clocks)
> - max_sustainable_clocks = kzalloc(sizeof(struct smu_11_0_max_sustainable_clocks),
> - GFP_KERNEL);
> - else
> - max_sustainable_clocks = smu->smu_table.max_sustainable_clocks;
> -
> - smu->smu_table.max_sustainable_clocks = (void *)max_sustainable_clocks;
> -
> max_sustainable_clocks->uclock = smu->smu_table.boot_values.uclk / 100;
> max_sustainable_clocks->soc_clock = smu->smu_table.boot_values.socclk / 100;
> max_sustainable_clocks->dcef_clock = smu->smu_table.boot_values.dcefclk / 100;
> @@ -1930,24 +1964,11 @@ int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool initialize,
> int ret = 0;
>
> if (initialize) {
> - if (table_context->overdrive_table) {
> - return -EINVAL;
> - }
> - table_context->overdrive_table = kzalloc(overdrive_table_size, GFP_KERNEL);
> - if (!table_context->overdrive_table) {
> - return -ENOMEM;
> - }
> ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, false);
> if (ret) {
> pr_err("Failed to export overdrive table!\n");
> return ret;
> }
> - if (!table_context->boot_overdrive_table) {
> - table_context->boot_overdrive_table = kmemdup(table_context->overdrive_table, overdrive_table_size, GFP_KERNEL);
> - if (!table_context->boot_overdrive_table) {
> - return -ENOMEM;
> - }
> - }
> }
> ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, true);
> if (ret) {
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/9] drm/amd/powerplay: clean up the APIs for bootup clocks
2020-06-01 7:29 ` [PATCH 4/9] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
@ 2020-06-02 15:01 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:01 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Combine and simplify the logics for retrieving bootup
> clocks.
>
> Change-Id: Ifca28c454f3769dece0cc705ba054ff34db0ab60
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 -
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 1 -
> .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 -
> drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 2 -
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 1 -
> drivers/gpu/drm/amd/powerplay/smu_internal.h | 2 -
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 141 +++++++-----------
> 7 files changed, 51 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 70c7b3fdee79..9bafa6b3e123 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1132,10 +1132,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> if (ret)
> return ret;
>
> - ret = smu_get_clk_info_from_vbios(smu);
> - if (ret)
> - return ret;
> -
> /*
> * check if the format_revision in vbios is up to pptable header
> * version, and the structure size is not 0.
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index e856ad36ab01..902c8cfa4a3b 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -2423,7 +2423,6 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> .check_fw_status = smu_v11_0_check_fw_status,
> .setup_pptable = smu_v11_0_setup_pptable,
> .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> - .get_clk_info_from_vbios = smu_v11_0_get_clk_info_from_vbios,
> .check_pptable = smu_v11_0_check_pptable,
> .parse_pptable = smu_v11_0_parse_pptable,
> .populate_smc_tables = smu_v11_0_populate_smc_pptable,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 5bb1ac821aeb..223678e329a5 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -505,7 +505,6 @@ struct pptable_funcs {
> int (*check_fw_status)(struct smu_context *smu);
> int (*setup_pptable)(struct smu_context *smu);
> int (*get_vbios_bootup_values)(struct smu_context *smu);
> - int (*get_clk_info_from_vbios)(struct smu_context *smu);
> int (*check_pptable)(struct smu_context *smu);
> int (*parse_pptable)(struct smu_context *smu);
> int (*populate_smc_tables)(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> index 71f829ab306e..5b785816aa64 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -161,8 +161,6 @@ int smu_v11_0_setup_pptable(struct smu_context *smu);
>
> int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu);
>
> -int smu_v11_0_get_clk_info_from_vbios(struct smu_context *smu);
> -
> int smu_v11_0_check_pptable(struct smu_context *smu);
>
> int smu_v11_0_parse_pptable(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 652728f18271..bea6a96b5afb 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -2320,7 +2320,6 @@ static const struct pptable_funcs navi10_ppt_funcs = {
> .check_fw_status = smu_v11_0_check_fw_status,
> .setup_pptable = smu_v11_0_setup_pptable,
> .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> - .get_clk_info_from_vbios = smu_v11_0_get_clk_info_from_vbios,
> .check_pptable = smu_v11_0_check_pptable,
> .parse_pptable = smu_v11_0_parse_pptable,
> .populate_smc_tables = smu_v11_0_populate_smc_pptable,
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> index 6c59eeef2590..a31df7f4e91a 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
> +++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> @@ -47,8 +47,6 @@
>
> #define smu_get_vbios_bootup_values(smu) \
> ((smu)->ppt_funcs->get_vbios_bootup_values ? (smu)->ppt_funcs->get_vbios_bootup_values((smu)) : 0)
> -#define smu_get_clk_info_from_vbios(smu) \
> - ((smu)->ppt_funcs->get_clk_info_from_vbios ? (smu)->ppt_funcs->get_clk_info_from_vbios((smu)) : 0)
> #define smu_check_pptable(smu) \
> ((smu)->ppt_funcs->check_pptable ? (smu)->ppt_funcs->check_pptable((smu)) : 0)
> #define smu_parse_pptable(smu) \
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 3b22f66e3fbc..be6dca8c6014 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -558,6 +558,32 @@ int smu_v11_0_fini_power(struct smu_context *smu)
> return 0;
> }
>
> +static int smu_v11_0_atom_get_smu_clockinfo(struct amdgpu_device *adev,
> + uint8_t clk_id,
> + uint8_t syspll_id,
> + uint32_t *clk_freq)
> +{
> + struct atom_get_smu_clock_info_parameters_v3_1 input = {0};
> + struct atom_get_smu_clock_info_output_parameters_v3_1 *output;
> + int ret, index;
> +
> + input.clk_id = clk_id;
> + input.syspll_id = syspll_id;
> + input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> + index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> + getsmuclockinfo);
> +
> + ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> + (uint32_t *)&input);
> + if (ret)
> + return -EINVAL;
> +
> + output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> + *clk_freq = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> +
> + return 0;
> +}
> +
> int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu)
> {
> int ret, index;
> @@ -616,102 +642,37 @@ int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu)
> smu->smu_table.boot_values.format_revision = header->format_revision;
> smu->smu_table.boot_values.content_revision = header->content_revision;
>
> - return 0;
> -}
> + smu_v11_0_atom_get_smu_clockinfo(smu->adev,
> + (uint8_t)SMU11_SYSPLL0_SOCCLK_ID,
> + (uint8_t)0,
> + &smu->smu_table.boot_values.socclk);
>
> -int smu_v11_0_get_clk_info_from_vbios(struct smu_context *smu)
> -{
> - int ret, index;
> - struct amdgpu_device *adev = smu->adev;
> - struct atom_get_smu_clock_info_parameters_v3_1 input = {0};
> - struct atom_get_smu_clock_info_output_parameters_v3_1 *output;
> + smu_v11_0_atom_get_smu_clockinfo(smu->adev,
> + (uint8_t)SMU11_SYSPLL0_DCEFCLK_ID,
> + (uint8_t)0,
> + &smu->smu_table.boot_values.dcefclk);
>
> - input.clk_id = SMU11_SYSPLL0_SOCCLK_ID;
> - input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> - index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> - getsmuclockinfo);
> -
> - ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> - (uint32_t *)&input);
> - if (ret)
> - return -EINVAL;
> + smu_v11_0_atom_get_smu_clockinfo(smu->adev,
> + (uint8_t)SMU11_SYSPLL0_ECLK_ID,
> + (uint8_t)0,
> + &smu->smu_table.boot_values.eclk);
>
> - output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> - smu->smu_table.boot_values.socclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> + smu_v11_0_atom_get_smu_clockinfo(smu->adev,
> + (uint8_t)SMU11_SYSPLL0_VCLK_ID,
> + (uint8_t)0,
> + &smu->smu_table.boot_values.vclk);
>
> - memset(&input, 0, sizeof(input));
> - input.clk_id = SMU11_SYSPLL0_DCEFCLK_ID;
> - input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> - index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> - getsmuclockinfo);
> -
> - ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> - (uint32_t *)&input);
> - if (ret)
> - return -EINVAL;
> -
> - output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> - smu->smu_table.boot_values.dcefclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> -
> - memset(&input, 0, sizeof(input));
> - input.clk_id = SMU11_SYSPLL0_ECLK_ID;
> - input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> - index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> - getsmuclockinfo);
> -
> - ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> - (uint32_t *)&input);
> - if (ret)
> - return -EINVAL;
> -
> - output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> - smu->smu_table.boot_values.eclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> -
> - memset(&input, 0, sizeof(input));
> - input.clk_id = SMU11_SYSPLL0_VCLK_ID;
> - input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> - index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> - getsmuclockinfo);
> -
> - ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> - (uint32_t *)&input);
> - if (ret)
> - return -EINVAL;
> -
> - output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> - smu->smu_table.boot_values.vclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> -
> - memset(&input, 0, sizeof(input));
> - input.clk_id = SMU11_SYSPLL0_DCLK_ID;
> - input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> - index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> - getsmuclockinfo);
> -
> - ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> - (uint32_t *)&input);
> - if (ret)
> - return -EINVAL;
> -
> - output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> - smu->smu_table.boot_values.dclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> + smu_v11_0_atom_get_smu_clockinfo(smu->adev,
> + (uint8_t)SMU11_SYSPLL0_DCLK_ID,
> + (uint8_t)0,
> + &smu->smu_table.boot_values.dclk);
>
> if ((smu->smu_table.boot_values.format_revision == 3) &&
> - (smu->smu_table.boot_values.content_revision >= 2)) {
> - memset(&input, 0, sizeof(input));
> - input.clk_id = SMU11_SYSPLL1_0_FCLK_ID;
> - input.syspll_id = SMU11_SYSPLL1_2_ID;
> - input.command = GET_SMU_CLOCK_INFO_V3_1_GET_CLOCK_FREQ;
> - index = get_index_into_master_table(atom_master_list_of_command_functions_v2_1,
> - getsmuclockinfo);
> -
> - ret = amdgpu_atom_execute_table(adev->mode_info.atom_context, index,
> - (uint32_t *)&input);
> - if (ret)
> - return -EINVAL;
> -
> - output = (struct atom_get_smu_clock_info_output_parameters_v3_1 *)&input;
> - smu->smu_table.boot_values.fclk = le32_to_cpu(output->atom_smu_outputclkfreq.smu_clock_freq_hz) / 10000;
> - }
> + (smu->smu_table.boot_values.content_revision >= 2))
> + smu_v11_0_atom_get_smu_clockinfo(smu->adev,
> + (uint8_t)SMU11_SYSPLL1_0_FCLK_ID,
> + (uint8_t)SMU11_SYSPLL1_2_ID,
> + &smu->smu_table.boot_values.fclk);
>
> return 0;
> }
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup
2020-06-01 7:29 ` [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
@ 2020-06-02 15:04 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:04 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Combine and simplify the logics for setup pptable.
>
> Change-Id: I062f15eab586050593afd960432c4c70fbdd5d41
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 17 ----
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 66 ++++++++-----
> .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 5 -
> drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 4 -
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 92 ++++++++++---------
> drivers/gpu/drm/amd/powerplay/smu_internal.h | 10 --
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 21 -----
> 7 files changed, 89 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 9bafa6b3e123..b079ac6325d0 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1132,23 +1132,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> if (ret)
> return ret;
>
> - /*
> - * check if the format_revision in vbios is up to pptable header
> - * version, and the structure size is not 0.
> - */
> - ret = smu_check_pptable(smu);
> - if (ret)
> - return ret;
> -
> - /*
> - * Parse pptable format and fill PPTable_t smc_pptable to
> - * smu_table_context structure. And read the smc_dpm_table from vbios,
> - * then fill it into smc_pptable.
> - */
> - ret = smu_parse_pptable(smu);
> - if (ret)
> - return ret;
> -
> /*
> * Send msg GetDriverIfVersion to check if the return value is equal
> * with DRIVER_IF_VERSION of smc header.
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 902c8cfa4a3b..c5c23126ec2d 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -487,33 +487,33 @@ static int arcturus_set_default_dpm_table(struct smu_context *smu)
>
> static int arcturus_check_powerplay_table(struct smu_context *smu)
> {
> + struct smu_table_context *table_context = &smu->smu_table;
> + struct smu_11_0_powerplay_table *powerplay_table =
> + table_context->power_play_table;
> + struct smu_baco_context *smu_baco = &smu->smu_baco;
> +
> + mutex_lock(&smu_baco->mutex);
> + if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> + powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> + smu_baco->platform_support = true;
> + mutex_unlock(&smu_baco->mutex);
> +
> + table_context->thermal_controller_type =
> + powerplay_table->thermal_controller_type;
> +
> return 0;
> }
>
> static int arcturus_store_powerplay_table(struct smu_context *smu)
> {
> - struct smu_11_0_powerplay_table *powerplay_table = NULL;
> struct smu_table_context *table_context = &smu->smu_table;
> - struct smu_baco_context *smu_baco = &smu->smu_baco;
> - int ret = 0;
> -
> - if (!table_context->power_play_table)
> - return -EINVAL;
> -
> - powerplay_table = table_context->power_play_table;
> + struct smu_11_0_powerplay_table *powerplay_table =
> + table_context->power_play_table;
>
> memcpy(table_context->driver_pptable, &powerplay_table->smc_pptable,
> sizeof(PPTable_t));
>
> - table_context->thermal_controller_type = powerplay_table->thermal_controller_type;
> -
> - mutex_lock(&smu_baco->mutex);
> - if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> - powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> - smu_baco->platform_support = true;
> - mutex_unlock(&smu_baco->mutex);
> -
> - return ret;
> + return 0;
> }
>
> static int arcturus_append_powerplay_table(struct smu_context *smu)
> @@ -544,6 +544,29 @@ static int arcturus_append_powerplay_table(struct smu_context *smu)
> return 0;
> }
>
> +static int arcturus_setup_pptable(struct smu_context *smu)
> +{
> + int ret = 0;
> +
> + ret = smu_v11_0_setup_pptable(smu);
> + if (ret)
> + return ret;
> +
> + ret = arcturus_store_powerplay_table(smu);
> + if (ret)
> + return ret;
> +
> + ret = arcturus_append_powerplay_table(smu);
> + if (ret)
> + return ret;
> +
> + ret = arcturus_check_powerplay_table(smu);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> static int arcturus_run_btc(struct smu_context *smu)
> {
> int ret = 0;
> @@ -2383,10 +2406,6 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> /* internal structurs allocations */
> .tables_init = arcturus_tables_init,
> .alloc_dpm_context = arcturus_allocate_dpm_context,
> - /* pptable related */
> - .check_powerplay_table = arcturus_check_powerplay_table,
> - .store_powerplay_table = arcturus_store_powerplay_table,
> - .append_powerplay_table = arcturus_append_powerplay_table,
> /* init dpm */
> .get_allowed_feature_mask = arcturus_get_allowed_feature_mask,
> /* btc */
> @@ -2421,10 +2440,9 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> .init_power = smu_v11_0_init_power,
> .fini_power = smu_v11_0_fini_power,
> .check_fw_status = smu_v11_0_check_fw_status,
> - .setup_pptable = smu_v11_0_setup_pptable,
> + /* pptable related */
> + .setup_pptable = arcturus_setup_pptable,
> .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> - .check_pptable = smu_v11_0_check_pptable,
> - .parse_pptable = smu_v11_0_parse_pptable,
> .populate_smc_tables = smu_v11_0_populate_smc_pptable,
> .check_fw_version = smu_v11_0_check_fw_version,
> .write_pptable = smu_v11_0_write_pptable,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 223678e329a5..14f4a850b553 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -417,9 +417,6 @@ struct i2c_adapter;
>
> struct pptable_funcs {
> int (*alloc_dpm_context)(struct smu_context *smu);
> - int (*store_powerplay_table)(struct smu_context *smu);
> - int (*check_powerplay_table)(struct smu_context *smu);
> - int (*append_powerplay_table)(struct smu_context *smu);
> int (*get_smu_msg_index)(struct smu_context *smu, uint32_t index);
> int (*get_smu_clk_index)(struct smu_context *smu, uint32_t index);
> int (*get_smu_feature_index)(struct smu_context *smu, uint32_t index);
> @@ -505,8 +502,6 @@ struct pptable_funcs {
> int (*check_fw_status)(struct smu_context *smu);
> int (*setup_pptable)(struct smu_context *smu);
> int (*get_vbios_bootup_values)(struct smu_context *smu);
> - int (*check_pptable)(struct smu_context *smu);
> - int (*parse_pptable)(struct smu_context *smu);
> int (*populate_smc_tables)(struct smu_context *smu);
> int (*check_fw_version)(struct smu_context *smu);
> int (*powergate_sdma)(struct smu_context *smu, bool gate);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> index 5b785816aa64..51868dc33238 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -161,10 +161,6 @@ int smu_v11_0_setup_pptable(struct smu_context *smu);
>
> int smu_v11_0_get_vbios_bootup_values(struct smu_context *smu);
>
> -int smu_v11_0_check_pptable(struct smu_context *smu);
> -
> -int smu_v11_0_parse_pptable(struct smu_context *smu);
> -
> int smu_v11_0_populate_smc_pptable(struct smu_context *smu);
>
> int smu_v11_0_check_fw_version(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index bea6a96b5afb..db38fb10524d 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -417,6 +417,29 @@ navi10_get_allowed_feature_mask(struct smu_context *smu,
>
> static int navi10_check_powerplay_table(struct smu_context *smu)
> {
> + struct smu_table_context *table_context = &smu->smu_table;
> + struct smu_11_0_powerplay_table *powerplay_table =
> + table_context->power_play_table;
> + struct smu_baco_context *smu_baco = &smu->smu_baco;
> +
> + if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
> + smu->dc_controlled_by_gpio = true;
> +
> + mutex_lock(&smu_baco->mutex);
> + if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> + powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> + smu_baco->platform_support = true;
> + mutex_unlock(&smu_baco->mutex);
> +
> + table_context->thermal_controller_type =
> + powerplay_table->thermal_controller_type;
> +
> + /*
> + * Instead of having its own buffer space and get overdrive_table copied,
> + * smu->od_settings just points to the actual overdrive_table
> + */
> + smu->od_settings = &powerplay_table->overdrive_table;
> +
> return 0;
> }
>
> @@ -475,30 +498,37 @@ static int navi10_append_powerplay_table(struct smu_context *smu)
>
> static int navi10_store_powerplay_table(struct smu_context *smu)
> {
> - struct smu_11_0_powerplay_table *powerplay_table = NULL;
> struct smu_table_context *table_context = &smu->smu_table;
> - struct smu_baco_context *smu_baco = &smu->smu_baco;
> -
> - if (!table_context->power_play_table)
> - return -EINVAL;
> -
> - powerplay_table = table_context->power_play_table;
> + struct smu_11_0_powerplay_table *powerplay_table =
> + table_context->power_play_table;
>
> memcpy(table_context->driver_pptable, &powerplay_table->smc_pptable,
> sizeof(PPTable_t));
>
> - table_context->thermal_controller_type = powerplay_table->thermal_controller_type;
> + return 0;
> +}
>
> - if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
> - smu->dc_controlled_by_gpio = true;
> +static int navi10_setup_pptable(struct smu_context *smu)
> +{
> + int ret = 0;
>
> - mutex_lock(&smu_baco->mutex);
> - if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
> - powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
> - smu_baco->platform_support = true;
> - mutex_unlock(&smu_baco->mutex);
> + ret = smu_v11_0_setup_pptable(smu);
> + if (ret)
> + return ret;
>
> - return 0;
> + ret = navi10_store_powerplay_table(smu);
> + if (ret)
> + return ret;
> +
> + ret = navi10_append_powerplay_table(smu);
> + if (ret)
> + return ret;
> +
> + ret = navi10_check_powerplay_table(smu);
> + if (ret)
> + return ret;
> +
> + return ret;
> }
>
> static int navi10_tables_init(struct smu_context *smu, struct smu_table *tables)
> @@ -1927,24 +1957,6 @@ static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
> return 0;
> }
>
> -static int navi10_setup_od_limits(struct smu_context *smu) {
> - struct smu_11_0_overdrive_table *overdrive_table = NULL;
> - struct smu_11_0_powerplay_table *powerplay_table = NULL;
> -
> - if (!smu->smu_table.power_play_table) {
> - pr_err("powerplay table uninitialized!\n");
> - return -ENOENT;
> - }
> - powerplay_table = (struct smu_11_0_powerplay_table *)smu->smu_table.power_play_table;
> - overdrive_table = &powerplay_table->overdrive_table;
> - if (!smu->od_settings) {
> - smu->od_settings = kmemdup(overdrive_table, sizeof(struct smu_11_0_overdrive_table), GFP_KERNEL);
> - } else {
> - memcpy(smu->od_settings, overdrive_table, sizeof(struct smu_11_0_overdrive_table));
> - }
> - return 0;
> -}
> -
> static bool navi10_is_baco_supported(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> @@ -1968,11 +1980,6 @@ static int navi10_set_default_od_settings(struct smu_context *smu, bool initiali
> od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
> boot_od_table = (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> if (initialize) {
> - ret = navi10_setup_od_limits(smu);
> - if (ret) {
> - pr_err("Failed to retrieve board OD limits\n");
> - return ret;
> - }
> if (od_table) {
> if (!od_table->GfxclkVolt1) {
> ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> @@ -2274,9 +2281,6 @@ static int navi10_disable_umc_cdr_12gbps_workaround(struct smu_context *smu)
> static const struct pptable_funcs navi10_ppt_funcs = {
> .tables_init = navi10_tables_init,
> .alloc_dpm_context = navi10_allocate_dpm_context,
> - .store_powerplay_table = navi10_store_powerplay_table,
> - .check_powerplay_table = navi10_check_powerplay_table,
> - .append_powerplay_table = navi10_append_powerplay_table,
> .get_smu_msg_index = navi10_get_smu_msg_index,
> .get_smu_clk_index = navi10_get_smu_clk_index,
> .get_smu_feature_index = navi10_get_smu_feature_index,
> @@ -2318,10 +2322,8 @@ static const struct pptable_funcs navi10_ppt_funcs = {
> .init_power = smu_v11_0_init_power,
> .fini_power = smu_v11_0_fini_power,
> .check_fw_status = smu_v11_0_check_fw_status,
> - .setup_pptable = smu_v11_0_setup_pptable,
> + .setup_pptable = navi10_setup_pptable,
> .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> - .check_pptable = smu_v11_0_check_pptable,
> - .parse_pptable = smu_v11_0_parse_pptable,
> .populate_smc_tables = smu_v11_0_populate_smc_pptable,
> .check_fw_version = smu_v11_0_check_fw_version,
> .write_pptable = smu_v11_0_write_pptable,
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> index a31df7f4e91a..33086f94267a 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
> +++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> @@ -47,10 +47,6 @@
>
> #define smu_get_vbios_bootup_values(smu) \
> ((smu)->ppt_funcs->get_vbios_bootup_values ? (smu)->ppt_funcs->get_vbios_bootup_values((smu)) : 0)
> -#define smu_check_pptable(smu) \
> - ((smu)->ppt_funcs->check_pptable ? (smu)->ppt_funcs->check_pptable((smu)) : 0)
> -#define smu_parse_pptable(smu) \
> - ((smu)->ppt_funcs->parse_pptable ? (smu)->ppt_funcs->parse_pptable((smu)) : 0)
> #define smu_populate_smc_tables(smu) \
> ((smu)->ppt_funcs->populate_smc_tables ? (smu)->ppt_funcs->populate_smc_tables((smu)) : 0)
> #define smu_check_fw_version(smu) \
> @@ -96,12 +92,6 @@ static inline int smu_send_smc_msg(struct smu_context *smu, enum smu_message_typ
> ((smu)->ppt_funcs->is_dpm_running ? (smu)->ppt_funcs->is_dpm_running((smu)) : 0)
> #define smu_notify_display_change(smu) \
> ((smu)->ppt_funcs->notify_display_change? (smu)->ppt_funcs->notify_display_change((smu)) : 0)
> -#define smu_store_powerplay_table(smu) \
> - ((smu)->ppt_funcs->store_powerplay_table ? (smu)->ppt_funcs->store_powerplay_table((smu)) : 0)
> -#define smu_check_powerplay_table(smu) \
> - ((smu)->ppt_funcs->check_powerplay_table ? (smu)->ppt_funcs->check_powerplay_table((smu)) : 0)
> -#define smu_append_powerplay_table(smu) \
> - ((smu)->ppt_funcs->append_powerplay_table ? (smu)->ppt_funcs->append_powerplay_table((smu)) : 0)
> #define smu_set_default_dpm_table(smu) \
> ((smu)->ppt_funcs->set_default_dpm_table ? (smu)->ppt_funcs->set_default_dpm_table((smu)) : 0)
> #define smu_populate_umd_state_clk(smu) \
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index be6dca8c6014..7a97a4510c6d 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -725,27 +725,6 @@ int smu_v11_0_notify_memory_pool_location(struct smu_context *smu)
> return ret;
> }
>
> -int smu_v11_0_check_pptable(struct smu_context *smu)
> -{
> - int ret;
> -
> - ret = smu_check_powerplay_table(smu);
> - return ret;
> -}
> -
> -int smu_v11_0_parse_pptable(struct smu_context *smu)
> -{
> - int ret;
> -
> - ret = smu_store_powerplay_table(smu);
> - if (ret)
> - return -EINVAL;
> -
> - ret = smu_append_powerplay_table(smu);
> -
> - return ret;
> -}
> -
> int smu_v11_0_populate_smc_pptable(struct smu_context *smu)
> {
> int ret;
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/9] drm/amd/powerplay: clean up the overdrive settings
2020-06-01 7:30 ` [PATCH 6/9] drm/amd/powerplay: clean up the overdrive settings Evan Quan
@ 2020-06-02 15:06 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:06 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Eliminate the buffer allocation and drop the unnecessary
> overdrive table uploading.
>
> Change-Id: I8ba5383a330e6d5355cea219147500c1b4a43f47
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 +-
> .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
> drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 2 -
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 74 +++++++++----------
> drivers/gpu/drm/amd/powerplay/smu_internal.h | 4 +-
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 20 -----
> 6 files changed, 38 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index b079ac6325d0..9b81b6519a96 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1215,7 +1215,7 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> if (ret)
> return ret;
>
> - ret = smu_set_default_od_settings(smu, initialize);
> + ret = smu_set_default_od_settings(smu);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 14f4a850b553..4aa63dc79124 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -480,7 +480,7 @@ struct pptable_funcs {
> uint32_t *value);
> int (*get_thermal_temperature_range)(struct smu_context *smu, struct smu_temperature_range *range);
> int (*get_uclk_dpm_states)(struct smu_context *smu, uint32_t *clocks_in_khz, uint32_t *num_states);
> - int (*set_default_od_settings)(struct smu_context *smu, bool initialize);
> + int (*set_default_od_settings)(struct smu_context *smu);
> int (*set_performance_level)(struct smu_context *smu, enum amd_dpm_forced_level level);
> int (*display_disable_memory_clock_switch)(struct smu_context *smu, bool disable_memory_clock_switch);
> void (*dump_pptable)(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> index 51868dc33238..8d317e05f65b 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -258,8 +258,6 @@ int smu_v11_0_set_soft_freq_limited_range(struct smu_context *smu, enum smu_clk_
>
> int smu_v11_0_override_pcie_parameters(struct smu_context *smu);
>
> -int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool initialize, size_t overdrive_table_size);
> -
> uint32_t smu_v11_0_get_max_power_limit(struct smu_context *smu);
>
> int smu_v11_0_set_performance_level(struct smu_context *smu,
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index db38fb10524d..caa4355b601e 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -1969,55 +1969,49 @@ static bool navi10_is_baco_supported(struct smu_context *smu)
> return (val & RCC_BIF_STRAP0__STRAP_PX_CAPABLE_MASK) ? true : false;
> }
>
> -static int navi10_set_default_od_settings(struct smu_context *smu, bool initialize) {
> - OverDriveTable_t *od_table, *boot_od_table;
> +static int navi10_set_default_od_settings(struct smu_context *smu)
> +{
> + OverDriveTable_t *od_table =
> + (OverDriveTable_t *)smu->smu_table.overdrive_table;
> + OverDriveTable_t *boot_od_table =
> + (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> int ret = 0;
>
> - ret = smu_v11_0_set_default_od_settings(smu, initialize, sizeof(OverDriveTable_t));
> - if (ret)
> + ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, (void *)od_table, false);
> + if (ret) {
> + pr_err("Failed to get overdrive table!\n");
> return ret;
> + }
>
> - od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
> - boot_od_table = (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
> - if (initialize) {
> - if (od_table) {
> - if (!od_table->GfxclkVolt1) {
> - ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> - &od_table->GfxclkVolt1,
> - od_table->GfxclkFreq1);
> - if (ret)
> - od_table->GfxclkVolt1 = 0;
> - if (boot_od_table)
> - boot_od_table->GfxclkVolt1 = od_table->GfxclkVolt1;
> - }
> -
> - if (!od_table->GfxclkVolt2) {
> - ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> - &od_table->GfxclkVolt2,
> - od_table->GfxclkFreq2);
> - if (ret)
> - od_table->GfxclkVolt2 = 0;
> - if (boot_od_table)
> - boot_od_table->GfxclkVolt2 = od_table->GfxclkVolt2;
> - }
> + if (!od_table->GfxclkVolt1) {
> + ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> + &od_table->GfxclkVolt1,
> + od_table->GfxclkFreq1);
> + if (ret)
> + return ret;
> + }
>
> - if (!od_table->GfxclkVolt3) {
> - ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> - &od_table->GfxclkVolt3,
> - od_table->GfxclkFreq3);
> - if (ret)
> - od_table->GfxclkVolt3 = 0;
> - if (boot_od_table)
> - boot_od_table->GfxclkVolt3 = od_table->GfxclkVolt3;
> - }
> - }
> + if (!od_table->GfxclkVolt2) {
> + ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> + &od_table->GfxclkVolt2,
> + od_table->GfxclkFreq2);
> + if (ret)
> + return ret;
> }
>
> - if (od_table) {
> - navi10_dump_od_table(od_table);
> + if (!od_table->GfxclkVolt3) {
> + ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
> + &od_table->GfxclkVolt3,
> + od_table->GfxclkFreq3);
> + if (ret)
> + return ret;
> }
>
> - return ret;
> + memcpy(boot_od_table, od_table, sizeof(OverDriveTable_t));
> +
> + navi10_dump_od_table(od_table);
> +
> + return 0;
> }
>
> static int navi10_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TABLE_COMMAND type, long input[], uint32_t size) {
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> index 33086f94267a..0c7d5f0b1cd1 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
> +++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> @@ -70,8 +70,8 @@
> ((smu)->ppt_funcs->system_features_control ? (smu)->ppt_funcs->system_features_control((smu), (en)) : 0)
> #define smu_init_max_sustainable_clocks(smu) \
> ((smu)->ppt_funcs->init_max_sustainable_clocks ? (smu)->ppt_funcs->init_max_sustainable_clocks((smu)) : 0)
> -#define smu_set_default_od_settings(smu, initialize) \
> - ((smu)->ppt_funcs->set_default_od_settings ? (smu)->ppt_funcs->set_default_od_settings((smu), (initialize)) : 0)
> +#define smu_set_default_od_settings(smu) \
> + ((smu)->ppt_funcs->set_default_od_settings ? (smu)->ppt_funcs->set_default_od_settings((smu)) : 0)
>
> #define smu_send_smc_msg_with_param(smu, msg, param, read_arg) \
> ((smu)->ppt_funcs->send_smc_msg_with_param? (smu)->ppt_funcs->send_smc_msg_with_param((smu), (msg), (param), (read_arg)) : 0)
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 7a97a4510c6d..891781a5c0d4 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1898,26 +1898,6 @@ int smu_v11_0_override_pcie_parameters(struct smu_context *smu)
>
> }
>
> -int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool initialize, size_t overdrive_table_size)
> -{
> - struct smu_table_context *table_context = &smu->smu_table;
> - int ret = 0;
> -
> - if (initialize) {
> - ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, false);
> - if (ret) {
> - pr_err("Failed to export overdrive table!\n");
> - return ret;
> - }
> - }
> - ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0, table_context->overdrive_table, true);
> - if (ret) {
> - pr_err("Failed to import overdrive table!\n");
> - return ret;
> - }
> - return ret;
> -}
> -
> int smu_v11_0_set_performance_level(struct smu_context *smu,
> enum amd_dpm_forced_level level)
> {
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations
2020-06-01 7:30 ` [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations Evan Quan
@ 2020-06-02 15:11 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:11 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan <evan.quan@amd.com> 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 <evan.quan@amd.com>
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 = &adev->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, &smu->default_power_limit, false, false);
> + if (ret)
> + return ret;
> +
> smu_handle_task(&adev->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, &adev->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;
> +
> + return ret;
> +}
> +
> +static int smu_disable_dpms(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> + int ret = 0;
> + bool use_baco = !smu->is_apu &&
> + ((adev->in_gpu_reset &&
> + (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> + ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
>
> /*
> - * 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.
> + * For custom pptable uploading, skip the DPM features
> + * disable process on Navi1x ASICs.
> + * - As the gfx related features are under control of
> + * RLC on those ASICs. RLC reinitialization will be
> + * needed to reenable them. That will cost much more
> + * efforts.
> + *
> + * - SMU firmware can handle the DPM reenablement
> + * properly.
> */
> - if (initialize) {
> - ret = smu_populate_smc_tables(smu);
> - if (ret)
> - return ret;
> + if (smu->uploading_custom_pp_table &&
> + (adev->asic_type >= CHIP_NAVI10) &&
> + (adev->asic_type <= CHIP_NAVI12))
> + return 0;
>
> - ret = smu_init_max_sustainable_clocks(smu);
> - if (ret)
> + /*
> + * Disable all enabled SMU features.
> + * This should be handled in SMU FW, as a backup
> + * driver can issue call to SMU FW until sequence
> + * in SMU FW is operational.
> + */
> + ret = smu_system_features_control(smu, false);
> + if (ret) {
> + pr_err("Failed to disable smu features.\n");
> + return ret;
> + }
> +
> + /*
> + * For baco, need to leave BACO feature enabled
> + *
> + * Correct the way for checking whether SMU_FEATURE_BACO_BIT
> + * is supported.
> + *
> + * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
> + * always return false as the 'smu_system_features_control(smu, false)'
> + * was just issued above which disabled all SMU features.
> + *
> + * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
> + * now for the checking.
> + */
> + if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
> + ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
> + if (ret) {
> + pr_warn("set BACO feature enabled failed, return %d\n", ret);
> return ret;
> + }
> }
>
> - ret = smu_override_pcie_parameters(smu);
> - if (ret)
> - return ret;
> + return ret;
> +}
>
> - ret = smu_set_default_od_settings(smu);
> - if (ret)
> - return ret;
> +static int smu_internal_hw_cleanup(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> + int ret = 0;
>
> - if (initialize) {
> - ret = smu_populate_umd_state_clk(smu);
> - if (ret)
> - return ret;
> + smu_i2c_eeprom_fini(smu, &adev->pm.smu_i2c);
>
> - ret = smu_get_power_limit(smu, &smu->default_power_limit, false, false);
> - if (ret)
> - return ret;
> + ret = smu_disable_thermal_alert(smu);
> + if (ret) {
> + pr_warn("Fail to stop thermal control!\n");
> + return ret;
> }
>
> - /*
> - * Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools.
> - */
> - ret = smu_set_tool_table_location(smu);
> -
> - if (!smu_is_dpm_running(smu))
> - pr_info("dpm has been disabled\n");
> + ret = smu_disable_dpms(smu);
> + if (ret)
> + return ret;
>
> - return ret;
> + return 0;
> }
>
> static int smu_start_smc_engine(struct smu_context *smu)
> @@ -1257,10 +1336,20 @@ static int smu_start_smc_engine(struct smu_context *smu)
>
> if (smu->ppt_funcs->check_fw_status) {
> ret = smu->ppt_funcs->check_fw_status(smu);
> - if (ret)
> + if (ret) {
> pr_err("SMC is not ready\n");
> + 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;
> +
> return ret;
> }
>
> @@ -1289,99 +1378,24 @@ static int smu_hw_init(void *handle)
> if (!smu->pm_enabled)
> return 0;
>
> - ret = smu_feature_init_dpm(smu);
> - if (ret)
> - goto failed;
> -
> - ret = smu_smc_table_hw_init(smu, true);
> - if (ret)
> - goto failed;
> -
> - /*
> - * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
> - * pool location.
> - */
> - ret = smu_notify_memory_pool_location(smu);
> + /* get boot_values from vbios to set revision, gfxclk, and etc. */
> + ret = smu_get_vbios_bootup_values(smu);
> if (ret)
> - goto failed;
> + return ret;
>
> - ret = smu_enable_thermal_alert(smu);
> + ret = smu_setup_pptable(smu);
> if (ret)
> - goto failed;
> + return ret;
>
> - ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
> + ret = smu_internal_hw_setup(smu);
> if (ret)
> - goto failed;
> + return ret;
>
> adev->pm.dpm_enabled = true;
>
> pr_info("SMU is initialized successfully!\n");
>
> return 0;
> -
> -failed:
> - return ret;
> -}
> -
> -static int smu_disable_dpms(struct smu_context *smu)
> -{
> - struct amdgpu_device *adev = smu->adev;
> - int ret = 0;
> - bool use_baco = !smu->is_apu &&
> - ((adev->in_gpu_reset &&
> - (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> - ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
> -
> - /*
> - * For custom pptable uploading, skip the DPM features
> - * disable process on Navi1x ASICs.
> - * - As the gfx related features are under control of
> - * RLC on those ASICs. RLC reinitialization will be
> - * needed to reenable them. That will cost much more
> - * efforts.
> - *
> - * - SMU firmware can handle the DPM reenablement
> - * properly.
> - */
> - if (smu->uploading_custom_pp_table &&
> - (adev->asic_type >= CHIP_NAVI10) &&
> - (adev->asic_type <= CHIP_NAVI12))
> - return 0;
> -
> - /*
> - * Disable all enabled SMU features.
> - * This should be handled in SMU FW, as a backup
> - * driver can issue call to SMU FW until sequence
> - * in SMU FW is operational.
> - */
> - ret = smu_system_features_control(smu, false);
> - if (ret) {
> - pr_err("Failed to disable smu features.\n");
> - return ret;
> - }
> -
> - /*
> - * For baco, need to leave BACO feature enabled
> - *
> - * Correct the way for checking whether SMU_FEATURE_BACO_BIT
> - * is supported.
> - *
> - * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
> - * always return false as the 'smu_system_features_control(smu, false)'
> - * was just issued above which disabled all SMU features.
> - *
> - * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
> - * now for the checking.
> - */
> - if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
> - ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
> - if (ret) {
> - pr_warn("set BACO feature enabled failed, return %d\n", ret);
> - return ret;
> - }
> - }
> -
> - return ret;
> }
>
> static int smu_hw_fini(void *handle)
> @@ -1404,19 +1418,9 @@ static int smu_hw_fini(void *handle)
>
> adev->pm.dpm_enabled = false;
>
> - smu_i2c_eeprom_fini(smu, &adev->pm.smu_i2c);
> -
> - ret = smu_disable_thermal_alert(smu);
> - if (ret) {
> - pr_warn("Fail to stop thermal control!\n");
> - return ret;
> - }
> -
> - ret = smu_disable_dpms(smu);
> - if (ret) {
> - pr_warn("Fail to stop Dpms!\n");
> + ret = smu_internal_hw_cleanup(smu);
> + if (ret)
> return ret;
> - }
>
> return 0;
> }
> @@ -1434,6 +1438,8 @@ int smu_reset(struct smu_context *smu)
> if (ret)
> return ret;
>
> + ret = smu_late_init(adev);
> +
> return ret;
> }
>
> @@ -1451,15 +1457,7 @@ static int smu_suspend(void *handle)
>
> adev->pm.dpm_enabled = false;
>
> - smu_i2c_eeprom_fini(smu, &adev->pm.smu_i2c);
> -
> - ret = smu_disable_thermal_alert(smu);
> - if (ret) {
> - pr_warn("Fail to stop thermal control!\n");
> - return ret;
> - }
> -
> - ret = smu_disable_dpms(smu);
> + ret = smu_internal_hw_cleanup(smu);
> if (ret)
> return ret;
>
> @@ -1491,20 +1489,12 @@ static int smu_resume(void *handle)
> ret = smu_start_smc_engine(smu);
> if (ret) {
> pr_err("SMU is not ready yet!\n");
> - goto failed;
> + return ret;
> }
>
> - ret = smu_smc_table_hw_init(smu, false);
> - if (ret)
> - goto failed;
> -
> - ret = smu_enable_thermal_alert(smu);
> - if (ret)
> - goto failed;
> -
> - ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
> + ret = smu_internal_hw_setup(smu);
> if (ret)
> - goto failed;
> + return ret;
>
> if (smu->is_apu)
> smu_set_gfx_cgpg(&adev->smu, true);
> @@ -1516,9 +1506,6 @@ static int smu_resume(void *handle)
> pr_info("SMU is resumed successfully!\n");
>
> return 0;
> -
> -failed:
> - return ret;
> }
>
> int smu_display_configuration_change(struct smu_context *smu,
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation
2020-06-01 7:30 ` [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation Evan Quan
@ 2020-06-02 15:12 ` Alex Deucher
2020-06-02 21:18 ` Luben Tuikov
1 sibling, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:12 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Since the structure comes with only several bytes.
>
> Change-Id: Ie9df0db543fdd4cf5b963a286ef40dee03c436bf
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 3 ---
> drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +++------------
> 3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index e55c6458b212..b353ac1b0f07 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1121,9 +1121,6 @@ static int smu_sw_fini(void *handle)
> struct smu_context *smu = &adev->smu;
> int ret;
>
> - kfree(smu->irq_source);
> - smu->irq_source = NULL;
> -
> ret = smu_smc_table_sw_fini(smu);
> if (ret) {
> pr_err("Failed to sw fini smc table!\n");
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 4aa63dc79124..7fed2556213f 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -356,7 +356,7 @@ struct smu_baco_context
> struct smu_context
> {
> struct amdgpu_device *adev;
> - struct amdgpu_irq_src *irq_source;
> + struct amdgpu_irq_src irq_source;
>
> const struct pptable_funcs *ppt_funcs;
> struct mutex mutex;
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 891781a5c0d4..e2b1c619151f 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1167,7 +1167,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context *smu)
> if (ret)
> return ret;
>
> - ret = amdgpu_irq_get(adev, smu->irq_source, 0);
> + ret = amdgpu_irq_get(adev, &smu->irq_source, 0);
> if (ret)
> return ret;
>
> @@ -1191,7 +1191,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context *smu)
>
> int smu_v11_0_disable_thermal_alert(struct smu_context *smu)
> {
> - return amdgpu_irq_put(smu->adev, smu->irq_source, 0);
> + return amdgpu_irq_put(smu->adev, &smu->irq_source, 0);
> }
>
> static uint16_t convert_to_vddc(uint8_t vid)
> @@ -1607,18 +1607,9 @@ static const struct amdgpu_irq_src_funcs smu_v11_0_irq_funcs =
> int smu_v11_0_register_irq_handler(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> - struct amdgpu_irq_src *irq_src = smu->irq_source;
> + struct amdgpu_irq_src *irq_src = &smu->irq_source;
> int ret = 0;
>
> - /* already register */
> - if (irq_src)
> - return 0;
> -
> - irq_src = kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> - if (!irq_src)
> - return -ENOMEM;
> - smu->irq_source = irq_src;
> -
> irq_src->num_types = 1;
> irq_src->funcs = &smu_v11_0_irq_funcs;
>
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 9/9] drm/amd/powerplay: add firmware cleanup on sw_fini
2020-06-01 7:30 ` [PATCH 9/9] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
@ 2020-06-02 15:12 ` Alex Deucher
0 siblings, 0 replies; 20+ messages in thread
From: Alex Deucher @ 2020-06-02 15:12 UTC (permalink / raw)
To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan <evan.quan@amd.com> wrote:
>
> To avoid possible memory leak.
>
> Change-Id: I4740eac7fc2c6e934ec8f503e5a98057f0902f4a
> Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 ++
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 1 +
> drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 +
> drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 2 ++
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 1 +
> drivers/gpu/drm/amd/powerplay/smu_internal.h | 2 ++
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 9 +++++++++
> 7 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index b353ac1b0f07..197fef6f59a8 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1127,6 +1127,8 @@ static int smu_sw_fini(void *handle)
> return ret;
> }
>
> + smu_fini_microcode(smu);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index c5c23126ec2d..db2d86e3953b 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -2435,6 +2435,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> .i2c_eeprom_fini = arcturus_i2c_eeprom_control_fini,
> .init_microcode = smu_v11_0_init_microcode,
> .load_microcode = smu_v11_0_load_microcode,
> + .fini_microcode = smu_v11_0_fini_microcode,
> .init_smc_tables = smu_v11_0_init_smc_tables,
> .fini_smc_tables = smu_v11_0_fini_smc_tables,
> .init_power = smu_v11_0_init_power,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 7fed2556213f..718aecde88c0 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -495,6 +495,7 @@ struct pptable_funcs {
> int (*get_dpm_clock_table)(struct smu_context *smu, struct dpm_clocks *clock_table);
> int (*init_microcode)(struct smu_context *smu);
> int (*load_microcode)(struct smu_context *smu);
> + void (*fini_microcode)(struct smu_context *smu);
> int (*init_smc_tables)(struct smu_context *smu);
> int (*fini_smc_tables)(struct smu_context *smu);
> int (*init_power)(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> index 8d317e05f65b..4da5f5e87c81 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -145,6 +145,8 @@ enum smu_v11_0_baco_seq {
>
> int smu_v11_0_init_microcode(struct smu_context *smu);
>
> +void smu_v11_0_fini_microcode(struct smu_context *smu);
> +
> int smu_v11_0_load_microcode(struct smu_context *smu);
>
> int smu_v11_0_init_smc_tables(struct smu_context *smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index caa4355b601e..ebbbe38dfb63 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -2311,6 +2311,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
> .update_pcie_parameters = navi10_update_pcie_parameters,
> .init_microcode = smu_v11_0_init_microcode,
> .load_microcode = smu_v11_0_load_microcode,
> + .fini_microcode = smu_v11_0_fini_microcode,
> .init_smc_tables = smu_v11_0_init_smc_tables,
> .fini_smc_tables = smu_v11_0_fini_smc_tables,
> .init_power = smu_v11_0_init_power,
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> index 0c7d5f0b1cd1..dbdb870011d3 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
> +++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
> @@ -27,6 +27,8 @@
>
> #define smu_init_microcode(smu) \
> ((smu)->ppt_funcs->init_microcode ? (smu)->ppt_funcs->init_microcode((smu)) : 0)
> +#define smu_fini_microcode(smu) \
> + ((smu)->ppt_funcs->fini_microcode ? (smu)->ppt_funcs->fini_microcode((smu)) : 0)
> #define smu_init_smc_tables(smu) \
> ((smu)->ppt_funcs->init_smc_tables ? (smu)->ppt_funcs->init_smc_tables((smu)) : 0)
> #define smu_fini_smc_tables(smu) \
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index e2b1c619151f..10ae4575ccb2 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -195,6 +195,15 @@ int smu_v11_0_init_microcode(struct smu_context *smu)
> return err;
> }
>
> +void smu_v11_0_fini_microcode(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> +
> + release_firmware(adev->pm.fw);
> + adev->pm.fw = NULL;
> + adev->pm.fw_version = 0;
> +}
> +
> int smu_v11_0_load_microcode(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation
2020-06-01 7:30 ` [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation Evan Quan
2020-06-02 15:12 ` Alex Deucher
@ 2020-06-02 21:18 ` Luben Tuikov
1 sibling, 0 replies; 20+ messages in thread
From: Luben Tuikov @ 2020-06-02 21:18 UTC (permalink / raw)
To: Evan Quan, amd-gfx; +Cc: alexander.deucher
On 2020-06-01 3:30 a.m., Evan Quan wrote:
> Since the structure comes with only several bytes.
>
This is not a good commit message as it doesn't describe
what is being done. It evokes the "Yes? Then what?" questions
from a reader.
Perhaps a better one would be:
Allocate the struct amdgpu_irq_src on the stack,
since it is only several bytes in size.
Regards,
Luben
> Change-Id: Ie9df0db543fdd4cf5b963a286ef40dee03c436bf
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 3 ---
> drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +++------------
> 3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index e55c6458b212..b353ac1b0f07 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1121,9 +1121,6 @@ static int smu_sw_fini(void *handle)
> struct smu_context *smu = &adev->smu;
> int ret;
>
> - kfree(smu->irq_source);
> - smu->irq_source = NULL;
> -
> ret = smu_smc_table_sw_fini(smu);
> if (ret) {
> pr_err("Failed to sw fini smc table!\n");
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 4aa63dc79124..7fed2556213f 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -356,7 +356,7 @@ struct smu_baco_context
> struct smu_context
> {
> struct amdgpu_device *adev;
> - struct amdgpu_irq_src *irq_source;
> + struct amdgpu_irq_src irq_source;
>
> const struct pptable_funcs *ppt_funcs;
> struct mutex mutex;
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 891781a5c0d4..e2b1c619151f 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1167,7 +1167,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context *smu)
> if (ret)
> return ret;
>
> - ret = amdgpu_irq_get(adev, smu->irq_source, 0);
> + ret = amdgpu_irq_get(adev, &smu->irq_source, 0);
> if (ret)
> return ret;
>
> @@ -1191,7 +1191,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context *smu)
>
> int smu_v11_0_disable_thermal_alert(struct smu_context *smu)
> {
> - return amdgpu_irq_put(smu->adev, smu->irq_source, 0);
> + return amdgpu_irq_put(smu->adev, &smu->irq_source, 0);
> }
>
> static uint16_t convert_to_vddc(uint8_t vid)
> @@ -1607,18 +1607,9 @@ static const struct amdgpu_irq_src_funcs smu_v11_0_irq_funcs =
> int smu_v11_0_register_irq_handler(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> - struct amdgpu_irq_src *irq_src = smu->irq_source;
> + struct amdgpu_irq_src *irq_src = &smu->irq_source;
> int ret = 0;
>
> - /* already register */
> - if (irq_src)
> - return 0;
> -
> - irq_src = kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> - if (!irq_src)
> - return -ENOMEM;
> - smu->irq_source = irq_src;
> -
> irq_src->num_types = 1;
> irq_src->funcs = &smu_v11_0_irq_funcs;
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
2020-06-02 14:54 ` Alex Deucher
@ 2020-06-04 4:51 ` Quan, Evan
0 siblings, 0 replies; 20+ messages in thread
From: Quan, Evan @ 2020-06-04 4:51 UTC (permalink / raw)
To: Alex Deucher; +Cc: Deucher, Alexander, amd-gfx list
[AMD Official Use Only - Internal Distribution Only]
I refreshed the patch set based on the latest code and just sent them out.
And patch 8 and this one were splitted into several small patches.
-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Tuesday, June 2, 2020 10:55 PM
To: Quan, Evan <Evan.Quan@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan <evan.quan@amd.com> wrote:
>
> To make code more clean and readable by moving ASIC specific code to
> its own file, more code sharing and dropping unused code.
There seem to be multiple things going on here. It's kind of hard to follow all of the changes. Maybe split this patch up? One additional comment below.
Alex
>
> Change-Id: I6b299f9e98c7678b48281cbed9beb17b644bb4cc
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 213
> ++++++++------------- drivers/gpu/drm/amd/powerplay/navi10_ppt.c |
> 19 ++
> 2 files changed, 102 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 4998ea942760..b4f108cb52fa 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -817,22 +817,10 @@ int smu_get_atom_data_table(struct smu_context *smu, uint32_t table,
> return 0;
> }
>
> -static int smu_initialize_pptable(struct smu_context *smu) -{
> - /* TODO */
> - return 0;
> -}
> -
> static int smu_smc_table_sw_init(struct smu_context *smu) {
> int ret;
>
> - ret = smu_initialize_pptable(smu);
> - if (ret) {
> - pr_err("Failed to init smu_initialize_pptable!\n");
> - return ret;
> - }
> -
> /**
> * Create smu_table structure, and init smc tables such as
> * TABLE_PPTABLE, TABLE_WATERMARKS, TABLE_SMU_METRICS, and etc.
> @@ -860,6 +848,12 @@ static int smu_smc_table_sw_fini(struct
> smu_context *smu) {
> int ret;
>
> + ret = smu_fini_power(smu);
> + if (ret) {
> + pr_err("Failed to init smu_fini_power!\n");
> + return ret;
> + }
> +
> ret = smu_fini_smc_tables(smu);
> if (ret) {
> pr_err("Failed to smu_fini_smc_tables!\n"); @@ -950,12
> +944,6 @@ static int smu_sw_fini(void *handle)
> return ret;
> }
>
> - ret = smu_fini_power(smu);
> - if (ret) {
> - pr_err("Failed to init smu_fini_power!\n");
> - return ret;
> - }
> -
> return 0;
> }
>
> @@ -1125,36 +1113,22 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
> if (ret)
> return ret;
>
> - if (adev->asic_type == CHIP_NAVI10) {
> - if ((adev->pdev->device == 0x731f && (adev->pdev->revision == 0xc2 ||
> - adev->pdev->revision == 0xc3 ||
> - adev->pdev->revision == 0xca ||
> - adev->pdev->revision == 0xcb)) ||
> - (adev->pdev->device == 0x66af && (adev->pdev->revision == 0xf3 ||
> - adev->pdev->revision == 0xf4 ||
> - adev->pdev->revision == 0xf5 ||
> - adev->pdev->revision == 0xf6))) {
> - 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_disable_umc_cdr_12gbps_workaround(smu);
> + if (ret) {
> + pr_err("Workaround failed to disable UMC CDR feature on 12Gbps SKU!\n");
> + return ret;
> }
>
> - if (smu->ppt_funcs->set_power_source) {
> - /*
> - * For Navi1X, manually switch it to AC mode as PMFW
> - * may boot it with DC mode.
> - */
> - if (adev->pm.ac_power)
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_AC);
> - else
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_DC);
> - if (ret) {
> - pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ? "AC" : "DC");
> - return ret;
> - }
> + /*
> + * For Navi1X, manually switch it to AC mode as PMFW
> + * may boot it with DC mode.
> + */
> + ret = smu_set_power_source(smu,
> + adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
> + SMU_POWER_SOURCE_DC);
> + if (ret) {
> + pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ? "AC" : "DC");
> + return ret;
> }
>
> ret = smu_notify_display_change(smu); @@ -1362,9 +1336,65 @@
> static int smu_hw_init(void *handle)
> return ret;
> }
>
> -static int smu_stop_dpms(struct smu_context *smu)
> +static int smu_disable_dpms(struct smu_context *smu)
> {
> - return smu_system_features_control(smu, false);
> + struct amdgpu_device *adev = smu->adev;
> + int ret = 0;
> + bool use_baco = !smu->is_apu &&
> + ((adev->in_gpu_reset &&
> + (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> + ((adev->in_runpm || adev->in_hibernate) &&
> + amdgpu_asic_supports_baco(adev)));
> +
> + /*
> + * For custom pptable uploading, skip the DPM features
> + * disable process on Navi1x ASICs.
> + * - As the gfx related features are under control of
> + * RLC on those ASICs. RLC reinitialization will be
> + * needed to reenable them. That will cost much more
> + * efforts.
> + *
> + * - SMU firmware can handle the DPM reenablement
> + * properly.
> + */
> + if (smu->uploading_custom_pp_table &&
> + (adev->asic_type >= CHIP_NAVI10) &&
> + (adev->asic_type <= CHIP_NAVI12))
> + return 0;
> +
> + /*
> + * Disable all enabled SMU features.
> + * This should be handled in SMU FW, as a backup
> + * driver can issue call to SMU FW until sequence
> + * in SMU FW is operational.
> + */
> + ret = smu_system_features_control(smu, false);
> + if (ret) {
> + pr_err("Failed to disable smu features.\n");
> + return ret;
> + }
> +
> + /*
> + * For baco, need to leave BACO feature enabled
> + *
> + * Correct the way for checking whether SMU_FEATURE_BACO_BIT
> + * is supported.
> + *
> + * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
> + * always return false as the 'smu_system_features_control(smu, false)'
> + * was just issued above which disabled all SMU features.
> + *
> + * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
> + * now for the checking.
> + */
> + if (use_baco && (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0)) {
> + ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
> + if (ret) {
> + pr_warn("set BACO feature enabled failed, return %d\n", ret);
> + return ret;
> + }
> + }
> +
> + return ret;
> }
>
> static int smu_hw_fini(void *handle)
> @@ -1396,25 +1426,10 @@ static int smu_hw_fini(void *handle)
> return ret;
> }
>
> - /*
> - * For custom pptable uploading, skip the DPM features
> - * disable process on Navi1x ASICs.
> - * - As the gfx related features are under control of
> - * RLC on those ASICs. RLC reinitialization will be
> - * needed to reenable them. That will cost much more
> - * efforts.
> - *
> - * - SMU firmware can handle the DPM reenablement
> - * properly.
> - */
> - if (!smu->uploading_custom_pp_table ||
> - !((adev->asic_type >= CHIP_NAVI10) &&
> - (adev->asic_type <= CHIP_NAVI12))) {
> - ret = smu_stop_dpms(smu);
> - if (ret) {
> - pr_warn("Fail to stop Dpms!\n");
> - return ret;
> - }
> + ret = smu_disable_dpms(smu);
> + if (ret) {
> + pr_warn("Fail to stop Dpms!\n");
> + return ret;
> }
>
> kfree(table_context->driver_pptable);
> @@ -1453,68 +1468,6 @@ int smu_reset(struct smu_context *smu)
> return ret;
> }
>
> -static int smu_disable_dpm(struct smu_context *smu) -{
> - struct amdgpu_device *adev = smu->adev;
> - uint32_t smu_version;
> - int ret = 0;
> - bool use_baco = !smu->is_apu &&
> - ((adev->in_gpu_reset &&
> - (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> - ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
> -
> - ret = smu_get_smc_version(smu, NULL, &smu_version);
> - if (ret) {
> - pr_err("Failed to get smu version.\n");
> - return ret;
> - }
> -
> - /*
> - * Disable all enabled SMU features.
> - * This should be handled in SMU FW, as a backup
> - * driver can issue call to SMU FW until sequence
> - * in SMU FW is operational.
> - */
> - ret = smu_system_features_control(smu, false);
> - if (ret) {
> - pr_err("Failed to disable smu features.\n");
> - return ret;
> - }
> -
> - /*
> - * Arcturus does not have BACO bit in disable feature mask.
> - * Enablement of BACO bit on Arcturus should be skipped.
> - */
> - if (adev->asic_type == CHIP_ARCTURUS) {
> - if (use_baco && (smu_version > 0x360e00))
> - return 0;
> - }
> -
> - /* For baco, need to leave BACO feature enabled */
> - if (use_baco) {
> - /*
> - * Correct the way for checking whether SMU_FEATURE_BACO_BIT
> - * is supported.
> - *
> - * Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will
> - * always return false as the 'smu_system_features_control(smu, false)'
> - * was just issued above which disabled all SMU features.
> - *
> - * Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is used
> - * now for the checking.
> - */
> - if (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 0) {
> - ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
> - if (ret) {
> - pr_warn("set BACO feature enabled failed, return %d\n", ret);
> - return ret;
> - }
> - }
> - }
> -
> - return ret;
> -}
> -
> static int smu_suspend(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> @@ -1537,7 +1490,7 @@ static int smu_suspend(void *handle)
> return ret;
> }
>
> - ret = smu_disable_dpm(smu);
> + ret = smu_disable_dpms(smu);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 68142f6798c6..652728f18271 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -2209,12 +2209,31 @@ static int navi10_dummy_pstate_control(struct smu_context *smu, bool enable)
> return result;
> }
>
> +static inline bool navi10_need_umc_cdr_12gbps_workaround(struct
> +amdgpu_device *adev) {
> + if (adev->asic_type != CHIP_NAVI10)
> + return false;
> +
> + if ((adev->pdev->device == 0x731f && (adev->pdev->revision == 0xc2 ||
> + adev->pdev->revision == 0xc3 || adev->pdev->revision == 0xca ||
> + adev->pdev->revision == 0xcb)) ||
> + (adev->pdev->device == 0x66af && (adev->pdev->revision == 0xf3 ||
> + adev->pdev->revision == 0xf4 || adev->pdev->revision == 0xf5 ||
> + adev->pdev->revision == 0xf6)))
> + return true;
> + else
> + return false;
> +}
Do we need a separate function for this or can we just inline this code in the function below?
> +
> static int navi10_disable_umc_cdr_12gbps_workaround(struct
> smu_context *smu) {
> uint32_t uclk_count, uclk_min, uclk_max;
> uint32_t smu_version;
> int ret = 0;
>
> + if (!navi10_need_umc_cdr_12gbps_workaround(smu->adev))
> + return 0;
> +
> ret = smu_get_smc_version(smu, NULL, &smu_version);
> if (ret)
> return ret;
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cev
> an.quan%40amd.com%7C85aa3357a2a44d8b3b7a08d80704e68f%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C637267064920288932&sdata=8UmDsGkNmF0VT
> 07vt%2Bk2k6HHBFm9H4EXgLmZ2ccsjf8%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-06-04 4:51 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01 7:29 [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Evan Quan
2020-06-01 7:29 ` [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes Evan Quan
2020-06-02 14:54 ` Alex Deucher
2020-06-04 4:51 ` Quan, Evan
2020-06-01 7:29 ` [PATCH 3/9] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
2020-06-02 15:00 ` Alex Deucher
2020-06-01 7:29 ` [PATCH 4/9] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
2020-06-02 15:01 ` Alex Deucher
2020-06-01 7:29 ` [PATCH 5/9] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
2020-06-02 15:04 ` Alex Deucher
2020-06-01 7:30 ` [PATCH 6/9] drm/amd/powerplay: clean up the overdrive settings Evan Quan
2020-06-02 15:06 ` Alex Deucher
2020-06-01 7:30 ` [PATCH 7/9] drm/amd/powerplay: clean up the SMU hw setup operations Evan Quan
2020-06-02 15:11 ` Alex Deucher
2020-06-01 7:30 ` [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation Evan Quan
2020-06-02 15:12 ` Alex Deucher
2020-06-02 21:18 ` Luben Tuikov
2020-06-01 7:30 ` [PATCH 9/9] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
2020-06-02 15:12 ` Alex Deucher
2020-06-02 14:49 ` [PATCH 1/9] drm/amd/powerplay: drop unnecessary CHIP_ARCTURUS guard Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.