[PATCH v4 01/10] drm/amd/pm: Add support for DPM policies

Kamal, Asad Asad.Kamal at amd.com
Wed May 15 07:57:18 UTC 2024


[AMD Official Use Only - AMD Internal Distribution Only]

Series is
Reviewed-by: Asad Kamal <asad.kamal at amd.com>

Thanks & Regards
Asad

-----Original Message-----
From: Wang, Yang(Kevin) <KevinYang.Wang at amd.com>
Sent: Wednesday, May 15, 2024 1:10 PM
To: Lazar, Lijo <Lijo.Lazar at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; Kamal, Asad <Asad.Kamal at amd.com>; Ma, Le <Le.Ma at amd.com>
Subject: RE: [PATCH v4 01/10] drm/amd/pm: Add support for DPM policies

[AMD Official Use Only - AMD Internal Distribution Only]

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Lijo Lazar
Sent: Tuesday, May 14, 2024 7:06 PM
To: amd-gfx at lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; Kamal, Asad <Asad.Kamal at amd.com>; Ma, Le <Le.Ma at amd.com>
Subject: [PATCH v4 01/10] drm/amd/pm: Add support for DPM policies

Add support to set/get information about different DPM policies. The support is only available on SOCs which use swsmu architecture.

A DPM policy type may be defined with different levels. For example, a policy may be defined to select Pstate preference and then later a pstate preference may be chosen.

Signed-off-by: Lijo Lazar <lijo.lazar at amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang at amd.com>
---
v2: Add NULL checks before accessing smu_dpm_policy_ctxt
v3: Rebase to add device_attr_id__pm_policy
v4: Use macro to define policy type for consistency.

 .../gpu/drm/amd/include/kgd_pp_interface.h    | 16 +++
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c           | 29 ++++++
 drivers/gpu/drm/amd/pm/amdgpu_pm.c            | 94 ++++++++++++++++++
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       |  4 +
 drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h        |  1 +
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 98 +++++++++++++++++++
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 31 ++++++
 7 files changed, 273 insertions(+)

diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 805c9d37a2b4..8ed9aa9a990d 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -273,6 +273,22 @@ enum pp_xgmi_plpd_mode {
        XGMI_PLPD_COUNT,
 };

+enum pp_pm_policy {
+       PP_PM_POLICY_NONE = -1,
+       PP_PM_POLICY_SOC_PSTATE = 0,
+       PP_PM_POLICY_NUM,
+};
+
+enum pp_policy_soc_pstate {
+       SOC_PSTATE_DEFAULT = 0,
+       SOC_PSTATE_0,
+       SOC_PSTATE_1,
+       SOC_PSTATE_2,
+       SOC_PSTAT_COUNT,
+};
+
+#define PP_POLICY_MAX_LEVELS 5
+
 #define PP_GROUP_MASK        0xF0000000
 #define PP_GROUP_SHIFT       28

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index eee919577b44..b443906484e7 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -411,6 +411,35 @@ int amdgpu_dpm_set_xgmi_plpd_mode(struct amdgpu_device *adev, int mode)
        return ret;
 }

