amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/16] drm/amd/powerplay: eliminate asic type check
@ 2020-06-04  4:46 Evan Quan
  2020-06-04  4:46 ` [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks Evan Quan
                   ` (15 more replies)
  0 siblings, 16 replies; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

By moving ASIC specific code into its own file.

Change-Id: Ib6775c1c8c5211ea45db6c3fb604a8279411ab37
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 59 +++++++------------
 drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  |  8 +--
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c    | 18 ++++++
 .../drm/amd/powerplay/sienna_cichlid_ppt.c    |  6 +-
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c     |  5 --
 5 files changed, 45 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 78ab0d46eddd..b2c1ad95edf7 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1055,12 +1055,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
 		return 0;
 	}
 
-	if (adev->asic_type != CHIP_ARCTURUS &&
-	    adev->asic_type != CHIP_SIENNA_CICHLID) {
-		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. */
@@ -1134,21 +1131,10 @@ 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) {
@@ -1166,20 +1152,17 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
 		}
 	}
 
-	if (adev->asic_type != CHIP_ARCTURUS &&
-	    adev->asic_type != CHIP_SIENNA_CICHLID) {
-		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
@@ -1196,11 +1179,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 df7b408319f7..1de5304d12bf 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2463,16 +2463,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,
@@ -2496,7 +2496,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,
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 68142f6798c6..a43c16befffd 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -2209,12 +2209,30 @@ 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))
+		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;
diff --git a/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
index ce16cabb0780..f83df6adcbce 100644
--- a/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
@@ -2463,16 +2463,16 @@ static const struct pptable_funcs sienna_cichlid_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,
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 06b376a1396d..d4509ceb7854 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -836,11 +836,6 @@ int smu_v11_0_set_tool_table_location(struct smu_context *smu)
 int smu_v11_0_init_display_count(struct smu_context *smu, uint32_t count)
 {
 	int ret = 0;
-	struct amdgpu_device *adev = smu->adev;
-
-	/* Sienna_Cichlid do not support to change display num currently */
-	if (adev->asic_type == CHIP_SIENNA_CICHLID)
-		return 0;
 
 	if (!smu->pm_enabled)
 		return ret;
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 20:50   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 03/16] drm/amd/powerplay: implement a common API for dpms disablement Evan Quan
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Minor code cleanups.

Change-Id: I6d240241e78cae17288c1d49dbae6ab1796b1128
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 74 ++++---------------
 .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |  2 -
 2 files changed, 16 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index b2c1ad95edf7..bd0fe71dffde 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -720,30 +720,6 @@ int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask
 	return ret;
 }
 
-int smu_feature_set_supported(struct smu_context *smu,
-			      enum smu_feature_mask mask,
-			      bool enable)
-{
-	struct smu_feature *feature = &smu->smu_feature;
-	int feature_id;
-	int ret = 0;
-
-	feature_id = smu_feature_get_index(smu, mask);
-	if (feature_id < 0)
-		return -EINVAL;
-
-	WARN_ON(feature_id > feature->feature_num);
-
-	mutex_lock(&feature->mutex);
-	if (enable)
-		test_and_set_bit(feature_id, feature->supported);
-	else
-		test_and_clear_bit(feature_id, feature->supported);
-	mutex_unlock(&feature->mutex);
-
-	return ret;
-}
-
 static int smu_set_funcs(struct amdgpu_device *adev)
 {
 	struct smu_context *smu = &adev->smu;
@@ -823,22 +799,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.
@@ -1137,19 +1101,16 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
 		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);
@@ -2138,15 +2099,12 @@ int smu_set_ac_dc(struct smu_context *smu)
 		return 0;
 
 	mutex_lock(&smu->mutex);
-	if (smu->ppt_funcs->set_power_source) {
-		if (smu->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",
-			       smu->adev->pm.ac_power ? "AC" : "DC");
-	}
+	ret = smu_set_power_source(smu,
+				   smu->adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
+				   SMU_POWER_SOURCE_DC);
+	if (ret)
+		pr_err("Failed to switch to %s mode!\n",
+		       smu->adev->pm.ac_power ? "AC" : "DC");
 	mutex_unlock(&smu->mutex);
 
 	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 13fc5773ba45..210432187ceb 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -670,8 +670,6 @@ extern int smu_feature_set_enabled(struct smu_context *smu,
 				   enum smu_feature_mask mask, bool enable);
 extern int smu_feature_is_supported(struct smu_context *smu,
 				    enum smu_feature_mask mask);
-extern int smu_feature_set_supported(struct smu_context *smu,
-				     enum smu_feature_mask mask, bool enable);
 
 int smu_update_table(struct smu_context *smu, enum smu_table_id table_index, int argument,
 		     void *table_data, bool drv2smu);
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/16] drm/amd/powerplay: implement a common API for dpms disablement
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
  2020-06-04  4:46 ` [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 20:51   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 04/16] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

So that code can be shared between .hw_fini and .suspend.

Change-Id: I4a0eeb7cdecbf5b24fac3d0fe1d8fcb1ca9f0b0a
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 178 +++++++++------------
 1 file changed, 77 insertions(+), 101 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index bd0fe71dffde..87205abb1e8b 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1318,9 +1318,77 @@ 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;
+
+	/*
+	 * For Sienna_Cichlid, PMFW will handle the features disablement properly
+	 * on BACO in. Driver involvement is unnecessary.
+	 */
+	if ((adev->asic_type == CHIP_SIENNA_CICHLID) &&
+	     use_baco)
+		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;
+		}
+	}
+
+	if (adev->asic_type >= CHIP_NAVI10 &&
+	    adev->gfx.rlc.funcs->stop)
+		adev->gfx.rlc.funcs->stop(adev);
+
+	return ret;
 }
 
 static int smu_hw_fini(void *handle)
@@ -1352,25 +1420,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);
@@ -1409,78 +1462,11 @@ 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;
 	struct smu_context *smu = &adev->smu;
 	int ret;
-	bool use_baco = !smu->is_apu &&
-		((adev->in_gpu_reset &&
-		  (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
-		 (adev->in_runpm && amdgpu_asic_supports_baco(adev)));
-
 
 	if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
 		return 0;
@@ -1498,19 +1484,9 @@ static int smu_suspend(void *handle)
 		return ret;
 	}
 
-	/*
-	 * For Sienna_Cichlid, PMFW will handle the features disablement properly
-	 * on BACO in. Driver involvement is unnecessary.
-	 */
-	if ((adev->asic_type != CHIP_SIENNA_CICHLID) || !use_baco) {
-		ret = smu_disable_dpm(smu);
-		if (ret)
-			return ret;
-
-		if (adev->asic_type >= CHIP_NAVI10 &&
-		    adev->gfx.rlc.funcs->stop)
-			adev->gfx.rlc.funcs->stop(adev);
-	}
+	ret = smu_disable_dpms(smu);
+	if (ret)
+		return ret;
 
 	smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
 
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/16] drm/amd/powerplay: centralize all buffer allocation in sw_init phase
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
  2020-06-04  4:46 ` [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks Evan Quan
  2020-06-04  4:46 ` [PATCH 03/16] drm/amd/powerplay: implement a common API for dpms disablement Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04  4:46 ` [PATCH 05/16] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 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>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 342 ++++++++++-----------
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c  | 105 ++++---
 2 files changed, 229 insertions(+), 218 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 87205abb1e8b..0e459186a2bf 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -799,6 +799,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;
@@ -823,6 +964,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;
 }
 
