[PATCH 1/1] amdgpu/pm: remove code duplication in show_power_cap calls

Wang, Kevin(Yang) Kevin1.Wang at amd.com
Wed Jun 23 06:46:54 UTC 2021


[AMD Official Use Only]

Reviewed-by: Kevin Wang <kevin1.wang at amd.com>

Best Regards,
Kevin

________________________________
From: Powell, Darren <Darren.Powell at amd.com>
Sent: Wednesday, June 23, 2021 10:34 AM
To: amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell at amd.com>
Subject: [PATCH 1/1] amdgpu/pm: remove code duplication in show_power_cap calls

v2: reorder to check pointers before calling pm_runtime_* functions

 created generic function and call with enum from
 * amdgpu_hwmon_show_power_cap_max
 * amdgpu_hwmon_show_power_cap
 * amdgpu_hwmon_show_power_cap_default

=== Test ===
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}

cp pp_show_power_cap.txt{,.old}
lspci -nn | grep "VGA\|Display" > pp_show_power_cap.test.log
FILES="
power1_cap
power1_cap_max
power1_cap_default "

for f in $FILES
do
  echo  $f = `cat $HWMON_DIR/$f` >> pp_show_power_cap.test.log
done

Signed-off-by: Darren Powell <darren.powell at amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c | 96 ++++++------------------------
 1 file changed, 19 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index b2335a1d3f98..fa63160e6405 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2901,14 +2901,14 @@ static ssize_t amdgpu_hwmon_show_power_cap_min(struct device *dev,
         return sprintf(buf, "%i\n", 0);
 }

-static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
-                                        struct device_attribute *attr,
-                                        char *buf)
+static ssize_t amdgpu_hwmon_show_power_cap_generic(struct device *dev,
+                                  struct device_attribute *attr,
+                                  char *buf,
+                                  enum pp_power_limit_level pp_limit_level)
 {
         struct amdgpu_device *adev = dev_get_drvdata(dev);
         const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
         enum pp_power_type power_type = to_sensor_dev_attr(attr)->index;
-       enum pp_power_limit_level pp_limit_level = PP_PWR_LIMIT_MAX;
         uint32_t limit;
         ssize_t size;
         int r;
@@ -2918,17 +2918,17 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
         if (adev->in_suspend && !adev->in_runpm)
                 return -EPERM;

+       if ( !(pp_funcs && pp_funcs->get_power_limit))
+               return -ENODATA;
+
         r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
         if (r < 0) {
                 pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
                 return r;
         }

-       if (pp_funcs && pp_funcs->get_power_limit)
-               r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
-                                             pp_limit_level, power_type);
-       else
-               r = -ENODATA;
+       r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+                                     pp_limit_level, power_type);

         if (!r)
                 size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
@@ -2941,85 +2941,27 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
         return size;
 }

-static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
+static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
                                          struct device_attribute *attr,
                                          char *buf)
 {
-       struct amdgpu_device *adev = dev_get_drvdata(dev);
-       const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
-       enum pp_power_type power_type = to_sensor_dev_attr(attr)->index;
-       enum pp_power_limit_level pp_limit_level = PP_PWR_LIMIT_CURRENT;
-       uint32_t limit;
-       ssize_t size;
-       int r;
-
-       if (amdgpu_in_reset(adev))
-               return -EPERM;
-       if (adev->in_suspend && !adev->in_runpm)
-               return -EPERM;
-
-       r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
-       if (r < 0) {
-               pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-               return r;
-       }
-
-       if (pp_funcs && pp_funcs->get_power_limit)
-               r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
-                                             pp_limit_level, power_type);
-       else
-               r = -ENODATA;
-
-       if (!r)
-               size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
-       else
-               size = snprintf(buf, PAGE_SIZE, "\n");
-
-       pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
-       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
+       return amdgpu_hwmon_show_power_cap_generic(dev, attr, buf, PP_PWR_LIMIT_MAX);
+}

-       return size;
+static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
+                                        struct device_attribute *attr,
+                                        char *buf)
+{
+       return amdgpu_hwmon_show_power_cap_generic(dev, attr, buf, PP_PWR_LIMIT_CURRENT);
 }

 static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
                                          struct device_attribute *attr,
                                          char *buf)
 {
-       struct amdgpu_device *adev = dev_get_drvdata(dev);
-       const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
-       enum pp_power_type power_type = to_sensor_dev_attr(attr)->index;
-       enum pp_power_limit_level pp_limit_level = PP_PWR_LIMIT_DEFAULT;
-       uint32_t limit;
-       ssize_t size;
-       int r;
-
-       if (amdgpu_in_reset(adev))
-               return -EPERM;
-       if (adev->in_suspend && !adev->in_runpm)
-               return -EPERM;
-
-       r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
-       if (r < 0) {
-               pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-               return r;
-       }
-
-       if (pp_funcs && pp_funcs->get_power_limit)
-               r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
-                                             pp_limit_level, power_type);
-       else
-               r = -ENODATA;
-
-       if (!r)
-               size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
-       else
-               size = snprintf(buf, PAGE_SIZE, "\n");
-
-       pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
-       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-
-       return size;
+       return amdgpu_hwmon_show_power_cap_generic(dev, attr, buf, PP_PWR_LIMIT_DEFAULT);
 }
+
 static ssize_t amdgpu_hwmon_show_power_label(struct device *dev,
                                          struct device_attribute *attr,
                                          char *buf)

base-commit: 1971ad5abcd8575efd12d2e6639fd8f58210a8d8
--
2.31.1

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210623/2de9eb09/attachment-0001.htm>


More information about the amd-gfx mailing list