+ssize_t amdgpu_dpm_get_pm_policy_info(struct amdgpu_device *adev, char
+*buf) {
+       struct smu_context *smu = adev->powerplay.pp_handle;
+       int ret = -EOPNOTSUPP;
+
+       if (is_support_sw_smu(adev)) {
+               mutex_lock(&adev->pm.mutex);
+               ret = smu_get_pm_policy_info(smu, buf);
+               mutex_unlock(&adev->pm.mutex);
+       }
+
+       return ret;
+}
+
+int amdgpu_dpm_set_pm_policy(struct amdgpu_device *adev, int policy_type,
+                            int policy_level) {
+       struct smu_context *smu = adev->powerplay.pp_handle;
+       int ret = -EOPNOTSUPP;
+
+       if (is_support_sw_smu(adev)) {
+               mutex_lock(&adev->pm.mutex);
+               ret = smu_set_pm_policy(smu, policy_type, policy_level);
+               mutex_unlock(&adev->pm.mutex);
+       }
+
+       return ret;
+}
+
 int amdgpu_dpm_enable_mgpu_fan_boost(struct amdgpu_device *adev)  {
        void *pp_handle = adev->powerplay.pp_handle; diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 110f2fc31754..6dab0b085239 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2278,6 +2278,98 @@ static ssize_t amdgpu_set_xgmi_plpd_policy(struct device *dev,
        return count;
 }

+static ssize_t amdgpu_get_pm_policy(struct device *dev,
+                                   struct device_attribute *attr, char *buf) {
+       struct drm_device *ddev = dev_get_drvdata(dev);
+       struct amdgpu_device *adev = drm_to_adev(ddev);
+
+       if (amdgpu_in_reset(adev))
+               return -EPERM;
+       if (adev->in_suspend && !adev->in_runpm)
+               return -EPERM;
+
+       return amdgpu_dpm_get_pm_policy_info(adev, buf); }
+
+#define STR_SOC_PSTATE_POLICY "soc_pstate"
[Kevin]:

Better to move above macro to top of file.

Best Regards,
Kevin
+
+static ssize_t amdgpu_set_pm_policy(struct device *dev,
+                                   struct device_attribute *attr,
+                                   const char *buf, size_t count) {
+       struct drm_device *ddev = dev_get_drvdata(dev);
+       struct amdgpu_device *adev = drm_to_adev(ddev);
+       int policy_type, ret, num_params = 0;
+       char delimiter[] = " \n\t";
+       char tmp_buf[128];
+       char *tmp, *param;
+       long val;
+
+       if (amdgpu_in_reset(adev))
+               return -EPERM;
+       if (adev->in_suspend && !adev->in_runpm)
+               return -EPERM;
+
+       count = min(count, sizeof(tmp_buf));
+       memcpy(tmp_buf, buf, count);
+       tmp_buf[count - 1] = '\0';
+       tmp = tmp_buf;
+
+       tmp = skip_spaces(tmp);
+       if (strncmp(tmp, STR_SOC_PSTATE_POLICY, strlen(STR_SOC_PSTATE_POLICY)) == 0) {
+               policy_type = PP_PM_POLICY_SOC_PSTATE;
+               tmp += strlen(STR_SOC_PSTATE_POLICY);
+       } else {
+               return -EINVAL;
+       }
+
+       tmp = skip_spaces(tmp);
+       while ((param = strsep(&tmp, delimiter))) {
+               if (!strlen(param)) {
+                       tmp = skip_spaces(tmp);
+                       continue;
+               }
+               ret = kstrtol(param, 0, &val);
+               if (ret)
+                       return -EINVAL;
+               num_params++;
+               if (num_params > 1)
+                       return -EINVAL;
+       }
+
+       if (num_params != 1)
+               return -EINVAL;
+
+       ret = pm_runtime_get_sync(ddev->dev);
+       if (ret < 0) {
+               pm_runtime_put_autosuspend(ddev->dev);
+               return ret;
+       }
+
+       ret = amdgpu_dpm_set_pm_policy(adev, policy_type, val);
+
+       pm_runtime_mark_last_busy(ddev->dev);
+       pm_runtime_put_autosuspend(ddev->dev);
+
+       if (ret)
+               return ret;
+
+       return count;
+}
+
+static int amdgpu_pm_policy_attr_update(struct amdgpu_device *adev,
+                                        struct amdgpu_device_attr *attr,
+                                        uint32_t mask,
+                                        enum amdgpu_device_attr_states *states) {
+       if (amdgpu_dpm_get_pm_policy_info(adev, NULL) == -EOPNOTSUPP)
+               *states = ATTR_STATE_UNSUPPORTED;
+
+       return 0;
+}
+
+
 static struct amdgpu_device_attr amdgpu_device_attrs[] = {
        AMDGPU_DEVICE_ATTR_RW(power_dpm_state,                          ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
        AMDGPU_DEVICE_ATTR_RW(power_dpm_force_performance_level,        ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
@@ -2326,6 +2418,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[] = {
        AMDGPU_DEVICE_ATTR_RW(smartshift_bias,                          ATTR_FLAG_BASIC,
                              .attr_update = ss_bias_attr_update),
        AMDGPU_DEVICE_ATTR_RW(xgmi_plpd_policy,                         ATTR_FLAG_BASIC),
+       AMDGPU_DEVICE_ATTR_RW(pm_policy,                                ATTR_FLAG_BASIC,
+                             .attr_update =
+ amdgpu_pm_policy_attr_update),
        AMDGPU_DEVICE_ATTR_RO(pm_metrics,                               ATTR_FLAG_BASIC,
                              .attr_update = amdgpu_pm_metrics_attr_update),  }; diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 501f8c726e8d..1455db9c3789 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -598,4 +598,8 @@ enum pp_smu_status amdgpu_dpm_get_uclk_dpm_states(struct amdgpu_device *adev,
                                                  unsigned int *num_states);  int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
                                   struct dpm_clocks *clock_table);
+int amdgpu_dpm_set_pm_policy(struct amdgpu_device *adev, int policy_type,
+                            int policy_level); ssize_t
+amdgpu_dpm_get_pm_policy_info(struct amdgpu_device *adev, char *buf);
+
 #endif
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
index 448ba3a14584..6ec9fca045e0 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
@@ -79,6 +79,7 @@ enum amdgpu_device_attr_id {
        device_attr_id__smartshift_bias,
        device_attr_id__xgmi_plpd_policy,
        device_attr_id__pm_metrics,
+       device_attr_id__pm_policy,
        device_attr_id__count,
 };

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index e61aa4418d44..df9ff377ebfd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -3498,6 +3498,104 @@ static int smu_get_prv_buffer_details(void *handle, void **addr, size_t *size)
        return 0;
 }

+static void smu_print_dpm_policy(struct smu_dpm_policy *policy, char *sysbuf,
+                                size_t *size) {
+       size_t offset = *size;
+       int level;
+
+       offset += sysfs_emit_at(sysbuf, offset, "%s \n", policy->desc->name);
+       for_each_set_bit(level, &policy->level_mask, PP_POLICY_MAX_LEVELS) {
+               if (level == policy->current_level)
+                       offset += sysfs_emit_at(
+                               sysbuf, offset, "%d : %s*\n", level,
+                               policy->desc->get_desc(policy, level));
+               else
+                       offset += sysfs_emit_at(
+                               sysbuf, offset, "%d : %s\n", level,
+                               policy->desc->get_desc(policy, level));
+       }
[Kevin]:

It is better to add 'offset' check to avoid buffer overflow.
After fixes, the patch Series is,

Reviewed-by: Yang Wang <kevinyang.wang at amd.com>

Best Regards,
Kevin
+
+       *size = offset;
+}
+
+ssize_t smu_get_pm_policy_info(struct smu_context *smu, char *sysbuf) {
+       struct smu_dpm_context *dpm_ctxt = &(smu->smu_dpm);
+       struct smu_dpm_policy_ctxt *policy_ctxt;
+       struct smu_dpm_policy *dpm_policy;
+       size_t offset = 0;
+       int i;
+
+       policy_ctxt = dpm_ctxt->dpm_policies;
+       if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled || !policy_ctxt ||
+           !policy_ctxt->policy_mask)
+               return -EOPNOTSUPP;
+
+       if (!sysbuf)
+               return -EINVAL;
+
+       for_each_set_bit(i, &policy_ctxt->policy_mask, PP_PM_POLICY_NUM) {
+               dpm_policy = &policy_ctxt->policies[i];
+               if (!dpm_policy->level_mask || !dpm_policy->desc)
+                       continue;
+               smu_print_dpm_policy(dpm_policy, sysbuf, &offset);
+       }
+
+       return offset;
+}
+
+struct smu_dpm_policy *smu_get_pm_policy(struct smu_context *smu,
+                                        enum pp_pm_policy p_type) {
+       struct smu_dpm_context *dpm_ctxt = &(smu->smu_dpm);
+       struct smu_dpm_policy_ctxt *policy_ctxt;
+       int i;
+
+       policy_ctxt = dpm_ctxt->dpm_policies;
+       if (!policy_ctxt)
+               return NULL;
+
+       for_each_set_bit(i, &policy_ctxt->policy_mask, PP_PM_POLICY_NUM) {
+               if (policy_ctxt->policies[i].policy_type == p_type)
+                       return &policy_ctxt->policies[i];
+       }
+
+       return NULL;
+}
+
+int smu_set_pm_policy(struct smu_context *smu, enum pp_pm_policy p_type,
+                     int level)
+{
+       struct smu_dpm_context *dpm_ctxt = &(smu->smu_dpm);
+       struct smu_dpm_policy *dpm_policy = NULL;
+       struct smu_dpm_policy_ctxt *policy_ctxt;
+       int ret = -EOPNOTSUPP;
+
+       policy_ctxt = dpm_ctxt->dpm_policies;
+       if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled || !policy_ctxt ||
+           !policy_ctxt->policy_mask)
+               return ret;
+
+       if (level < 0 || level >= PP_POLICY_MAX_LEVELS)
+               return -EINVAL;
+
+       dpm_policy = smu_get_pm_policy(smu, p_type);
+
+       if (!dpm_policy || !dpm_policy->level_mask || !dpm_policy->set_policy)
+               return ret;
+
+       if (dpm_policy->current_level == level)
+               return 0;
+
+       ret = dpm_policy->set_policy(smu, level);
+
+       if (!ret)
+               dpm_policy->current_level = level;
+
+       return ret;
+}
+
 int smu_set_xgmi_plpd_mode(struct smu_context *smu,
                           enum pp_xgmi_plpd_mode mode)  { 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 0917dec8efe3..ee5b9701038c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -362,6 +362,27 @@ struct smu_table_context {
        void                            *gpu_metrics_table;
 };

+struct smu_context;
+struct smu_dpm_policy;
+
+struct smu_dpm_policy_desc {
+       const char *name;
+       char *(*get_desc)(struct smu_dpm_policy *dpm_policy, int level);
+};
+
+struct smu_dpm_policy {
+       struct smu_dpm_policy_desc *desc;
+       enum pp_pm_policy policy_type;
+       unsigned long level_mask;
+       int current_level;
+       int (*set_policy)(struct smu_context *ctxt, int level); };
+
+struct smu_dpm_policy_ctxt{
+       struct smu_dpm_policy policies[PP_PM_POLICY_NUM];
+       unsigned long policy_mask;
+};
+
 struct smu_dpm_context {
        uint32_t dpm_context_size;
        void *dpm_context;
@@ -372,6 +393,7 @@ struct smu_dpm_context {
        struct smu_power_state *dpm_request_power_state;
        struct smu_power_state *dpm_current_power_state;
        struct mclock_latency_table *mclk_latency_table;
+       struct smu_dpm_policy_ctxt *dpm_policies;
 };

 struct smu_power_gate {
@@ -1551,6 +1573,11 @@ typedef struct {
        uint32_t                MmHubPadding[8];
 } WifiBandEntryTable_t;

+#define STR_SOC_PSTATE_POLICY "soc_pstate"
+
+struct smu_dpm_policy *smu_get_pm_policy(struct smu_context *smu,
+                                        enum pp_pm_policy p_type);
+
 #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4)  int smu_get_power_limit(void *handle,
                        uint32_t *limit, @@ -1598,5 +1625,9 @@ void amdgpu_smu_stb_debug_fs_init(struct amdgpu_device *adev);  int smu_send_hbm_bad_pages_num(struct smu_context *smu, uint32_t size);  int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size);  int smu_send_rma_reason(struct smu_context *smu);
+int smu_set_pm_policy(struct smu_context *smu, enum pp_pm_policy p_type,
+                     int level);
+ssize_t smu_get_pm_policy_info(struct smu_context *smu, char *sysbuf);
+
 #endif
 #endif
--
2.25.1




More information about the amd-gfx mailing list