Re: [PATCH 1/2] drm/amd/pp: Refine code in powerplay for Cz/Vega10

2018-02-22 Thread Alex Deucher 
On Thu, Feb 22, 2018 at 7:48 AM, Rex Zhu  wrote:
> Move dpm check functions on CZ/Vega10 to smu backend
> function table.
>
> Change-Id: I084f586ced5dbbcded54176a06b857c75303e553
> Signed-off-by: Rex Zhu 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 27 
> ++
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h |  1 -
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 15 ++--
>  drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c   | 24 +++
>  drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.h   |  2 ++
>  .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c   | 13 +++
>  6 files changed, 43 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index 5a7b99f..4b48765 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -1009,32 +1009,9 @@ static void cz_reset_acp_boot_level(struct pp_hwmgr 
> *hwmgr)
> cz_hwmgr->acp_boot_level = 0xff;
>  }
>
> -static bool cz_dpm_check_smu_features(struct pp_hwmgr *hwmgr,
> -   unsigned long check_feature)
> -{
> -   int result;
> -   unsigned long features;
> -
> -   result = smum_send_msg_to_smc_with_parameter(hwmgr, 
> PPSMC_MSG_GetFeatureStatus, 0);
> -   if (result == 0) {
> -   features = smum_get_argument(hwmgr);
> -   if (features & check_feature)
> -   return true;
> -   }
> -
> -   return false;
> -}
> -
> -static bool cz_check_for_dpm_enabled(struct pp_hwmgr *hwmgr)
> -{
> -   if (cz_dpm_check_smu_features(hwmgr, 
> SMU_EnabledFeatureScoreboard_SclkDpmOn))
> -   return true;
> -   return false;
> -}
> -
>  static int cz_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
>  {
> -   if (!cz_check_for_dpm_enabled(hwmgr)) {
> +   if (!smum_is_dpm_running(hwmgr)) {
> pr_info("dpm has been disabled\n");
> return 0;
> }
> @@ -1049,7 +1026,7 @@ static int cz_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
>
>  static int cz_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
>  {
> -   if (cz_check_for_dpm_enabled(hwmgr)) {
> +   if (smum_is_dpm_running(hwmgr)) {
> pr_info("dpm has been enabled\n");
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h
> index 468c739..b56720a 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h
> @@ -171,7 +171,6 @@ struct cz_power_state {
>  #define DPMFlags_Debug 0x8000
>
>  #define SMU_EnabledFeatureScoreboard_AcpDpmOn   0x0001 /* bit 0 */
> -#define SMU_EnabledFeatureScoreboard_SclkDpmOn0x0020
>  #define SMU_EnabledFeatureScoreboard_UvdDpmOn   0x0080 /* bit 23 */
>  #define SMU_EnabledFeatureScoreboard_VceDpmOn   0x0100 /* bit 24 */
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index bd77a91..ecea677 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -949,17 +949,6 @@ static int vega10_setup_asic_task(struct pp_hwmgr *hwmgr)
> return 0;
>  }
>
> -static bool vega10_is_dpm_running(struct pp_hwmgr *hwmgr)
> -{
> -   uint32_t features_enabled;
> -
> -   if (!vega10_get_smc_features(hwmgr, _enabled)) {
> -   if (features_enabled & SMC_DPM_FEATURES)
> -   return true;
> -   }
> -   return false;
> -}
> -
>  /**
>  * Remove repeated voltage values and create table with unique values.
>  *
> @@ -2912,7 +2901,7 @@ static int vega10_enable_dpm_tasks(struct pp_hwmgr 
> *hwmgr)
>
> vega10_enable_disable_PCC_limit_feature(hwmgr, true);
>
> -   tmp_result = (!vega10_is_dpm_running(hwmgr)) ? 0 : -1;
> +   tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(!tmp_result,
> "DPM is already running right , skipping 
> re-enablement!",
> return 0);
> @@ -4740,7 +4729,7 @@ static int vega10_disable_dpm_tasks(struct pp_hwmgr 
> *hwmgr)
>  {
> int tmp_result, result = 0;
>
> -   tmp_result = (vega10_is_dpm_running(hwmgr)) ? 0 : -1;
> +   tmp_result = (smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(tmp_result == 0,
> "DPM is not running right now, no need to disable 
> DPM!",
> return 0);
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c 
> b/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
> index 4d3aff3..1eadd64 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
> +++

[PATCH 1/2] drm/amd/pp: Refine code in powerplay for Cz/Vega10

2018-02-22 Thread Rex Zhu 
Move dpm check functions on CZ/Vega10 to smu backend
function table.

Change-Id: I084f586ced5dbbcded54176a06b857c75303e553
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 27 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h |  1 -
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 15 ++--
 drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c   | 24 +++
 drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.h   |  2 ++
 .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c   | 13 +++
 6 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
index 5a7b99f..4b48765 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
@@ -1009,32 +1009,9 @@ static void cz_reset_acp_boot_level(struct pp_hwmgr 
*hwmgr)
cz_hwmgr->acp_boot_level = 0xff;
 }
 
-static bool cz_dpm_check_smu_features(struct pp_hwmgr *hwmgr,
-   unsigned long check_feature)
-{
-   int result;
-   unsigned long features;
-
-   result = smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_GetFeatureStatus, 0);
-   if (result == 0) {
-   features = smum_get_argument(hwmgr);
-   if (features & check_feature)
-   return true;
-   }
-
-   return false;
-}
-
-static bool cz_check_for_dpm_enabled(struct pp_hwmgr *hwmgr)
-{
-   if (cz_dpm_check_smu_features(hwmgr, 
SMU_EnabledFeatureScoreboard_SclkDpmOn))
-   return true;
-   return false;
-}
-
 static int cz_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
 {
-   if (!cz_check_for_dpm_enabled(hwmgr)) {
+   if (!smum_is_dpm_running(hwmgr)) {
pr_info("dpm has been disabled\n");
return 0;
}
@@ -1049,7 +1026,7 @@ static int cz_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
 
 static int cz_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 {
-   if (cz_check_for_dpm_enabled(hwmgr)) {
+   if (smum_is_dpm_running(hwmgr)) {
pr_info("dpm has been enabled\n");
return 0;
}
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h
index 468c739..b56720a 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.h
@@ -171,7 +171,6 @@ struct cz_power_state {
 #define DPMFlags_Debug 0x8000
 
 #define SMU_EnabledFeatureScoreboard_AcpDpmOn   0x0001 /* bit 0 */
-#define SMU_EnabledFeatureScoreboard_SclkDpmOn0x0020
 #define SMU_EnabledFeatureScoreboard_UvdDpmOn   0x0080 /* bit 23 */
 #define SMU_EnabledFeatureScoreboard_VceDpmOn   0x0100 /* bit 24 */
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index bd77a91..ecea677 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -949,17 +949,6 @@ static int vega10_setup_asic_task(struct pp_hwmgr *hwmgr)
return 0;
 }
 
-static bool vega10_is_dpm_running(struct pp_hwmgr *hwmgr)
-{
-   uint32_t features_enabled;
-
-   if (!vega10_get_smc_features(hwmgr, _enabled)) {
-   if (features_enabled & SMC_DPM_FEATURES)
-   return true;
-   }
-   return false;
-}
-
 /**
 * Remove repeated voltage values and create table with unique values.
 *
@@ -2912,7 +2901,7 @@ static int vega10_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 
vega10_enable_disable_PCC_limit_feature(hwmgr, true);
 
-   tmp_result = (!vega10_is_dpm_running(hwmgr)) ? 0 : -1;
+   tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
PP_ASSERT_WITH_CODE(!tmp_result,
"DPM is already running right , skipping 
re-enablement!",
return 0);
@@ -4740,7 +4729,7 @@ static int vega10_disable_dpm_tasks(struct pp_hwmgr 
*hwmgr)
 {
int tmp_result, result = 0;
 
-   tmp_result = (vega10_is_dpm_running(hwmgr)) ? 0 : -1;
+   tmp_result = (smum_is_dpm_running(hwmgr)) ? 0 : -1;
PP_ASSERT_WITH_CODE(tmp_result == 0,
"DPM is not running right now, no need to disable DPM!",
return 0);
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
index 4d3aff3..1eadd64 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
@@ -855,6 +855,29 @@ static int cz_smu_fini(struct pp_hwmgr *hwmgr)
return 0;
 }
 
+static bool cz_dpm_check_smu_features(struct pp_hwmgr *hwmgr,
+   unsigned long check_feature)
+{
+   int result;
+   unsigned long features;
+
+   result =