@@ -830,6 +982,20 @@ 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");
+		return ret;
+	}
+
 	ret = smu_fini_smc_tables(smu);
 	if (ret) {
 		pr_err("Failed to smu_fini_smc_tables!\n");
@@ -920,91 +1086,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;
-}
-
-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;
 }
 
@@ -1045,13 +1126,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,
@@ -1169,68 +1243,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;
@@ -1288,10 +1300,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.
@@ -1395,7 +1403,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))
@@ -1426,23 +1433,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 d4509ceb7854..45d4fd2526eb 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -439,25 +439,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)
@@ -468,6 +510,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);
@@ -730,18 +783,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;
@@ -982,17 +1023,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;
@@ -1938,24 +1972,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.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/16] drm/amd/powerplay: clean up the APIs for bootup clocks
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (2 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 04/16] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04  4:46 ` [PATCH 06/16] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 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>
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 -
 .../drm/amd/powerplay/sienna_cichlid_ppt.c    |   1 -
 drivers/gpu/drm/amd/powerplay/smu_internal.h  |   2 -
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c     | 141 +++++++-----------
 8 files changed, 51 insertions(+), 102 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 0e459186a2bf..b2dad4d09d88 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1114,10 +1114,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 1de5304d12bf..747fdd315575 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2457,7 +2457,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 210432187ceb..cab64f896c4b 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -506,7 +506,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 4682a2fd4381..79736b1e3506 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -162,8 +162,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 a43c16befffd..85c7cceef9dc 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -2319,7 +2319,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/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
index f83df6adcbce..3405bdcbfbd9 100644
--- a/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
@@ -2457,7 +2457,6 @@ static const struct pptable_funcs sienna_cichlid_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 c9440c978402..496a401615c6 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 45d4fd2526eb..ef825327974c 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -565,6 +565,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;
@@ -623,102 +649,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.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/16] drm/amd/powerplay: clean up the APIs for pptable setup
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (3 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 05/16] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04  4:46 ` [PATCH 07/16] drm/amd/powerplay: clean up the overdrive settings Evan Quan
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 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>
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 ++++++++++---------
 .../drm/amd/powerplay/sienna_cichlid_ppt.c    | 59 ++++++++----
 drivers/gpu/drm/amd/powerplay/smu_internal.h  | 10 --
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c     | 21 -----
 8 files changed, 128 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index b2dad4d09d88..de9e52ad9e25 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1114,23 +1114,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 747fdd315575..9eb57bec27e1 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -489,33 +489,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)
@@ -546,6 +546,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;
@@ -2416,10 +2439,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 */
@@ -2455,10 +2474,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 cab64f896c4b..69ad51161635 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);
@@ -506,8 +503,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 79736b1e3506..e9c71e5a8093 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -162,10 +162,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 85c7cceef9dc..110845922724 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,
@@ -2273,9 +2280,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,
@@ -2317,10 +2321,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/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
index 3405bdcbfbd9..d637fc4b72ac 100644
--- a/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
@@ -359,6 +359,20 @@ sienna_cichlid_get_allowed_feature_mask(struct smu_context *smu,
 
 static int sienna_cichlid_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;
 }
 
@@ -470,27 +484,37 @@ static int sienna_cichlid_append_powerplay_table(struct smu_context *smu)
 
 static int sienna_cichlid_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;
+}
 
-	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);
+static int sienna_cichlid_setup_pptable(struct smu_context *smu)
+{
+	int ret = 0;
 
-	return 0;
+	ret = smu_v11_0_setup_pptable(smu);
+	if (ret)
+		return ret;
+
+	ret = sienna_cichlid_store_powerplay_table(smu);
+	if (ret)
+		return ret;
+
+	ret = sienna_cichlid_append_powerplay_table(smu);
+	if (ret)
+		return ret;
+
+	ret = sienna_cichlid_check_powerplay_table(smu);
+	if (ret)
+		return ret;
+
+	return ret;
 }
 
 static int sienna_cichlid_tables_init(struct smu_context *smu, struct smu_table *tables)
@@ -2411,9 +2435,6 @@ static void sienna_cichlid_dump_pptable(struct smu_context *smu)
 static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
 	.tables_init = sienna_cichlid_tables_init,
 	.alloc_dpm_context = sienna_cichlid_allocate_dpm_context,
-	.store_powerplay_table = sienna_cichlid_store_powerplay_table,
-	.check_powerplay_table = sienna_cichlid_check_powerplay_table,
-	.append_powerplay_table = sienna_cichlid_append_powerplay_table,
 	.get_smu_msg_index = sienna_cichlid_get_smu_msg_index,
 	.get_smu_clk_index = sienna_cichlid_get_smu_clk_index,
 	.get_smu_feature_index = sienna_cichlid_get_smu_feature_index,
