* [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support
2022-05-23 11:08 [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support Badal Nilawar
@ 2022-05-23 11:08 ` Badal Nilawar
2022-05-23 13:45 ` Jani Nikula
` (2 more replies)
2022-05-23 11:08 ` [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support Badal Nilawar
` (4 subsequent siblings)
5 siblings, 3 replies; 18+ messages in thread
From: Badal Nilawar @ 2022-05-23 11:08 UTC (permalink / raw)
To: intel-gfx
From: Dale B Stimson <dale.b.stimson@intel.com>
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 19 +
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_driver.c | 5 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_hwmon.c | 410 ++++++++++++++++++
drivers/gpu/drm/i915/i915_hwmon.h | 44 ++
drivers/gpu/drm/i915/i915_reg.h | 15 +
drivers/gpu/drm/i915/intel_mchbar_regs.h | 9 +
9 files changed, 506 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
new file mode 100644
index 000000000000..c680dd842358
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -0,0 +1,19 @@
+What: /sys/devices/.../hwmon/hwmon<i>/power1_max
+Date: June 2021
+KernelVersion: 5.14
+Contact: dri-devel@lists.freedesktop.org
+Description: RW. Card reactive sustained (PL1/Tau) power limit in microwatts.
+
+ The power controller will throttle the operating frequency
+ if the power averaged over a window (typically seconds)
+ exceeds this limit.
+
+ Only supported for particular Intel i915 graphics platforms.
+
+What: /sys/devices/.../hwmon/hwmon<i>/power1_max_default
+Date: June 2021
+KernelVersion: 5.14
+Contact: dri-devel@lists.freedesktop.org
+Description: RO. Card default power limit (default TDP setting).
+
+ Only supported for particular Intel i915 graphics platforms.
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 7ae3b7d67fcf..c60cfacd4c47 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -19,6 +19,7 @@ config DRM_I915
select DRM_MIPI_DSI
select RELAY
select IRQ_WORK
+ select HWMON
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_CLASS_DEVICE if ACPI
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index d2b18f03a33c..e1d5912ea879 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -36,6 +36,7 @@ i915-y += i915_driver.o \
i915_drm_client.o \
i915_config.o \
i915_getparam.o \
+ i915_hwmon.o \
i915_ioctl.o \
i915_irq.o \
i915_mitigations.o \
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index b47746152d97..6f678587a771 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -80,6 +80,7 @@
#include "i915_drm_client.h"
#include "i915_drv.h"
#include "i915_getparam.h"
+#include "i915_hwmon.h"
#include "i915_ioc32.h"
#include "i915_ioctl.h"
#include "i915_irq.h"
@@ -702,6 +703,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
intel_gt_driver_register(to_gt(dev_priv));
+ i915_hwmon_register(dev_priv);
+
intel_display_driver_register(dev_priv);
intel_power_domains_enable(dev_priv);
@@ -728,6 +731,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
intel_display_driver_unregister(dev_priv);
+ i915_hwmon_unregister(dev_priv);
+
intel_gt_driver_unregister(to_gt(dev_priv));
i915_perf_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c5fc402d9c50..e16bedba2d84 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -769,6 +769,8 @@ struct drm_i915_private {
struct i915_perf perf;
+ struct i915_hwmon *hwmon;
+
/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
struct intel_gt gt0;
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
new file mode 100644
index 000000000000..b94c11f2517f
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+/*
+ * Power-related hwmon entries.
+ */
+
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/types.h>
+
+#include "i915_drv.h"
+#include "i915_hwmon.h"
+#include "intel_mchbar_regs.h"
+
+/*
+ * SF_* - scale factors for particular quantities according to hwmon spec.
+ * - power - microwatts
+ */
+#define SF_POWER 1000000
+
+#define FIELD_SHIFT(__mask) \
+ (BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
+ BUILD_BUG_ON_ZERO((__mask) == 0) + \
+ __bf_shf(__mask))
+
+static void
+_locked_with_pm_intel_uncore_rmw(struct i915_hwmon_drvdata *ddat,
+ i915_reg_t reg, u32 clear, u32 set)
+{
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ struct intel_uncore *uncore = ddat->dd_uncore;
+ intel_wakeref_t wakeref;
+
+ mutex_lock(&hwmon->hwmon_lock);
+
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+ intel_uncore_rmw(uncore, reg, clear, set);
+
+ mutex_unlock(&hwmon->hwmon_lock);
+}
+
+static u64
+_scale_and_shift(u32 in, u32 scale_factor, int nshift)
+{
+ u64 out = mul_u32_u32(scale_factor, in);
+
+ /* Shift, rounding to nearest */
+ if (nshift > 0)
+ out = (out + (1 << (nshift - 1))) >> nshift;
+ return out;
+}
+
+/*
+ * This function's return type of u64 allows for the case where the scaling
+ * of the field taken from the 32-bit register value might cause a result to
+ * exceed 32 bits.
+ */
+static u64
+_field_read_and_scale(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
+ u32 field_msk, int field_shift,
+ int nshift, u32 scale_factor)
+{
+ struct intel_uncore *uncore = ddat->dd_uncore;
+ intel_wakeref_t wakeref;
+ u32 reg_value;
+
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+ reg_value = intel_uncore_read(uncore, rgadr);
+
+ reg_value = (reg_value & field_msk) >> field_shift;
+
+ return _scale_and_shift(reg_value, scale_factor, nshift);
+}
+
+static void
+_field_scale_and_write(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
+ u32 field_msk, int field_shift,
+ int nshift, unsigned int scale_factor, long lval)
+{
+ u32 nval;
+ u32 bits_to_clear;
+ u32 bits_to_set;
+
+ /* Computation in 64-bits to avoid overflow. Round to nearest. */
+ nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
+
+ bits_to_clear = field_msk;
+ bits_to_set = (nval << field_shift) & field_msk;
+
+ _locked_with_pm_intel_uncore_rmw(ddat, rgadr,
+ bits_to_clear, bits_to_set);
+}
+
+static ssize_t
+i915_power1_max_default_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ u64 val = 0; /* uapi specifies to keep visible but return 0 if unsupported */
+
+ if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku))
+ val = _field_read_and_scale(ddat,
+ hwmon->rg.pkg_power_sku,
+ PKG_PKG_TDP,
+ FIELD_SHIFT(PKG_PKG_TDP),
+ hwmon->scl_shift_power,
+ SF_POWER);
+ return sysfs_emit(buf, "%llu\n", val);
+}
+
+static SENSOR_DEVICE_ATTR(power1_max_default, 0444,
+ i915_power1_max_default_show, NULL, 0);
+
+static struct attribute *hwmon_attributes[] = {
+ &sensor_dev_attr_power1_max_default.dev_attr.attr,
+ NULL
+};
+
+static umode_t hwmon_attributes_visible(struct kobject *kobj,
+ struct attribute *attr, int index)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
+ struct drm_i915_private *i915 = ddat->dd_uncore->i915;
+ i915_reg_t rgadr;
+
+ if (attr == &sensor_dev_attr_power1_max_default.dev_attr.attr)
+ return IS_DGFX(i915) ? attr->mode : 0;
+ else
+ return 0;
+
+ if (!i915_mmio_reg_valid(rgadr))
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group hwmon_attrgroup = {
+ .attrs = hwmon_attributes,
+ .is_visible = hwmon_attributes_visible,
+};
+
+static const struct attribute_group *hwmon_groups[] = {
+ &hwmon_attrgroup,
+ NULL
+};
+
+/*
+ * HWMON SENSOR TYPE = hwmon_power
+ * - Sustained Power (power1_max)
+ */
+static const u32 i915_config_power[] = {
+ HWMON_P_MAX,
+ 0
+};
+
+static const struct hwmon_channel_info i915_power = {
+ .type = hwmon_power,
+ .config = i915_config_power,
+};
+
+static const struct hwmon_channel_info *i915_info[] = {
+ &i915_power,
+ NULL
+};
+
+static umode_t
+i915_power_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr, int chan)
+{
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ i915_reg_t rgadr;
+
+ switch (attr) {
+ case hwmon_power_max:
+ rgadr = hwmon->rg.pkg_rapl_limit;
+ break;
+ default:
+ return 0;
+ }
+
+ if (!i915_mmio_reg_valid(rgadr))
+ return 0;
+
+ return 0664;
+}
+
+static int
+i915_power_read(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long *val)
+{
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ int ret = 0;
+
+ switch (attr) {
+ case hwmon_power_max:
+ *val = _field_read_and_scale(ddat,
+ hwmon->rg.pkg_rapl_limit,
+ PKG_PWR_LIM_1,
+ FIELD_SHIFT(PKG_PWR_LIM_1),
+ hwmon->scl_shift_power,
+ SF_POWER);
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ }
+
+ return ret;
+}
+
+static int
+i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
+{
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ int ret = 0;
+
+ switch (attr) {
+ case hwmon_power_max:
+ _field_scale_and_write(ddat,
+ hwmon->rg.pkg_rapl_limit,
+ PKG_PWR_LIM_1,
+ FIELD_SHIFT(PKG_PWR_LIM_1),
+ hwmon->scl_shift_power,
+ SF_POWER, val);
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ }
+
+ return ret;
+}
+
+static umode_t
+i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ struct i915_hwmon_drvdata *ddat = (struct i915_hwmon_drvdata *)drvdata;
+
+ switch (type) {
+ case hwmon_power:
+ return i915_power_is_visible(ddat, attr, channel);
+ default:
+ return 0;
+ }
+}
+
+static int
+i915_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long *val)
+{
+ struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
+
+ switch (type) {
+ case hwmon_power:
+ return i915_power_read(ddat, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+i915_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long val)
+{
+ struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
+
+ switch (type) {
+ case hwmon_power:
+ return i915_power_write(ddat, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static const struct hwmon_ops i915_hwmon_ops = {
+ .is_visible = i915_is_visible,
+ .read = i915_read,
+ .write = i915_write,
+};
+
+static const struct hwmon_chip_info i915_chip_info = {
+ .ops = &i915_hwmon_ops,
+ .info = i915_info,
+};
+
+static void
+i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
+{
+ struct i915_hwmon *hwmon = i915->hwmon;
+ struct intel_uncore *uncore = &i915->uncore;
+ struct i915_hwmon_drvdata *ddat = &hwmon->ddat;
+ intel_wakeref_t wakeref;
+ u32 val_sku_unit;
+ __le32 le_sku_unit;
+
+ if (IS_DG1(i915) || IS_DG2(i915)) {
+ hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
+ hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
+ hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
+ } else {
+ hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
+ hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
+ hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
+ }
+
+ wakeref = intel_runtime_pm_get(uncore->rpm);
+
+ /*
+ * The contents of register hwmon->rg.pkg_power_sku_unit do not change,
+ * so read it once and store the shift values.
+ *
+ * For some platforms, this value is defined as available "for all
+ * tiles", with the values consistent across all tiles.
+ * In this case, use the tile 0 value for all.
+ */
+ if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit))
+ val_sku_unit = intel_uncore_read(uncore,
+ hwmon->rg.pkg_power_sku_unit);
+ else
+ val_sku_unit = 0;
+
+ intel_runtime_pm_put(uncore->rpm, wakeref);
+
+ le_sku_unit = cpu_to_le32(val_sku_unit);
+ hwmon->scl_shift_power = le32_get_bits(le_sku_unit, PKG_PWR_UNIT);
+
+ /*
+ * The value of power1_max is reset to the default on reboot, but is
+ * not reset by a module unload/load sequence. To allow proper
+ * functioning after a module reload, the value for power1_max is
+ * restored to its original value at module unload time in
+ * i915_hwmon_unregister().
+ */
+ hwmon->power_max_initial_value =
+ (u32)_field_read_and_scale(ddat,
+ hwmon->rg.pkg_rapl_limit,
+ PKG_PWR_LIM_1,
+ FIELD_SHIFT(PKG_PWR_LIM_1),
+ hwmon->scl_shift_power, SF_POWER);
+}
+
+void i915_hwmon_register(struct drm_i915_private *i915)
+{
+ struct device *dev = i915->drm.dev;
+ struct i915_hwmon *hwmon;
+ struct device *hwmon_dev;
+ struct i915_hwmon_drvdata *ddat;
+
+ hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
+ if (!hwmon)
+ return;
+
+ i915->hwmon = hwmon;
+
+ mutex_init(&hwmon->hwmon_lock);
+
+ ddat = &hwmon->ddat;
+
+ ddat->dd_hwmon = hwmon;
+ ddat->dd_uncore = &i915->uncore;
+ snprintf(ddat->dd_name, sizeof(ddat->dd_name), "i915");
+
+ i915_hwmon_get_preregistration_info(i915);
+
+ /* hwmon_dev points to device hwmon<i> */
+ hwmon_dev = hwmon_device_register_with_info(dev, ddat->dd_name,
+ ddat,
+ &i915_chip_info,
+ hwmon_groups);
+
+ if (IS_ERR(hwmon_dev)) {
+ mutex_destroy(&hwmon->hwmon_lock);
+ i915->hwmon = NULL;
+ kfree(hwmon);
+ return;
+ }
+
+ ddat->dd_hwmon_dev = hwmon_dev;
+}
+
+void i915_hwmon_unregister(struct drm_i915_private *i915)
+{
+ struct i915_hwmon *hwmon;
+ struct i915_hwmon_drvdata *ddat;
+
+ hwmon = fetch_and_zero(&i915->hwmon);
+ if (!hwmon)
+ return;
+
+ ddat = &hwmon->ddat;
+
+ if (hwmon->power_max_initial_value) {
+ /* Restore power1_max. */
+ _field_scale_and_write(ddat, hwmon->rg.pkg_rapl_limit,
+ PKG_PWR_LIM_1,
+ FIELD_SHIFT(PKG_PWR_LIM_1),
+ hwmon->scl_shift_power,
+ SF_POWER,
+ hwmon->power_max_initial_value);
+ }
+
+ if (ddat->dd_hwmon_dev)
+ hwmon_device_unregister(ddat->dd_hwmon_dev);
+
+ mutex_destroy(&hwmon->hwmon_lock);
+
+ kfree(hwmon);
+}
diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
new file mode 100644
index 000000000000..396b758aefce
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef __INTEL_HWMON_H__
+#define __INTEL_HWMON_H__
+
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+#include "i915_reg.h"
+
+struct drm_i915_private;
+
+struct i915_hwmon_reg {
+ i915_reg_t pkg_power_sku_unit;
+ i915_reg_t pkg_power_sku;
+ i915_reg_t pkg_rapl_limit;
+};
+
+struct i915_hwmon_drvdata {
+ struct i915_hwmon *dd_hwmon;
+ struct intel_uncore *dd_uncore;
+ struct device *dd_hwmon_dev;
+ char dd_name[12];
+};
+
+struct i915_hwmon {
+ struct i915_hwmon_drvdata ddat;
+
+ struct mutex hwmon_lock; /* counter overflow logic and rmw */
+
+ struct i915_hwmon_reg rg;
+
+ u32 power_max_initial_value;
+
+ int scl_shift_power;
+};
+
+void i915_hwmon_register(struct drm_i915_private *i915);
+void i915_hwmon_unregister(struct drm_i915_private *i915);
+#endif
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d8579ab9384c..1c570c706ff8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1866,6 +1866,21 @@
#define POWER_LIMIT_4_MASK REG_BIT(9)
#define POWER_LIMIT_1_MASK REG_BIT(11)
#define POWER_LIMIT_2_MASK REG_BIT(12)
+/*
+ * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
+ * Used herein as a 64-bit register.
+ * These masks are defined using GENMASK_ULL as REG_GENMASK is limited to u32
+ * and as GENMASK is "long" and therefore 32-bits on a 32-bit system.
+ * PKG_PKG_TDP, PKG_MIN_PWR, and PKG_MAX_PWR are scaled in the same way as
+ * PKG_PWR_LIM_*, above.
+ * PKG_MAX_WIN has sub-fields for x and y, and has the value: is 1.x * 2^y.
+ */
+#define PKG_PKG_TDP GENMASK_ULL(14, 0)
+#define PKG_MIN_PWR GENMASK_ULL(30, 16)
+#define PKG_MAX_PWR GENMASK_ULL(46, 32)
+#define PKG_MAX_WIN GENMASK_ULL(54, 48)
+#define PKG_MAX_WIN_Y GENMASK_ULL(54, 53)
+#define PKG_MAX_WIN_X GENMASK_ULL(52, 48)
#define CHV_CLK_CTL1 _MMIO(0x101100)
#define VLV_CLK_CTL2 _MMIO(0x101104)
diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
index 2aad2f0cc8db..247561d7974c 100644
--- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
+++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
@@ -191,11 +191,20 @@
#define GEN6_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
#define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
+#define PCU_PACKAGE_POWER_SKU_UNIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
+#define PKG_PWR_UNIT REG_GENMASK(3, 0)
+#define PKG_TIME_UNIT REG_GENMASK(19, 16)
+
#define GEN6_RP_STATE_CAP _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
#define RP0_CAP_MASK REG_GENMASK(7, 0)
#define RP1_CAP_MASK REG_GENMASK(15, 8)
#define RPN_CAP_MASK REG_GENMASK(23, 16)
+#define PCU_PACKAGE_RAPL_LIMIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
+#define PKG_PWR_LIM_1 REG_GENMASK(14, 0)
+#define PKG_PWR_LIM_1_EN REG_BIT(15)
+#define PKG_PWR_LIM_1_TIME REG_GENMASK(23, 17)
+
/* snb MCH registers for priority tuning */
#define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
#define SSKPD_NEW_WM0_MASK_HSW REG_GENMASK64(63, 56)
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support Badal Nilawar
@ 2022-05-23 13:45 ` Jani Nikula
2022-05-23 13:46 ` Jani Nikula
2022-05-24 3:10 ` Dixit, Ashutosh
2 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2022-05-23 13:45 UTC (permalink / raw)
To: Badal Nilawar, intel-gfx
On Mon, 23 May 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> As part of the System Managemenent Interface (SMI), use the HWMON
> subsystem to display power utilization.
>
> Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 19 +
> drivers/gpu/drm/i915/Kconfig | 1 +
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_driver.c | 5 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_hwmon.c | 410 ++++++++++++++++++
> drivers/gpu/drm/i915/i915_hwmon.h | 44 ++
> drivers/gpu/drm/i915/i915_reg.h | 15 +
> drivers/gpu/drm/i915/intel_mchbar_regs.h | 9 +
> 9 files changed, 506 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> new file mode 100644
> index 000000000000..c680dd842358
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -0,0 +1,19 @@
> +What: /sys/devices/.../hwmon/hwmon<i>/power1_max
> +Date: June 2021
> +KernelVersion: 5.14
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RW. Card reactive sustained (PL1/Tau) power limit in microwatts.
> +
> + The power controller will throttle the operating frequency
> + if the power averaged over a window (typically seconds)
> + exceeds this limit.
> +
> + Only supported for particular Intel i915 graphics platforms.
> +
> +What: /sys/devices/.../hwmon/hwmon<i>/power1_max_default
> +Date: June 2021
> +KernelVersion: 5.14
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RO. Card default power limit (default TDP setting).
> +
> + Only supported for particular Intel i915 graphics platforms.
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 7ae3b7d67fcf..c60cfacd4c47 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -19,6 +19,7 @@ config DRM_I915
> select DRM_MIPI_DSI
> select RELAY
> select IRQ_WORK
> + select HWMON
Is this really what we want, though? Maybe depends on, or make it work
with either setting. See Documentation/kbuild/kconfig-language.rst on
select.
> # i915 depends on ACPI_VIDEO when ACPI is enabled
> # but for select to work, need to select ACPI_VIDEO's dependencies, ick
> select BACKLIGHT_CLASS_DEVICE if ACPI
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index d2b18f03a33c..e1d5912ea879 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -36,6 +36,7 @@ i915-y += i915_driver.o \
> i915_drm_client.o \
> i915_config.o \
> i915_getparam.o \
> + i915_hwmon.o \
> i915_ioctl.o \
> i915_irq.o \
> i915_mitigations.o \
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b47746152d97..6f678587a771 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -80,6 +80,7 @@
> #include "i915_drm_client.h"
> #include "i915_drv.h"
> #include "i915_getparam.h"
> +#include "i915_hwmon.h"
> #include "i915_ioc32.h"
> #include "i915_ioctl.h"
> #include "i915_irq.h"
> @@ -702,6 +703,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>
> intel_gt_driver_register(to_gt(dev_priv));
>
> + i915_hwmon_register(dev_priv);
> +
> intel_display_driver_register(dev_priv);
>
> intel_power_domains_enable(dev_priv);
> @@ -728,6 +731,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>
> intel_display_driver_unregister(dev_priv);
>
> + i915_hwmon_unregister(dev_priv);
> +
> intel_gt_driver_unregister(to_gt(dev_priv));
>
> i915_perf_unregister(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c5fc402d9c50..e16bedba2d84 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -769,6 +769,8 @@ struct drm_i915_private {
>
> struct i915_perf perf;
>
> + struct i915_hwmon *hwmon;
> +
That should need a forward declaration.
> /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
> struct intel_gt gt0;
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> new file mode 100644
> index 000000000000..b94c11f2517f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +/*
> + * Power-related hwmon entries.
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/types.h>
> +
> +#include "i915_drv.h"
> +#include "i915_hwmon.h"
> +#include "intel_mchbar_regs.h"
> +
> +/*
> + * SF_* - scale factors for particular quantities according to hwmon spec.
> + * - power - microwatts
> + */
> +#define SF_POWER 1000000
> +
> +#define FIELD_SHIFT(__mask) \
> + (BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
> + BUILD_BUG_ON_ZERO((__mask) == 0) + \
> + __bf_shf(__mask))
> +
> +static void
> +_locked_with_pm_intel_uncore_rmw(struct i915_hwmon_drvdata *ddat,
> + i915_reg_t reg, u32 clear, u32 set)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + struct intel_uncore *uncore = ddat->dd_uncore;
> + intel_wakeref_t wakeref;
> +
> + mutex_lock(&hwmon->hwmon_lock);
> +
> + with_intel_runtime_pm(uncore->rpm, wakeref)
> + intel_uncore_rmw(uncore, reg, clear, set);
> +
> + mutex_unlock(&hwmon->hwmon_lock);
> +}
> +
> +static u64
> +_scale_and_shift(u32 in, u32 scale_factor, int nshift)
> +{
> + u64 out = mul_u32_u32(scale_factor, in);
> +
> + /* Shift, rounding to nearest */
> + if (nshift > 0)
> + out = (out + (1 << (nshift - 1))) >> nshift;
> + return out;
> +}
> +
> +/*
> + * This function's return type of u64 allows for the case where the scaling
> + * of the field taken from the 32-bit register value might cause a result to
> + * exceed 32 bits.
> + */
> +static u64
> +_field_read_and_scale(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
> + u32 field_msk, int field_shift,
> + int nshift, u32 scale_factor)
> +{
> + struct intel_uncore *uncore = ddat->dd_uncore;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + with_intel_runtime_pm(uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(uncore, rgadr);
> +
> + reg_value = (reg_value & field_msk) >> field_shift;
> +
> + return _scale_and_shift(reg_value, scale_factor, nshift);
> +}
> +
> +static void
> +_field_scale_and_write(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
> + u32 field_msk, int field_shift,
> + int nshift, unsigned int scale_factor, long lval)
> +{
> + u32 nval;
> + u32 bits_to_clear;
> + u32 bits_to_set;
> +
> + /* Computation in 64-bits to avoid overflow. Round to nearest. */
> + nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
> +
> + bits_to_clear = field_msk;
> + bits_to_set = (nval << field_shift) & field_msk;
> +
> + _locked_with_pm_intel_uncore_rmw(ddat, rgadr,
> + bits_to_clear, bits_to_set);
> +}
> +
> +static ssize_t
> +i915_power1_max_default_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + u64 val = 0; /* uapi specifies to keep visible but return 0 if unsupported */
> +
> + if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku))
> + val = _field_read_and_scale(ddat,
> + hwmon->rg.pkg_power_sku,
> + PKG_PKG_TDP,
> + FIELD_SHIFT(PKG_PKG_TDP),
> + hwmon->scl_shift_power,
> + SF_POWER);
> + return sysfs_emit(buf, "%llu\n", val);
> +}
> +
> +static SENSOR_DEVICE_ATTR(power1_max_default, 0444,
> + i915_power1_max_default_show, NULL, 0);
> +
> +static struct attribute *hwmon_attributes[] = {
> + &sensor_dev_attr_power1_max_default.dev_attr.attr,
> + NULL
> +};
> +
> +static umode_t hwmon_attributes_visible(struct kobject *kobj,
> + struct attribute *attr, int index)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> + struct drm_i915_private *i915 = ddat->dd_uncore->i915;
> + i915_reg_t rgadr;
> +
> + if (attr == &sensor_dev_attr_power1_max_default.dev_attr.attr)
> + return IS_DGFX(i915) ? attr->mode : 0;
> + else
> + return 0;
> +
> + if (!i915_mmio_reg_valid(rgadr))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static const struct attribute_group hwmon_attrgroup = {
> + .attrs = hwmon_attributes,
> + .is_visible = hwmon_attributes_visible,
> +};
> +
> +static const struct attribute_group *hwmon_groups[] = {
> + &hwmon_attrgroup,
> + NULL
> +};
> +
> +/*
> + * HWMON SENSOR TYPE = hwmon_power
> + * - Sustained Power (power1_max)
> + */
> +static const u32 i915_config_power[] = {
> + HWMON_P_MAX,
> + 0
> +};
> +
> +static const struct hwmon_channel_info i915_power = {
> + .type = hwmon_power,
> + .config = i915_config_power,
> +};
> +
> +static const struct hwmon_channel_info *i915_info[] = {
> + &i915_power,
> + NULL
> +};
> +
> +static umode_t
> +i915_power_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr, int chan)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + i915_reg_t rgadr;
> +
> + switch (attr) {
> + case hwmon_power_max:
> + rgadr = hwmon->rg.pkg_rapl_limit;
> + break;
> + default:
> + return 0;
> + }
> +
> + if (!i915_mmio_reg_valid(rgadr))
> + return 0;
> +
> + return 0664;
> +}
> +
> +static int
> +i915_power_read(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + int ret = 0;
> +
> + switch (attr) {
> + case hwmon_power_max:
> + *val = _field_read_and_scale(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power,
> + SF_POWER);
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + int ret = 0;
> +
> + switch (attr) {
> + case hwmon_power_max:
> + _field_scale_and_write(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power,
> + SF_POWER, val);
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static umode_t
> +i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + struct i915_hwmon_drvdata *ddat = (struct i915_hwmon_drvdata *)drvdata;
> +
> + switch (type) {
> + case hwmon_power:
> + return i915_power_is_visible(ddat, attr, channel);
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +i915_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_power:
> + return i915_power_read(ddat, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int
> +i915_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_power:
> + return i915_power_write(ddat, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static const struct hwmon_ops i915_hwmon_ops = {
> + .is_visible = i915_is_visible,
> + .read = i915_read,
> + .write = i915_write,
> +};
> +
> +static const struct hwmon_chip_info i915_chip_info = {
> + .ops = &i915_hwmon_ops,
> + .info = i915_info,
> +};
> +
> +static void
> +i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
> +{
> + struct i915_hwmon *hwmon = i915->hwmon;
> + struct intel_uncore *uncore = &i915->uncore;
> + struct i915_hwmon_drvdata *ddat = &hwmon->ddat;
> + intel_wakeref_t wakeref;
> + u32 val_sku_unit;
> + __le32 le_sku_unit;
> +
> + if (IS_DG1(i915) || IS_DG2(i915)) {
> + hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
> + hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> + hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> + } else {
> + hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> + hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> + hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> + }
> +
> + wakeref = intel_runtime_pm_get(uncore->rpm);
> +
> + /*
> + * The contents of register hwmon->rg.pkg_power_sku_unit do not change,
> + * so read it once and store the shift values.
> + *
> + * For some platforms, this value is defined as available "for all
> + * tiles", with the values consistent across all tiles.
> + * In this case, use the tile 0 value for all.
> + */
> + if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit))
> + val_sku_unit = intel_uncore_read(uncore,
> + hwmon->rg.pkg_power_sku_unit);
> + else
> + val_sku_unit = 0;
> +
> + intel_runtime_pm_put(uncore->rpm, wakeref);
> +
> + le_sku_unit = cpu_to_le32(val_sku_unit);
> + hwmon->scl_shift_power = le32_get_bits(le_sku_unit, PKG_PWR_UNIT);
> +
> + /*
> + * The value of power1_max is reset to the default on reboot, but is
> + * not reset by a module unload/load sequence. To allow proper
> + * functioning after a module reload, the value for power1_max is
> + * restored to its original value at module unload time in
> + * i915_hwmon_unregister().
> + */
> + hwmon->power_max_initial_value =
> + (u32)_field_read_and_scale(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power, SF_POWER);
> +}
> +
> +void i915_hwmon_register(struct drm_i915_private *i915)
> +{
> + struct device *dev = i915->drm.dev;
> + struct i915_hwmon *hwmon;
> + struct device *hwmon_dev;
> + struct i915_hwmon_drvdata *ddat;
> +
> + hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
> + if (!hwmon)
> + return;
> +
> + i915->hwmon = hwmon;
> +
> + mutex_init(&hwmon->hwmon_lock);
> +
> + ddat = &hwmon->ddat;
> +
> + ddat->dd_hwmon = hwmon;
> + ddat->dd_uncore = &i915->uncore;
> + snprintf(ddat->dd_name, sizeof(ddat->dd_name), "i915");
> +
> + i915_hwmon_get_preregistration_info(i915);
> +
> + /* hwmon_dev points to device hwmon<i> */
> + hwmon_dev = hwmon_device_register_with_info(dev, ddat->dd_name,
> + ddat,
> + &i915_chip_info,
> + hwmon_groups);
> +
> + if (IS_ERR(hwmon_dev)) {
> + mutex_destroy(&hwmon->hwmon_lock);
> + i915->hwmon = NULL;
> + kfree(hwmon);
> + return;
> + }
> +
> + ddat->dd_hwmon_dev = hwmon_dev;
> +}
> +
> +void i915_hwmon_unregister(struct drm_i915_private *i915)
> +{
> + struct i915_hwmon *hwmon;
> + struct i915_hwmon_drvdata *ddat;
> +
> + hwmon = fetch_and_zero(&i915->hwmon);
> + if (!hwmon)
> + return;
> +
> + ddat = &hwmon->ddat;
> +
> + if (hwmon->power_max_initial_value) {
> + /* Restore power1_max. */
> + _field_scale_and_write(ddat, hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power,
> + SF_POWER,
> + hwmon->power_max_initial_value);
> + }
> +
> + if (ddat->dd_hwmon_dev)
> + hwmon_device_unregister(ddat->dd_hwmon_dev);
> +
> + mutex_destroy(&hwmon->hwmon_lock);
> +
> + kfree(hwmon);
> +}
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> new file mode 100644
> index 000000000000..396b758aefce
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * Copyright © 2020 Intel Corporation
It's 2022.
> + */
> +
> +#ifndef __INTEL_HWMON_H__
> +#define __INTEL_HWMON_H__
Does not match the file name.
> +
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +#include "i915_reg.h"
> +
> +struct drm_i915_private;
> +
> +struct i915_hwmon_reg {
> + i915_reg_t pkg_power_sku_unit;
> + i915_reg_t pkg_power_sku;
> + i915_reg_t pkg_rapl_limit;
> +};
> +
> +struct i915_hwmon_drvdata {
> + struct i915_hwmon *dd_hwmon;
> + struct intel_uncore *dd_uncore;
> + struct device *dd_hwmon_dev;
> + char dd_name[12];
> +};
> +
> +struct i915_hwmon {
> + struct i915_hwmon_drvdata ddat;
> +
> + struct mutex hwmon_lock; /* counter overflow logic and rmw */
> +
> + struct i915_hwmon_reg rg;
> +
> + u32 power_max_initial_value;
> +
> + int scl_shift_power;
> +};
AFAICT all of the above can be placed in i915_hwmon.c. You only store
the pointer in struct drm_i915_private. Hide this stuff whenever you
can. This lets you clean up the includes and forward declarations too.
> +
> +void i915_hwmon_register(struct drm_i915_private *i915);
> +void i915_hwmon_unregister(struct drm_i915_private *i915);
Blank line here.
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d8579ab9384c..1c570c706ff8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1866,6 +1866,21 @@
> #define POWER_LIMIT_4_MASK REG_BIT(9)
> #define POWER_LIMIT_1_MASK REG_BIT(11)
> #define POWER_LIMIT_2_MASK REG_BIT(12)
> +/*
> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
> + * Used herein as a 64-bit register.
> + * These masks are defined using GENMASK_ULL as REG_GENMASK is limited to u32
> + * and as GENMASK is "long" and therefore 32-bits on a 32-bit system.
> + * PKG_PKG_TDP, PKG_MIN_PWR, and PKG_MAX_PWR are scaled in the same way as
> + * PKG_PWR_LIM_*, above.
> + * PKG_MAX_WIN has sub-fields for x and y, and has the value: is 1.x * 2^y.
> + */
> +#define PKG_PKG_TDP GENMASK_ULL(14, 0)
> +#define PKG_MIN_PWR GENMASK_ULL(30, 16)
> +#define PKG_MAX_PWR GENMASK_ULL(46, 32)
> +#define PKG_MAX_WIN GENMASK_ULL(54, 48)
> +#define PKG_MAX_WIN_Y GENMASK_ULL(54, 53)
> +#define PKG_MAX_WIN_X GENMASK_ULL(52, 48)
REG_GENMASK64().
>
> #define CHV_CLK_CTL1 _MMIO(0x101100)
> #define VLV_CLK_CTL2 _MMIO(0x101104)
> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> index 2aad2f0cc8db..247561d7974c 100644
> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> @@ -191,11 +191,20 @@
>
> #define GEN6_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
> #define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
> +#define PCU_PACKAGE_POWER_SKU_UNIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
> +#define PKG_PWR_UNIT REG_GENMASK(3, 0)
> +#define PKG_TIME_UNIT REG_GENMASK(19, 16)
> +
> #define GEN6_RP_STATE_CAP _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
> #define RP0_CAP_MASK REG_GENMASK(7, 0)
> #define RP1_CAP_MASK REG_GENMASK(15, 8)
> #define RPN_CAP_MASK REG_GENMASK(23, 16)
>
> +#define PCU_PACKAGE_RAPL_LIMIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
> +#define PKG_PWR_LIM_1 REG_GENMASK(14, 0)
> +#define PKG_PWR_LIM_1_EN REG_BIT(15)
> +#define PKG_PWR_LIM_1_TIME REG_GENMASK(23, 17)
> +
> /* snb MCH registers for priority tuning */
> #define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
> #define SSKPD_NEW_WM0_MASK_HSW REG_GENMASK64(63, 56)
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support Badal Nilawar
2022-05-23 13:45 ` Jani Nikula
@ 2022-05-23 13:46 ` Jani Nikula
2022-05-24 3:10 ` Dixit, Ashutosh
2 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2022-05-23 13:46 UTC (permalink / raw)
To: Badal Nilawar, intel-gfx
On Mon, 23 May 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> As part of the System Managemenent Interface (SMI), use the HWMON
> subsystem to display power utilization.
>
> Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 19 +
> drivers/gpu/drm/i915/Kconfig | 1 +
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_driver.c | 5 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_hwmon.c | 410 ++++++++++++++++++
> drivers/gpu/drm/i915/i915_hwmon.h | 44 ++
> drivers/gpu/drm/i915/i915_reg.h | 15 +
> drivers/gpu/drm/i915/intel_mchbar_regs.h | 9 +
> 9 files changed, 506 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> new file mode 100644
> index 000000000000..c680dd842358
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -0,0 +1,19 @@
> +What: /sys/devices/.../hwmon/hwmon<i>/power1_max
> +Date: June 2021
> +KernelVersion: 5.14
Please check the dates and versions in all these files.
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RW. Card reactive sustained (PL1/Tau) power limit in microwatts.
> +
> + The power controller will throttle the operating frequency
> + if the power averaged over a window (typically seconds)
> + exceeds this limit.
> +
> + Only supported for particular Intel i915 graphics platforms.
> +
> +What: /sys/devices/.../hwmon/hwmon<i>/power1_max_default
> +Date: June 2021
> +KernelVersion: 5.14
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RO. Card default power limit (default TDP setting).
> +
> + Only supported for particular Intel i915 graphics platforms.
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 7ae3b7d67fcf..c60cfacd4c47 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -19,6 +19,7 @@ config DRM_I915
> select DRM_MIPI_DSI
> select RELAY
> select IRQ_WORK
> + select HWMON
> # i915 depends on ACPI_VIDEO when ACPI is enabled
> # but for select to work, need to select ACPI_VIDEO's dependencies, ick
> select BACKLIGHT_CLASS_DEVICE if ACPI
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index d2b18f03a33c..e1d5912ea879 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -36,6 +36,7 @@ i915-y += i915_driver.o \
> i915_drm_client.o \
> i915_config.o \
> i915_getparam.o \
> + i915_hwmon.o \
> i915_ioctl.o \
> i915_irq.o \
> i915_mitigations.o \
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b47746152d97..6f678587a771 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -80,6 +80,7 @@
> #include "i915_drm_client.h"
> #include "i915_drv.h"
> #include "i915_getparam.h"
> +#include "i915_hwmon.h"
> #include "i915_ioc32.h"
> #include "i915_ioctl.h"
> #include "i915_irq.h"
> @@ -702,6 +703,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>
> intel_gt_driver_register(to_gt(dev_priv));
>
> + i915_hwmon_register(dev_priv);
> +
> intel_display_driver_register(dev_priv);
>
> intel_power_domains_enable(dev_priv);
> @@ -728,6 +731,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>
> intel_display_driver_unregister(dev_priv);
>
> + i915_hwmon_unregister(dev_priv);
> +
> intel_gt_driver_unregister(to_gt(dev_priv));
>
> i915_perf_unregister(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c5fc402d9c50..e16bedba2d84 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -769,6 +769,8 @@ struct drm_i915_private {
>
> struct i915_perf perf;
>
> + struct i915_hwmon *hwmon;
> +
> /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
> struct intel_gt gt0;
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> new file mode 100644
> index 000000000000..b94c11f2517f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +/*
> + * Power-related hwmon entries.
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/types.h>
> +
> +#include "i915_drv.h"
> +#include "i915_hwmon.h"
> +#include "intel_mchbar_regs.h"
> +
> +/*
> + * SF_* - scale factors for particular quantities according to hwmon spec.
> + * - power - microwatts
> + */
> +#define SF_POWER 1000000
> +
> +#define FIELD_SHIFT(__mask) \
> + (BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
> + BUILD_BUG_ON_ZERO((__mask) == 0) + \
> + __bf_shf(__mask))
> +
> +static void
> +_locked_with_pm_intel_uncore_rmw(struct i915_hwmon_drvdata *ddat,
> + i915_reg_t reg, u32 clear, u32 set)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + struct intel_uncore *uncore = ddat->dd_uncore;
> + intel_wakeref_t wakeref;
> +
> + mutex_lock(&hwmon->hwmon_lock);
> +
> + with_intel_runtime_pm(uncore->rpm, wakeref)
> + intel_uncore_rmw(uncore, reg, clear, set);
> +
> + mutex_unlock(&hwmon->hwmon_lock);
> +}
> +
> +static u64
> +_scale_and_shift(u32 in, u32 scale_factor, int nshift)
> +{
> + u64 out = mul_u32_u32(scale_factor, in);
> +
> + /* Shift, rounding to nearest */
> + if (nshift > 0)
> + out = (out + (1 << (nshift - 1))) >> nshift;
> + return out;
> +}
> +
> +/*
> + * This function's return type of u64 allows for the case where the scaling
> + * of the field taken from the 32-bit register value might cause a result to
> + * exceed 32 bits.
> + */
> +static u64
> +_field_read_and_scale(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
> + u32 field_msk, int field_shift,
> + int nshift, u32 scale_factor)
> +{
> + struct intel_uncore *uncore = ddat->dd_uncore;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + with_intel_runtime_pm(uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(uncore, rgadr);
> +
> + reg_value = (reg_value & field_msk) >> field_shift;
> +
> + return _scale_and_shift(reg_value, scale_factor, nshift);
> +}
> +
> +static void
> +_field_scale_and_write(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
> + u32 field_msk, int field_shift,
> + int nshift, unsigned int scale_factor, long lval)
> +{
> + u32 nval;
> + u32 bits_to_clear;
> + u32 bits_to_set;
> +
> + /* Computation in 64-bits to avoid overflow. Round to nearest. */
> + nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
> +
> + bits_to_clear = field_msk;
> + bits_to_set = (nval << field_shift) & field_msk;
> +
> + _locked_with_pm_intel_uncore_rmw(ddat, rgadr,
> + bits_to_clear, bits_to_set);
> +}
> +
> +static ssize_t
> +i915_power1_max_default_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + u64 val = 0; /* uapi specifies to keep visible but return 0 if unsupported */
> +
> + if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku))
> + val = _field_read_and_scale(ddat,
> + hwmon->rg.pkg_power_sku,
> + PKG_PKG_TDP,
> + FIELD_SHIFT(PKG_PKG_TDP),
> + hwmon->scl_shift_power,
> + SF_POWER);
> + return sysfs_emit(buf, "%llu\n", val);
> +}
> +
> +static SENSOR_DEVICE_ATTR(power1_max_default, 0444,
> + i915_power1_max_default_show, NULL, 0);
> +
> +static struct attribute *hwmon_attributes[] = {
> + &sensor_dev_attr_power1_max_default.dev_attr.attr,
> + NULL
> +};
> +
> +static umode_t hwmon_attributes_visible(struct kobject *kobj,
> + struct attribute *attr, int index)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> + struct drm_i915_private *i915 = ddat->dd_uncore->i915;
> + i915_reg_t rgadr;
> +
> + if (attr == &sensor_dev_attr_power1_max_default.dev_attr.attr)
> + return IS_DGFX(i915) ? attr->mode : 0;
> + else
> + return 0;
> +
> + if (!i915_mmio_reg_valid(rgadr))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static const struct attribute_group hwmon_attrgroup = {
> + .attrs = hwmon_attributes,
> + .is_visible = hwmon_attributes_visible,
> +};
> +
> +static const struct attribute_group *hwmon_groups[] = {
> + &hwmon_attrgroup,
> + NULL
> +};
> +
> +/*
> + * HWMON SENSOR TYPE = hwmon_power
> + * - Sustained Power (power1_max)
> + */
> +static const u32 i915_config_power[] = {
> + HWMON_P_MAX,
> + 0
> +};
> +
> +static const struct hwmon_channel_info i915_power = {
> + .type = hwmon_power,
> + .config = i915_config_power,
> +};
> +
> +static const struct hwmon_channel_info *i915_info[] = {
> + &i915_power,
> + NULL
> +};
> +
> +static umode_t
> +i915_power_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr, int chan)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + i915_reg_t rgadr;
> +
> + switch (attr) {
> + case hwmon_power_max:
> + rgadr = hwmon->rg.pkg_rapl_limit;
> + break;
> + default:
> + return 0;
> + }
> +
> + if (!i915_mmio_reg_valid(rgadr))
> + return 0;
> +
> + return 0664;
> +}
> +
> +static int
> +i915_power_read(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + int ret = 0;
> +
> + switch (attr) {
> + case hwmon_power_max:
> + *val = _field_read_and_scale(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power,
> + SF_POWER);
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + int ret = 0;
> +
> + switch (attr) {
> + case hwmon_power_max:
> + _field_scale_and_write(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power,
> + SF_POWER, val);
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static umode_t
> +i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + struct i915_hwmon_drvdata *ddat = (struct i915_hwmon_drvdata *)drvdata;
> +
> + switch (type) {
> + case hwmon_power:
> + return i915_power_is_visible(ddat, attr, channel);
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +i915_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_power:
> + return i915_power_read(ddat, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int
> +i915_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_power:
> + return i915_power_write(ddat, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static const struct hwmon_ops i915_hwmon_ops = {
> + .is_visible = i915_is_visible,
> + .read = i915_read,
> + .write = i915_write,
> +};
> +
> +static const struct hwmon_chip_info i915_chip_info = {
> + .ops = &i915_hwmon_ops,
> + .info = i915_info,
> +};
> +
> +static void
> +i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
> +{
> + struct i915_hwmon *hwmon = i915->hwmon;
> + struct intel_uncore *uncore = &i915->uncore;
> + struct i915_hwmon_drvdata *ddat = &hwmon->ddat;
> + intel_wakeref_t wakeref;
> + u32 val_sku_unit;
> + __le32 le_sku_unit;
> +
> + if (IS_DG1(i915) || IS_DG2(i915)) {
> + hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
> + hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> + hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> + } else {
> + hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> + hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> + hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> + }
> +
> + wakeref = intel_runtime_pm_get(uncore->rpm);
> +
> + /*
> + * The contents of register hwmon->rg.pkg_power_sku_unit do not change,
> + * so read it once and store the shift values.
> + *
> + * For some platforms, this value is defined as available "for all
> + * tiles", with the values consistent across all tiles.
> + * In this case, use the tile 0 value for all.
> + */
> + if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit))
> + val_sku_unit = intel_uncore_read(uncore,
> + hwmon->rg.pkg_power_sku_unit);
> + else
> + val_sku_unit = 0;
> +
> + intel_runtime_pm_put(uncore->rpm, wakeref);
> +
> + le_sku_unit = cpu_to_le32(val_sku_unit);
> + hwmon->scl_shift_power = le32_get_bits(le_sku_unit, PKG_PWR_UNIT);
> +
> + /*
> + * The value of power1_max is reset to the default on reboot, but is
> + * not reset by a module unload/load sequence. To allow proper
> + * functioning after a module reload, the value for power1_max is
> + * restored to its original value at module unload time in
> + * i915_hwmon_unregister().
> + */
> + hwmon->power_max_initial_value =
> + (u32)_field_read_and_scale(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power, SF_POWER);
> +}
> +
> +void i915_hwmon_register(struct drm_i915_private *i915)
> +{
> + struct device *dev = i915->drm.dev;
> + struct i915_hwmon *hwmon;
> + struct device *hwmon_dev;
> + struct i915_hwmon_drvdata *ddat;
> +
> + hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
> + if (!hwmon)
> + return;
> +
> + i915->hwmon = hwmon;
> +
> + mutex_init(&hwmon->hwmon_lock);
> +
> + ddat = &hwmon->ddat;
> +
> + ddat->dd_hwmon = hwmon;
> + ddat->dd_uncore = &i915->uncore;
> + snprintf(ddat->dd_name, sizeof(ddat->dd_name), "i915");
> +
> + i915_hwmon_get_preregistration_info(i915);
> +
> + /* hwmon_dev points to device hwmon<i> */
> + hwmon_dev = hwmon_device_register_with_info(dev, ddat->dd_name,
> + ddat,
> + &i915_chip_info,
> + hwmon_groups);
> +
> + if (IS_ERR(hwmon_dev)) {
> + mutex_destroy(&hwmon->hwmon_lock);
> + i915->hwmon = NULL;
> + kfree(hwmon);
> + return;
> + }
> +
> + ddat->dd_hwmon_dev = hwmon_dev;
> +}
> +
> +void i915_hwmon_unregister(struct drm_i915_private *i915)
> +{
> + struct i915_hwmon *hwmon;
> + struct i915_hwmon_drvdata *ddat;
> +
> + hwmon = fetch_and_zero(&i915->hwmon);
> + if (!hwmon)
> + return;
> +
> + ddat = &hwmon->ddat;
> +
> + if (hwmon->power_max_initial_value) {
> + /* Restore power1_max. */
> + _field_scale_and_write(ddat, hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + FIELD_SHIFT(PKG_PWR_LIM_1),
> + hwmon->scl_shift_power,
> + SF_POWER,
> + hwmon->power_max_initial_value);
> + }
> +
> + if (ddat->dd_hwmon_dev)
> + hwmon_device_unregister(ddat->dd_hwmon_dev);
> +
> + mutex_destroy(&hwmon->hwmon_lock);
> +
> + kfree(hwmon);
> +}
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> new file mode 100644
> index 000000000000..396b758aefce
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#ifndef __INTEL_HWMON_H__
> +#define __INTEL_HWMON_H__
> +
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +#include "i915_reg.h"
> +
> +struct drm_i915_private;
> +
> +struct i915_hwmon_reg {
> + i915_reg_t pkg_power_sku_unit;
> + i915_reg_t pkg_power_sku;
> + i915_reg_t pkg_rapl_limit;
> +};
> +
> +struct i915_hwmon_drvdata {
> + struct i915_hwmon *dd_hwmon;
> + struct intel_uncore *dd_uncore;
> + struct device *dd_hwmon_dev;
> + char dd_name[12];
> +};
> +
> +struct i915_hwmon {
> + struct i915_hwmon_drvdata ddat;
> +
> + struct mutex hwmon_lock; /* counter overflow logic and rmw */
> +
> + struct i915_hwmon_reg rg;
> +
> + u32 power_max_initial_value;
> +
> + int scl_shift_power;
> +};
> +
> +void i915_hwmon_register(struct drm_i915_private *i915);
> +void i915_hwmon_unregister(struct drm_i915_private *i915);
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d8579ab9384c..1c570c706ff8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1866,6 +1866,21 @@
> #define POWER_LIMIT_4_MASK REG_BIT(9)
> #define POWER_LIMIT_1_MASK REG_BIT(11)
> #define POWER_LIMIT_2_MASK REG_BIT(12)
> +/*
> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
> + * Used herein as a 64-bit register.
> + * These masks are defined using GENMASK_ULL as REG_GENMASK is limited to u32
> + * and as GENMASK is "long" and therefore 32-bits on a 32-bit system.
> + * PKG_PKG_TDP, PKG_MIN_PWR, and PKG_MAX_PWR are scaled in the same way as
> + * PKG_PWR_LIM_*, above.
> + * PKG_MAX_WIN has sub-fields for x and y, and has the value: is 1.x * 2^y.
> + */
> +#define PKG_PKG_TDP GENMASK_ULL(14, 0)
> +#define PKG_MIN_PWR GENMASK_ULL(30, 16)
> +#define PKG_MAX_PWR GENMASK_ULL(46, 32)
> +#define PKG_MAX_WIN GENMASK_ULL(54, 48)
> +#define PKG_MAX_WIN_Y GENMASK_ULL(54, 53)
> +#define PKG_MAX_WIN_X GENMASK_ULL(52, 48)
>
> #define CHV_CLK_CTL1 _MMIO(0x101100)
> #define VLV_CLK_CTL2 _MMIO(0x101104)
> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> index 2aad2f0cc8db..247561d7974c 100644
> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> @@ -191,11 +191,20 @@
>
> #define GEN6_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
> #define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
> +#define PCU_PACKAGE_POWER_SKU_UNIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
> +#define PKG_PWR_UNIT REG_GENMASK(3, 0)
> +#define PKG_TIME_UNIT REG_GENMASK(19, 16)
> +
> #define GEN6_RP_STATE_CAP _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
> #define RP0_CAP_MASK REG_GENMASK(7, 0)
> #define RP1_CAP_MASK REG_GENMASK(15, 8)
> #define RPN_CAP_MASK REG_GENMASK(23, 16)
>
> +#define PCU_PACKAGE_RAPL_LIMIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
> +#define PKG_PWR_LIM_1 REG_GENMASK(14, 0)
> +#define PKG_PWR_LIM_1_EN REG_BIT(15)
> +#define PKG_PWR_LIM_1_TIME REG_GENMASK(23, 17)
> +
> /* snb MCH registers for priority tuning */
> #define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
> #define SSKPD_NEW_WM0_MASK_HSW REG_GENMASK64(63, 56)
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support Badal Nilawar
2022-05-23 13:45 ` Jani Nikula
2022-05-23 13:46 ` Jani Nikula
@ 2022-05-24 3:10 ` Dixit, Ashutosh
2 siblings, 0 replies; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-05-24 3:10 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:39 -0700, Badal Nilawar wrote:
>
A few initial comments.
> +static void
> +i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
> +{
> + struct i915_hwmon *hwmon = i915->hwmon;
> + struct intel_uncore *uncore = &i915->uncore;
> + struct i915_hwmon_drvdata *ddat = &hwmon->ddat;
> + intel_wakeref_t wakeref;
> + u32 val_sku_unit;
> + __le32 le_sku_unit;
> +
> + if (IS_DG1(i915) || IS_DG2(i915)) {
> + hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
> + hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> + hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> + } else {
> + hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> + hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> + hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> + }
> +
> + wakeref = intel_runtime_pm_get(uncore->rpm);
Let's just use with_intel_runtime_pm().
> +
> + /*
> + * The contents of register hwmon->rg.pkg_power_sku_unit do not change,
> + * so read it once and store the shift values.
> + *
> + * For some platforms, this value is defined as available "for all
> + * tiles", with the values consistent across all tiles.
> + * In this case, use the tile 0 value for all.
If we are going to introduce multiple tiles "later", let's introduce this
comment later too.
> + */
> + if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit))
> + val_sku_unit = intel_uncore_read(uncore,
> + hwmon->rg.pkg_power_sku_unit);
> + else
> + val_sku_unit = 0;
> +
> + intel_runtime_pm_put(uncore->rpm, wakeref);
> +
> + le_sku_unit = cpu_to_le32(val_sku_unit);
> + hwmon->scl_shift_power = le32_get_bits(le_sku_unit, PKG_PWR_UNIT);
Do we need such endianness conversions, typically we don't do them?
> +
> + /*
> + * The value of power1_max is reset to the default on reboot, but is
> + * not reset by a module unload/load sequence. To allow proper
> + * functioning after a module reload, the value for power1_max is
> + * restored to its original value at module unload time in
> + * i915_hwmon_unregister().
> + */
Because on production systems typically modules are not reloaded, I am not
sure if we need to take care of this save/restore. Also there may be other
ways of doing this e.g.:
https://patchwork.freedesktop.org/patch/483988/?series=102665&rev=3
That is above we just expose the default or init values but don't expose
them.
In order for such issues to be reviewed/debated, I would submit this
save/restore part as a separate patch, so decouple it from the
hwmon_power_max patch.
> +void i915_hwmon_register(struct drm_i915_private *i915);
> +void i915_hwmon_unregister(struct drm_i915_private *i915);
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d8579ab9384c..1c570c706ff8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1866,6 +1866,21 @@
> #define POWER_LIMIT_4_MASK REG_BIT(9)
> #define POWER_LIMIT_1_MASK REG_BIT(11)
> #define POWER_LIMIT_2_MASK REG_BIT(12)
> +/*
> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
> + * Used herein as a 64-bit register.
> + * These masks are defined using GENMASK_ULL as REG_GENMASK is limited to u32
> + * and as GENMASK is "long" and therefore 32-bits on a 32-bit system.
> + * PKG_PKG_TDP, PKG_MIN_PWR, and PKG_MAX_PWR are scaled in the same way as
> + * PKG_PWR_LIM_*, above.
> + * PKG_MAX_WIN has sub-fields for x and y, and has the value: is 1.x * 2^y.
> + */
> +#define PKG_PKG_TDP GENMASK_ULL(14, 0)
> +#define PKG_MIN_PWR GENMASK_ULL(30, 16)
> +#define PKG_MAX_PWR GENMASK_ULL(46, 32)
> +#define PKG_MAX_WIN GENMASK_ULL(54, 48)
> +#define PKG_MAX_WIN_Y GENMASK_ULL(54, 53)
> +#define PKG_MAX_WIN_X GENMASK_ULL(52, 48)
>
> #define CHV_CLK_CTL1 _MMIO(0x101100)
> #define VLV_CLK_CTL2 _MMIO(0x101104)
> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> index 2aad2f0cc8db..247561d7974c 100644
> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> @@ -191,11 +191,20 @@
>
> #define GEN6_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
> #define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
> +#define PCU_PACKAGE_POWER_SKU_UNIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
> +#define PKG_PWR_UNIT REG_GENMASK(3, 0)
> +#define PKG_TIME_UNIT REG_GENMASK(19, 16)
> +
> #define GEN6_RP_STATE_CAP _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
> #define RP0_CAP_MASK REG_GENMASK(7, 0)
> #define RP1_CAP_MASK REG_GENMASK(15, 8)
> #define RPN_CAP_MASK REG_GENMASK(23, 16)
>
> +#define PCU_PACKAGE_RAPL_LIMIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
> +#define PKG_PWR_LIM_1 REG_GENMASK(14, 0)
> +#define PKG_PWR_LIM_1_EN REG_BIT(15)
> +#define PKG_PWR_LIM_1_TIME REG_GENMASK(23, 17)
> +
I think we should remove #define's which are not actually used in the patch
and introduce them in the patches in which they are used.
> /* snb MCH registers for priority tuning */
> #define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
> #define SSKPD_NEW_WM0_MASK_HSW REG_GENMASK64(63, 56)
> --
> 2.25.1
>
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support
2022-05-23 11:08 [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support Badal Nilawar
2022-05-23 11:08 ` [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support Badal Nilawar
@ 2022-05-23 11:08 ` Badal Nilawar
2022-05-25 4:55 ` Dixit, Ashutosh
2022-05-25 4:56 ` Dixit, Ashutosh
2022-05-23 11:08 ` [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Badal Nilawar @ 2022-05-23 11:08 UTC (permalink / raw)
To: intel-gfx
From: Dale B Stimson <dale.b.stimson@intel.com>
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display energy utilization
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 16 ++
drivers/gpu/drm/i915/i915_hwmon.c | 155 +++++++++++++++++-
drivers/gpu/drm/i915/i915_hwmon.h | 11 ++
drivers/gpu/drm/i915/intel_mchbar_regs.h | 2 +
4 files changed, 183 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index c680dd842358..7574421c326f 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -1,3 +1,19 @@
+What: /sys/devices/.../hwmon/hwmon<i>/energy1_input
+Date: June 2021
+KernelVersion: 5.14
+Contact: dri-devel@lists.freedesktop.org
+Description: RO. Energy input of device in microjoules.
+
+ The returned textual representation is an unsigned integer
+ number that can be stored in 64-bits. Warning: The hardware
+ register is 32-bits wide and can overflow by wrapping around.
+ A single wrap-around between calls to read this value can
+ be detected and will be accounted for in the returned value.
+ At a power consumption of 1 watt, the 32-bit hardware register
+ would wrap-around approximately every 3 days.
+
+ Only supported for particular Intel i915 graphics platforms.
+
What: /sys/devices/.../hwmon/hwmon<i>/power1_max
Date: June 2021
KernelVersion: 5.14
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index b94c11f2517f..b35c4de73f30 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -18,8 +18,10 @@
/*
* SF_* - scale factors for particular quantities according to hwmon spec.
* - power - microwatts
+ * - energy - microjoules
*/
#define SF_POWER 1000000
+#define SF_ENERGY 1000000
#define FIELD_SHIFT(__mask) \
(BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
@@ -94,6 +96,136 @@ _field_scale_and_write(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
bits_to_clear, bits_to_set);
}
+/*
+ * _i915_energy1_input_sub - A custom function to obtain energy1_input.
+ * Use a custom function instead of the usual hwmon helpers in order to
+ * guarantee 64-bits of result to user-space.
+ * Units are microjoules.
+ *
+ * The underlying hardware register is 32-bits and is subject to overflow.
+ * This function compensates for overflow of the 32-bit register by detecting
+ * wrap-around and incrementing an overflow counter.
+ * This only works if the register is sampled often enough to avoid
+ * missing an instance of overflow - achieved either by repeated
+ * queries through the API, or via a possible timer (future - TBD) that
+ * ensures values are read often enough to catch all overflows.
+ *
+ * How long before overflow? For example, with an example scaling bit
+ * shift of 14 bits (see register *PACKAGE_POWER_SKU_UNIT) and a power draw of
+ * 1000 watts, the 32-bit counter will overflow in approximately 4.36 minutes.
+ *
+ * Examples:
+ * 1 watt: (2^32 >> 14) / 1 W / (60 * 60 * 24) secs/day -> 3 days
+ * 1000 watts: (2^32 >> 14) / 1000 W / 60 secs/min -> 4.36 minutes
+ */
+static int
+_i915_energy1_input_sub(struct i915_hwmon_drvdata *ddat, u64 *energy)
+{
+ struct intel_uncore *uncore = ddat->dd_uncore;
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ struct i915_energy_info *pei = &ddat->dd_ei;
+ int nshift = hwmon->scl_shift_energy;
+ intel_wakeref_t wakeref;
+ u32 reg_value;
+ u64 vlo;
+ u64 vhi;
+ i915_reg_t rgaddr;
+
+ rgaddr = hwmon->rg.energy_status_all;
+
+ if (!i915_mmio_reg_valid(rgaddr))
+ return -EOPNOTSUPP;
+
+ mutex_lock(&hwmon->hwmon_lock);
+
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+ reg_value = intel_uncore_read(uncore, rgaddr);
+
+ /*
+ * The u32 register concatenated with the u32 overflow counter
+ * gives an effective energy counter size of 64-bits. However, the
+ * computations below are done modulo 2^96 to avoid overflow during
+ * scaling in the conversion to microjoules.
+ *
+ * The low-order 64-bits of the resulting quantity are returned to
+ * the caller in units of microjoules, encoded into a decimal string.
+ *
+ * For a power of 1000 watts, 64 bits in units of microjoules will
+ * overflow after 584 years.
+ */
+
+ if (pei->energy_counter_prev > reg_value)
+ pei->energy_counter_overflow++;
+
+ pei->energy_counter_prev = reg_value;
+
+ /*
+ * 64-bit variables vlo and vhi are used for the scaling process.
+ * The 96-bit counter value is composed from the two 64-bit variables
+ * vhi and vlo thusly: counter == vhi << 32 + vlo .
+ * The 32-bits of overlap between the two variables is convenient for
+ * handling overflows out of vlo.
+ */
+
+ vlo = reg_value;
+ vhi = pei->energy_counter_overflow;
+
+ mutex_unlock(&hwmon->hwmon_lock);
+
+ vlo = SF_ENERGY * vlo;
+
+ /* Prepare to round to nearest */
+ if (nshift > 0)
+ vlo += 1 << (nshift - 1);
+
+ /*
+ * Anything in the upper-32 bits of vlo gets added into vhi here,
+ * and then cleared from vlo.
+ */
+ vhi = (SF_ENERGY * vhi) + (vlo >> 32);
+ vlo &= 0xffffffffULL;
+
+ /*
+ * Apply the right shift.
+ * - vlo shifted by itself.
+ * - vlo receiving what's shifted out of vhi.
+ * - vhi shifted by itself
+ */
+ vlo = vlo >> nshift;
+ vlo |= (vhi << (32 - nshift)) & 0xffffffffULL;
+ vhi = vhi >> nshift;
+
+ /* Combined to get a 64-bit result in vlo. */
+ vlo |= (vhi << 32);
+
+ *energy = vlo;
+
+ return 0;
+}
+
+static ssize_t
+i915_energy1_input_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
+ ssize_t ret = 0;
+ u64 energy;
+
+ if (!_i915_energy1_input_sub(ddat, &energy))
+ ret = sysfs_emit(buf, "%llu\n", energy);
+
+ return ret;
+}
+
+int
+i915_energy_status_get(struct drm_i915_private *i915, u64 *energy)
+{
+ struct i915_hwmon *hwmon = i915->hwmon;
+ struct i915_hwmon_drvdata *ddat = &hwmon->ddat;
+
+ return _i915_energy1_input_sub(ddat, energy);
+}
+
static ssize_t
i915_power1_max_default_show(struct device *dev, struct device_attribute *attr,
char *buf)
@@ -114,9 +246,12 @@ i915_power1_max_default_show(struct device *dev, struct device_attribute *attr,
static SENSOR_DEVICE_ATTR(power1_max_default, 0444,
i915_power1_max_default_show, NULL, 0);
+static SENSOR_DEVICE_ATTR(energy1_input, 0444,
+ i915_energy1_input_show, NULL, 0);
static struct attribute *hwmon_attributes[] = {
&sensor_dev_attr_power1_max_default.dev_attr.attr,
+ &sensor_dev_attr_energy1_input.dev_attr.attr,
NULL
};
@@ -126,9 +261,12 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
struct device *dev = kobj_to_dev(kobj);
struct i915_hwmon_drvdata *ddat = dev_get_drvdata(dev);
struct drm_i915_private *i915 = ddat->dd_uncore->i915;
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
i915_reg_t rgadr;
- if (attr == &sensor_dev_attr_power1_max_default.dev_attr.attr)
+ if (attr == &sensor_dev_attr_energy1_input.dev_attr.attr)
+ rgadr = hwmon->rg.energy_status_all;
+ else if (attr == &sensor_dev_attr_power1_max_default.dev_attr.attr)
return IS_DGFX(i915) ? attr->mode : 0;
else
return 0;
@@ -291,6 +429,7 @@ i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
struct i915_hwmon *hwmon = i915->hwmon;
struct intel_uncore *uncore = &i915->uncore;
struct i915_hwmon_drvdata *ddat = &hwmon->ddat;
+ struct i915_energy_info *pei;
intel_wakeref_t wakeref;
u32 val_sku_unit;
__le32 le_sku_unit;
@@ -299,10 +438,14 @@ i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
+ hwmon->rg.energy_status_all = PCU_PACKAGE_ENERGY_STATUS;
+ hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
} else {
hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
+ hwmon->rg.energy_status_all = INVALID_MMIO_REG;
+ hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
}
wakeref = intel_runtime_pm_get(uncore->rpm);
@@ -321,10 +464,20 @@ i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
else
val_sku_unit = 0;
+ pei = &ddat->dd_ei;
+ pei->energy_counter_overflow = 0;
+
+ if (i915_mmio_reg_valid(hwmon->rg.energy_status_all))
+ pei->energy_counter_prev =
+ intel_uncore_read(uncore, hwmon->rg.energy_status_all);
+ else
+ pei->energy_counter_prev = 0;
+
intel_runtime_pm_put(uncore->rpm, wakeref);
le_sku_unit = cpu_to_le32(val_sku_unit);
hwmon->scl_shift_power = le32_get_bits(le_sku_unit, PKG_PWR_UNIT);
+ hwmon->scl_shift_energy = le32_get_bits(le_sku_unit, PKG_ENERGY_UNIT);
/*
* The value of power1_max is reset to the default on reboot, but is
diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
index 396b758aefce..1c70a7056be7 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.h
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -18,12 +18,20 @@ struct i915_hwmon_reg {
i915_reg_t pkg_power_sku_unit;
i915_reg_t pkg_power_sku;
i915_reg_t pkg_rapl_limit;
+ i915_reg_t energy_status_all;
+ i915_reg_t energy_status_tile;
+};
+
+struct i915_energy_info {
+ u32 energy_counter_overflow;
+ u32 energy_counter_prev;
};
struct i915_hwmon_drvdata {
struct i915_hwmon *dd_hwmon;
struct intel_uncore *dd_uncore;
struct device *dd_hwmon_dev;
+ struct i915_energy_info dd_ei; /* Energy info for energy1_input */
char dd_name[12];
};
@@ -37,8 +45,11 @@ struct i915_hwmon {
u32 power_max_initial_value;
int scl_shift_power;
+ int scl_shift_energy;
};
void i915_hwmon_register(struct drm_i915_private *i915);
void i915_hwmon_unregister(struct drm_i915_private *i915);
+
+int i915_energy_status_get(struct drm_i915_private *i915, u64 *energy);
#endif
diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
index 247561d7974c..d6dc6bc95627 100644
--- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
+++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
@@ -193,7 +193,9 @@
#define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
#define PCU_PACKAGE_POWER_SKU_UNIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
#define PKG_PWR_UNIT REG_GENMASK(3, 0)
+#define PKG_ENERGY_UNIT REG_GENMASK(12, 8)
#define PKG_TIME_UNIT REG_GENMASK(19, 16)
+#define PCU_PACKAGE_ENERGY_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x593c)
#define GEN6_RP_STATE_CAP _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
#define RP0_CAP_MASK REG_GENMASK(7, 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support Badal Nilawar
@ 2022-05-25 4:55 ` Dixit, Ashutosh
2022-05-25 4:56 ` Dixit, Ashutosh
1 sibling, 0 replies; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-05-25 4:55 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:40 -0700, Badal Nilawar wrote:
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index b94c11f2517f..b35c4de73f30 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -18,8 +18,10 @@
> /*
> * SF_* - scale factors for particular quantities according to hwmon spec.
> * - power - microwatts
> + * - energy - microjoules
> */
> #define SF_POWER 1000000
> +#define SF_ENERGY 1000000
>
> #define FIELD_SHIFT(__mask) \
> (BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
> @@ -94,6 +96,136 @@ _field_scale_and_write(struct i915_hwmon_drvdata *ddat, i915_reg_t rgadr,
> bits_to_clear, bits_to_set);
> }
>
> +/*
> + * _i915_energy1_input_sub - A custom function to obtain energy1_input.
> + * Use a custom function instead of the usual hwmon helpers in order to
> + * guarantee 64-bits of result to user-space.
We need to look into whether energy needs to be a custom interface or can
it just be the standard interface. The justification for custom interface
is that energy needs to be a u64 but the standard interface is already a
long which is at least 63 bits already. So the question is is that extra
bit needed which justifies a custom interface?
> + * Units are microjoules.
> + *
> + * The underlying hardware register is 32-bits and is subject to overflow.
> + * This function compensates for overflow of the 32-bit register by detecting
> + * wrap-around and incrementing an overflow counter.
> + * This only works if the register is sampled often enough to avoid
> + * missing an instance of overflow - achieved either by repeated
> + * queries through the API, or via a possible timer (future - TBD) that
> + * ensures values are read often enough to catch all overflows.
> + *
> + * How long before overflow? For example, with an example scaling bit
> + * shift of 14 bits (see register *PACKAGE_POWER_SKU_UNIT) and a power draw of
> + * 1000 watts, the 32-bit counter will overflow in approximately 4.36 minutes.
> + *
> + * Examples:
> + * 1 watt: (2^32 >> 14) / 1 W / (60 * 60 * 24) secs/day -> 3 days
> + * 1000 watts: (2^32 >> 14) / 1000 W / 60 secs/min -> 4.36 minutes
We need to "sample" the energy often enough to avoid 32 bit counter
overflow. But that can be a later patch.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support Badal Nilawar
2022-05-25 4:55 ` Dixit, Ashutosh
@ 2022-05-25 4:56 ` Dixit, Ashutosh
2022-05-25 5:25 ` Gupta, Anshuman
1 sibling, 1 reply; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-05-25 4:56 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:40 -0700, Badal Nilawar wrote:
>
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> As part of the System Managemenent Interface (SMI), use the HWMON
> subsystem to display energy utilization
Energy seems to be hardly ever exposed in the kernel. Searching for
'hwmon_energy' shows only a single hit in drivers/hwmon/scmi-hwmon.c. We
should see if there are real consumers for this and the reason for exposing
this.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support
2022-05-25 4:56 ` Dixit, Ashutosh
@ 2022-05-25 5:25 ` Gupta, Anshuman
0 siblings, 0 replies; 18+ messages in thread
From: Gupta, Anshuman @ 2022-05-25 5:25 UTC (permalink / raw)
To: Dixit, Ashutosh, Nilawar, Badal; +Cc: intel-gfx@lists.freedesktop.org
> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: Wednesday, May 25, 2022 10:27 AM
> To: Nilawar, Badal <badal.nilawar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Ewins, Jon <jon.ewins@intel.com>; Tauro,
> Riana <riana.tauro@intel.com>; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>
> Subject: Re: [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support
>
> On Mon, 23 May 2022 04:08:40 -0700, Badal Nilawar wrote:
> >
> > From: Dale B Stimson <dale.b.stimson@intel.com>
> >
> > As part of the System Managemenent Interface (SMI), use the HWMON
> > subsystem to display energy utilization
>
> Energy seems to be hardly ever exposed in the kernel. Searching for
> 'hwmon_energy' shows only a single hit in drivers/hwmon/scmi-hwmon.c.
> We should see if there are real consumers for this and the reason for
> exposing this.
rc6_idle test opens energy sys fs to test the Entergy numbers via rapl interface(AFAIR that interface is via x86 MSR) but that interface is not available to use for dgfx. If we can use hwmon energy interface. It can be leverage for IGT tests?
Thanks,
Anshuman Gupta.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support
2022-05-23 11:08 [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support Badal Nilawar
2022-05-23 11:08 ` [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Add HWMON power sensor support Badal Nilawar
2022-05-23 11:08 ` [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Add HWMON energy support Badal Nilawar
@ 2022-05-23 11:08 ` Badal Nilawar
2022-05-23 13:47 ` Jani Nikula
` (3 more replies)
2022-05-23 17:03 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add HWMON support Patchwork
` (2 subsequent siblings)
5 siblings, 4 replies; 18+ messages in thread
From: Badal Nilawar @ 2022-05-23 11:08 UTC (permalink / raw)
To: intel-gfx
From: Riana Tauro <riana.tauro@intel.com>
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display current voltage
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 8 +++
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 ++
drivers/gpu/drm/i915/i915_hwmon.c | 57 +++++++++++++++++++
drivers/gpu/drm/i915/i915_hwmon.h | 1 +
4 files changed, 70 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 7574421c326f..c70e06dd0333 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -33,3 +33,11 @@ Contact: dri-devel@lists.freedesktop.org
Description: RO. Card default power limit (default TDP setting).
Only supported for particular Intel i915 graphics platforms.
+
+What: /sys/devices/.../hwmon/hwmon<i>/in0_input
+Date: May 2022
+KernelVersion: 5.18
+Contact: dri-devel@lists.freedesktop.org
+Description: RO. Current Voltage in millivolt.
+
+ Only supported for particular Intel i915 graphics platforms.
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 7246eb870c7e..0adeca083de1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1475,6 +1475,10 @@
#define GEN6_GT_GFX_RC6p _MMIO(0x13810c)
#define GEN6_GT_GFX_RC6pp _MMIO(0x138110)
#define VLV_RENDER_C0_COUNT _MMIO(0x138118)
+
+#define GEN12_RPSTAT1 _MMIO(0x1381b4)
+#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
+
#define VLV_MEDIA_C0_COUNT _MMIO(0x13811c)
#define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index b35c4de73f30..e5f4293a8fdb 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -12,6 +12,7 @@
#include <linux/types.h>
#include "i915_drv.h"
+#include "gt/intel_gt_regs.h"
#include "i915_hwmon.h"
#include "intel_mchbar_regs.h"
@@ -301,8 +302,23 @@ static const struct hwmon_channel_info i915_power = {
.config = i915_config_power,
};
+/*
+ * HWMON SENSOR TYPE = hwmon_in
+ * - Voltage Input value (in0_input)
+ */
+static const u32 i915_config_in[] = {
+ HWMON_I_INPUT,
+ 0
+};
+
+static const struct hwmon_channel_info i915_in = {
+ .type = hwmon_in,
+ .config = i915_config_in,
+};
+
static const struct hwmon_channel_info *i915_info[] = {
&i915_power,
+ &i915_in,
NULL
};
@@ -370,6 +386,41 @@ i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
return ret;
}
+static umode_t
+i915_in_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr)
+{
+ struct drm_i915_private *i915 = ddat->dd_uncore->i915;
+
+ switch (attr) {
+ case hwmon_in_input:
+ return (IS_DG1(i915) || IS_DG2(i915)) ? 0444 : 0;
+ default:
+ return 0;
+ }
+
+ return 0444;
+}
+
+static int
+i915_in_read(struct i915_hwmon_drvdata *ddat, u32 attr, long *val)
+{
+ struct i915_hwmon *hwmon = ddat->dd_hwmon;
+ intel_wakeref_t wakeref;
+ u32 reg_value;
+
+ switch (attr) {
+ case hwmon_in_input:
+ with_intel_runtime_pm(ddat->dd_uncore->rpm, wakeref)
+ reg_value = intel_uncore_read(ddat->dd_uncore, hwmon->rg.gt_perf_status);
+ *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
static umode_t
i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
u32 attr, int channel)
@@ -379,6 +430,8 @@ i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
switch (type) {
case hwmon_power:
return i915_power_is_visible(ddat, attr, channel);
+ case hwmon_in:
+ return i915_in_is_visible(ddat, attr);
default:
return 0;
}
@@ -393,6 +446,8 @@ i915_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
switch (type) {
case hwmon_power:
return i915_power_read(ddat, attr, channel, val);
+ case hwmon_in:
+ return i915_in_read(ddat, attr, val);
default:
return -EOPNOTSUPP;
}
@@ -440,12 +495,14 @@ i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
hwmon->rg.energy_status_all = PCU_PACKAGE_ENERGY_STATUS;
hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
+ hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
} else {
hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
hwmon->rg.energy_status_all = INVALID_MMIO_REG;
hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
+ hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
}
wakeref = intel_runtime_pm_get(uncore->rpm);
diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
index 1c70a7056be7..90ae4acae74e 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.h
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -20,6 +20,7 @@ struct i915_hwmon_reg {
i915_reg_t pkg_rapl_limit;
i915_reg_t energy_status_all;
i915_reg_t energy_status_tile;
+ i915_reg_t gt_perf_status;
};
struct i915_energy_info {
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
@ 2022-05-23 13:47 ` Jani Nikula
2022-05-25 5:01 ` Dixit, Ashutosh
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2022-05-23 13:47 UTC (permalink / raw)
To: Badal Nilawar, intel-gfx
On Mon, 23 May 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> From: Riana Tauro <riana.tauro@intel.com>
>
> As part of the System Managemenent Interface (SMI), use the HWMON
> subsystem to display current voltage
>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 8 +++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 ++
> drivers/gpu/drm/i915/i915_hwmon.c | 57 +++++++++++++++++++
> drivers/gpu/drm/i915/i915_hwmon.h | 1 +
> 4 files changed, 70 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 7574421c326f..c70e06dd0333 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -33,3 +33,11 @@ Contact: dri-devel@lists.freedesktop.org
> Description: RO. Card default power limit (default TDP setting).
>
> Only supported for particular Intel i915 graphics platforms.
> +
> +What: /sys/devices/.../hwmon/hwmon<i>/in0_input
> +Date: May 2022
> +KernelVersion: 5.18
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RO. Current Voltage in millivolt.
> +
> + Only supported for particular Intel i915 graphics platforms.
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 7246eb870c7e..0adeca083de1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1475,6 +1475,10 @@
> #define GEN6_GT_GFX_RC6p _MMIO(0x13810c)
> #define GEN6_GT_GFX_RC6pp _MMIO(0x138110)
> #define VLV_RENDER_C0_COUNT _MMIO(0x138118)
> +
> +#define GEN12_RPSTAT1 _MMIO(0x1381b4)
> +#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> +
> #define VLV_MEDIA_C0_COUNT _MMIO(0x13811c)
>
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index b35c4de73f30..e5f4293a8fdb 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -12,6 +12,7 @@
> #include <linux/types.h>
>
> #include "i915_drv.h"
> +#include "gt/intel_gt_regs.h"
> #include "i915_hwmon.h"
> #include "intel_mchbar_regs.h"
Sort order.
>
> @@ -301,8 +302,23 @@ static const struct hwmon_channel_info i915_power = {
> .config = i915_config_power,
> };
>
> +/*
> + * HWMON SENSOR TYPE = hwmon_in
> + * - Voltage Input value (in0_input)
> + */
> +static const u32 i915_config_in[] = {
> + HWMON_I_INPUT,
> + 0
> +};
> +
> +static const struct hwmon_channel_info i915_in = {
> + .type = hwmon_in,
> + .config = i915_config_in,
> +};
> +
> static const struct hwmon_channel_info *i915_info[] = {
> &i915_power,
> + &i915_in,
> NULL
> };
>
> @@ -370,6 +386,41 @@ i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
> return ret;
> }
>
> +static umode_t
> +i915_in_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr)
> +{
> + struct drm_i915_private *i915 = ddat->dd_uncore->i915;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + return (IS_DG1(i915) || IS_DG2(i915)) ? 0444 : 0;
> + default:
> + return 0;
> + }
> +
> + return 0444;
> +}
> +
> +static int
> +i915_in_read(struct i915_hwmon_drvdata *ddat, u32 attr, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + with_intel_runtime_pm(ddat->dd_uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(ddat->dd_uncore, hwmon->rg.gt_perf_status);
> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> static umode_t
> i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> u32 attr, int channel)
> @@ -379,6 +430,8 @@ i915_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> switch (type) {
> case hwmon_power:
> return i915_power_is_visible(ddat, attr, channel);
> + case hwmon_in:
> + return i915_in_is_visible(ddat, attr);
> default:
> return 0;
> }
> @@ -393,6 +446,8 @@ i915_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> switch (type) {
> case hwmon_power:
> return i915_power_read(ddat, attr, channel, val);
> + case hwmon_in:
> + return i915_in_read(ddat, attr, val);
> default:
> return -EOPNOTSUPP;
> }
> @@ -440,12 +495,14 @@ i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
> hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> hwmon->rg.energy_status_all = PCU_PACKAGE_ENERGY_STATUS;
> hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
> + hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
> } else {
> hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> hwmon->rg.energy_status_all = INVALID_MMIO_REG;
> hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
> + hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
> }
>
> wakeref = intel_runtime_pm_get(uncore->rpm);
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> index 1c70a7056be7..90ae4acae74e 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.h
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -20,6 +20,7 @@ struct i915_hwmon_reg {
> i915_reg_t pkg_rapl_limit;
> i915_reg_t energy_status_all;
> i915_reg_t energy_status_tile;
> + i915_reg_t gt_perf_status;
> };
>
> struct i915_energy_info {
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-05-23 13:47 ` Jani Nikula
@ 2022-05-25 5:01 ` Dixit, Ashutosh
2022-06-10 22:35 ` Dixit, Ashutosh
2022-06-11 0:55 ` Dixit, Ashutosh
3 siblings, 0 replies; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-05-25 5:01 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:41 -0700, Badal Nilawar wrote:
>
> +static umode_t
> +i915_in_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr)
> +{
> + struct drm_i915_private *i915 = ddat->dd_uncore->i915;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + return (IS_DG1(i915) || IS_DG2(i915)) ? 0444 : 0;
Nit but maybe we should change this to:
return i915_mmio_reg_valid() ? 0444 : 0;
So that the platform checks are in one place in
i915_hwmon_get_preregistration_info()?
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-05-23 13:47 ` Jani Nikula
2022-05-25 5:01 ` Dixit, Ashutosh
@ 2022-06-10 22:35 ` Dixit, Ashutosh
2022-06-11 0:45 ` Dixit, Ashutosh
2022-06-11 0:55 ` Dixit, Ashutosh
3 siblings, 1 reply; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-06-10 22:35 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:41 -0700, Badal Nilawar wrote:
>
> @@ -370,6 +386,41 @@ i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
> return ret;
> }
>
> +static umode_t
> +i915_in_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr)
> +{
> + struct drm_i915_private *i915 = ddat->dd_uncore->i915;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + return (IS_DG1(i915) || IS_DG2(i915)) ? 0444 : 0;
> + default:
> + return 0;
> + }
> +
> + return 0444;
Don't return 0444 by default, let's just delete this line (return 0 by
default from the switch statement).
> +}
> +
> +static int
> +i915_in_read(struct i915_hwmon_drvdata *ddat, u32 attr, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + with_intel_runtime_pm(ddat->dd_uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(ddat->dd_uncore, hwmon->rg.gt_perf_status);
> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
Don't return 0 by default, let's just delete this line (return -EOPNOTSUPP
by default from the switch statement).
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support
2022-06-10 22:35 ` Dixit, Ashutosh
@ 2022-06-11 0:45 ` Dixit, Ashutosh
0 siblings, 0 replies; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-06-11 0:45 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Fri, 10 Jun 2022 15:35:23 -0700, Dixit, Ashutosh wrote:
>
> On Mon, 23 May 2022 04:08:41 -0700, Badal Nilawar wrote:
> >
> > @@ -370,6 +386,41 @@ i915_power_write(struct i915_hwmon_drvdata *ddat, u32 attr, int chan, long val)
> > return ret;
> > }
> >
> > +static umode_t
> > +i915_in_is_visible(const struct i915_hwmon_drvdata *ddat, u32 attr)
> > +{
> > + struct drm_i915_private *i915 = ddat->dd_uncore->i915;
> > +
> > + switch (attr) {
> > + case hwmon_in_input:
> > + return (IS_DG1(i915) || IS_DG2(i915)) ? 0444 : 0;
> > + default:
> > + return 0;
> > + }
> > +
> > + return 0444;
>
> Don't return 0444 by default, let's just delete this line (return 0 by
> default from the switch statement).
>
> > +}
> > +
> > +static int
> > +i915_in_read(struct i915_hwmon_drvdata *ddat, u32 attr, long *val)
> > +{
> > + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> > + intel_wakeref_t wakeref;
> > + u32 reg_value;
> > +
> > + switch (attr) {
> > + case hwmon_in_input:
> > + with_intel_runtime_pm(ddat->dd_uncore->rpm, wakeref)
> > + reg_value = intel_uncore_read(ddat->dd_uncore, hwmon->rg.gt_perf_status);
> > + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
>
> Don't return 0 by default, let's just delete this line (return -EOPNOTSUPP
> by default from the switch statement).
Sorry this one is ok, or return 0 from 'case hwmon_in_input' and delete
this line.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support
2022-05-23 11:08 ` [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
` (2 preceding siblings ...)
2022-06-10 22:35 ` Dixit, Ashutosh
@ 2022-06-11 0:55 ` Dixit, Ashutosh
3 siblings, 0 replies; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-06-11 0:55 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:41 -0700, Badal Nilawar wrote:
>
> +static int
> +i915_in_read(struct i915_hwmon_drvdata *ddat, u32 attr, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->dd_hwmon;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + with_intel_runtime_pm(ddat->dd_uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(ddat->dd_uncore, hwmon->rg.gt_perf_status);
> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
A comment here that voltage is in units of 2.5 mV would be nice.
> @@ -440,12 +495,14 @@ i915_hwmon_get_preregistration_info(struct drm_i915_private *i915)
> hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> hwmon->rg.energy_status_all = PCU_PACKAGE_ENERGY_STATUS;
> hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
> + hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
> } else {
> hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> hwmon->rg.energy_status_all = INVALID_MMIO_REG;
> hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
> + hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
Looks like this is supportable on XEHPSDV too, so let's add that too.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add HWMON support
2022-05-23 11:08 [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support Badal Nilawar
` (2 preceding siblings ...)
2022-05-23 11:08 ` [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
@ 2022-05-23 17:03 ` Patchwork
2022-05-23 17:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-05-24 1:36 ` [Intel-gfx] [PATCH 0/3] " Dixit, Ashutosh
5 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2022-05-23 17:03 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add HWMON support
URL : https://patchwork.freedesktop.org/series/104278/
State : warning
== Summary ==
Error: dim checkpatch failed
461a66ade2f0 drm/i915/hwmon: Add HWMON power sensor support
Traceback (most recent call last):
File "scripts/spdxcheck.py", line 10, in <module>
import git
ModuleNotFoundError: No module named 'git'
Traceback (most recent call last):
File "scripts/spdxcheck.py", line 10, in <module>
import git
ModuleNotFoundError: No module named 'git'
-:15: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#15:
new file mode 100644
-:135: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__mask' - possible side-effects?
#135: FILE: drivers/gpu/drm/i915/i915_hwmon.c:24:
+#define FIELD_SHIFT(__mask) \
+ (BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
+ BUILD_BUG_ON_ZERO((__mask) == 0) + \
+ __bf_shf(__mask))
total: 0 errors, 1 warnings, 1 checks, 559 lines checked
14d8fb223c66 drm/i915/hwmon: Add HWMON energy support
629e24ca4b59 drm/i915/hwmon: Add HWMON current voltage support
^ permalink raw reply [flat|nested] 18+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add HWMON support
2022-05-23 11:08 [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support Badal Nilawar
` (3 preceding siblings ...)
2022-05-23 17:03 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add HWMON support Patchwork
@ 2022-05-23 17:26 ` Patchwork
2022-05-24 1:36 ` [Intel-gfx] [PATCH 0/3] " Dixit, Ashutosh
5 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2022-05-23 17:26 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 22672 bytes --]
== Series Details ==
Series: drm/i915: Add HWMON support
URL : https://patchwork.freedesktop.org/series/104278/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11691 -> Patchwork_104278v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_104278v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_104278v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/index.html
Participating hosts (45 -> 44)
------------------------------
Additional (2): bat-dg2-8 fi-tgl-u2
Missing (3): fi-bsw-kefka fi-icl-u2 fi-bsw-n3050
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_104278v1:
### IGT changes ###
#### Possible regressions ####
* igt@core_hotunplug@unbind-rebind:
- fi-bsw-nick: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/fi-bsw-nick/igt@core_hotunplug@unbind-rebind.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-bsw-nick/igt@core_hotunplug@unbind-rebind.html
New tests
---------
New tests have been introduced between CI_DRM_11691 and Patchwork_104278v1:
### New IGT tests (112) ###
* igt@dmabuf@all@dma_fence_chain:
- Statuses : 36 pass(s)
- Exec time: [5.22, 7.62] s
* igt@gem_busy@busy:
- Statuses : 1 skip(s)
- Exec time: [0.0] s
* igt@gem_busy@busy@all:
- Statuses : 42 pass(s)
- Exec time: [0.01, 0.32] s
* igt@gem_exec_parallel@engines:
- Statuses : 1 skip(s)
- Exec time: [0.0] s
* igt@gem_exec_parallel@engines@basic:
- Statuses : 41 pass(s) 1 skip(s)
- Exec time: [0.00, 10.67] s
* igt@gem_exec_parallel@engines@contexts:
- Statuses : 39 pass(s) 3 skip(s)
- Exec time: [0.00, 16.30] s
* igt@gem_exec_parallel@engines@fds:
- Statuses : 37 pass(s) 5 skip(s)
- Exec time: [0.0, 15.30] s
* igt@gem_exec_store@basic:
- Statuses : 41 pass(s) 2 skip(s)
- Exec time: [0.0, 0.17] s
* igt@gem_wait@busy:
- Statuses : 1 skip(s)
- Exec time: [0.0] s
* igt@gem_wait@busy@all:
- Statuses : 42 pass(s)
- Exec time: [0.51, 0.60] s
* igt@gem_wait@wait:
- Statuses : 1 skip(s)
- Exec time: [0.0] s
* igt@gem_wait@wait@all:
- Statuses : 42 pass(s)
- Exec time: [1.01, 1.11] s
* igt@kms_flip@basic-flip-vs-dpms@a-dp1:
- Statuses : 3 pass(s)
- Exec time: [0.70, 0.85] s
* igt@kms_flip@basic-flip-vs-dpms@a-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.61, 0.74] s
* igt@kms_flip@basic-flip-vs-dpms@a-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.55, 2.11] s
* igt@kms_flip@basic-flip-vs-dpms@a-edp1:
- Statuses : 1 dmesg-warn(s) 8 pass(s)
- Exec time: [2.66, 3.43] s
* igt@kms_flip@basic-flip-vs-dpms@a-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.64, 0.88] s
* igt@kms_flip@basic-flip-vs-dpms@a-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.65, 1.08] s
* igt@kms_flip@basic-flip-vs-dpms@a-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.78] s
* igt@kms_flip@basic-flip-vs-dpms@a-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.68, 1.70] s
* igt@kms_flip@basic-flip-vs-dpms@b-dp1:
- Statuses : 3 pass(s)
- Exec time: [0.70, 0.81] s
* igt@kms_flip@basic-flip-vs-dpms@b-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.60, 0.73] s
* igt@kms_flip@basic-flip-vs-dpms@b-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.35, 1.67] s
* igt@kms_flip@basic-flip-vs-dpms@b-edp1:
- Statuses : 9 pass(s)
- Exec time: [2.29, 2.51] s
* igt@kms_flip@basic-flip-vs-dpms@b-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.64, 0.85] s
* igt@kms_flip@basic-flip-vs-dpms@b-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.63, 1.02] s
* igt@kms_flip@basic-flip-vs-dpms@b-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.41] s
* igt@kms_flip@basic-flip-vs-dpms@b-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.65, 1.37] s
* igt@kms_flip@basic-flip-vs-dpms@c-dp1:
- Statuses : 2 pass(s)
- Exec time: [0.77, 0.81] s
* igt@kms_flip@basic-flip-vs-dpms@c-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.60, 0.72] s
* igt@kms_flip@basic-flip-vs-dpms@c-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.34, 1.64] s
* igt@kms_flip@basic-flip-vs-dpms@c-edp1:
- Statuses : 9 pass(s)
- Exec time: [2.30, 2.51] s
* igt@kms_flip@basic-flip-vs-dpms@c-hdmi-a1:
- Statuses : 7 pass(s)
- Exec time: [0.65, 0.85] s
* igt@kms_flip@basic-flip-vs-dpms@c-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.61, 1.08] s
* igt@kms_flip@basic-flip-vs-dpms@c-vga1:
- Statuses : 3 pass(s)
- Exec time: [0.72, 0.86] s
* igt@kms_flip@basic-flip-vs-dpms@d-dsi1:
- Statuses : 1 pass(s)
- Exec time: [1.61] s
* igt@kms_flip@basic-flip-vs-dpms@d-edp1:
- Statuses : 3 pass(s)
- Exec time: [2.30, 2.37] s
* igt@kms_flip@basic-flip-vs-modeset@a-dp1:
- Statuses : 3 pass(s)
- Exec time: [0.72, 0.87] s
* igt@kms_flip@basic-flip-vs-modeset@a-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.62, 0.75] s
* igt@kms_flip@basic-flip-vs-modeset@a-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.56, 2.10] s
* igt@kms_flip@basic-flip-vs-modeset@a-edp1:
- Statuses : 2 dmesg-warn(s) 7 pass(s)
- Exec time: [2.66, 3.81] s
* igt@kms_flip@basic-flip-vs-modeset@a-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.71, 0.91] s
* igt@kms_flip@basic-flip-vs-modeset@a-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.66, 1.16] s
* igt@kms_flip@basic-flip-vs-modeset@a-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.80] s
* igt@kms_flip@basic-flip-vs-modeset@a-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.71, 1.59] s
* igt@kms_flip@basic-flip-vs-modeset@b-dp1:
- Statuses : 3 pass(s)
- Exec time: [0.71, 0.79] s
* igt@kms_flip@basic-flip-vs-modeset@b-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.62, 0.73] s
* igt@kms_flip@basic-flip-vs-modeset@b-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.34, 1.59] s
* igt@kms_flip@basic-flip-vs-modeset@b-edp1:
- Statuses : 9 pass(s)
- Exec time: [2.28, 3.46] s
* igt@kms_flip@basic-flip-vs-modeset@b-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.60, 0.81] s
* igt@kms_flip@basic-flip-vs-modeset@b-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.59, 0.91] s
* igt@kms_flip@basic-flip-vs-modeset@b-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.40] s
* igt@kms_flip@basic-flip-vs-modeset@b-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.66, 1.28] s
* igt@kms_flip@basic-flip-vs-modeset@c-dp1:
- Statuses : 2 pass(s)
- Exec time: [0.76, 0.80] s
* igt@kms_flip@basic-flip-vs-modeset@c-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.61, 0.75] s
* igt@kms_flip@basic-flip-vs-modeset@c-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.34, 1.55] s
* igt@kms_flip@basic-flip-vs-modeset@c-edp1:
- Statuses : 9 pass(s)
- Exec time: [2.31, 3.45] s
* igt@kms_flip@basic-flip-vs-modeset@c-hdmi-a1:
- Statuses : 7 pass(s)
- Exec time: [0.59, 0.81] s
* igt@kms_flip@basic-flip-vs-modeset@c-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.59, 0.90] s
* igt@kms_flip@basic-flip-vs-modeset@c-vga1:
- Statuses : 3 pass(s)
- Exec time: [0.73, 0.88] s
* igt@kms_flip@basic-flip-vs-modeset@d-dsi1:
- Statuses : 1 pass(s)
- Exec time: [1.58] s
* igt@kms_flip@basic-flip-vs-modeset@d-edp1:
- Statuses : 3 pass(s)
- Exec time: [2.37, 3.46] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1:
- Statuses : 3 pass(s)
- Exec time: [1.04, 1.16] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.90, 1.07] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.87, 1.97] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1:
- Statuses : 9 pass(s)
- Exec time: [1.97, 2.75] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.97, 1.25] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.95, 1.41] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.97] s
* igt@kms_flip@basic-flip-vs-wf_vblank@a-vga1:
- Statuses : 10 pass(s)
- Exec time: [1.02, 1.88] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1:
- Statuses : 3 pass(s)
- Exec time: [1.02, 1.15] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.90, 1.07] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.72, 1.90] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
- Statuses : 9 pass(s)
- Exec time: [1.98, 2.50] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.92, 1.20] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.90, 1.31] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.91] s
* igt@kms_flip@basic-flip-vs-wf_vblank@b-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.96, 1.63] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-dp1:
- Statuses : 2 pass(s)
- Exec time: [1.0, 1.01] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.90, 1.07] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.67, 1.90] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1:
- Statuses : 9 pass(s)
- Exec time: [1.97, 2.50] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a1:
- Statuses : 7 pass(s)
- Exec time: [0.90, 1.19] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.90, 1.32] s
* igt@kms_flip@basic-flip-vs-wf_vblank@c-vga1:
- Statuses : 3 pass(s)
- Exec time: [1.12, 1.16] s
* igt@kms_flip@basic-flip-vs-wf_vblank@d-dsi1:
- Statuses : 1 pass(s)
- Exec time: [1.65] s
* igt@kms_flip@basic-flip-vs-wf_vblank@d-edp1:
- Statuses : 3 pass(s)
- Exec time: [1.93, 2.24] s
* igt@kms_flip@basic-plain-flip@a-dp1:
- Statuses : 3 pass(s)
- Exec time: [0.73, 0.80] s
* igt@kms_flip@basic-plain-flip@a-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.62, 0.70] s
* igt@kms_flip@basic-plain-flip@a-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.52, 1.63] s
* igt@kms_flip@basic-plain-flip@a-edp1:
- Statuses : 9 pass(s)
- Exec time: [1.75, 2.47] s
* igt@kms_flip@basic-plain-flip@a-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.68, 0.88] s
* igt@kms_flip@basic-plain-flip@a-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.67, 1.13] s
* igt@kms_flip@basic-plain-flip@a-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.58] s
* igt@kms_flip@basic-plain-flip@a-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.74, 1.59] s
* igt@kms_flip@basic-plain-flip@b-dp1:
- Statuses : 3 pass(s)
- Exec time: [0.71, 0.72] s
* igt@kms_flip@basic-plain-flip@b-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.62, 0.68] s
* igt@kms_flip@basic-plain-flip@b-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.43, 1.47] s
* igt@kms_flip@basic-plain-flip@b-edp1:
- Statuses : 9 pass(s)
- Exec time: [1.72, 2.25] s
* igt@kms_flip@basic-plain-flip@b-hdmi-a1:
- Statuses : 9 pass(s)
- Exec time: [0.61, 0.83] s
* igt@kms_flip@basic-plain-flip@b-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.61, 1.02] s
* igt@kms_flip@basic-plain-flip@b-lvds1:
- Statuses : 1 pass(s)
- Exec time: [1.50] s
* igt@kms_flip@basic-plain-flip@b-vga1:
- Statuses : 10 pass(s)
- Exec time: [0.67, 1.38] s
* igt@kms_flip@basic-plain-flip@c-dp1:
- Statuses : 2 pass(s)
- Exec time: [0.72, 0.73] s
* igt@kms_flip@basic-plain-flip@c-dp2:
- Statuses : 3 pass(s)
- Exec time: [0.62, 0.69] s
* igt@kms_flip@basic-plain-flip@c-dsi1:
- Statuses : 3 pass(s)
- Exec time: [1.43, 1.51] s
* igt@kms_flip@basic-plain-flip@c-edp1:
- Statuses : 9 pass(s)
- Exec time: [1.73, 2.24] s
* igt@kms_flip@basic-plain-flip@c-hdmi-a1:
- Statuses : 7 pass(s)
- Exec time: [0.61, 0.83] s
* igt@kms_flip@basic-plain-flip@c-hdmi-a2:
- Statuses : 5 pass(s)
- Exec time: [0.61, 1.03] s
* igt@kms_flip@basic-plain-flip@c-vga1:
- Statuses : 3 pass(s)
- Exec time: [0.71, 0.84] s
* igt@kms_flip@basic-plain-flip@d-dsi1:
- Statuses : 1 pass(s)
- Exec time: [1.46] s
* igt@kms_flip@basic-plain-flip@d-edp1:
- Statuses : 3 pass(s)
- Exec time: [1.74, 1.78] s
Known issues
------------
Here are the changes found in Patchwork_104278v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-tgl-u2: NOTRUN -> [SKIP][3] ([i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html
* igt@i915_selftest@live@hangcheck:
- bat-dg1-6: [PASS][4] -> [DMESG-FAIL][5] ([i915#4494] / [i915#4957])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
* igt@i915_selftest@live@reset:
- fi-bdw-5557u: [PASS][6] -> [INCOMPLETE][7] ([i915#6000])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/fi-bdw-5557u/igt@i915_selftest@live@reset.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-bdw-5557u/igt@i915_selftest@live@reset.html
* igt@kms_busy@basic@flip:
- fi-tgl-u2: NOTRUN -> [DMESG-WARN][8] ([i915#402]) +1 similar issue
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@kms_busy@basic@flip.html
* igt@kms_busy@basic@modeset:
- bat-adlp-4: [PASS][9] -> [DMESG-WARN][10] ([i915#3576]) +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/bat-adlp-4/igt@kms_busy@basic@modeset.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/bat-adlp-4/igt@kms_busy@basic@modeset.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-pnv-d510: NOTRUN -> [SKIP][11] ([fdo#109271])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-pnv-d510/igt@kms_chamelium@common-hpd-after-suspend.html
- fi-rkl-11600: NOTRUN -> [SKIP][12] ([fdo#111827])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-rkl-11600/igt@kms_chamelium@common-hpd-after-suspend.html
* igt@kms_chamelium@dp-hpd-fast:
- fi-tgl-u2: NOTRUN -> [SKIP][13] ([fdo#109284] / [fdo#111827]) +7 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@kms_chamelium@dp-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-tgl-u2: NOTRUN -> [SKIP][14] ([i915#4103]) +1 similar issue
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-tgl-u2: NOTRUN -> [SKIP][15] ([fdo#109285])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-tgl-u2: NOTRUN -> [SKIP][16] ([i915#3555])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-userptr:
- fi-tgl-u2: NOTRUN -> [SKIP][17] ([i915#3301])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-tgl-u2/igt@prime_vgem@basic-userptr.html
#### Possible fixes ####
* igt@i915_selftest@live@requests:
- fi-pnv-d510: [DMESG-FAIL][18] ([i915#4528]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/fi-pnv-d510/igt@i915_selftest@live@requests.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-pnv-d510/igt@i915_selftest@live@requests.html
* igt@i915_selftest@live@workarounds:
- {bat-adlp-6}: [DMESG-FAIL][20] -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/bat-adlp-6/igt@i915_selftest@live@workarounds.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/bat-adlp-6/igt@i915_selftest@live@workarounds.html
- bat-adlp-4: [DMESG-WARN][22] -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/bat-adlp-4/igt@i915_selftest@live@workarounds.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/bat-adlp-4/igt@i915_selftest@live@workarounds.html
* igt@kms_flip@basic-flip-vs-modeset@b-edp1 (NEW):
- bat-adlp-4: [DMESG-WARN][24] ([i915#3576]) -> [PASS][25] +1 similar issue
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@b-edp1.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@b-edp1.html
* igt@kms_force_connector_basic@force-connector-state:
- {bat-adlp-6}: [DMESG-WARN][26] ([i915#3576]) -> [PASS][27] +2 similar issues
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/bat-adlp-6/igt@kms_force_connector_basic@force-connector-state.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/bat-adlp-6/igt@kms_force_connector_basic@force-connector-state.html
#### Warnings ####
* igt@i915_selftest@live@hangcheck:
- fi-hsw-g3258: [INCOMPLETE][28] ([i915#3303] / [i915#4785]) -> [INCOMPLETE][29] ([i915#4785])
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-rkl-11600: [INCOMPLETE][30] ([i915#5982]) -> [FAIL][31] ([fdo#103375])
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11691/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
[i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
[i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
[i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
[i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153
[i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
[i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763
[i915#5885]: https://gitlab.freedesktop.org/drm/intel/issues/5885
[i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903
[i915#5982]: https://gitlab.freedesktop.org/drm/intel/issues/5982
[i915#6000]: https://gitlab.freedesktop.org/drm/intel/issues/6000
Build changes
-------------
* Linux: CI_DRM_11691 -> Patchwork_104278v1
CI-20190529: 20190529
CI_DRM_11691: 20762208303d98e86cd19306752ee2c830fe4f2b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6485: 51663917b40d36086cc1c555ce4f67b22937694d @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_104278v1: 20762208303d98e86cd19306752ee2c830fe4f2b @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
6bdab94e0086 drm/i915/hwmon: Add HWMON current voltage support
f52dae00490e drm/i915/hwmon: Add HWMON energy support
651bec8e7652 drm/i915/hwmon: Add HWMON power sensor support
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104278v1/index.html
[-- Attachment #2: Type: text/html, Size: 25877 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support
2022-05-23 11:08 [Intel-gfx] [PATCH 0/3] drm/i915: Add HWMON support Badal Nilawar
` (4 preceding siblings ...)
2022-05-23 17:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2022-05-24 1:36 ` Dixit, Ashutosh
5 siblings, 0 replies; 18+ messages in thread
From: Dixit, Ashutosh @ 2022-05-24 1:36 UTC (permalink / raw)
To: Badal Nilawar; +Cc: intel-gfx
On Mon, 23 May 2022 04:08:38 -0700, Badal Nilawar wrote:
>
> This series adds the HWMON support for DG1, DG2
>
> Dale B Stimson (2):
> drm/i915/hwmon: Add HWMON power sensor support
> drm/i915/hwmon: Add HWMON energy support
I would suggest a slight reorganization of the series. I think the first
patch should just add the HWMON infrastrusture into i915. Subsequent
patches should add the various hwmon features exposed in sysfs, mostly one
patch per feature.
So at least for now I'd add a first patch adding the HWMON infra to
i915. And follow on with the feature patches. Thanks.
> Riana Tauro (1):
> drm/i915/hwmon: Add HWMON current voltage support
>
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 43 ++
> drivers/gpu/drm/i915/Kconfig | 1 +
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 +
> drivers/gpu/drm/i915/i915_driver.c | 5 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_hwmon.c | 620 ++++++++++++++++++
> drivers/gpu/drm/i915/i915_hwmon.h | 56 ++
> drivers/gpu/drm/i915/i915_reg.h | 15 +
> drivers/gpu/drm/i915/intel_mchbar_regs.h | 11 +
> 10 files changed, 758 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread