Re: [PATCH V3 5/7] drm/amd/pm: drop the cache for enabled ppfeatures

2022-01-28 Thread Deucher, Alexander 
[Public]

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 5/7] drm/amd/pm: drop the cache for enabled ppfeatures

The following scenarios make the driver cache for enabled ppfeatures
outdated and invalid:
  - Other tools interact with PMFW to change the enabled ppfeatures.
  - PMFW may enable/disable some features behind driver's back. E.g.
for sienna_cichild, on gfxoff entering, PMFW will disable gfx
related DPM features. All those are performed without driver's
notice.
Also considering driver does not actually interact with PMFW such
frequently, the benefit brought by such cache is very limited.

Signed-off-by: Evan Quan 
Change-Id: I20ed58ab216e930c7a5d223be1eb99146889f2b3
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  1 -
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |  1 -
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c| 23 +-
 .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  | 16 +--
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 23 +-
 .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  | 16 +--
 drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 46 +--
 7 files changed, 17 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 803068cb5079..59be1c822b2c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -950,7 +950,6 @@ static int smu_sw_init(void *handle)
 smu->pool_size = adev->pm.smu_prv_buffer_size;
 smu->smu_feature.feature_num = SMU_FEATURE_MAX;
 bitmap_zero(smu->smu_feature.supported, SMU_FEATURE_MAX);
-   bitmap_zero(smu->smu_feature.enabled, SMU_FEATURE_MAX);
 bitmap_zero(smu->smu_feature.allowed, SMU_FEATURE_MAX);

 mutex_init(>message_lock);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 8cd1c3bb595a..721b4080d3e6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -390,7 +390,6 @@ struct smu_feature
 uint32_t feature_num;
 DECLARE_BITMAP(supported, SMU_FEATURE_MAX);
 DECLARE_BITMAP(allowed, SMU_FEATURE_MAX);
