RE: [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
[AMD Official Use Only - Internal Distribution Only]
I refreshed the patch set based on the latest code and just sent them out.
And patch 8 and this one were splitted into several small patches.
-Original Message-
From: Alex Deucher
Sent: Tuesday, June 2, 2020 10:55 PM
To: Quan, Evan
Cc: amd-gfx list ; Deucher, Alexander
Subject: Re: [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan wrote:
>
> To make code more clean and readable by moving ASIC specific code to
> its own file, more code sharing and dropping unused code.
There seem to be multiple things going on here. It's kind of hard to follow
all of the changes. Maybe split this patch up? One additional comment below.
Alex
>
> Change-Id: I6b299f9e98c7678b48281cbed9beb17b644bb4cc
> Signed-off-by: Evan Quan
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 213
> - drivers/gpu/drm/amd/powerplay/navi10_ppt.c |
> 19 ++
> 2 files changed, 102 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 4998ea942760..b4f108cb52fa 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -817,22 +817,10 @@ int smu_get_atom_data_table(struct smu_context *smu,
> uint32_t table,
> return 0;
> }
>
> -static int smu_initialize_pptable(struct smu_context *smu) -{
> - /* TODO */
> - return 0;
> -}
> -
> static int smu_smc_table_sw_init(struct smu_context *smu) {
> int ret;
>
> - ret = smu_initialize_pptable(smu);
> - if (ret) {
> - pr_err("Failed to init smu_initialize_pptable!\n");
> - return ret;
> - }
> -
> /**
> * Create smu_table structure, and init smc tables such as
> * TABLE_PPTABLE, TABLE_WATERMARKS, TABLE_SMU_METRICS, and etc.
> @@ -860,6 +848,12 @@ static int smu_smc_table_sw_fini(struct
> smu_context *smu) {
> int ret;
>
> + ret = smu_fini_power(smu);
> + if (ret) {
> + pr_err("Failed to init smu_fini_power!\n");
> + return ret;
> + }
> +
> ret = smu_fini_smc_tables(smu);
> if (ret) {
> pr_err("Failed to smu_fini_smc_tables!\n"); @@ -950,12
> +944,6 @@ static int smu_sw_fini(void *handle)
> return ret;
> }
>
> - ret = smu_fini_power(smu);
> - if (ret) {
> - pr_err("Failed to init smu_fini_power!\n");
> - return ret;
> - }
> -
> return 0;
> }
>
> @@ -1125,36 +1113,22 @@ static int smu_smc_table_hw_init(struct smu_context
> *smu,
> if (ret)
> return ret;
>
> - if (adev->asic_type == CHIP_NAVI10) {
> - if ((adev->pdev->device == 0x731f && (adev->pdev->revision ==
> 0xc2 ||
> - adev->pdev->revision ==
> 0xc3 ||
> - adev->pdev->revision ==
> 0xca ||
> - adev->pdev->revision ==
> 0xcb)) ||
> - (adev->pdev->device == 0x66af && (adev->pdev->revision ==
> 0xf3 ||
> - adev->pdev->revision ==
> 0xf4 ||
> - adev->pdev->revision ==
> 0xf5 ||
> - adev->pdev->revision ==
> 0xf6))) {
> - ret = smu_disable_umc_cdr_12gbps_workaround(smu);
> - if (ret) {
> - pr_err("Workaround failed to disable UMC CDR
> feature on 12Gbps SKU!\n");
> - return ret;
> - }
> - }
> + ret = smu_disable_umc_cdr_12gbps_workaround(smu);
> + if (ret) {
> + pr_err("Workaround failed to disable UMC CDR feature on
> 12Gbps SKU!\n");
> + return ret;
> }
>
> - if (smu->ppt_funcs->set_power_source) {
> - /*
> -* For Navi1X, manually switch it to AC mode as PMFW
> -* may boot it with DC mode.
> -*/
> - if (adev->pm.ac_power)
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_AC);
> - else
> - ret = smu
Re: [PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
On Mon, Jun 1, 2020 at 3:30 AM Evan Quan wrote:
>
> To make code more clean and readable by moving ASIC
> specific code to its own file, more code sharing and
> dropping unused code.
There seem to be multiple things going on here. It's kind of hard to
follow all of the changes. Maybe split this patch up? One additional
comment below.
Alex
>
> Change-Id: I6b299f9e98c7678b48281cbed9beb17b644bb4cc
> Signed-off-by: Evan Quan
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 213 -
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 19 ++
> 2 files changed, 102 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 4998ea942760..b4f108cb52fa 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -817,22 +817,10 @@ int smu_get_atom_data_table(struct smu_context *smu,
> uint32_t table,
> return 0;
> }
>
> -static int smu_initialize_pptable(struct smu_context *smu)
> -{
> - /* TODO */
> - return 0;
> -}
> -
> static int smu_smc_table_sw_init(struct smu_context *smu)
> {
> int ret;
>
> - ret = smu_initialize_pptable(smu);
> - if (ret) {
> - pr_err("Failed to init smu_initialize_pptable!\n");
> - return ret;
> - }
> -
> /**
> * Create smu_table structure, and init smc tables such as
> * TABLE_PPTABLE, TABLE_WATERMARKS, TABLE_SMU_METRICS, and etc.
> @@ -860,6 +848,12 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
> {
> int ret;
>
> + ret = smu_fini_power(smu);
> + if (ret) {
> + pr_err("Failed to init smu_fini_power!\n");
> + return ret;
> + }
> +
> ret = smu_fini_smc_tables(smu);
> if (ret) {
> pr_err("Failed to smu_fini_smc_tables!\n");
> @@ -950,12 +944,6 @@ static int smu_sw_fini(void *handle)
> return ret;
> }
>
> - ret = smu_fini_power(smu);
> - if (ret) {
> - pr_err("Failed to init smu_fini_power!\n");
> - return ret;
> - }
> -
> return 0;
> }
>
> @@ -1125,36 +1113,22 @@ static int smu_smc_table_hw_init(struct smu_context
> *smu,
> if (ret)
> return ret;
>
> - if (adev->asic_type == CHIP_NAVI10) {
> - if ((adev->pdev->device == 0x731f && (adev->pdev->revision ==
> 0xc2 ||
> - adev->pdev->revision ==
> 0xc3 ||
> - adev->pdev->revision ==
> 0xca ||
> - adev->pdev->revision ==
> 0xcb)) ||
> - (adev->pdev->device == 0x66af && (adev->pdev->revision ==
> 0xf3 ||
> - adev->pdev->revision ==
> 0xf4 ||
> - adev->pdev->revision ==
> 0xf5 ||
> - adev->pdev->revision ==
> 0xf6))) {
> - ret = smu_disable_umc_cdr_12gbps_workaround(smu);
> - if (ret) {
> - pr_err("Workaround failed to disable UMC CDR
> feature on 12Gbps SKU!\n");
> - return ret;
> - }
> - }
> + ret = smu_disable_umc_cdr_12gbps_workaround(smu);
> + if (ret) {
> + pr_err("Workaround failed to disable UMC CDR feature on
> 12Gbps SKU!\n");
> + return ret;
> }
>
> - if (smu->ppt_funcs->set_power_source) {
> - /*
> -* For Navi1X, manually switch it to AC mode as PMFW
> -* may boot it with DC mode.
> -*/
> - if (adev->pm.ac_power)
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_AC);
> - else
> - ret = smu_set_power_source(smu, SMU_POWER_SOURCE_DC);
> - if (ret) {
> - pr_err("Failed to switch to %s mode!\n",
> adev->pm.ac_power ? "AC" : "DC");
> - return ret;
> - }
> + /*
> +* For Navi1X, manually switch it to AC mode as PMFW
> +* may boot it with DC mode.
> +*/
> + ret = smu_set_power_source(smu,
> + adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
> + SMU_POWER_SOURCE_DC);
> + if (ret) {
> + pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ?
> "AC" : "DC");
> + return ret;
> }
>
> ret = smu_notify_display_change(smu);
> @@ -1362,9 +1336,65 @@ static int smu_hw_init(void *handle)
> return ret;
> }
>
> -static int smu_stop_dpms(struct smu_context *smu)
> +static int smu_di
[PATCH 2/9] drm/amd/powerplay: some cosmetic fixes
To make code more clean and readable by moving ASIC
specific code to its own file, more code sharing and
dropping unused code.
Change-Id: I6b299f9e98c7678b48281cbed9beb17b644bb4cc
Signed-off-by: Evan Quan
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 213 -
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 19 ++
2 files changed, 102 insertions(+), 130 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 4998ea942760..b4f108cb52fa 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -817,22 +817,10 @@ int smu_get_atom_data_table(struct smu_context *smu,
uint32_t table,
return 0;
}
-static int smu_initialize_pptable(struct smu_context *smu)
-{
- /* TODO */
- return 0;
-}
-
static int smu_smc_table_sw_init(struct smu_context *smu)
{
int ret;
- ret = smu_initialize_pptable(smu);
- if (ret) {
- pr_err("Failed to init smu_initialize_pptable!\n");
- return ret;
- }
-
/**
* Create smu_table structure, and init smc tables such as
* TABLE_PPTABLE, TABLE_WATERMARKS, TABLE_SMU_METRICS, and etc.
@@ -860,6 +848,12 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
{
int ret;
+ ret = smu_fini_power(smu);
+ if (ret) {
+ pr_err("Failed to init smu_fini_power!\n");
+ return ret;
+ }
+
ret = smu_fini_smc_tables(smu);
if (ret) {
pr_err("Failed to smu_fini_smc_tables!\n");
@@ -950,12 +944,6 @@ static int smu_sw_fini(void *handle)
return ret;
}
- ret = smu_fini_power(smu);
- if (ret) {
- pr_err("Failed to init smu_fini_power!\n");
- return ret;
- }
-
return 0;
}
@@ -1125,36 +1113,22 @@ static int smu_smc_table_hw_init(struct smu_context
*smu,
if (ret)
return ret;
- if (adev->asic_type == CHIP_NAVI10) {
- if ((adev->pdev->device == 0x731f && (adev->pdev->revision ==
0xc2 ||
- adev->pdev->revision ==
0xc3 ||
- adev->pdev->revision ==
0xca ||
- adev->pdev->revision ==
0xcb)) ||
- (adev->pdev->device == 0x66af && (adev->pdev->revision ==
0xf3 ||
- adev->pdev->revision ==
0xf4 ||
- adev->pdev->revision ==
0xf5 ||
- adev->pdev->revision ==
0xf6))) {
- ret = smu_disable_umc_cdr_12gbps_workaround(smu);
- if (ret) {
- pr_err("Workaround failed to disable UMC CDR
feature on 12Gbps SKU!\n");
- return ret;
- }
- }
+ ret = smu_disable_umc_cdr_12gbps_workaround(smu);
+ if (ret) {
+ pr_err("Workaround failed to disable UMC CDR feature on 12Gbps
SKU!\n");
+ return ret;
}
- if (smu->ppt_funcs->set_power_source) {
- /*
-* For Navi1X, manually switch it to AC mode as PMFW
-* may boot it with DC mode.
-*/
- if (adev->pm.ac_power)
- ret = smu_set_power_source(smu, SMU_POWER_SOURCE_AC);
- else
- ret = smu_set_power_source(smu, SMU_POWER_SOURCE_DC);
- if (ret) {
- pr_err("Failed to switch to %s mode!\n",
adev->pm.ac_power ? "AC" : "DC");
- return ret;
- }
+ /*
+* For Navi1X, manually switch it to AC mode as PMFW
+* may boot it with DC mode.
+*/
+ ret = smu_set_power_source(smu,
+ adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
+ SMU_POWER_SOURCE_DC);
+ if (ret) {
+ pr_err("Failed to switch to %s mode!\n", adev->pm.ac_power ?
"AC" : "DC");
+ return ret;
}
ret = smu_notify_display_change(smu);
@@ -1362,9 +1336,65 @@ static int smu_hw_init(void *handle)
return ret;
}
-static int smu_stop_dpms(struct smu_context *smu)
+static int smu_disable_dpms(struct smu_context *smu)
{
- return smu_system_features_control(smu, false);
+ struct amdgpu_device *adev = smu->adev;
+ int ret = 0;
+ bool use_baco = !smu->is_apu &&
+ ((adev->in_gpu_reset &&
+ (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
+((adev->in_runpm || adev->in_hibernate) &&
amdgpu_asic_supports_baco(adev)));
+
+ /*
+* For custom pptable uplo