@@ -2455,10 +2476,8 @@ static const struct pptable_funcs sienna_cichlid_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 = sienna_cichlid_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 496a401615c6..d2b0b9b2f841 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 ef825327974c..377986a1d492 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -732,27 +732,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.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/16] drm/amd/powerplay: clean up the overdrive settings
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (4 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 06/16] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04  4:46 ` [PATCH 08/16] drm/amd/powerplay: postpone operations not must for hw setup to late_init Evan Quan
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 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>
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 de9e52ad9e25..cd7a3d3efa23 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1197,7 +1197,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 69ad51161635..b2de042493ad 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 e9c71e5a8093..c442fc992d2e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -259,8 +259,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 110845922724..4c1c4af2249b 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 d2b0b9b2f841..9dce366a6f5f 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 377986a1d492..5f3125ec5850 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1906,26 +1906,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.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/16] drm/amd/powerplay: postpone operations not must for hw setup to late_init
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (5 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 07/16] drm/amd/powerplay: clean up the overdrive settings Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 20:56   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 09/16] drm/amd/powerplay: move those operations not needed for resume out Evan Quan
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

So that we do not need to perform those unnecessary operations again on
resume.

Change-Id: I90f8a8d68762b5f88d7477934128a17bf67e3341
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 57 +++++++++++-----------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index cd7a3d3efa23..78fb2b42fc93 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -769,10 +769,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_get_unique_id(smu);
 
 	smu_handle_task(&adev->smu,
@@ -1178,39 +1204,10 @@ static int smu_smc_table_hw_init(struct smu_context *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.
-	 */
-	if (initialize) {
-		ret = smu_populate_smc_tables(smu);
-		if (ret)
-			return ret;
-
-		ret = smu_init_max_sustainable_clocks(smu);
-		if (ret)
-			return ret;
-	}
-
 	ret = smu_override_pcie_parameters(smu);
 	if (ret)
 		return ret;
 
-	ret = smu_set_default_od_settings(smu);
-	if (ret)
-		return ret;
-
-	if (initialize) {
-		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;
-	}
-
 	/*
 	 * Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools.
 	 */
@@ -1428,6 +1425,8 @@ int smu_reset(struct smu_context *smu)
 	if (ret)
 		return ret;
 
+	ret = smu_late_init(adev);
+
 	return ret;
 }
 
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/16] drm/amd/powerplay: move those operations not needed for resume out
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (6 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 08/16] drm/amd/powerplay: postpone operations not must for hw setup to late_init Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 20:58   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 10/16] drm/amd/powerplay: max code sharing between .hw_init and .resume Evan Quan
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Since smu_smc_table_hw_init() is needed for both .hw_init and .resume.
By doing this, we can drop unnecessary operations on resume.

Change-Id: I2af6277efaa9adba2de69161e20e54c4aa10a411
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 40 +++++++++++-----------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 78fb2b42fc93..3bd6b9a5b63c 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1130,25 +1130,6 @@ 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;
-
-		/*
-		 * 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;
-	}
-
 	ret = smu_set_driver_table_location(smu);
 	if (ret)
 		return ret;
@@ -1236,10 +1217,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;
 }
 
@@ -1268,6 +1259,15 @@ static int smu_hw_init(void *handle)
 	if (!smu->pm_enabled)
 		return 0;
 
+	/* 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_feature_init_dpm(smu);
 	if (ret)
 		goto failed;
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/16] drm/amd/powerplay: max code sharing between .hw_init and .resume
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (7 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 09/16] drm/amd/powerplay: move those operations not needed for resume out Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 21:00   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 11/16] drm/amd/powerplay: resort those operations performed in hw setup Evan Quan
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Then redundant code can be dropped.

Change-Id: Icbafbb7ffc8189a09f4236786aea6702ee73f9f4
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 42 ++++++++++------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 3bd6b9a5b63c..4c1f7c36b74b 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1193,10 +1193,28 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
 	 * Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools.
 	 */
 	ret = smu_set_tool_table_location(smu);
+	if (ret)
+		return ret;
 
 	if (!smu_is_dpm_running(smu))
 		pr_info("dpm has been disabled\n");
 
+	/*
+	 * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
+	 * pool location.
+	 */
+	ret = smu_notify_memory_pool_location(smu);
+	if (ret)
+		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;
+
 	return ret;
 }
 
@@ -1276,22 +1294,6 @@ static int smu_hw_init(void *handle)
 	if (ret)
 		goto failed;
 
-	/*
-	 * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
-	 * pool location.
-	 */
-	ret = smu_notify_memory_pool_location(smu);
-	if (ret)
-		goto failed;
-
-	ret = smu_enable_thermal_alert(smu);
-	if (ret)
-		goto failed;
-
-	ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
-	if (ret)
-		goto failed;
-
 	adev->pm.dpm_enabled = true;
 
 	pr_info("SMU is initialized successfully!\n");
@@ -1488,14 +1490,6 @@ static int smu_resume(void *handle)
 	if (ret)
 		goto failed;
 
-	ret = smu_enable_thermal_alert(smu);
-	if (ret)
-		goto failed;
-
-	ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
-	if (ret)
-		goto failed;
-
 	if (smu->is_apu)
 		smu_set_gfx_cgpg(&adev->smu, true);
 
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/16] drm/amd/powerplay: resort those operations performed in hw setup
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (8 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 10/16] drm/amd/powerplay: max code sharing between .hw_init and .resume Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 21:02   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 12/16] drm/amd/powerplay: better namings Evan Quan
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Those common operations(for all ASICs) are placed first and followed
by ASIC specific ones. While the display related are placed at the last.

Change-Id: Id45caee98273c8c0b9c1c9f2713fcf8106e02000
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 61 +++++++++++-----------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 4c1f7c36b74b..b3410d111316 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1134,6 +1134,21 @@ static int smu_smc_table_hw_init(struct smu_context *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;
+
+	/*
+	 * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
+	 * pool location.
+	 */
+	ret = smu_notify_memory_pool_location(smu);
+	if (ret)
+		return ret;
+
 	/* smu_dump_pptable(smu); */
 	/*
 	 * Copy pptable bo in the vram to smc with SMU MSGs such as
@@ -1147,6 +1162,7 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
 	ret = smu_run_btc(smu);
 	if (ret)
 		return ret;
+
 	ret = smu_feature_set_allowed_mask(smu);
 	if (ret)
 		return ret;
@@ -1155,6 +1171,21 @@ 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_override_pcie_parameters(smu);
+	if (ret)
+		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_disable_umc_cdr_12gbps_workaround(smu);
 	if (ret) {
 		pr_err("Workaround failed to disable UMC CDR feature on 12Gbps SKU!\n");
@@ -1185,36 +1216,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
 	if (ret)
 		return ret;
 
-	ret = smu_override_pcie_parameters(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;
-
-	if (!smu_is_dpm_running(smu))
-		pr_info("dpm has been disabled\n");
-
-	/*
-	 * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
-	 * pool location.
-	 */
-	ret = smu_notify_memory_pool_location(smu);
-	if (ret)
-		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;
-
 	return ret;
 }
 
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/16] drm/amd/powerplay: better namings
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (9 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 11/16] drm/amd/powerplay: resort those operations performed in hw setup Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 21:07   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 13/16] drm/amd/powerplay: max code sharing between .hw_fini and .suspend Evan Quan
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

And some minor changes as dropping unused parameter and label
internal used API as static.

Change-Id: I0af0aea029dc4fc7d8e150ab6ec984e9a5f1a74a
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 12 +++++-------
 drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  2 --
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index b3410d111316..55ffbf1cf086 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -638,7 +638,7 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void *buf, size_t size)
 	return ret;
 }
 
-int smu_feature_init_dpm(struct smu_context *smu)
+static int smu_get_driver_allowed_feature_mask(struct smu_context *smu)
 {
 	struct smu_feature *feature = &smu->smu_feature;
 	int ret = 0;
@@ -662,7 +662,6 @@ int smu_feature_init_dpm(struct smu_context *smu)
 	return ret;
 }
 
-
 int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask)
 {
 	struct smu_feature *feature = &smu->smu_feature;
@@ -1115,8 +1114,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;
@@ -1287,11 +1285,11 @@ static int smu_hw_init(void *handle)
 	if (ret)
 		return ret;
 
-	ret = smu_feature_init_dpm(smu);
+	ret = smu_get_driver_allowed_feature_mask(smu);
 	if (ret)
 		goto failed;
 
-	ret = smu_smc_table_hw_init(smu, true);
+	ret = smu_internal_hw_setup(smu);
 	if (ret)
 		goto failed;
 
@@ -1487,7 +1485,7 @@ static int smu_resume(void *handle)
 		goto failed;
 	}
 
-	ret = smu_smc_table_hw_init(smu, false);
+	ret = smu_internal_hw_setup(smu);
 	if (ret)
 		goto failed;
 
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index b2de042493ad..5f5a210668a1 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -656,8 +656,6 @@ extern const struct amd_ip_funcs smu_ip_funcs;
 extern const struct amdgpu_ip_block_version smu_v11_0_ip_block;
 extern const struct amdgpu_ip_block_version smu_v12_0_ip_block;
 
-extern int smu_feature_init_dpm(struct smu_context *smu);
-
 extern int smu_feature_is_enabled(struct smu_context *smu,
 				  enum smu_feature_mask mask);
 extern int smu_feature_set_enabled(struct smu_context *smu,
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 13/16] drm/amd/powerplay: max code sharing between .hw_fini and .suspend
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (10 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 12/16] drm/amd/powerplay: better namings Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 21:08   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 14/16] drm/amd/powerplay: allocate the struct amdgpu_irq_src on the stack Evan Quan
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Thus redundant code can be dropped.

Change-Id: I672f84ed5856da53b7f8f915b2f24ca11cd4b228
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 44 +++++++++++-----------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 55ffbf1cf086..65f5edcaa405 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1376,6 +1376,26 @@ static int smu_disable_dpms(struct smu_context *smu)
 	return ret;
 }
 
+static int smu_internal_hw_cleanup(struct smu_context *smu)
+{
+	struct amdgpu_device *adev = smu->adev;
+	int ret = 0;
+
+	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)
+		return ret;
+
+	return 0;
+}
+
 static int smu_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -1396,19 +1416,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;
 }
@@ -1445,15 +1455,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;
 
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 14/16] drm/amd/powerplay: allocate the struct amdgpu_irq_src on the stack
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (11 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 13/16] drm/amd/powerplay: max code sharing between .hw_fini and .suspend Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 21:13   ` Alex Deucher
  2020-06-04  4:46 ` [PATCH 15/16] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Since it is only several bytes in size.

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 65f5edcaa405..23eb64ae7432 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1102,9 +1102,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 5f5a210668a1..90bb61c159fb 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 5f3125ec5850..6c53488acd68 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1174,7 +1174,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;
 
@@ -1198,7 +1198,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)
@@ -1615,18 +1615,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.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 15/16] drm/amd/powerplay: add firmware cleanup on sw_fini
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (12 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 14/16] drm/amd/powerplay: allocate the struct amdgpu_irq_src on the stack Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04  4:46 ` [PATCH 16/16] drm/amd/powerplay: skip BACO feature on DPMs disablement Evan Quan
  2020-06-04 20:49 ` [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Alex Deucher
  15 siblings, 0 replies; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 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>
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 23eb64ae7432..d9a9d9723be1 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1108,6 +1108,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 9eb57bec27e1..05abfdedcf37 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2469,6 +2469,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.get_unique_id = arcturus_get_unique_id,
 	.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 90bb61c159fb..c0e761d4b31a 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -496,6 +496,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 c442fc992d2e..91fe6f9b4c98 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
@@ -146,6 +146,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 4c1c4af2249b..8dd916a8e8f8 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -2310,6 +2310,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 9dce366a6f5f..927d75ff1ac9 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 6c53488acd68..87132a58c985 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -199,6 +199,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.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 16/16] drm/amd/powerplay: skip BACO feature on DPMs disablement
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (13 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 15/16] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
@ 2020-06-04  4:46 ` Evan Quan
  2020-06-04 21:14   ` Alex Deucher
  2020-06-04 20:49 ` [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Alex Deucher
  15 siblings, 1 reply; 29+ messages in thread
From: Evan Quan @ 2020-06-04  4:46 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Evan Quan

Instead of disabling and reenabling it later.

Change-Id: I90775202178f3b7695f42f39ce240bbfd51a1346
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 72 ++++++++++------------
 1 file changed, 31 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index d9a9d9723be1..b645bba1d201 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -110,28 +110,32 @@ static int smu_feature_update_enable_state(struct smu_context *smu,
 					   bool enabled)
 {
 	struct smu_feature *feature = &smu->smu_feature;
-	uint32_t feature_low = 0, feature_high = 0;
 	int ret = 0;
 
-	feature_low = (feature_mask >> 0 ) & 0xffffffff;
-	feature_high = (feature_mask >> 32) & 0xffffffff;
-
 	if (enabled) {
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesLow,
-						  feature_low, NULL);
+		ret = smu_send_smc_msg_with_param(smu,
+						  SMU_MSG_EnableSmuFeaturesLow,
+						  lower_32_bits(feature_mask),
+						  NULL);
 		if (ret)
 			return ret;
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesHigh,
-						  feature_high, NULL);
+		ret = smu_send_smc_msg_with_param(smu,
+						  SMU_MSG_EnableSmuFeaturesHigh,
+						  upper_32_bits(feature_mask),
+						  NULL);
 		if (ret)
 			return ret;
 	} else {
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesLow,
-						  feature_low, NULL);
+		ret = smu_send_smc_msg_with_param(smu,
+						  SMU_MSG_DisableSmuFeaturesLow,
+						  lower_32_bits(feature_mask),
+						  NULL);
 		if (ret)
 			return ret;
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesHigh,
-						  feature_high, NULL);
+		ret = smu_send_smc_msg_with_param(smu,
+						  SMU_MSG_DisableSmuFeaturesHigh,
+						  upper_32_bits(feature_mask),
+						  NULL);
 		if (ret)
 			return ret;
 	}
