Re: [PATCH] drm/amd/pm: fix enabled features retrieving on Renoir and Cyan Skillfish

2022-02-09 Thread Alex Deucher 
On Wed, Feb 9, 2022 at 8:47 PM Evan Quan  wrote:
>
> For Cyan Skillfish and Renoir, there is no interface provided by PMFW
> to retrieve the enabled features. So, we assume all features are enabled.
>
> Fixes: 7ade3ca9cdb5 ("drm/amd/pm: correct the usage for 'supported' member of 
> smu_feature structure")
>
> Signed-off-by: Evan Quan 

Acked-by: Alex Deucher 

> Change-Id: I1231f146405a229a11aa7ac608c8c932d3c90ee4
> --
> v1->v2:
>   - add back the logic for supporting those ASICs which have
> no feature_map available
> v2->v3:
>   - update the check for smu_cmn_feature_is_enabled to use a more
> generic way instead of asic type
>
> Change-Id: I7dfa453ffc086f5364848f7f32decd57a5a5b0e6
> ---
>  .../amd/pm/swsmu/smu11/cyan_skillfish_ppt.c   |  1 +
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 27 ++-
>  drivers/gpu/drm/amd/pm/swsmu/smu_internal.h   |  2 +-
>  3 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> index 2b38a9154dd4..b3a0f3fb3e65 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> @@ -562,6 +562,7 @@ static const struct pptable_funcs 
> cyan_skillfish_ppt_funcs = {
> .fini_smc_tables = smu_v11_0_fini_smc_tables,
> .read_sensor = cyan_skillfish_read_sensor,
> .print_clk_levels = cyan_skillfish_print_clk_levels,
> +   .get_enabled_mask = smu_cmn_get_enabled_mask,
> .is_dpm_running = cyan_skillfish_is_dpm_running,
> .get_gpu_metrics = cyan_skillfish_get_gpu_metrics,
> .od_edit_dpm_table = cyan_skillfish_od_edit_dpm_table,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 2a6b752a6996..4c12abcd995d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -500,7 +500,17 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
> uint64_t enabled_features;
> int feature_id;
>
> -   if (smu->is_apu && adev->family < AMDGPU_FAMILY_VGH)
> +   if (smu_cmn_get_enabled_mask(smu, &enabled_features)) {
> +   dev_err(adev->dev, "Failed to retrieve enabled 
> ppfeatures!\n");
> +   return 0;
> +   }
> +
> +   /*
> +* For Renoir and Cyan Skillfish, they are assumed to have all 
> features
> +* enabled. Also considering they have no feature_map available, the
> +* check here can avoid unwanted feature_map check below.
> +*/
> +   if (enabled_features == ULLONG_MAX)
> return 1;
>
> feature_id = smu_cmn_to_asic_specific_index(smu,
> @@ -509,11 +519,6 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
> if (feature_id < 0)
> return 0;
>
> -   if (smu_cmn_get_enabled_mask(smu, &enabled_features)) {
> -   dev_err(adev->dev, "Failed to retrieve enabled 
> ppfeatures!\n");
> -   return 0;
> -   }
> -
> return test_bit(feature_id, (unsigned long *)&enabled_features);
>  }
>
> @@ -559,7 +564,7 @@ int smu_cmn_get_enabled_mask(struct smu_context *smu,
> feature_mask_high = &((uint32_t *)feature_mask)[1];
>
> switch (adev->ip_versions[MP1_HWIP][0]) {
> -   case IP_VERSION(11, 0, 8):
> +   /* For Vangogh and Yellow Carp */
> case IP_VERSION(11, 5, 0):
> case IP_VERSION(13, 0, 1):
> case IP_VERSION(13, 0, 3):
> @@ -575,8 +580,16 @@ int smu_cmn_get_enabled_mask(struct smu_context *smu,
>   1,
>   feature_mask_high);
> break;
> +   /*
> +* For Cyan Skillfish and Renoir, there is no interface provided by 
> PMFW
> +* to retrieve the enabled features. So, we assume all features are 
> enabled.
> +* TODO: add other APU ASICs which suffer from the same issue here
> +*/
> +   case IP_VERSION(11, 0, 8):
> case IP_VERSION(12, 0, 0):
> case IP_VERSION(12, 0, 1):
> +   memset(feature_mask, 0xff, sizeof(*feature_mask));
> +   break;
> /* other dGPU ASICs */
> default:
> ret = smu_cmn_send_smc_msg(smu,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h 
> b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> index 530be44e00ec..15bcf72b8e56 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> @@ -55,7 +55,7 @@
>  #define smu_send_smc_msg(smu, msg, read_arg)   
> smu_ppt_funcs(send_smc_msg, 0, smu, msg, read_arg)
>  #define smu_init_display_count(smu, count) 
> smu_ppt_funcs(init_display_count, 0, smu, count)
>  #define smu_feature_set_allowed_mask(smu)

Re: [PATCH] drm/amd/pm: fix enabled features retrieving on Renoir and Cyan Skillfish

2022-02-09 Thread Nathan Chancellor 
On Thu, Feb 10, 2022 at 09:47:00AM +0800, Evan Quan wrote:
> For Cyan Skillfish and Renoir, there is no interface provided by PMFW
> to retrieve the enabled features. So, we assume all features are enabled.
> 
> Fixes: 7ade3ca9cdb5 ("drm/amd/pm: correct the usage for 'supported' member of 
> smu_feature structure")
> 
> Signed-off-by: Evan Quan 
> Change-Id: I1231f146405a229a11aa7ac608c8c932d3c90ee4

Tested-by: Nathan Chancellor 

> --
> v1->v2:
>   - add back the logic for supporting those ASICs which have
> no feature_map available
> v2->v3:
>   - update the check for smu_cmn_feature_is_enabled to use a more
> generic way instead of asic type
> 
> Change-Id: I7dfa453ffc086f5364848f7f32decd57a5a5b0e6
> ---
>  .../amd/pm/swsmu/smu11/cyan_skillfish_ppt.c   |  1 +
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 27 ++-
>  drivers/gpu/drm/amd/pm/swsmu/smu_internal.h   |  2 +-
>  3 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> index 2b38a9154dd4..b3a0f3fb3e65 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> @@ -562,6 +562,7 @@ static const struct pptable_funcs 
> cyan_skillfish_ppt_funcs = {
>   .fini_smc_tables = smu_v11_0_fini_smc_tables,
>   .read_sensor = cyan_skillfish_read_sensor,
>   .print_clk_levels = cyan_skillfish_print_clk_levels,
> + .get_enabled_mask = smu_cmn_get_enabled_mask,
>   .is_dpm_running = cyan_skillfish_is_dpm_running,
>   .get_gpu_metrics = cyan_skillfish_get_gpu_metrics,
>   .od_edit_dpm_table = cyan_skillfish_od_edit_dpm_table,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 2a6b752a6996..4c12abcd995d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -500,7 +500,17 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
>   uint64_t enabled_features;
>   int feature_id;
>  
> - if (smu->is_apu && adev->family < AMDGPU_FAMILY_VGH)
> + if (smu_cmn_get_enabled_mask(smu, &enabled_features)) {
> + dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
> + return 0;
> + }
> +
> + /*
> +  * For Renoir and Cyan Skillfish, they are assumed to have all features
> +  * enabled. Also considering they have no feature_map available, the
> +  * check here can avoid unwanted feature_map check below.
> +  */
> + if (enabled_features == ULLONG_MAX)
>   return 1;
>  
>   feature_id = smu_cmn_to_asic_specific_index(smu,
> @@ -509,11 +519,6 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
>   if (feature_id < 0)
>   return 0;
>  
> - if (smu_cmn_get_enabled_mask(smu, &enabled_features)) {
> - dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
> - return 0;
> - }
> -
>   return test_bit(feature_id, (unsigned long *)&enabled_features);
>  }
>  
> @@ -559,7 +564,7 @@ int smu_cmn_get_enabled_mask(struct smu_context *smu,
>   feature_mask_high = &((uint32_t *)feature_mask)[1];
>  
>   switch (adev->ip_versions[MP1_HWIP][0]) {
> - case IP_VERSION(11, 0, 8):
> + /* For Vangogh and Yellow Carp */
>   case IP_VERSION(11, 5, 0):
>   case IP_VERSION(13, 0, 1):
>   case IP_VERSION(13, 0, 3):
> @@ -575,8 +580,16 @@ int smu_cmn_get_enabled_mask(struct smu_context *smu,
> 1,
> feature_mask_high);
>   break;
> + /*
> +  * For Cyan Skillfish and Renoir, there is no interface provided by PMFW
> +  * to retrieve the enabled features. So, we assume all features are 
> enabled.
> +  * TODO: add other APU ASICs which suffer from the same issue here
> +  */
> + case IP_VERSION(11, 0, 8):
>   case IP_VERSION(12, 0, 0):
>   case IP_VERSION(12, 0, 1):
> + memset(feature_mask, 0xff, sizeof(*feature_mask));
> + break;
>   /* other dGPU ASICs */
>   default:
>   ret = smu_cmn_send_smc_msg(smu,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h 
> b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> index 530be44e00ec..15bcf72b8e56 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> @@ -55,7 +55,7 @@
>  #define smu_send_smc_msg(smu, msg, read_arg) 
> smu_ppt_funcs(send_smc_msg, 0, smu, msg, read_arg)
>  #define smu_init_display_count(smu, count)   
> smu_ppt_funcs(init_display_count, 0, smu, count)
>  #define smu_feature_set_allowed_mask(smu)
> smu_ppt_funcs(set_allowed_mask, 0, smu)
> -#define smu_feature_get_enabled_mask(s

[PATCH] drm/amd/pm: fix enabled features retrieving on Renoir and Cyan Skillfish

2022-02-09 Thread Evan Quan 
For Cyan Skillfish and Renoir, there is no interface provided by PMFW
to retrieve the enabled features. So, we assume all features are enabled.

Fixes: 7ade3ca9cdb5 ("drm/amd/pm: correct the usage for 'supported' member of 
smu_feature structure")

Signed-off-by: Evan Quan 
Change-Id: I1231f146405a229a11aa7ac608c8c932d3c90ee4
--
v1->v2:
  - add back the logic for supporting those ASICs which have
no feature_map available
v2->v3:
  - update the check for smu_cmn_feature_is_enabled to use a more
generic way instead of asic type

Change-Id: I7dfa453ffc086f5364848f7f32decd57a5a5b0e6
---
 .../amd/pm/swsmu/smu11/cyan_skillfish_ppt.c   |  1 +
 drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 27 ++-
 drivers/gpu/drm/amd/pm/swsmu/smu_internal.h   |  2 +-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
index 2b38a9154dd4..b3a0f3fb3e65 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
@@ -562,6 +562,7 @@ static const struct pptable_funcs cyan_skillfish_ppt_funcs 
= {
.fini_smc_tables = smu_v11_0_fini_smc_tables,
.read_sensor = cyan_skillfish_read_sensor,
.print_clk_levels = cyan_skillfish_print_clk_levels,
+   .get_enabled_mask = smu_cmn_get_enabled_mask,
.is_dpm_running = cyan_skillfish_is_dpm_running,
.get_gpu_metrics = cyan_skillfish_get_gpu_metrics,
.od_edit_dpm_table = cyan_skillfish_od_edit_dpm_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index 2a6b752a6996..4c12abcd995d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -500,7 +500,17 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
uint64_t enabled_features;
int feature_id;
 
-   if (smu->is_apu && adev->family < AMDGPU_FAMILY_VGH)
+   if (smu_cmn_get_enabled_mask(smu, &enabled_features)) {
+   dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
+   return 0;
+   }
+
+   /*
+* For Renoir and Cyan Skillfish, they are assumed to have all features
+* enabled. Also considering they have no feature_map available, the
+* check here can avoid unwanted feature_map check below.
+*/
+   if (enabled_features == ULLONG_MAX)
return 1;
 
feature_id = smu_cmn_to_asic_specific_index(smu,
@@ -509,11 +519,6 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
if (feature_id < 0)
return 0;
 
-   if (smu_cmn_get_enabled_mask(smu, &enabled_features)) {
-   dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
-   return 0;
-   }
-
return test_bit(feature_id, (unsigned long *)&enabled_features);
 }
 
@@ -559,7 +564,7 @@ int smu_cmn_get_enabled_mask(struct smu_context *smu,
feature_mask_high = &((uint32_t *)feature_mask)[1];
 
switch (adev->ip_versions[MP1_HWIP][0]) {
-   case IP_VERSION(11, 0, 8):
+   /* For Vangogh and Yellow Carp */
case IP_VERSION(11, 5, 0):
case IP_VERSION(13, 0, 1):
case IP_VERSION(13, 0, 3):
@@ -575,8 +580,16 @@ int smu_cmn_get_enabled_mask(struct smu_context *smu,
  1,
  feature_mask_high);
break;
+   /*
+* For Cyan Skillfish and Renoir, there is no interface provided by PMFW
+* to retrieve the enabled features. So, we assume all features are 
enabled.
+* TODO: add other APU ASICs which suffer from the same issue here
+*/
+   case IP_VERSION(11, 0, 8):
case IP_VERSION(12, 0, 0):
case IP_VERSION(12, 0, 1):
+   memset(feature_mask, 0xff, sizeof(*feature_mask));
+   break;
/* other dGPU ASICs */
default:
ret = smu_cmn_send_smc_msg(smu,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h 
b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
index 530be44e00ec..15bcf72b8e56 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
@@ -55,7 +55,7 @@
 #define smu_send_smc_msg(smu, msg, read_arg)   
smu_ppt_funcs(send_smc_msg, 0, smu, msg, read_arg)
 #define smu_init_display_count(smu, count) 
smu_ppt_funcs(init_display_count, 0, smu, count)
 #define smu_feature_set_allowed_mask(smu)  
smu_ppt_funcs(set_allowed_mask, 0, smu)
-#define smu_feature_get_enabled_mask(smu, mask)
smu_ppt_funcs(get_enabled_mask, 0, smu, mask)
+#define smu_feature_get_enabled_mask(smu, mask)
smu_ppt_funcs(get_enabled_mask, -EOPNO