Re: [PATCH V3 4/7] drm/amd/pm: correct the usage for 'supported' member of smu_feature structure

2022-02-08 Thread Nathan Chancellor
Hi Evan,

On Fri, Jan 28, 2022 at 03:04:52PM +0800, Evan Quan wrote:
> The supported features should be retrieved just after EnableAllDpmFeatures 
> message
> complete. And the check(whether some dpm feature is supported) is only needed 
> when we
> decide to enable or disable it.
> 
> Signed-off-by: Evan Quan 
> Change-Id: I07c9a5ac5290cd0d88a40ce1768d393156419b5a
> ---
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 11 +++
>  drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  8 
>  .../gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 10 +-
>  drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c|  3 ---
>  drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  |  5 +
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c|  3 ---
>  drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  |  3 ---
>  7 files changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ae48cc5aa567..803068cb5079 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1057,8 +1057,10 @@ static int smu_get_thermal_temperature_range(struct 
> smu_context *smu)
>  
>  static int smu_smc_hw_setup(struct smu_context *smu)
>  {
> + struct smu_feature *feature = >smu_feature;
>   struct amdgpu_device *adev = smu->adev;
>   uint32_t pcie_gen = 0, pcie_width = 0;
> + uint64_t features_supported;
>   int ret = 0;
>  
>   if (adev->in_suspend && smu_is_dpm_running(smu)) {
> @@ -1138,6 +1140,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>   return ret;
>   }
>  
> + ret = smu_feature_get_enabled_mask(smu, _supported);
> + if (ret) {
> + dev_err(adev->dev, "Failed to retrieve supported dpm 
> features!\n");
> + return ret;
> + }
> + bitmap_copy(feature->supported,
> + (unsigned long *)_supported,
> + feature->feature_num);
> +
>   if (!smu_is_dpm_running(smu))
>   dev_info(adev->dev, "dpm has been disabled\n");
>  
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 84cbde3f913d..f55ead5f9aba 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -1624,8 +1624,8 @@ static int navi10_display_config_changed(struct 
> smu_context *smu)
>   int ret = 0;
>  
>   if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> - smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> - smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
> + smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> + smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
>   ret = smu_cmn_send_smc_msg_with_param(smu, 
> SMU_MSG_NumOfDisplays,
> 
> smu->display_config->num_display,
> NULL);
> @@ -1860,13 +1860,13 @@ static int navi10_notify_smc_display_config(struct 
> smu_context *smu)
>   min_clocks.dcef_clock_in_sr = 
> smu->display_config->min_dcef_deep_sleep_set_clk;
>   min_clocks.memory_clock = smu->display_config->min_mem_set_clock;
>  
> - if (smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT)) {
> + if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT)) {
>   clock_req.clock_type = amd_pp_dcef_clock;
>   clock_req.clock_freq_in_khz = min_clocks.dcef_clock * 10;
>  
>   ret = smu_v11_0_display_clock_voltage_request(smu, _req);
>   if (!ret) {
> - if (smu_cmn_feature_is_supported(smu, 
> SMU_FEATURE_DS_DCEFCLK_BIT)) {
> + if (smu_cmn_feature_is_enabled(smu, 
> SMU_FEATURE_DS_DCEFCLK_BIT)) {
>   ret = smu_cmn_send_smc_msg_with_param(smu,
> 
> SMU_MSG_SetMinDeepSleepDcefclk,
> 
> min_clocks.dcef_clock_in_sr/100,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index b6759f8b5167..804e1c98238d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -1280,8 +1280,8 @@ static int sienna_cichlid_display_config_changed(struct 
> smu_context *smu)
>   int ret = 0;
>  
>   if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> - smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> - smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
> + smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> + 

Re: [PATCH V3 4/7] drm/amd/pm: correct the usage for 'supported' member of smu_feature structure

2022-01-28 Thread Deucher, Alexander
[AMD Official Use Only]

Reviewed-by: Alex Deucher 

From: Quan, Evan 
Sent: Friday, January 28, 2022 2:04 AM
To: amd-gfx@lists.freedesktop.org 
Cc: Deucher, Alexander ; Lazar, Lijo 
; Quan, Evan 
Subject: [PATCH V3 4/7] drm/amd/pm: correct the usage for 'supported' member of 
smu_feature structure

The supported features should be retrieved just after EnableAllDpmFeatures 
message
complete. And the check(whether some dpm feature is supported) is only needed 
when we
decide to enable or disable it.

Signed-off-by: Evan Quan 
Change-Id: I07c9a5ac5290cd0d88a40ce1768d393156419b5a
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 11 +++
 drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  8 
 .../gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 10 +-
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c|  3 ---
 drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  |  5 +
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c|  3 ---
 drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  |  3 ---
 7 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index ae48cc5aa567..803068cb5079 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1057,8 +1057,10 @@ static int smu_get_thermal_temperature_range(struct 
smu_context *smu)

 static int smu_smc_hw_setup(struct smu_context *smu)
 {
+   struct smu_feature *feature = >smu_feature;
 struct amdgpu_device *adev = smu->adev;
 uint32_t pcie_gen = 0, pcie_width = 0;
+   uint64_t features_supported;
 int ret = 0;

 if (adev->in_suspend && smu_is_dpm_running(smu)) {
@@ -1138,6 +1140,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
 return ret;
 }

+   ret = smu_feature_get_enabled_mask(smu, _supported);
+   if (ret) {
+   dev_err(adev->dev, "Failed to retrieve supported dpm 
features!\n");
+   return ret;
+   }
+   bitmap_copy(feature->supported,
+   (unsigned long *)_supported,
+   feature->feature_num);
+
 if (!smu_is_dpm_running(smu))
 dev_info(adev->dev, "dpm has been disabled\n");

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 84cbde3f913d..f55ead5f9aba 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1624,8 +1624,8 @@ static int navi10_display_config_changed(struct 
smu_context *smu)
 int ret = 0;

 if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
+   smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
+   smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
 ret = smu_cmn_send_smc_msg_with_param(smu, 
SMU_MSG_NumOfDisplays,
   
smu->display_config->num_display,
   NULL);
@@ -1860,13 +1860,13 @@ static int navi10_notify_smc_display_config(struct 
smu_context *smu)
 min_clocks.dcef_clock_in_sr = 
smu->display_config->min_dcef_deep_sleep_set_clk;
 min_clocks.memory_clock = smu->display_config->min_mem_set_clock;

-   if (smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT)) {
+   if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT)) {
 clock_req.clock_type = amd_pp_dcef_clock;
 clock_req.clock_freq_in_khz = min_clocks.dcef_clock * 10;

 ret = smu_v11_0_display_clock_voltage_request(smu, _req);
 if (!ret) {
-   if (smu_cmn_feature_is_supported(smu, 
SMU_FEATURE_DS_DCEFCLK_BIT)) {
+   if (smu_cmn_feature_is_enabled(smu, 
SMU_FEATURE_DS_DCEFCLK_BIT)) {
 ret = smu_cmn_send_smc_msg_with_param(smu,
   
SMU_MSG_SetMinDeepSleepDcefclk,
   
min_clocks.dcef_clock_in_sr/100,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index b6759f8b5167..804e1c98238d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1280,8 +1280,8 @@ static int sienna_cichlid_display_config_changed(struct 
smu_context *smu)
 int ret = 0;

 if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
-   

[PATCH V3 4/7] drm/amd/pm: correct the usage for 'supported' member of smu_feature structure

2022-01-27 Thread Evan Quan
The supported features should be retrieved just after EnableAllDpmFeatures 
message
complete. And the check(whether some dpm feature is supported) is only needed 
when we
decide to enable or disable it.

Signed-off-by: Evan Quan 
Change-Id: I07c9a5ac5290cd0d88a40ce1768d393156419b5a
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 11 +++
 drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  8 
 .../gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 10 +-
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c|  3 ---
 drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  |  5 +
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c|  3 ---
 drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  |  3 ---
 7 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index ae48cc5aa567..803068cb5079 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1057,8 +1057,10 @@ static int smu_get_thermal_temperature_range(struct 
smu_context *smu)
 
 static int smu_smc_hw_setup(struct smu_context *smu)
 {
+   struct smu_feature *feature = >smu_feature;
struct amdgpu_device *adev = smu->adev;
uint32_t pcie_gen = 0, pcie_width = 0;
+   uint64_t features_supported;
int ret = 0;
 
if (adev->in_suspend && smu_is_dpm_running(smu)) {
@@ -1138,6 +1140,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
return ret;
}
 
+   ret = smu_feature_get_enabled_mask(smu, _supported);
+   if (ret) {
+   dev_err(adev->dev, "Failed to retrieve supported dpm 
features!\n");
+   return ret;
+   }
+   bitmap_copy(feature->supported,
+   (unsigned long *)_supported,
+   feature->feature_num);
+
if (!smu_is_dpm_running(smu))
dev_info(adev->dev, "dpm has been disabled\n");
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 84cbde3f913d..f55ead5f9aba 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1624,8 +1624,8 @@ static int navi10_display_config_changed(struct 
smu_context *smu)
int ret = 0;
 
if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
+   smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
+   smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
ret = smu_cmn_send_smc_msg_with_param(smu, 
SMU_MSG_NumOfDisplays,
  
smu->display_config->num_display,
  NULL);
@@ -1860,13 +1860,13 @@ static int navi10_notify_smc_display_config(struct 
smu_context *smu)
min_clocks.dcef_clock_in_sr = 
smu->display_config->min_dcef_deep_sleep_set_clk;
min_clocks.memory_clock = smu->display_config->min_mem_set_clock;
 
-   if (smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT)) {
+   if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT)) {
clock_req.clock_type = amd_pp_dcef_clock;
clock_req.clock_freq_in_khz = min_clocks.dcef_clock * 10;
 
ret = smu_v11_0_display_clock_voltage_request(smu, _req);
if (!ret) {
-   if (smu_cmn_feature_is_supported(smu, 
SMU_FEATURE_DS_DCEFCLK_BIT)) {
+   if (smu_cmn_feature_is_enabled(smu, 
SMU_FEATURE_DS_DCEFCLK_BIT)) {
ret = smu_cmn_send_smc_msg_with_param(smu,
  
SMU_MSG_SetMinDeepSleepDcefclk,
  
min_clocks.dcef_clock_in_sr/100,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index b6759f8b5167..804e1c98238d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1280,8 +1280,8 @@ static int sienna_cichlid_display_config_changed(struct 
smu_context *smu)
int ret = 0;
 
if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
-   smu_cmn_feature_is_supported(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
+   smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_DCEFCLK_BIT) &&
+   smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_SOCCLK_BIT)) {
 #if 0
ret = smu_cmn_send_smc_msg_with_param(smu, 
SMU_MSG_NumOfDisplays,