@@ -1305,6 +1309,7 @@ static int smu_hw_init(void *handle)
 static int smu_disable_dpms(struct smu_context *smu)
 {
 	struct amdgpu_device *adev = smu->adev;
+	uint64_t features_to_disable;
 	int ret = 0;
 	bool use_baco = !smu->is_apu &&
 		((adev->in_gpu_reset &&
@@ -1336,36 +1341,21 @@ static int smu_disable_dpms(struct smu_context *smu)
 		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.
+	 * For gpu reset, runpm and hibernation through BACO,
+	 * BACO feature has to be kept enabled.
 	 */
-	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;
-		}
+	if (use_baco && smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
+		features_to_disable = U64_MAX &
+			~(1ULL << smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT));
+		ret = smu_feature_update_enable_state(smu,
+						      features_to_disable,
+						      0);
+		if (ret)
+			pr_err("Failed to disable smu features except BACO.\n");
+	} else {
+		ret = smu_system_features_control(smu, false);
+		if (ret)
+			pr_err("Failed to disable smu features.\n");
 	}
 
 	if (adev->asic_type >= CHIP_NAVI10 &&
-- 
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/16] drm/amd/powerplay: eliminate asic type check
  2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
                   ` (14 preceding siblings ...)
  2020-06-04  4:46 ` [PATCH 16/16] drm/amd/powerplay: skip BACO feature on DPMs disablement Evan Quan
@ 2020-06-04 20:49 ` Alex Deucher
  15 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 20:49 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:46 AM Evan Quan <evan.quan@amd.com> wrote:
>
> By moving ASIC specific code into its own file.

You might want to clarify that the macros check if the asic has the
callback, so no need to explicitly check.  With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> Change-Id: Ib6775c1c8c5211ea45db6c3fb604a8279411ab37
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 59 +++++++------------
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  |  8 +--
>  drivers/gpu/drm/amd/powerplay/navi10_ppt.c    | 18 ++++++
>  .../drm/amd/powerplay/sienna_cichlid_ppt.c    |  6 +-
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c     |  5 --
>  5 files changed, 45 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 78ab0d46eddd..b2c1ad95edf7 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1055,12 +1055,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>                 return 0;
>         }
>
> -       if (adev->asic_type != CHIP_ARCTURUS &&
> -           adev->asic_type != CHIP_SIENNA_CICHLID) {
> -               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. */
> @@ -1134,21 +1131,10 @@ 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) {
> @@ -1166,20 +1152,17 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>                 }
>         }
>
> -       if (adev->asic_type != CHIP_ARCTURUS &&
> -           adev->asic_type != CHIP_SIENNA_CICHLID) {
> -               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
> @@ -1196,11 +1179,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 df7b408319f7..1de5304d12bf 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -2463,16 +2463,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,
> @@ -2496,7 +2496,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,
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 68142f6798c6..a43c16befffd 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -2209,12 +2209,30 @@ 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))
> +               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;
> diff --git a/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
> index ce16cabb0780..f83df6adcbce 100644
> --- a/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
> @@ -2463,16 +2463,16 @@ static const struct pptable_funcs sienna_cichlid_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,
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 06b376a1396d..d4509ceb7854 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -836,11 +836,6 @@ int smu_v11_0_set_tool_table_location(struct smu_context *smu)
>  int smu_v11_0_init_display_count(struct smu_context *smu, uint32_t count)
>  {
>         int ret = 0;
> -       struct amdgpu_device *adev = smu->adev;
> -
> -       /* Sienna_Cichlid do not support to change display num currently */
> -       if (adev->asic_type == CHIP_SIENNA_CICHLID)
> -               return 0;
>
>         if (!smu->pm_enabled)
>                 return ret;
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks
  2020-06-04  4:46 ` [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks Evan Quan
@ 2020-06-04 20:50   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 20:50 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:46 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Minor code cleanups.
>
> Change-Id: I6d240241e78cae17288c1d49dbae6ab1796b1128
> 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    | 74 ++++---------------
>  .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |  2 -
>  2 files changed, 16 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index b2c1ad95edf7..bd0fe71dffde 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -720,30 +720,6 @@ int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask
>         return ret;
>  }
>
> -int smu_feature_set_supported(struct smu_context *smu,
> -                             enum smu_feature_mask mask,
> -                             bool enable)
> -{
> -       struct smu_feature *feature = &smu->smu_feature;
> -       int feature_id;
> -       int ret = 0;
> -
> -       feature_id = smu_feature_get_index(smu, mask);
> -       if (feature_id < 0)
> -               return -EINVAL;
> -
> -       WARN_ON(feature_id > feature->feature_num);
> -
> -       mutex_lock(&feature->mutex);
> -       if (enable)
> -               test_and_set_bit(feature_id, feature->supported);
> -       else
> -               test_and_clear_bit(feature_id, feature->supported);
> -       mutex_unlock(&feature->mutex);
> -
> -       return ret;
> -}
> -
>  static int smu_set_funcs(struct amdgpu_device *adev)
>  {
>         struct smu_context *smu = &adev->smu;
> @@ -823,22 +799,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.
> @@ -1137,19 +1101,16 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>                 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);
> @@ -2138,15 +2099,12 @@ int smu_set_ac_dc(struct smu_context *smu)
>                 return 0;
>
>         mutex_lock(&smu->mutex);
> -       if (smu->ppt_funcs->set_power_source) {
> -               if (smu->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",
> -                              smu->adev->pm.ac_power ? "AC" : "DC");
> -       }
> +       ret = smu_set_power_source(smu,
> +                                  smu->adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
> +                                  SMU_POWER_SOURCE_DC);
> +       if (ret)
> +               pr_err("Failed to switch to %s mode!\n",
> +                      smu->adev->pm.ac_power ? "AC" : "DC");
>         mutex_unlock(&smu->mutex);
>
>         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 13fc5773ba45..210432187ceb 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -670,8 +670,6 @@ extern int smu_feature_set_enabled(struct smu_context *smu,
>                                    enum smu_feature_mask mask, bool enable);
>  extern int smu_feature_is_supported(struct smu_context *smu,
>                                     enum smu_feature_mask mask);
> -extern int smu_feature_set_supported(struct smu_context *smu,
> -                                    enum smu_feature_mask mask, bool enable);
>
>  int smu_update_table(struct smu_context *smu, enum smu_table_id table_index, int argument,
>                      void *table_data, bool drv2smu);
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 03/16] drm/amd/powerplay: implement a common API for dpms disablement
  2020-06-04  4:46 ` [PATCH 03/16] drm/amd/powerplay: implement a common API for dpms disablement Evan Quan
@ 2020-06-04 20:51   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 20:51 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> So that code can be shared between .hw_fini and .suspend.
>
> Change-Id: I4a0eeb7cdecbf5b24fac3d0fe1d8fcb1ca9f0b0a
> 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 | 178 +++++++++------------
>  1 file changed, 77 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index bd0fe71dffde..87205abb1e8b 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1318,9 +1318,77 @@ 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;
> +
> +       /*
> +        * For Sienna_Cichlid, PMFW will handle the features disablement properly
> +        * on BACO in. Driver involvement is unnecessary.
> +        */
> +       if ((adev->asic_type == CHIP_SIENNA_CICHLID) &&
> +            use_baco)
> +               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;
> +               }
> +       }
> +
> +       if (adev->asic_type >= CHIP_NAVI10 &&
> +           adev->gfx.rlc.funcs->stop)
> +               adev->gfx.rlc.funcs->stop(adev);
> +
> +       return ret;
>  }
>
>  static int smu_hw_fini(void *handle)
> @@ -1352,25 +1420,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);
> @@ -1409,78 +1462,11 @@ 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;
>         struct smu_context *smu = &adev->smu;
>         int ret;
> -       bool use_baco = !smu->is_apu &&
> -               ((adev->in_gpu_reset &&
> -                 (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> -                (adev->in_runpm && amdgpu_asic_supports_baco(adev)));
> -
>
>         if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
>                 return 0;
> @@ -1498,19 +1484,9 @@ static int smu_suspend(void *handle)
>                 return ret;
>         }
>
> -       /*
> -        * For Sienna_Cichlid, PMFW will handle the features disablement properly
> -        * on BACO in. Driver involvement is unnecessary.
> -        */
> -       if ((adev->asic_type != CHIP_SIENNA_CICHLID) || !use_baco) {
> -               ret = smu_disable_dpm(smu);
> -               if (ret)
> -                       return ret;
> -
> -               if (adev->asic_type >= CHIP_NAVI10 &&
> -                   adev->gfx.rlc.funcs->stop)
> -                       adev->gfx.rlc.funcs->stop(adev);
> -       }
> +       ret = smu_disable_dpms(smu);
> +       if (ret)
> +               return ret;
>
>         smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
>
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 08/16] drm/amd/powerplay: postpone operations not must for hw setup to late_init
  2020-06-04  4:46 ` [PATCH 08/16] drm/amd/powerplay: postpone operations not must for hw setup to late_init Evan Quan
