On 2/13/23 21:33, Ashutosh Dixit wrote:
On ATSM the PL1 power limit is disabled at power up. The previous uapi
assumed that the PL1 limit is always enabled and therefore did not have a
notion of a disabled PL1 limit. This results in erroneous PL1 limit values
when PL1 limit is disabled. For example at power up, the disabled ATSM PL1
limit is shown as 0 which means a low PL1 limit whereas the limit being
disabled actually implies a high effective PL1 limit value.

To get round this problem, expose power1_max_enable as a custom hwmon
attribute. power1_max_enable can be used in conjunction with power1_max to
interpret power1_max (PL1 limit) values correctly. It can also be used to
enable/disable the PL1 power limit.

Signed-off-by: Ashutosh Dixit <ashutosh.di...@intel.com>
---
  .../ABI/testing/sysfs-driver-intel-i915-hwmon |  7 +++
  drivers/gpu/drm/i915/i915_hwmon.c             | 48 +++++++++++++++++--
  2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon 
b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 2d6a472eef885..edd94a44b4570 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -18,6 +18,13 @@ Description: RW. Card reactive sustained  (PL1/Tau) power 
limit in microwatts.
Only supported for particular Intel i915 graphics platforms. +What: /sys/devices/.../hwmon/hwmon<i>/power1_max_enable

This is not a standard hwmon attribute. The standard attribute would be 
power1_enable.

So from hwmon perspective this is a NACK.

Guenter

+Date:          May 2023
+KernelVersion: 6.3
+Contact:       intel-...@lists.freedesktop.org
+Description:   RW. Enable/disable the PL1 power limit (power1_max).
+
+               Only supported for particular Intel i915 graphics platforms.
  What:         /sys/devices/.../hwmon/hwmon<i>/power1_rated_max
  Date:         February 2023
  KernelVersion:        6.2
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c 
b/drivers/gpu/drm/i915/i915_hwmon.c
index 7c20a6f47b92e..5665869d8602b 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -230,13 +230,52 @@ hwm_power1_max_interval_store(struct device *dev,
                                            PKG_PWR_LIM_1_TIME, rxy);
        return count;
  }
+static SENSOR_DEVICE_ATTR_RW(power1_max_interval, hwm_power1_max_interval, 0);
-static SENSOR_DEVICE_ATTR(power1_max_interval, 0664,
-                         hwm_power1_max_interval_show,
-                         hwm_power1_max_interval_store, 0);
+static ssize_t
+hwm_power1_max_enable_show(struct device *dev, struct device_attribute *attr, 
char *buf)
+{
+       struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+       intel_wakeref_t wakeref;
+       u32 r;
+
+       with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
+               r = intel_uncore_read(ddat->uncore, 
ddat->hwmon->rg.pkg_rapl_limit);
+
+       return sysfs_emit(buf, "%u\n", !!(r & PKG_PWR_LIM_1_EN));
+}
+
+static ssize_t
+hwm_power1_max_enable_store(struct device *dev, struct device_attribute *attr,
+                           const char *buf, size_t count)
+{
+       struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+       intel_wakeref_t wakeref;
+       u32 en, r;
+       bool _en;
+       int ret;
+
+       ret = kstrtobool(buf, &_en);
+       if (ret)
+               return ret;
+
+       en = REG_FIELD_PREP(PKG_PWR_LIM_1_EN, _en);
+       hwm_locked_with_pm_intel_uncore_rmw(ddat, 
ddat->hwmon->rg.pkg_rapl_limit,
+                                           PKG_PWR_LIM_1_EN, en);
+
+       /* Verify, because PL1 limit cannot be disabled on all platforms */
+       with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
+               r = intel_uncore_read(ddat->uncore, 
ddat->hwmon->rg.pkg_rapl_limit);
+       if ((r & PKG_PWR_LIM_1_EN) != en)
+               return -EPERM;
+
+       return count;
+}
+static SENSOR_DEVICE_ATTR_RW(power1_max_enable, hwm_power1_max_enable, 0);
static struct attribute *hwm_attributes[] = {
        &sensor_dev_attr_power1_max_interval.dev_attr.attr,
+       &sensor_dev_attr_power1_max_enable.dev_attr.attr,
        NULL
  };
@@ -247,7 +286,8 @@ static umode_t hwm_attributes_visible(struct kobject *kobj,
        struct hwm_drvdata *ddat = dev_get_drvdata(dev);
        struct i915_hwmon *hwmon = ddat->hwmon;
- if (attr == &sensor_dev_attr_power1_max_interval.dev_attr.attr)
+       if (attr == &sensor_dev_attr_power1_max_interval.dev_attr.attr ||
+           attr == &sensor_dev_attr_power1_max_enable.dev_attr.attr)
                return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 
attr->mode : 0;
return 0;

Reply via email to