-   DECLARE_BITMAP(enabled, SMU_FEATURE_MAX);
 };

 struct smu_clocks {
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index d36b64371492..d71155a66f97 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -798,27 +798,8 @@ int smu_v11_0_set_allowed_mask(struct smu_context *smu)
 int smu_v11_0_system_features_control(struct smu_context *smu,
  bool en)
 {
-   struct smu_feature *feature = >smu_feature;
-   uint64_t feature_mask;
-   int ret = 0;
-
-   ret = smu_cmn_send_smc_msg(smu, (en ? SMU_MSG_EnableAllSmuFeatures :
-SMU_MSG_DisableAllSmuFeatures), NULL);
-   if (ret)
-   return ret;
-
-   bitmap_zero(feature->enabled, feature->feature_num);
-
-   if (en) {
-   ret = smu_cmn_get_enabled_mask(smu, _mask);
-   if (ret)
-   return ret;
-
-   bitmap_copy(feature->enabled, (unsigned long *)_mask,
-   feature->feature_num);
-   }
-
-   return ret;
+   return smu_cmn_send_smc_msg(smu, (en ? SMU_MSG_EnableAllSmuFeatures :
+ SMU_MSG_DisableAllSmuFeatures), NULL);
 }

 int smu_v11_0_notify_display_change(struct smu_context *smu)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 478151e72889..96a5b31f708d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -1947,27 +1947,13 @@ static int vangogh_get_dpm_clock_table(struct 
smu_context *smu, struct dpm_clock
 static int vangogh_system_features_control(struct smu_context *smu, bool en)
 {
 struct amdgpu_device *adev = smu->adev;
-   struct smu_feature *feature = >smu_feature;
-   uint64_t feature_mask;
 int ret = 0;

 if (adev->pm.fw_version >= 0x43f1700 && !en)
 ret = smu_cmn_send_smc_msg_with_param(smu, 
SMU_MSG_RlcPowerNotify,
   RLC_STATUS_OFF, NULL);

-   bitmap_zero(feature->enabled, feature->feature_num);
-
-   if (!en)
-   return ret;
-
-   ret = smu_cmn_get_enabled_mask(smu, _mask);
-   if (ret)
-   return ret;
-
-   bitmap_copy(feature->enabled, (unsigned long *)_ma

[PATCH V3 5/7] drm/amd/pm: drop the cache for enabled ppfeatures

2022-01-27 Thread Evan Quan 
The following scenarios make the driver cache for enabled ppfeatures
outdated and invalid:
  - Other tools interact with PMFW to change the enabled ppfeatures.
  - PMFW may enable/disable some features behind driver's back. E.g.
for sienna_cichild, on gfxoff entering, PMFW will disable gfx
related DPM features. All those are performed without driver's
notice.
Also considering driver does not actually interact with PMFW such
frequently, the benefit brought by such cache is very limited.

Signed-off-by: Evan Quan 
Change-Id: I20ed58ab216e930c7a5d223be1eb99146889f2b3
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  1 -
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |  1 -
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c| 23 +-
 .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  | 16 +--
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 23 +-
 .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  | 16 +--
 drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 46 +--
 7 files changed, 17 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 803068cb5079..59be1c822b2c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -950,7 +950,6 @@ static int smu_sw_init(void *handle)
smu->pool_size = adev->pm.smu_prv_buffer_size;
smu->smu_feature.feature_num = SMU_FEATURE_MAX;
bitmap_zero(smu->smu_feature.supported, SMU_FEATURE_MAX);
-   bitmap_zero(smu->smu_feature.enabled, SMU_FEATURE_MAX);
bitmap_zero(smu->smu_feature.allowed, SMU_FEATURE_MAX);
 
mutex_init(>message_lock);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 8cd1c3bb595a..721b4080d3e6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -390,7 +390,6 @@ struct smu_feature
uint32_t feature_num;
DECLARE_BITMAP(supported, SMU_FEATURE_MAX);
DECLARE_BITMAP(allowed, SMU_FEATURE_MAX);
-   DECLARE_BITMAP(enabled, SMU_FEATURE_MAX);
 };
 
 struct smu_clocks {
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index d36b64371492..d71155a66f97 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -798,27 +798,8 @@ int smu_v11_0_set_allowed_mask(struct smu_context *smu)
 int smu_v11_0_system_features_control(struct smu_context *smu,
 bool en)
 {
-   struct smu_feature *feature = >smu_feature;
-   uint64_t feature_mask;
-   int ret = 0;
-
-   ret = smu_cmn_send_smc_msg(smu, (en ? SMU_MSG_EnableAllSmuFeatures :
-SMU_MSG_DisableAllSmuFeatures), NULL);
-   if (ret)
-   return ret;
-
-   bitmap_zero(feature->enabled, feature->feature_num);
-
-   if (en) {
-   ret = smu_cmn_get_enabled_mask(smu, _mask);
-   if (ret)
-   return ret;
-
-   bitmap_copy(feature->enabled, (unsigned long *)_mask,
-   feature->feature_num);
-   }
-
-   return ret;
+   return smu_cmn_send_smc_msg(smu, (en ? SMU_MSG_EnableAllSmuFeatures :
+ SMU_MSG_DisableAllSmuFeatures), NULL);
 }
 
 int smu_v11_0_notify_display_change(struct smu_context *smu)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 478151e72889..96a5b31f708d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -1947,27 +1947,13 @@ static int vangogh_get_dpm_clock_table(struct 
smu_context *smu, struct dpm_clock
 static int vangogh_system_features_control(struct smu_context *smu, bool en)
 {
struct amdgpu_device *adev = smu->adev;
-   struct smu_feature *feature = >smu_feature;
-   uint64_t feature_mask;
int ret = 0;
 
if (adev->pm.fw_version >= 0x43f1700 && !en)
ret = smu_cmn_send_smc_msg_with_param(smu, 
SMU_MSG_RlcPowerNotify,
  RLC_STATUS_OFF, NULL);
 
-   bitmap_zero(feature->enabled, feature->feature_num);
-
-   if (!en)
-   return ret;
-
-   ret = smu_cmn_get_enabled_mask(smu, _mask);
-   if (ret)
-   return ret;
-
-   bitmap_copy(feature->enabled, (unsigned long *)_mask,
-   feature->feature_num);
-
-   return 0;
+   return ret;
 }
 
 static int vangogh_post_smu_init(struct smu_context *smu)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 92b5c1108a2e..f0ab1dc3ca59 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++