@ 2020-06-04 20:56   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 20:56 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> So that we do not need to perform those unnecessary operations again on
> resume.
>
> Change-Id: I90f8a8d68762b5f88d7477934128a17bf67e3341
> Signed-off-by: Evan Quan <evan.quan@amd.com>

For the patch subject, I think it would be clearer as:

drm/amd/powerplay: postpone operations not required for hw setup to late_init

With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 57 +++++++++++-----------
>  1 file changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index cd7a3d3efa23..78fb2b42fc93 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -769,10 +769,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_get_unique_id(smu);
>
>         smu_handle_task(&adev->smu,
> @@ -1178,39 +1204,10 @@ static int smu_smc_table_hw_init(struct smu_context *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.
> -        */
> -       if (initialize) {
> -               ret = smu_populate_smc_tables(smu);
> -               if (ret)
> -                       return ret;
> -
> -               ret = smu_init_max_sustainable_clocks(smu);
> -               if (ret)
> -                       return ret;
> -       }
> -
>         ret = smu_override_pcie_parameters(smu);
>         if (ret)
>                 return ret;
>
> -       ret = smu_set_default_od_settings(smu);
> -       if (ret)
> -               return ret;
> -
> -       if (initialize) {
> -               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;
> -       }
> -
>         /*
>          * Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools.
>          */
> @@ -1428,6 +1425,8 @@ int smu_reset(struct smu_context *smu)
>         if (ret)
>                 return ret;
>
> +       ret = smu_late_init(adev);
> +
>         return ret;
>  }
>
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 09/16] drm/amd/powerplay: move those operations not needed for resume out
  2020-06-04  4:46 ` [PATCH 09/16] drm/amd/powerplay: move those operations not needed for resume out Evan Quan
@ 2020-06-04 20:58   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 20:58 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Since smu_smc_table_hw_init() is needed for both .hw_init and .resume.
> By doing this, we can drop unnecessary operations on resume.
>
> Change-Id: I2af6277efaa9adba2de69161e20e54c4aa10a411
> 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 | 40 +++++++++++-----------
>  1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 78fb2b42fc93..3bd6b9a5b63c 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1130,25 +1130,6 @@ 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;
> -
> -               /*
> -                * 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;
> -       }
> -
>         ret = smu_set_driver_table_location(smu);
>         if (ret)
>                 return ret;
> @@ -1236,10 +1217,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;
>  }
>
> @@ -1268,6 +1259,15 @@ static int smu_hw_init(void *handle)
>         if (!smu->pm_enabled)
>                 return 0;
>
> +       /* 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_feature_init_dpm(smu);
>         if (ret)
>                 goto failed;
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 10/16] drm/amd/powerplay: max code sharing between .hw_init and .resume
  2020-06-04  4:46 ` [PATCH 10/16] drm/amd/powerplay: max code sharing between .hw_init and .resume Evan Quan
@ 2020-06-04 21:00   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:00 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Then redundant code can be dropped.
>
> Change-Id: Icbafbb7ffc8189a09f4236786aea6702ee73f9f4
> Signed-off-by: Evan Quan <evan.quan@amd.com>

Subject could be clarified as:

drm/amd/powerplay: maximize code sharing between .hw_init and .resume

With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 42 ++++++++++------------
>  1 file changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 3bd6b9a5b63c..4c1f7c36b74b 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1193,10 +1193,28 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>          * Set PMSTATUSLOG table bo address with SetToolsDramAddr MSG for tools.
>          */
>         ret = smu_set_tool_table_location(smu);
> +       if (ret)
> +               return ret;
>
>         if (!smu_is_dpm_running(smu))
>                 pr_info("dpm has been disabled\n");
>
> +       /*
> +        * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
> +        * pool location.
> +        */
> +       ret = smu_notify_memory_pool_location(smu);
> +       if (ret)
> +               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;
> +
>         return ret;
>  }
>
> @@ -1276,22 +1294,6 @@ static int smu_hw_init(void *handle)
>         if (ret)
>                 goto failed;
>
> -       /*
> -        * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
> -        * pool location.
> -        */
> -       ret = smu_notify_memory_pool_location(smu);
> -       if (ret)
> -               goto failed;
> -
> -       ret = smu_enable_thermal_alert(smu);
> -       if (ret)
> -               goto failed;
> -
> -       ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
> -       if (ret)
> -               goto failed;
> -
>         adev->pm.dpm_enabled = true;
>
>         pr_info("SMU is initialized successfully!\n");
> @@ -1488,14 +1490,6 @@ static int smu_resume(void *handle)
>         if (ret)
>                 goto failed;
>
> -       ret = smu_enable_thermal_alert(smu);
> -       if (ret)
> -               goto failed;
> -
> -       ret = smu_i2c_eeprom_init(smu, &adev->pm.smu_i2c);
> -       if (ret)
> -               goto failed;
> -
>         if (smu->is_apu)
>                 smu_set_gfx_cgpg(&adev->smu, true);
>
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 11/16] drm/amd/powerplay: resort those operations performed in hw setup
  2020-06-04  4:46 ` [PATCH 11/16] drm/amd/powerplay: resort those operations performed in hw setup Evan Quan
@ 2020-06-04 21:02   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:02 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Those common operations(for all ASICs) are placed first and followed
> by ASIC specific ones. While the display related are placed at the last.
>
> Change-Id: Id45caee98273c8c0b9c1c9f2713fcf8106e02000
> Signed-off-by: Evan Quan <evan.quan@amd.com>

Typo in the subject and clarification:

drm/amd/powerplay: sort the operations performed in hw setup

With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 61 +++++++++++-----------
>  1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 4c1f7c36b74b..b3410d111316 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1134,6 +1134,21 @@ static int smu_smc_table_hw_init(struct smu_context *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;
> +
> +       /*
> +        * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
> +        * pool location.
> +        */
> +       ret = smu_notify_memory_pool_location(smu);
> +       if (ret)
> +               return ret;
> +
>         /* smu_dump_pptable(smu); */
>         /*
>          * Copy pptable bo in the vram to smc with SMU MSGs such as
> @@ -1147,6 +1162,7 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>         ret = smu_run_btc(smu);
>         if (ret)
>                 return ret;
> +
>         ret = smu_feature_set_allowed_mask(smu);
>         if (ret)
>                 return ret;
> @@ -1155,6 +1171,21 @@ 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_override_pcie_parameters(smu);
> +       if (ret)
> +               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_disable_umc_cdr_12gbps_workaround(smu);
>         if (ret) {
>                 pr_err("Workaround failed to disable UMC CDR feature on 12Gbps SKU!\n");
> @@ -1185,36 +1216,6 @@ static int smu_smc_table_hw_init(struct smu_context *smu,
>         if (ret)
>                 return ret;
>
> -       ret = smu_override_pcie_parameters(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;
> -
> -       if (!smu_is_dpm_running(smu))
> -               pr_info("dpm has been disabled\n");
> -
> -       /*
> -        * Use msg SetSystemVirtualDramAddr and DramLogSetDramAddr can notify
> -        * pool location.
> -        */
> -       ret = smu_notify_memory_pool_location(smu);
> -       if (ret)
> -               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;
> -
>         return ret;
>  }
>
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 12/16] drm/amd/powerplay: better namings
  2020-06-04  4:46 ` [PATCH 12/16] drm/amd/powerplay: better namings Evan Quan
@ 2020-06-04 21:07   ` Alex Deucher
  2020-06-04 21:09     ` Alex Deucher
  2020-06-05  5:53     ` Quan, Evan
  0 siblings, 2 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:07 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> And some minor changes as dropping unused parameter and label
> internal used API as static.
>
> Change-Id: I0af0aea029dc4fc7d8e150ab6ec984e9a5f1a74a
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 12 +++++-------
>  drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  2 --
>  2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index b3410d111316..55ffbf1cf086 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -638,7 +638,7 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void *buf, size_t size)
>         return ret;
>  }
>
> -int smu_feature_init_dpm(struct smu_context *smu)
> +static int smu_get_driver_allowed_feature_mask(struct smu_context *smu)
>  {
>         struct smu_feature *feature = &smu->smu_feature;
>         int ret = 0;
> @@ -662,7 +662,6 @@ int smu_feature_init_dpm(struct smu_context *smu)
>         return ret;
>  }
>
> -
>  int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask)
>  {
>         struct smu_feature *feature = &smu->smu_feature;
> @@ -1115,8 +1114,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)

The "internal" in the name made me think this was related to someting
in smu_internal.h.  Maybe call it smu_smc_hw_setup()?  I guess I could
go either way.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>  {
>         struct amdgpu_device *adev = smu->adev;
>         int ret;
> @@ -1287,11 +1285,11 @@ static int smu_hw_init(void *handle)
>         if (ret)
>                 return ret;
>
> -       ret = smu_feature_init_dpm(smu);
> +       ret = smu_get_driver_allowed_feature_mask(smu);
>         if (ret)
>                 goto failed;
>
> -       ret = smu_smc_table_hw_init(smu, true);
> +       ret = smu_internal_hw_setup(smu);
>         if (ret)
>                 goto failed;
>
> @@ -1487,7 +1485,7 @@ static int smu_resume(void *handle)
>                 goto failed;
>         }
>
> -       ret = smu_smc_table_hw_init(smu, false);
> +       ret = smu_internal_hw_setup(smu);
>         if (ret)
>                 goto failed;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index b2de042493ad..5f5a210668a1 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -656,8 +656,6 @@ extern const struct amd_ip_funcs smu_ip_funcs;
>  extern const struct amdgpu_ip_block_version smu_v11_0_ip_block;
>  extern const struct amdgpu_ip_block_version smu_v12_0_ip_block;
>
> -extern int smu_feature_init_dpm(struct smu_context *smu);
> -
>  extern int smu_feature_is_enabled(struct smu_context *smu,
>                                   enum smu_feature_mask mask);
>  extern int smu_feature_set_enabled(struct smu_context *smu,
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 13/16] drm/amd/powerplay: max code sharing between .hw_fini and .suspend
  2020-06-04  4:46 ` [PATCH 13/16] drm/amd/powerplay: max code sharing between .hw_fini and .suspend Evan Quan
@ 2020-06-04 21:08   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:08 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Thus redundant code can be dropped.
>
> Change-Id: I672f84ed5856da53b7f8f915b2f24ca11cd4b228
> Signed-off-by: Evan Quan <evan.quan@amd.com>

Clarify subject:

drm/amd/powerplay: maximize code sharing between .hw_fini and .suspend

With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 44 +++++++++++-----------
>  1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 55ffbf1cf086..65f5edcaa405 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1376,6 +1376,26 @@ static int smu_disable_dpms(struct smu_context *smu)
>         return ret;
>  }
>
> +static int smu_internal_hw_cleanup(struct smu_context *smu)
> +{
> +       struct amdgpu_device *adev = smu->adev;
> +       int ret = 0;
> +
> +       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)
> +               return ret;
> +
> +       return 0;
> +}
> +
>  static int smu_hw_fini(void *handle)
>  {
>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> @@ -1396,19 +1416,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;
>  }
> @@ -1445,15 +1455,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;
>
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 12/16] drm/amd/powerplay: better namings
  2020-06-04 21:07   ` Alex Deucher
@ 2020-06-04 21:09     ` Alex Deucher
  2020-06-05  5:53     ` Quan, Evan
  1 sibling, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:09 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 5:07 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
> >
> > And some minor changes as dropping unused parameter and label
> > internal used API as static.
> >
> > Change-Id: I0af0aea029dc4fc7d8e150ab6ec984e9a5f1a74a
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > ---
> >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 12 +++++-------
> >  drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  2 --
> >  2 files changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > index b3410d111316..55ffbf1cf086 100644
> > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > @@ -638,7 +638,7 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void *buf, size_t size)
> >         return ret;
> >  }
> >
> > -int smu_feature_init_dpm(struct smu_context *smu)
> > +static int smu_get_driver_allowed_feature_mask(struct smu_context *smu)
> >  {
> >         struct smu_feature *feature = &smu->smu_feature;
> >         int ret = 0;
> > @@ -662,7 +662,6 @@ int smu_feature_init_dpm(struct smu_context *smu)
> >         return ret;
> >  }
> >
> > -
> >  int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask)
> >  {
> >         struct smu_feature *feature = &smu->smu_feature;
> > @@ -1115,8 +1114,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)
>
> The "internal" in the name made me think this was related to someting
> in smu_internal.h.  Maybe call it smu_smc_hw_setup()?  I guess I could
> go either way.

Nevermind, I see you started using the "internal" naming in the next
patch as well. Either way is fine with me.

>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> >  {
> >         struct amdgpu_device *adev = smu->adev;
> >         int ret;
> > @@ -1287,11 +1285,11 @@ static int smu_hw_init(void *handle)
> >         if (ret)
> >                 return ret;
> >
> > -       ret = smu_feature_init_dpm(smu);
> > +       ret = smu_get_driver_allowed_feature_mask(smu);
> >         if (ret)
> >                 goto failed;
> >
> > -       ret = smu_smc_table_hw_init(smu, true);
> > +       ret = smu_internal_hw_setup(smu);
> >         if (ret)
> >                 goto failed;
> >
> > @@ -1487,7 +1485,7 @@ static int smu_resume(void *handle)
> >                 goto failed;
> >         }
> >
> > -       ret = smu_smc_table_hw_init(smu, false);
> > +       ret = smu_internal_hw_setup(smu);
> >         if (ret)
> >                 goto failed;
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> > index b2de042493ad..5f5a210668a1 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> > @@ -656,8 +656,6 @@ extern const struct amd_ip_funcs smu_ip_funcs;
> >  extern const struct amdgpu_ip_block_version smu_v11_0_ip_block;
> >  extern const struct amdgpu_ip_block_version smu_v12_0_ip_block;
> >
> > -extern int smu_feature_init_dpm(struct smu_context *smu);
> > -
> >  extern int smu_feature_is_enabled(struct smu_context *smu,
> >                                   enum smu_feature_mask mask);
> >  extern int smu_feature_set_enabled(struct smu_context *smu,
> > --
> > 2.27.0
> >
> > _______________________________________________
> > 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] 29+ messages in thread

* Re: [PATCH 14/16] drm/amd/powerplay: allocate the struct amdgpu_irq_src on the stack
  2020-06-04  4:46 ` [PATCH 14/16] drm/amd/powerplay: allocate the struct amdgpu_irq_src on the stack Evan Quan
@ 2020-06-04 21:13   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:13 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Since it is only several bytes in size.

I think the subject and description should be clarified a bit.  We are
not allocating it on the stack.  We are just moving the object to the
smu structure allocation rather than allocating it dynamically at
runtime.  We always allocate it anyway so no need to handle it
allocate it separately. With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> 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 65f5edcaa405..23eb64ae7432 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1102,9 +1102,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 5f5a210668a1..90bb61c159fb 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 5f3125ec5850..6c53488acd68 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1174,7 +1174,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;
>
> @@ -1198,7 +1198,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)
> @@ -1615,18 +1615,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.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 16/16] drm/amd/powerplay: skip BACO feature on DPMs disablement
  2020-06-04  4:46 ` [PATCH 16/16] drm/amd/powerplay: skip BACO feature on DPMs disablement Evan Quan
@ 2020-06-04 21:14   ` Alex Deucher
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Deucher @ 2020-06-04 21:14 UTC (permalink / raw)
  To: Evan Quan; +Cc: Deucher, Alexander, amd-gfx list

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Instead of disabling and reenabling it later.
>
> Change-Id: I90775202178f3b7695f42f39ce240bbfd51a1346
> 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 | 72 ++++++++++------------
>  1 file changed, 31 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index d9a9d9723be1..b645bba1d201 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -110,28 +110,32 @@ static int smu_feature_update_enable_state(struct smu_context *smu,
>                                            bool enabled)
>  {
>         struct smu_feature *feature = &smu->smu_feature;
> -       uint32_t feature_low = 0, feature_high = 0;
>         int ret = 0;
>
> -       feature_low = (feature_mask >> 0 ) & 0xffffffff;
> -       feature_high = (feature_mask >> 32) & 0xffffffff;
> -
>         if (enabled) {
> -               ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesLow,
> -                                                 feature_low, NULL);
> +               ret = smu_send_smc_msg_with_param(smu,
> +                                                 SMU_MSG_EnableSmuFeaturesLow,
> +                                                 lower_32_bits(feature_mask),
> +                                                 NULL);
>                 if (ret)
>                         return ret;
> -               ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesHigh,
> -                                                 feature_high, NULL);
> +               ret = smu_send_smc_msg_with_param(smu,
> +                                                 SMU_MSG_EnableSmuFeaturesHigh,
> +                                                 upper_32_bits(feature_mask),
> +                                                 NULL);
>                 if (ret)
>                         return ret;
>         } else {
> -               ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesLow,
> -                                                 feature_low, NULL);
> +               ret = smu_send_smc_msg_with_param(smu,
> +                                                 SMU_MSG_DisableSmuFeaturesLow,
> +                                                 lower_32_bits(feature_mask),
> +                                                 NULL);
>                 if (ret)
>                         return ret;
> -               ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesHigh,
> -                                                 feature_high, NULL);
> +               ret = smu_send_smc_msg_with_param(smu,
> +                                                 SMU_MSG_DisableSmuFeaturesHigh,
> +                                                 upper_32_bits(feature_mask),
> +                                                 NULL);
>                 if (ret)
>                         return ret;
>         }
> @@ -1305,6 +1309,7 @@ static int smu_hw_init(void *handle)
>  static int smu_disable_dpms(struct smu_context *smu)
>  {
>         struct amdgpu_device *adev = smu->adev;
> +       uint64_t features_to_disable;
>         int ret = 0;
>         bool use_baco = !smu->is_apu &&
>                 ((adev->in_gpu_reset &&
> @@ -1336,36 +1341,21 @@ static int smu_disable_dpms(struct smu_context *smu)
>                 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.
> +        * For gpu reset, runpm and hibernation through BACO,
> +        * BACO feature has to be kept enabled.
>          */
> -       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;
> -               }
> +       if (use_baco && smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
> +               features_to_disable = U64_MAX &
> +                       ~(1ULL << smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT));
> +               ret = smu_feature_update_enable_state(smu,
> +                                                     features_to_disable,
> +                                                     0);
> +               if (ret)
> +                       pr_err("Failed to disable smu features except BACO.\n");
> +       } else {
> +               ret = smu_system_features_control(smu, false);
> +               if (ret)
> +                       pr_err("Failed to disable smu features.\n");
>         }
>
>         if (adev->asic_type >= CHIP_NAVI10 &&
> --
> 2.27.0
>
> _______________________________________________
> 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] 29+ messages in thread

* RE: [PATCH 12/16] drm/amd/powerplay: better namings
  2020-06-04 21:07   ` Alex Deucher
  2020-06-04 21:09     ` Alex Deucher
@ 2020-06-05  5:53     ` Quan, Evan
  1 sibling, 0 replies; 29+ messages in thread
From: Quan, Evan @ 2020-06-05  5:53 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Deucher, Alexander, amd-gfx list

[AMD Official Use Only - Internal Distribution Only]

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Friday, June 5, 2020 5:07 AM
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 12/16] drm/amd/powerplay: better namings

On Thu, Jun 4, 2020 at 12:47 AM Evan Quan <evan.quan@amd.com> wrote:
>
> And some minor changes as dropping unused parameter and label internal
> used API as static.
>
> Change-Id: I0af0aea029dc4fc7d8e150ab6ec984e9a5f1a74a
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 12 +++++-------
>  drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  2 --
>  2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index b3410d111316..55ffbf1cf086 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -638,7 +638,7 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void *buf, size_t size)
>         return ret;
>  }
>
> -int smu_feature_init_dpm(struct smu_context *smu)
> +static int smu_get_driver_allowed_feature_mask(struct smu_context
> +*smu)
>  {
>         struct smu_feature *feature = &smu->smu_feature;
>         int ret = 0;
> @@ -662,7 +662,6 @@ int smu_feature_init_dpm(struct smu_context *smu)
>         return ret;
>  }
>
> -
>  int smu_feature_is_enabled(struct smu_context *smu, enum
> smu_feature_mask mask)  {
>         struct smu_feature *feature = &smu->smu_feature; @@ -1115,8
> +1114,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)

The "internal" in the name made me think this was related to someting in smu_internal.h.  Maybe call it smu_smc_hw_setup()?  I guess I could go either way.

[Quan, Evan] Thanks. This and other comments were addressed on code submitted.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>  {
>         struct amdgpu_device *adev = smu->adev;
>         int ret;
> @@ -1287,11 +1285,11 @@ static int smu_hw_init(void *handle)
>         if (ret)
>                 return ret;
>
> -       ret = smu_feature_init_dpm(smu);
> +       ret = smu_get_driver_allowed_feature_mask(smu);
>         if (ret)
>                 goto failed;
>
> -       ret = smu_smc_table_hw_init(smu, true);
> +       ret = smu_internal_hw_setup(smu);
>         if (ret)
>                 goto failed;
>
> @@ -1487,7 +1485,7 @@ static int smu_resume(void *handle)
>                 goto failed;
>         }
>
> -       ret = smu_smc_table_hw_init(smu, false);
> +       ret = smu_internal_hw_setup(smu);
>         if (ret)
>                 goto failed;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index b2de042493ad..5f5a210668a1 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -656,8 +656,6 @@ extern const struct amd_ip_funcs smu_ip_funcs;
> extern const struct amdgpu_ip_block_version smu_v11_0_ip_block;
> extern const struct amdgpu_ip_block_version smu_v12_0_ip_block;
>
> -extern int smu_feature_init_dpm(struct smu_context *smu);
> -
>  extern int smu_feature_is_enabled(struct smu_context *smu,
>                                   enum smu_feature_mask mask);  extern
> int smu_feature_set_enabled(struct smu_context *smu,
> --
> 2.27.0
>
> _______________________________________________
> 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&amp;data=02%7C01%7Cev
> an.quan%40amd.com%7Cc40db198d5ce43d1c5f408d808cb414c%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C637269016367160396&amp;sdata=XQbV2yqVdKZZY
> fM0j%2FZWQe2U53ik6untquO4FRp2OEU%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2020-06-05  5:53 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04  4:46 [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Evan Quan
2020-06-04  4:46 ` [PATCH 02/16] drm/amd/powerplay: drop unused APIs and unnecessary checks Evan Quan
2020-06-04 20:50   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 03/16] drm/amd/powerplay: implement a common API for dpms disablement Evan Quan
2020-06-04 20:51   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 04/16] drm/amd/powerplay: centralize all buffer allocation in sw_init phase Evan Quan
2020-06-04  4:46 ` [PATCH 05/16] drm/amd/powerplay: clean up the APIs for bootup clocks Evan Quan
2020-06-04  4:46 ` [PATCH 06/16] drm/amd/powerplay: clean up the APIs for pptable setup Evan Quan
2020-06-04  4:46 ` [PATCH 07/16] drm/amd/powerplay: clean up the overdrive settings Evan Quan
2020-06-04  4:46 ` [PATCH 08/16] drm/amd/powerplay: postpone operations not must for hw setup to late_init Evan Quan
2020-06-04 20:56   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 09/16] drm/amd/powerplay: move those operations not needed for resume out Evan Quan
2020-06-04 20:58   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 10/16] drm/amd/powerplay: max code sharing between .hw_init and .resume Evan Quan
2020-06-04 21:00   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 11/16] drm/amd/powerplay: resort those operations performed in hw setup Evan Quan
2020-06-04 21:02   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 12/16] drm/amd/powerplay: better namings Evan Quan
2020-06-04 21:07   ` Alex Deucher
2020-06-04 21:09     ` Alex Deucher
2020-06-05  5:53     ` Quan, Evan
2020-06-04  4:46 ` [PATCH 13/16] drm/amd/powerplay: max code sharing between .hw_fini and .suspend Evan Quan
2020-06-04 21:08   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 14/16] drm/amd/powerplay: allocate the struct amdgpu_irq_src on the stack Evan Quan
2020-06-04 21:13   ` Alex Deucher
2020-06-04  4:46 ` [PATCH 15/16] drm/amd/powerplay: add firmware cleanup on sw_fini Evan Quan
2020-06-04  4:46 ` [PATCH 16/16] drm/amd/powerplay: skip BACO feature on DPMs disablement Evan Quan
2020-06-04 21:14   ` Alex Deucher
2020-06-04 20:49 ` [PATCH 01/16] drm/amd/powerplay: eliminate asic type check Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).