* [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-26 13:30 ` Guenter Roeck
2022-08-29 17:26 ` Dixit, Ashutosh
2022-08-25 13:21 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
` (5 subsequent siblings)
6 siblings, 2 replies; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Dale B Stimson <dale.b.stimson@intel.com>
The i915 HWMON module will be used to expose voltage, power and energy
values for dGfx. Here we set up i915 hwmon infrastructure including i915
hwmon registration, basic data structures and functions.
v2:
- Create HWMON infra patch (Ashutosh)
- Fixed review comments (Jani)
- Remove "select HWMON" from i915/Kconfig (Jani)
v3: Use hwm_ prefix for static functions (Ashutosh)
v4: s/#ifdef CONFIG_HWMON/#if IS_REACHABLE(CONFIG_HWMON)/ since the former
doesn't work if hwmon is compiled as a module (Guenter)
v5: Fixed review comments (Jani)
Cc: Guenter Roeck <linux@roeck-us.net>
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>
---
drivers/gpu/drm/i915/Makefile | 3 +
drivers/gpu/drm/i915/i915_driver.c | 5 ++
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_hwmon.c | 136 +++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_hwmon.h | 20 +++++
5 files changed, 166 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 522ef9b4aff3..2b235f747490 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -208,6 +208,9 @@ i915-y += gt/uc/intel_uc.o \
# graphics system controller (GSC) support
i915-y += gt/intel_gsc.o
+# graphics hardware monitoring (HWMON) support
+i915-$(CONFIG_HWMON) += i915_hwmon.o
+
# modesetting core code
i915-y += \
display/hsw_ips.o \
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 1332c70370a6..248deecd26a5 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"
@@ -736,6 +737,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);
@@ -762,6 +765,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 69ce6db6a7c1..7b5b10df3404 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -705,6 +705,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..103dd543a214
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/types.h>
+
+#include "i915_drv.h"
+#include "i915_hwmon.h"
+#include "i915_reg.h"
+#include "intel_mchbar_regs.h"
+
+struct hwm_reg {
+};
+
+struct hwm_drvdata {
+ struct i915_hwmon *hwmon;
+ struct intel_uncore *uncore;
+ struct device *hwmon_dev;
+ char name[12];
+};
+
+struct i915_hwmon {
+ struct hwm_drvdata ddat;
+ struct mutex hwmon_lock; /* counter overflow logic and rmw */
+ struct hwm_reg rg;
+};
+
+static const struct hwmon_channel_info *hwm_info[] = {
+ NULL
+};
+
+static umode_t
+hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ default:
+ return 0;
+ }
+}
+
+static int
+hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long *val)
+{
+ switch (type) {
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long val)
+{
+ switch (type) {
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static const struct hwmon_ops hwm_ops = {
+ .is_visible = hwm_is_visible,
+ .read = hwm_read,
+ .write = hwm_write,
+};
+
+static const struct hwmon_chip_info hwm_chip_info = {
+ .ops = &hwm_ops,
+ .info = hwm_info,
+};
+
+static void
+hwm_get_preregistration_info(struct drm_i915_private *i915)
+{
+}
+
+void i915_hwmon_register(struct drm_i915_private *i915)
+{
+ struct device *dev = i915->drm.dev;
+ struct i915_hwmon *hwmon;
+ struct device *hwmon_dev;
+ struct hwm_drvdata *ddat;
+
+ /* hwmon is available only for dGfx */
+ if (!IS_DGFX(i915))
+ return;
+
+ hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
+ if (!hwmon)
+ return;
+
+ i915->hwmon = hwmon;
+ mutex_init(&hwmon->hwmon_lock);
+ ddat = &hwmon->ddat;
+
+ ddat->hwmon = hwmon;
+ ddat->uncore = &i915->uncore;
+ snprintf(ddat->name, sizeof(ddat->name), "i915");
+
+ hwm_get_preregistration_info(i915);
+
+ /* hwmon_dev points to device hwmon<i> */
+ hwmon_dev = hwmon_device_register_with_info(dev, ddat->name,
+ ddat,
+ &hwm_chip_info,
+ NULL);
+ if (IS_ERR(hwmon_dev)) {
+ mutex_destroy(&hwmon->hwmon_lock);
+ i915->hwmon = NULL;
+ kfree(hwmon);
+ return;
+ }
+
+ ddat->hwmon_dev = hwmon_dev;
+}
+
+void i915_hwmon_unregister(struct drm_i915_private *i915)
+{
+ struct i915_hwmon *hwmon;
+ struct hwm_drvdata *ddat;
+
+ hwmon = fetch_and_zero(&i915->hwmon);
+ if (!hwmon)
+ return;
+
+ ddat = &hwmon->ddat;
+ if (ddat->hwmon_dev)
+ hwmon_device_unregister(ddat->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..7ca9cf2c34c9
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef __I915_HWMON_H__
+#define __I915_HWMON_H__
+
+struct drm_i915_private;
+
+#if IS_REACHABLE(CONFIG_HWMON)
+void i915_hwmon_register(struct drm_i915_private *i915);
+void i915_hwmon_unregister(struct drm_i915_private *i915);
+#else
+static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
+static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
+#endif
+
+#endif /* __I915_HWMON_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure
2022-08-25 13:21 ` [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure Badal Nilawar
@ 2022-08-26 13:30 ` Guenter Roeck
2022-08-29 17:26 ` Dixit, Ashutosh
1 sibling, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2022-08-26 13:30 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-gfx, ashutosh.dixit, riana.tauro, anshuman.gupta,
jon.ewins, linux-hwmon
On Thu, Aug 25, 2022 at 06:51:12PM +0530, Badal Nilawar wrote:
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> The i915 HWMON module will be used to expose voltage, power and energy
> values for dGfx. Here we set up i915 hwmon infrastructure including i915
> hwmon registration, basic data structures and functions.
>
> v2:
> - Create HWMON infra patch (Ashutosh)
> - Fixed review comments (Jani)
> - Remove "select HWMON" from i915/Kconfig (Jani)
> v3: Use hwm_ prefix for static functions (Ashutosh)
> v4: s/#ifdef CONFIG_HWMON/#if IS_REACHABLE(CONFIG_HWMON)/ since the former
> doesn't work if hwmon is compiled as a module (Guenter)
> v5: Fixed review comments (Jani)
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> 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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +
> drivers/gpu/drm/i915/i915_driver.c | 5 ++
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_hwmon.c | 136 +++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_hwmon.h | 20 +++++
> 5 files changed, 166 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
> create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 522ef9b4aff3..2b235f747490 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -208,6 +208,9 @@ i915-y += gt/uc/intel_uc.o \
> # graphics system controller (GSC) support
> i915-y += gt/intel_gsc.o
>
> +# graphics hardware monitoring (HWMON) support
> +i915-$(CONFIG_HWMON) += i915_hwmon.o
> +
> # modesetting core code
> i915-y += \
> display/hsw_ips.o \
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 1332c70370a6..248deecd26a5 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"
> @@ -736,6 +737,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);
> @@ -762,6 +765,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 69ce6db6a7c1..7b5b10df3404 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -705,6 +705,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..103dd543a214
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/types.h>
> +
> +#include "i915_drv.h"
> +#include "i915_hwmon.h"
> +#include "i915_reg.h"
> +#include "intel_mchbar_regs.h"
> +
> +struct hwm_reg {
> +};
> +
> +struct hwm_drvdata {
> + struct i915_hwmon *hwmon;
> + struct intel_uncore *uncore;
> + struct device *hwmon_dev;
> + char name[12];
> +};
> +
> +struct i915_hwmon {
> + struct hwm_drvdata ddat;
> + struct mutex hwmon_lock; /* counter overflow logic and rmw */
> + struct hwm_reg rg;
> +};
> +
> +static const struct hwmon_channel_info *hwm_info[] = {
> + NULL
> +};
> +
> +static umode_t
> +hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + switch (type) {
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int
> +hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + switch (type) {
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static const struct hwmon_ops hwm_ops = {
> + .is_visible = hwm_is_visible,
> + .read = hwm_read,
> + .write = hwm_write,
> +};
> +
> +static const struct hwmon_chip_info hwm_chip_info = {
> + .ops = &hwm_ops,
> + .info = hwm_info,
> +};
> +
> +static void
> +hwm_get_preregistration_info(struct drm_i915_private *i915)
> +{
> +}
> +
> +void i915_hwmon_register(struct drm_i915_private *i915)
> +{
> + struct device *dev = i915->drm.dev;
> + struct i915_hwmon *hwmon;
> + struct device *hwmon_dev;
> + struct hwm_drvdata *ddat;
> +
> + /* hwmon is available only for dGfx */
> + if (!IS_DGFX(i915))
> + return;
> +
> + hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
> + if (!hwmon)
> + return;
> +
> + i915->hwmon = hwmon;
> + mutex_init(&hwmon->hwmon_lock);
> + ddat = &hwmon->ddat;
> +
> + ddat->hwmon = hwmon;
> + ddat->uncore = &i915->uncore;
> + snprintf(ddat->name, sizeof(ddat->name), "i915");
> +
> + hwm_get_preregistration_info(i915);
> +
> + /* hwmon_dev points to device hwmon<i> */
> + hwmon_dev = hwmon_device_register_with_info(dev, ddat->name,
> + ddat,
> + &hwm_chip_info,
> + NULL);
> + if (IS_ERR(hwmon_dev)) {
> + mutex_destroy(&hwmon->hwmon_lock);
> + i915->hwmon = NULL;
> + kfree(hwmon);
> + return;
> + }
> +
> + ddat->hwmon_dev = hwmon_dev;
> +}
> +
> +void i915_hwmon_unregister(struct drm_i915_private *i915)
> +{
> + struct i915_hwmon *hwmon;
> + struct hwm_drvdata *ddat;
> +
> + hwmon = fetch_and_zero(&i915->hwmon);
> + if (!hwmon)
> + return;
> +
> + ddat = &hwmon->ddat;
> + if (ddat->hwmon_dev)
> + hwmon_device_unregister(ddat->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..7ca9cf2c34c9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef __I915_HWMON_H__
> +#define __I915_HWMON_H__
> +
> +struct drm_i915_private;
> +
> +#if IS_REACHABLE(CONFIG_HWMON)
> +void i915_hwmon_register(struct drm_i915_private *i915);
> +void i915_hwmon_unregister(struct drm_i915_private *i915);
> +#else
> +static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
> +static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
> +#endif
> +
> +#endif /* __I915_HWMON_H__ */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure
2022-08-25 13:21 ` [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure Badal Nilawar
2022-08-26 13:30 ` Guenter Roeck
@ 2022-08-29 17:26 ` Dixit, Ashutosh
1 sibling, 0 replies; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-08-29 17:26 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On Thu, 25 Aug 2022 06:21:12 -0700, Badal Nilawar wrote:
>
A couple of minor observations below but otherwise this patch is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> new file mode 100644
> index 000000000000..103dd543a214
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
/snip/
> +struct hwm_reg {
> +};
> +
> +struct hwm_drvdata {
> + struct i915_hwmon *hwmon;
> + struct intel_uncore *uncore;
Instead of 'struct intel_uncore' we could have a 'struct intel_gt' here
since intel_gt is a higher level but I think uncore is fine and anyway has
a backpointer to intel_gt should we need it. So no changes needed.
> + struct device *hwmon_dev;
> + char name[12];
> +};
> +
> +struct i915_hwmon {
> + struct hwm_drvdata ddat;
> + struct mutex hwmon_lock; /* counter overflow logic and rmw */
> + struct hwm_reg rg;
> +};
Somebody looking at just this patch might wonder why we have two data
structs hwm_drvdata and i915_hwmon, rather than just one. The answer
becomes clear in a later patch and that of course is that i915 exposes
multiple hwmon devices. Anyway, just an observation, no changes required.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-08-25 13:21 ` [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-29 17:30 ` Dixit, Ashutosh
2022-09-12 14:09 ` Gupta, Anshuman
2022-08-25 13:21 ` [PATCH 3/7] drm/i915/hwmon: Power PL1 limit and TDP setting Badal Nilawar
` (4 subsequent siblings)
6 siblings, 2 replies; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Riana Tauro <riana.tauro@intel.com>
Use i915 HWMON subsystem to display current input voltage.
v2:
- Updated date and kernel version in feature description
- Fixed review comments (Ashutosh)
v3: Use macro HWMON_CHANNEL_INFO to define hwmon channel (Guenter)
v4:
- Fixed review comments (Ashutosh)
- Use hwm_ prefix for static functions (Ashutosh)
Cc: Guenter Roeck <linux@roeck-us.net>
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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 7 +++
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 ++
drivers/gpu/drm/i915/i915_hwmon.c | 47 +++++++++++++++++++
3 files changed, 57 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
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..24c4b7477d51
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -0,0 +1,7 @@
+What: /sys/devices/.../hwmon/hwmon<i>/in0_input
+Date: June 2022
+KernelVersion: 5.19
+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 94f9ddcfb3a5..5d4fbda4d326 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1508,6 +1508,9 @@
#define VLV_RENDER_C0_COUNT _MMIO(0x138118)
#define VLV_MEDIA_C0_COUNT _MMIO(0x13811c)
+#define GEN12_RPSTAT1 _MMIO(0x1381b4)
+#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
+
#define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
#define GEN11_CSME (31)
#define GEN11_GUNIT (28)
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 103dd543a214..2192d0fd4c66 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -11,8 +11,10 @@
#include "i915_hwmon.h"
#include "i915_reg.h"
#include "intel_mchbar_regs.h"
+#include "gt/intel_gt_regs.h"
struct hwm_reg {
+ i915_reg_t gt_perf_status;
};
struct hwm_drvdata {
@@ -29,14 +31,49 @@ struct i915_hwmon {
};
static const struct hwmon_channel_info *hwm_info[] = {
+ HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
NULL
};
+static umode_t
+hwm_in_is_visible(const struct hwm_drvdata *ddat, u32 attr)
+{
+ switch (attr) {
+ case hwmon_in_input:
+ return i915_mmio_reg_valid(ddat->hwmon->rg.gt_perf_status) ? 0444 : 0;
+ default:
+ return 0;
+ }
+}
+
+static int
+hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ intel_wakeref_t wakeref;
+ u32 reg_value;
+
+ switch (attr) {
+ case hwmon_in_input:
+ with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
+ reg_value = intel_uncore_read(ddat->uncore, hwmon->rg.gt_perf_status);
+ /* In units of 2.5 millivolt */
+ *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static umode_t
hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
u32 attr, int channel)
{
+ struct hwm_drvdata *ddat = (struct hwm_drvdata *)drvdata;
+
switch (type) {
+ case hwmon_in:
+ return hwm_in_is_visible(ddat, attr);
default:
return 0;
}
@@ -46,7 +83,11 @@ static int
hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
int channel, long *val)
{
+ struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+
switch (type) {
+ case hwmon_in:
+ return hwm_in_read(ddat, attr, val);
default:
return -EOPNOTSUPP;
}
@@ -76,6 +117,12 @@ static const struct hwmon_chip_info hwm_chip_info = {
static void
hwm_get_preregistration_info(struct drm_i915_private *i915)
{
+ struct i915_hwmon *hwmon = i915->hwmon;
+
+ if (IS_DG1(i915) || IS_DG2(i915))
+ hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
+ else
+ hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
}
void i915_hwmon_register(struct drm_i915_private *i915)
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-08-25 13:21 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
@ 2022-08-29 17:30 ` Dixit, Ashutosh
2022-09-15 14:40 ` Nilawar, Badal
2022-09-12 14:09 ` Gupta, Anshuman
1 sibling, 1 reply; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-08-29 17:30 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On Thu, 25 Aug 2022 06:21:13 -0700, Badal Nilawar wrote:
>
> From: Riana Tauro <riana.tauro@intel.com>
>
> Use i915 HWMON subsystem to display current input voltage.
A couple of suggestions to improve comments in this patch below and after
addressing those this patch is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 103dd543a214..2192d0fd4c66 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -11,8 +11,10 @@
> #include "i915_hwmon.h"
> #include "i915_reg.h"
> #include "intel_mchbar_regs.h"
> +#include "gt/intel_gt_regs.h"
In later patches we have added units for different quantities here. So I
think we should add those units for voltage to this patch too. It's in
Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon but I think it's
better to add to this file too otherwise if anyone looks at it is seems to
be missing.
So I would add the following to this patch:
/*
* SF_* - scale factors for particular quantities according to hwmon spec.
* - voltage - millivolts
*/
#define SF_VOLTAGE 1000
> +static int
> +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(ddat->uncore, hwmon->rg.gt_perf_status);
> + /* In units of 2.5 millivolt */
> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
Let's complete this comment to so that it is clear what's happening:
/* HW register value is in units of 2.5 millivolt */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-08-29 17:30 ` Dixit, Ashutosh
@ 2022-09-15 14:40 ` Nilawar, Badal
2022-09-21 0:02 ` Dixit, Ashutosh
0 siblings, 1 reply; 32+ messages in thread
From: Nilawar, Badal @ 2022-09-15 14:40 UTC (permalink / raw)
To: Dixit, Ashutosh
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On 29-08-2022 23:00, Dixit, Ashutosh wrote:
> On Thu, 25 Aug 2022 06:21:13 -0700, Badal Nilawar wrote:
>>
>> From: Riana Tauro <riana.tauro@intel.com>
>>
>> Use i915 HWMON subsystem to display current input voltage.
>
> A couple of suggestions to improve comments in this patch below and after
> addressing those this patch is:
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
>> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
>> index 103dd543a214..2192d0fd4c66 100644
>> --- a/drivers/gpu/drm/i915/i915_hwmon.c
>> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
>> @@ -11,8 +11,10 @@
>> #include "i915_hwmon.h"
>> #include "i915_reg.h"
>> #include "intel_mchbar_regs.h"
>> +#include "gt/intel_gt_regs.h"
>
> In later patches we have added units for different quantities here. So I
> think we should add those units for voltage to this patch too. It's in
> Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon but I think it's
> better to add to this file too otherwise if anyone looks at it is seems to
> be missing.
>
> So I would add the following to this patch:
>
> /*
> * SF_* - scale factors for particular quantities according to hwmon spec.
> * - voltage - millivolts
> */
Sure I will add above comment.
> #define SF_VOLTAGE 1000
we are not using above scale factor for voltage. As our scale factor is
2.5 millivolts shall I add like.
#define SF_VOLTAGE_MUL 25
#define SF_VOLTAGE_DIV 10
>
>> +static int
>> +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
>> +{
>> + struct i915_hwmon *hwmon = ddat->hwmon;
>> + intel_wakeref_t wakeref;
>> + u32 reg_value;
>> +
>> + switch (attr) {
>> + case hwmon_in_input:
>> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
>> + reg_value = intel_uncore_read(ddat->uncore, hwmon->rg.gt_perf_status);
>> + /* In units of 2.5 millivolt */
>> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
And use above scale factors here.
*val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) *
SF_VOLTAGE_MUL, SF_VOLTAGE_DIV);
Regards,
Badal
>
> Let's complete this comment to so that it is clear what's happening:
>
> /* HW register value is in units of 2.5 millivolt */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-09-15 14:40 ` Nilawar, Badal
@ 2022-09-21 0:02 ` Dixit, Ashutosh
0 siblings, 0 replies; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-09-21 0:02 UTC (permalink / raw)
To: Nilawar, Badal
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On Thu, 15 Sep 2022 07:40:37 -0700, Nilawar, Badal wrote:
>
> On 29-08-2022 23:00, Dixit, Ashutosh wrote:
> > On Thu, 25 Aug 2022 06:21:13 -0700, Badal Nilawar wrote:
> >>
> >> +static int
> >> +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> >> +{
> >> + struct i915_hwmon *hwmon = ddat->hwmon;
> >> + intel_wakeref_t wakeref;
> >> + u32 reg_value;
> >> +
> >> + switch (attr) {
> >> + case hwmon_in_input:
> >> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> >> + reg_value = intel_uncore_read(ddat->uncore, hwmon->rg.gt_perf_status);
> >> + /* In units of 2.5 millivolt */
> >> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
>
> And use above scale factors here.
> *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) *
> SF_VOLTAGE_MUL, SF_VOLTAGE_DIV);
> Regards,
> Badal
> >
> > Let's complete this comment to so that it is clear what's happening:
> >
> > /* HW register value is in units of 2.5 millivolt */
This was missed in the latest rev so if we could remember to add this that would be great.
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-08-25 13:21 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-08-29 17:30 ` Dixit, Ashutosh
@ 2022-09-12 14:09 ` Gupta, Anshuman
2022-09-12 16:37 ` Dixit, Ashutosh
1 sibling, 1 reply; 32+ messages in thread
From: Gupta, Anshuman @ 2022-09-12 14:09 UTC (permalink / raw)
To: Nilawar, Badal, intel-gfx
Cc: Dixit, Ashutosh, Tauro, Riana, Ewins, Jon, linux-hwmon
> -----Original Message-----
> From: Nilawar, Badal <badal.nilawar@intel.com>
> Sent: Thursday, August 25, 2022 6:51 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Dixit, Ashutosh <ashutosh.dixit@intel.com>; Tauro, Riana
> <riana.tauro@intel.com>; Gupta, Anshuman <anshuman.gupta@intel.com>;
> Ewins, Jon <jon.ewins@intel.com>; linux-hwmon@vger.kernel.org
> Subject: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
>
> From: Riana Tauro <riana.tauro@intel.com>
>
> Use i915 HWMON subsystem to display current input voltage.
>
> v2:
> - Updated date and kernel version in feature description
> - Fixed review comments (Ashutosh)
> v3: Use macro HWMON_CHANNEL_INFO to define hwmon channel (Guenter)
> v4:
> - Fixed review comments (Ashutosh)
> - Use hwm_ prefix for static functions (Ashutosh)
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> 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>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 7 +++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 ++
> drivers/gpu/drm/i915/i915_hwmon.c | 47 +++++++++++++++++++
> 3 files changed, 57 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>
> 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..24c4b7477d51
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -0,0 +1,7 @@
> +What: /sys/devices/.../hwmon/hwmon<i>/in0_input
> +Date: June 2022
> +KernelVersion: 5.19
> +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 94f9ddcfb3a5..5d4fbda4d326 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1508,6 +1508,9 @@
> #define VLV_RENDER_C0_COUNT _MMIO(0x138118)
> #define VLV_MEDIA_C0_COUNT _MMIO(0x13811c)
>
> +#define GEN12_RPSTAT1 _MMIO(0x1381b4)
> +#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> +
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 +
> ((x) * 4))
> #define GEN11_CSME (31)
> #define GEN11_GUNIT (28)
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
> b/drivers/gpu/drm/i915/i915_hwmon.c
> index 103dd543a214..2192d0fd4c66 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -11,8 +11,10 @@
> #include "i915_hwmon.h"
> #include "i915_reg.h"
> #include "intel_mchbar_regs.h"
> +#include "gt/intel_gt_regs.h"
>
> struct hwm_reg {
> + i915_reg_t gt_perf_status;
> };
>
> struct hwm_drvdata {
> @@ -29,14 +31,49 @@ struct i915_hwmon {
> };
>
> static const struct hwmon_channel_info *hwm_info[] = {
> + HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
> NULL
> };
>
> +static umode_t
> +hwm_in_is_visible(const struct hwm_drvdata *ddat, u32 attr) {
> + switch (attr) {
> + case hwmon_in_input:
> + return i915_mmio_reg_valid(ddat->hwmon->rg.gt_perf_status)
> ? 0444 : 0;
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val) {
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + intel_wakeref_t wakeref;
> + u32 reg_value;
> +
> + switch (attr) {
> + case hwmon_in_input:
Other attributes in this series take hwmon->lock before accessing i915 registers ,
So do we need lock here as well ?
Thanks,
Anshuman Gupta.
> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> + reg_value = intel_uncore_read(ddat->uncore, hwmon-
> >rg.gt_perf_status);
> + /* In units of 2.5 millivolt */
> + *val =
> DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) *
> 25, 10);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static umode_t
> hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> u32 attr, int channel)
> {
> + struct hwm_drvdata *ddat = (struct hwm_drvdata *)drvdata;
> +
> switch (type) {
> + case hwmon_in:
> + return hwm_in_is_visible(ddat, attr);
> default:
> return 0;
> }
> @@ -46,7 +83,11 @@ static int
> hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> int channel, long *val)
> {
> + struct hwm_drvdata *ddat = dev_get_drvdata(dev);
> +
> switch (type) {
> + case hwmon_in:
> + return hwm_in_read(ddat, attr, val);
> default:
> return -EOPNOTSUPP;
> }
> @@ -76,6 +117,12 @@ static const struct hwmon_chip_info hwm_chip_info = {
> static void hwm_get_preregistration_info(struct drm_i915_private *i915) {
> + struct i915_hwmon *hwmon = i915->hwmon;
> +
> + if (IS_DG1(i915) || IS_DG2(i915))
> + hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
> + else
> + hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
> }
>
> void i915_hwmon_register(struct drm_i915_private *i915)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-09-12 14:09 ` Gupta, Anshuman
@ 2022-09-12 16:37 ` Dixit, Ashutosh
2022-09-13 8:11 ` Gupta, Anshuman
0 siblings, 1 reply; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-09-12 16:37 UTC (permalink / raw)
To: Gupta, Anshuman
Cc: Nilawar, Badal, intel-gfx, Tauro, Riana, Ewins, Jon, linux-hwmon
On Mon, 12 Sep 2022 07:09:28 -0700, Gupta, Anshuman wrote:
>
> > +static int
> > +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val) {
> > + struct i915_hwmon *hwmon = ddat->hwmon;
> > + intel_wakeref_t wakeref;
> > + u32 reg_value;
> > +
> > + switch (attr) {
> > + case hwmon_in_input:
>
> Other attributes in this series take hwmon->lock before accessing i915
> registers , So do we need lock here as well ?
The lock is being taken only for RMW and for making sure energy counter
updates happen atomically. We are not taking the lock for just reads so IMO
no lock is needed here.
> > + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> > + reg_value = intel_uncore_read(ddat->uncore, hwmon-
> > >rg.gt_perf_status);
> > + /* In units of 2.5 millivolt */
> > + *val =
> > DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) *
> > 25, 10);
> > + return 0;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +}
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
2022-09-12 16:37 ` Dixit, Ashutosh
@ 2022-09-13 8:11 ` Gupta, Anshuman
[not found] ` <878rmn5mks.wl-ashutosh.dixit@intel.com>
0 siblings, 1 reply; 32+ messages in thread
From: Gupta, Anshuman @ 2022-09-13 8:11 UTC (permalink / raw)
To: Dixit, Ashutosh
Cc: Nilawar, Badal, intel-gfx, Tauro, Riana, Ewins, Jon, linux-hwmon
> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: Monday, September 12, 2022 10:08 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: Nilawar, Badal <badal.nilawar@intel.com>; intel-gfx@lists.freedesktop.org;
> Tauro, Riana <riana.tauro@intel.com>; Ewins, Jon <jon.ewins@intel.com>;
> linux-hwmon@vger.kernel.org
> Subject: Re: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage
> support
>
> On Mon, 12 Sep 2022 07:09:28 -0700, Gupta, Anshuman wrote:
> >
> > > +static int
> > > +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val) {
> > > + struct i915_hwmon *hwmon = ddat->hwmon;
> > > + intel_wakeref_t wakeref;
> > > + u32 reg_value;
> > > +
> > > + switch (attr) {
> > > + case hwmon_in_input:
> >
> > Other attributes in this series take hwmon->lock before accessing i915
> > registers , So do we need lock here as well ?
>
> The lock is being taken only for RMW and for making sure energy counter
> updates happen atomically. We are not taking the lock for just reads so IMO no
> lock is needed here.
If that is the case, then why it needs to grab a lock for rmw on different
Register ? Like in patch 3 of this series grabs hwmon->howmon_lock for rmw on
two different register pkg_power_sku_unit and pkg_rapl_limit. One register rmw
should be independent of other register here ?
Thanks,
Anshuman Gupta
>
> > > + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> > > + reg_value = intel_uncore_read(ddat->uncore, hwmon-
> > > >rg.gt_perf_status);
> > > + /* In units of 2.5 millivolt */
> > > + *val =
> > > DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value)
> * 25,
> > > 10);
> > > + return 0;
> > > + default:
> > > + return -EOPNOTSUPP;
> > > + }
> > > +}
>
> Thanks.
> --
> Ashutosh
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/7] drm/i915/hwmon: Power PL1 limit and TDP setting
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-08-25 13:21 ` [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure Badal Nilawar
2022-08-25 13:21 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-30 2:33 ` Dixit, Ashutosh
2022-08-25 13:21 ` [PATCH 4/7] drm/i915/hwmon: Show device level energy usage Badal Nilawar
` (3 subsequent siblings)
6 siblings, 1 reply; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Dale B Stimson <dale.b.stimson@intel.com>
Use i915 HWMON to display/modify dGfx power PL1 limit and TDP setting.
v2:
- Fix review comments (Ashutosh)
- Do not restore power1_max upon module unload/load sequence
because on production systems modules are always loaded
and not unloaded/reloaded (Ashutosh)
- Fix review comments (Jani)
- Remove endianness conversion (Ashutosh)
v3: Add power1_rated_max (Ashutosh)
v4:
- Use macro HWMON_CHANNEL_INFO to define power channel (Guenter)
- Update the date and kernel version in Documentation (Badal)
v5: Use hwm_ prefix for static functions (Ashutosh)
Cc: Guenter Roeck <linux@roeck-us.net>
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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 20 ++
drivers/gpu/drm/i915/i915_hwmon.c | 175 +++++++++++++++++-
drivers/gpu/drm/i915/i915_reg.h | 16 ++
drivers/gpu/drm/i915/intel_mchbar_regs.h | 6 +
4 files changed, 215 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 24c4b7477d51..9a2d10edfce8 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -5,3 +5,23 @@ Contact: dri-devel@lists.freedesktop.org
Description: RO. Current Voltage in millivolt.
Only supported for particular Intel i915 graphics platforms.
+
+What: /sys/devices/.../hwmon/hwmon<i>/power1_max
+Date: June 2022
+KernelVersion: 5.19
+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_rated_max
+Date: June 2022
+KernelVersion: 5.19
+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/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 2192d0fd4c66..922463da65bf 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -13,8 +13,22 @@
#include "intel_mchbar_regs.h"
#include "gt/intel_gt_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))
+
struct hwm_reg {
i915_reg_t gt_perf_status;
+ i915_reg_t pkg_power_sku_unit;
+ i915_reg_t pkg_power_sku;
+ i915_reg_t pkg_rapl_limit;
};
struct hwm_drvdata {
@@ -28,10 +42,69 @@ struct i915_hwmon {
struct hwm_drvdata ddat;
struct mutex hwmon_lock; /* counter overflow logic and rmw */
struct hwm_reg rg;
+ int scl_shift_power;
};
+static void
+hwm_locked_with_pm_intel_uncore_rmw(struct hwm_drvdata *ddat,
+ i915_reg_t reg, u32 clear, u32 set)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ struct intel_uncore *uncore = ddat->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);
+}
+
+/*
+ * 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
+hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
+ u32 field_msk, int field_shift,
+ int nshift, u32 scale_factor)
+{
+ struct intel_uncore *uncore = ddat->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 mul_u64_u32_shr(reg_value, scale_factor, nshift);
+}
+
+static void
+hwm_field_scale_and_write(struct hwm_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;
+
+ hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
+ bits_to_clear, bits_to_set);
+}
+
static const struct hwmon_channel_info *hwm_info[] = {
HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
+ HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
NULL
};
@@ -65,6 +138,67 @@ hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
}
}
+static umode_t
+hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+
+ switch (attr) {
+ case hwmon_power_max:
+ return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 : 0;
+ case hwmon_power_rated_max:
+ return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
+ default:
+ return 0;
+ }
+}
+
+static int
+hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+
+ switch (attr) {
+ case hwmon_power_max:
+ *val = hwm_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);
+ return 0;
+ case hwmon_power_rated_max:
+ *val = hwm_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 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+
+ switch (attr) {
+ case hwmon_power_max:
+ hwm_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);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static umode_t
hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
u32 attr, int channel)
@@ -74,6 +208,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
switch (type) {
case hwmon_in:
return hwm_in_is_visible(ddat, attr);
+ case hwmon_power:
+ return hwm_power_is_visible(ddat, attr, channel);
default:
return 0;
}
@@ -88,6 +224,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
switch (type) {
case hwmon_in:
return hwm_in_read(ddat, attr, val);
+ case hwmon_power:
+ return hwm_power_read(ddat, attr, channel, val);
default:
return -EOPNOTSUPP;
}
@@ -97,7 +235,11 @@ static int
hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
int channel, long val)
{
+ struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+
switch (type) {
+ case hwmon_power:
+ return hwm_power_write(ddat, attr, channel, val);
default:
return -EOPNOTSUPP;
}
@@ -118,11 +260,40 @@ static void
hwm_get_preregistration_info(struct drm_i915_private *i915)
{
struct i915_hwmon *hwmon = i915->hwmon;
+ struct intel_uncore *uncore = &i915->uncore;
+ intel_wakeref_t wakeref;
+ u32 val_sku_unit;
- if (IS_DG1(i915) || IS_DG2(i915))
+ if (IS_DG1(i915) || IS_DG2(i915)) {
hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
- else
+ 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.gt_perf_status = INVALID_MMIO_REG;
+ 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;
+ }
+
+ with_intel_runtime_pm(uncore->rpm, wakeref) {
+ /*
+ * 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;
+ }
+
+ hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
+ }
}
void i915_hwmon_register(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2e3aa684cf1b..fd13411a28d9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1866,6 +1866,22 @@
#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)
#define CLK_CTL2_CZCOUNT_30NS_SHIFT 28
diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
index ffc702b79579..b74df11977c6 100644
--- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
+++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
@@ -189,6 +189,10 @@
#define DG1_QCLK_RATIO_MASK REG_GENMASK(9, 2)
#define DG1_QCLK_REFERENCE REG_BIT(10)
+#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_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
#define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
#define GEN6_RP_STATE_CAP _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
@@ -198,6 +202,8 @@
#define GEN10_FREQ_INFO_REC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5ef0)
#define RPE_MASK REG_GENMASK(15, 8)
+#define PCU_PACKAGE_RAPL_LIMIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
+#define PKG_PWR_LIM_1 REG_GENMASK(14, 0)
/* snb MCH registers for priority tuning */
#define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 3/7] drm/i915/hwmon: Power PL1 limit and TDP setting
2022-08-25 13:21 ` [PATCH 3/7] drm/i915/hwmon: Power PL1 limit and TDP setting Badal Nilawar
@ 2022-08-30 2:33 ` Dixit, Ashutosh
0 siblings, 0 replies; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-08-30 2:33 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On Thu, 25 Aug 2022 06:21:14 -0700, Badal Nilawar wrote:
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 2192d0fd4c66..922463da65bf 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -13,8 +13,22 @@
> #include "intel_mchbar_regs.h"
> #include "gt/intel_gt_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))
Let's remove this macro, it's not needed, see below.
> +/*
> + * 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
> +hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> + u32 field_msk, int field_shift,
Let's remove field_shift arg.
> + int nshift, u32 scale_factor)
> +{
> + struct intel_uncore *uncore = ddat->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;
This is just 'REG_FIELD_GET(field_msk, reg_value)'.
> +
> + return mul_u64_u32_shr(reg_value, scale_factor, nshift);
> +}
> +
> +static void
> +hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> + u32 field_msk, int field_shift,
Let's remove field_shift arg.
> + 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;
This is just 'REG_FIELD_PREP(field_msk, nval)'.
> @@ -118,11 +260,40 @@ static void
> hwm_get_preregistration_info(struct drm_i915_private *i915)
> {
> struct i915_hwmon *hwmon = i915->hwmon;
> + struct intel_uncore *uncore = &i915->uncore;
> + intel_wakeref_t wakeref;
> + u32 val_sku_unit;
>
> - if (IS_DG1(i915) || IS_DG2(i915))
> + if (IS_DG1(i915) || IS_DG2(i915)) {
> hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
> - else
> + 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.gt_perf_status = INVALID_MMIO_REG;
> + 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;
> + }
> +
> + with_intel_runtime_pm(uncore->rpm, wakeref) {
> + /*
> + * 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.
> + */
Let's delete this 2nd paragraph above, if values are available per tile we
should just be using per tile counters consistently (this is only true for
energy in our case, that patch will need to be fixed).
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2e3aa684cf1b..fd13411a28d9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1866,6 +1866,22 @@
> #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)
Let's change this entire block above to just the following for this patch:
/* *_PACKAGE_POWER_SKU - SKU power and timing parameters */
#define PKG_PKG_TDP GENMASK_ULL(14, 0)
That is because none of the following #define's are used in this patch,
they will be added in the patches they are used (PKG_MIN_PWR and
PKG_MAX_PWR are not used at all). Also these fields are explained in
i915_hwmon.c, no need to explain so much in a common header file.
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 4/7] drm/i915/hwmon: Show device level energy usage
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
` (2 preceding siblings ...)
2022-08-25 13:21 ` [PATCH 3/7] drm/i915/hwmon: Power PL1 limit and TDP setting Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-30 3:14 ` Dixit, Ashutosh
2022-09-13 8:50 ` [Intel-gfx] " Tvrtko Ursulin
2022-08-25 13:21 ` [PATCH 5/7] drm/i915/hwmon: Expose card reactive critical power Badal Nilawar
` (2 subsequent siblings)
6 siblings, 2 replies; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Dale B Stimson <dale.b.stimson@intel.com>
Use i915 HWMON to display device level energy input.
v2:
- Updated the date and kernel version in feature description
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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 8 ++
drivers/gpu/drm/i915/i915_hwmon.c | 119 +++++++++++++++++-
drivers/gpu/drm/i915/i915_hwmon.h | 1 +
drivers/gpu/drm/i915/intel_mchbar_regs.h | 2 +
4 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 9a2d10edfce8..03d71c6869d3 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -25,3 +25,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>/energy1_input
+Date: June 2022
+KernelVersion: 5.19
+Contact: dri-devel@lists.freedesktop.org
+Description: RO. Energy input of device in microjoules.
+
+ Only supported for particular Intel i915 graphics platforms.
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 922463da65bf..e35f125be242 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -16,8 +16,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)) + \
@@ -29,12 +31,19 @@ struct hwm_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;
+};
+
+struct hwm_energy_info {
+ u32 reg_val_prev;
+ long accum_energy; /* Accumulated energy for energy1_input */
};
struct hwm_drvdata {
struct i915_hwmon *hwmon;
struct intel_uncore *uncore;
struct device *hwmon_dev;
+ struct hwm_energy_info ei; /* Energy info for energy1_input */
char name[12];
};
@@ -43,6 +52,7 @@ struct i915_hwmon {
struct mutex hwmon_lock; /* counter overflow logic and rmw */
struct hwm_reg rg;
int scl_shift_power;
+ int scl_shift_energy;
};
static void
@@ -102,9 +112,72 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
bits_to_clear, bits_to_set);
}
+/*
+ * hwm_energy - Obtain energy value
+ *
+ * The underlying energy hardware register is 32-bits and is subject to
+ * overflow. 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
+ *
+ * The function significantly increases overflow duration (from 4.36
+ * minutes) by accumulating the energy register into a 'long' as allowed by
+ * the hwmon API. Using x86_64 128 bit arithmetic (see mul_u64_u32_shr()),
+ * a 'long' of 63 bits, SF_ENERGY of 1e6 (~20 bits) and
+ * hwmon->scl_shift_energy of 14 bits we have 57 (63 - 20 + 14) bits before
+ * energy1_input overflows. This at 1000 W is an overflow duration of 278 years.
+ */
+static int
+hwm_energy(struct hwm_drvdata *ddat, long *energy)
+{
+ struct intel_uncore *uncore = ddat->uncore;
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ struct hwm_energy_info *ei = &ddat->ei;
+ intel_wakeref_t wakeref;
+ i915_reg_t rgaddr;
+ u32 reg_val;
+
+ 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_val = intel_uncore_read(uncore, rgaddr);
+
+ if (reg_val >= ei->reg_val_prev)
+ ei->accum_energy += reg_val - ei->reg_val_prev;
+ else
+ ei->accum_energy += UINT_MAX - ei->reg_val_prev + reg_val;
+ ei->reg_val_prev = reg_val;
+
+ *energy = mul_u64_u32_shr(ei->accum_energy, SF_ENERGY,
+ hwmon->scl_shift_energy);
+ mutex_unlock(&hwmon->hwmon_lock);
+
+ return 0;
+}
+
+int
+i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy)
+{
+ struct i915_hwmon *hwmon = i915->hwmon;
+ struct hwm_drvdata *ddat = &hwmon->ddat;
+
+ return hwm_energy(ddat, energy);
+}
+
static const struct hwmon_channel_info *hwm_info[] = {
HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
+ HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT),
NULL
};
@@ -199,6 +272,32 @@ hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
}
}
+static umode_t
+hwm_energy_is_visible(const struct hwm_drvdata *ddat, u32 attr)
+{
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ i915_reg_t rgaddr;
+
+ switch (attr) {
+ case hwmon_energy_input:
+ rgaddr = hwmon->rg.energy_status_all;
+ return i915_mmio_reg_valid(rgaddr) ? 0444 : 0;
+ default:
+ return 0;
+ }
+}
+
+static int
+hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
+{
+ switch (attr) {
+ case hwmon_energy_input:
+ return hwm_energy(ddat, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static umode_t
hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
u32 attr, int channel)
@@ -210,6 +309,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
return hwm_in_is_visible(ddat, attr);
case hwmon_power:
return hwm_power_is_visible(ddat, attr, channel);
+ case hwmon_energy:
+ return hwm_energy_is_visible(ddat, attr);
default:
return 0;
}
@@ -226,6 +327,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
return hwm_in_read(ddat, attr, val);
case hwmon_power:
return hwm_power_read(ddat, attr, channel, val);
+ case hwmon_energy:
+ return hwm_energy_read(ddat, attr, val);
default:
return -EOPNOTSUPP;
}
@@ -261,19 +364,23 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
{
struct i915_hwmon *hwmon = i915->hwmon;
struct intel_uncore *uncore = &i915->uncore;
+ struct hwm_drvdata *ddat = &hwmon->ddat;
intel_wakeref_t wakeref;
u32 val_sku_unit;
+ long energy;
if (IS_DG1(i915) || IS_DG2(i915)) {
hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
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;
} else {
hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
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;
}
with_intel_runtime_pm(uncore->rpm, wakeref) {
@@ -291,9 +398,17 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
} else {
val_sku_unit = 0;
}
-
- hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
}
+
+ hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
+ hwmon->scl_shift_energy = REG_FIELD_GET(PKG_ENERGY_UNIT, val_sku_unit);
+
+ /*
+ * Initialize 'struct hwm_energy_info', i.e. set fields to the
+ * first value of the energy register read
+ */
+ if (i915_mmio_reg_valid(hwmon->rg.energy_status_all))
+ hwm_energy(ddat, &energy);
}
void i915_hwmon_register(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
index 7ca9cf2c34c9..4e5b6c149f3a 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.h
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -17,4 +17,5 @@ static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
#endif
+int i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy);
#endif /* __I915_HWMON_H__ */
diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
index b74df11977c6..1014d0b7cc16 100644
--- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
+++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
@@ -191,7 +191,9 @@
#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_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
#define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 4/7] drm/i915/hwmon: Show device level energy usage
2022-08-25 13:21 ` [PATCH 4/7] drm/i915/hwmon: Show device level energy usage Badal Nilawar
@ 2022-08-30 3:14 ` Dixit, Ashutosh
2022-09-13 8:50 ` [Intel-gfx] " Tvrtko Ursulin
1 sibling, 0 replies; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-08-30 3:14 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On Thu, 25 Aug 2022 06:21:15 -0700, Badal Nilawar wrote:
>
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> Use i915 HWMON to display device level energy input.
>
> v2:
> - Updated the date and kernel version in feature description
A few things to delete below but since no other changes this is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 9a2d10edfce8..03d71c6869d3 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -25,3 +25,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>/energy1_input
> +Date: June 2022
> +KernelVersion: 5.19
Need to update kernel version on all patches now at least to 6.0 if not
6.1.
> +static int
> +hwm_energy(struct hwm_drvdata *ddat, long *energy)
> +{
> + struct intel_uncore *uncore = ddat->uncore;
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + struct hwm_energy_info *ei = &ddat->ei;
> + intel_wakeref_t wakeref;
> + i915_reg_t rgaddr;
> + u32 reg_val;
> +
> + rgaddr = hwmon->rg.energy_status_all;
> +
> + if (!i915_mmio_reg_valid(rgaddr))
> + return -EOPNOTSUPP;
Delete two lines above. Something like this is only needed if we have
i915_hwmon_energy_status_get() which we are deleting
below. hwm_energy_is_visible() takes care of making the sysfs node
invisible when something cannot be supported and has the same check.
> +int
> +i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy)
> +{
> + struct i915_hwmon *hwmon = i915->hwmon;
> + struct hwm_drvdata *ddat = &hwmon->ddat;
> +
> + return hwm_energy(ddat, energy);
> +}
Let's delete this function, there are no users for it at present.
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> index 7ca9cf2c34c9..4e5b6c149f3a 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.h
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -17,4 +17,5 @@ static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
> static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
> #endif
>
> +int i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy);
Delete.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH 4/7] drm/i915/hwmon: Show device level energy usage
2022-08-25 13:21 ` [PATCH 4/7] drm/i915/hwmon: Show device level energy usage Badal Nilawar
2022-08-30 3:14 ` Dixit, Ashutosh
@ 2022-09-13 8:50 ` Tvrtko Ursulin
2022-09-21 0:24 ` Dixit, Ashutosh
1 sibling, 1 reply; 32+ messages in thread
From: Tvrtko Ursulin @ 2022-09-13 8:50 UTC (permalink / raw)
To: Badal Nilawar, intel-gfx; +Cc: linux-hwmon
On 25/08/2022 14:21, Badal Nilawar wrote:
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> Use i915 HWMON to display device level energy input.
>
> v2:
> - Updated the date and kernel version in feature description
>
> 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>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 8 ++
> drivers/gpu/drm/i915/i915_hwmon.c | 119 +++++++++++++++++-
> drivers/gpu/drm/i915/i915_hwmon.h | 1 +
> drivers/gpu/drm/i915/intel_mchbar_regs.h | 2 +
> 4 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 9a2d10edfce8..03d71c6869d3 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -25,3 +25,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>/energy1_input
> +Date: June 2022
> +KernelVersion: 5.19
Date and kernel version will need updating throughout I think.
But why I am here actually is to ask if there are plans to make
intel_gpu_top support this? It would be nice to have since it reports
power for integrated.
Regards,
Tvrtko
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RO. Energy input of device in microjoules.
> +
> + Only supported for particular Intel i915 graphics platforms.
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 922463da65bf..e35f125be242 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -16,8 +16,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)) + \
> @@ -29,12 +31,19 @@ struct hwm_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;
> +};
> +
> +struct hwm_energy_info {
> + u32 reg_val_prev;
> + long accum_energy; /* Accumulated energy for energy1_input */
> };
>
> struct hwm_drvdata {
> struct i915_hwmon *hwmon;
> struct intel_uncore *uncore;
> struct device *hwmon_dev;
> + struct hwm_energy_info ei; /* Energy info for energy1_input */
> char name[12];
> };
>
> @@ -43,6 +52,7 @@ struct i915_hwmon {
> struct mutex hwmon_lock; /* counter overflow logic and rmw */
> struct hwm_reg rg;
> int scl_shift_power;
> + int scl_shift_energy;
> };
>
> static void
> @@ -102,9 +112,72 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> bits_to_clear, bits_to_set);
> }
>
> +/*
> + * hwm_energy - Obtain energy value
> + *
> + * The underlying energy hardware register is 32-bits and is subject to
> + * overflow. 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
> + *
> + * The function significantly increases overflow duration (from 4.36
> + * minutes) by accumulating the energy register into a 'long' as allowed by
> + * the hwmon API. Using x86_64 128 bit arithmetic (see mul_u64_u32_shr()),
> + * a 'long' of 63 bits, SF_ENERGY of 1e6 (~20 bits) and
> + * hwmon->scl_shift_energy of 14 bits we have 57 (63 - 20 + 14) bits before
> + * energy1_input overflows. This at 1000 W is an overflow duration of 278 years.
> + */
> +static int
> +hwm_energy(struct hwm_drvdata *ddat, long *energy)
> +{
> + struct intel_uncore *uncore = ddat->uncore;
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + struct hwm_energy_info *ei = &ddat->ei;
> + intel_wakeref_t wakeref;
> + i915_reg_t rgaddr;
> + u32 reg_val;
> +
> + 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_val = intel_uncore_read(uncore, rgaddr);
> +
> + if (reg_val >= ei->reg_val_prev)
> + ei->accum_energy += reg_val - ei->reg_val_prev;
> + else
> + ei->accum_energy += UINT_MAX - ei->reg_val_prev + reg_val;
> + ei->reg_val_prev = reg_val;
> +
> + *energy = mul_u64_u32_shr(ei->accum_energy, SF_ENERGY,
> + hwmon->scl_shift_energy);
> + mutex_unlock(&hwmon->hwmon_lock);
> +
> + return 0;
> +}
> +
> +int
> +i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy)
> +{
> + struct i915_hwmon *hwmon = i915->hwmon;
> + struct hwm_drvdata *ddat = &hwmon->ddat;
> +
> + return hwm_energy(ddat, energy);
> +}
> +
> static const struct hwmon_channel_info *hwm_info[] = {
> HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
> HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
> + HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT),
> NULL
> };
>
> @@ -199,6 +272,32 @@ hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
> }
> }
>
> +static umode_t
> +hwm_energy_is_visible(const struct hwm_drvdata *ddat, u32 attr)
> +{
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + i915_reg_t rgaddr;
> +
> + switch (attr) {
> + case hwmon_energy_input:
> + rgaddr = hwmon->rg.energy_status_all;
> + return i915_mmio_reg_valid(rgaddr) ? 0444 : 0;
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> +{
> + switch (attr) {
> + case hwmon_energy_input:
> + return hwm_energy(ddat, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static umode_t
> hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> u32 attr, int channel)
> @@ -210,6 +309,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> return hwm_in_is_visible(ddat, attr);
> case hwmon_power:
> return hwm_power_is_visible(ddat, attr, channel);
> + case hwmon_energy:
> + return hwm_energy_is_visible(ddat, attr);
> default:
> return 0;
> }
> @@ -226,6 +327,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> return hwm_in_read(ddat, attr, val);
> case hwmon_power:
> return hwm_power_read(ddat, attr, channel, val);
> + case hwmon_energy:
> + return hwm_energy_read(ddat, attr, val);
> default:
> return -EOPNOTSUPP;
> }
> @@ -261,19 +364,23 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> {
> struct i915_hwmon *hwmon = i915->hwmon;
> struct intel_uncore *uncore = &i915->uncore;
> + struct hwm_drvdata *ddat = &hwmon->ddat;
> intel_wakeref_t wakeref;
> u32 val_sku_unit;
> + long energy;
>
> if (IS_DG1(i915) || IS_DG2(i915)) {
> hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
> 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;
> } else {
> hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
> 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;
> }
>
> with_intel_runtime_pm(uncore->rpm, wakeref) {
> @@ -291,9 +398,17 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> } else {
> val_sku_unit = 0;
> }
> -
> - hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
> }
> +
> + hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
> + hwmon->scl_shift_energy = REG_FIELD_GET(PKG_ENERGY_UNIT, val_sku_unit);
> +
> + /*
> + * Initialize 'struct hwm_energy_info', i.e. set fields to the
> + * first value of the energy register read
> + */
> + if (i915_mmio_reg_valid(hwmon->rg.energy_status_all))
> + hwm_energy(ddat, &energy);
> }
>
> void i915_hwmon_register(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> index 7ca9cf2c34c9..4e5b6c149f3a 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.h
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -17,4 +17,5 @@ static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
> static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
> #endif
>
> +int i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy);
> #endif /* __I915_HWMON_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> index b74df11977c6..1014d0b7cc16 100644
> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> @@ -191,7 +191,9 @@
>
> #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_GT_PERF_STATUS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
> #define GEN6_RP_STATE_LIMITS _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH 4/7] drm/i915/hwmon: Show device level energy usage
2022-09-13 8:50 ` [Intel-gfx] " Tvrtko Ursulin
@ 2022-09-21 0:24 ` Dixit, Ashutosh
2022-09-21 7:43 ` Tvrtko Ursulin
0 siblings, 1 reply; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-09-21 0:24 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Badal Nilawar, intel-gfx, linux-hwmon, Riana Tauro, Jon Ewins
On Tue, 13 Sep 2022 01:50:08 -0700, Tvrtko Ursulin wrote:
>
Hi Tvrtko,
> On 25/08/2022 14:21, Badal Nilawar wrote:
> > From: Dale B Stimson <dale.b.stimson@intel.com>
> >
> > diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> > index 9a2d10edfce8..03d71c6869d3 100644
> > --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> > +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> > @@ -25,3 +25,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>/energy1_input
> > +Date: June 2022
> > +KernelVersion: 5.19
>
> Date and kernel version will need updating throughout I think.
>
> But why I am here actually is to ask if there are plans to make
> intel_gpu_top support this? It would be nice to have since it reports power
> for integrated.
There were no plans but now Riana has an IGT patch series which exposes a
unified inteface for rapl/hwmon (igfx/dgfx):
https://patchwork.freedesktop.org/series/108185/
So perhaps we can either have intel_gpu_top use this IGT lib or if it
doesn't, copy some functions to intel_gpu_top.
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH 4/7] drm/i915/hwmon: Show device level energy usage
2022-09-21 0:24 ` Dixit, Ashutosh
@ 2022-09-21 7:43 ` Tvrtko Ursulin
0 siblings, 0 replies; 32+ messages in thread
From: Tvrtko Ursulin @ 2022-09-21 7:43 UTC (permalink / raw)
To: Dixit, Ashutosh
Cc: Badal Nilawar, intel-gfx, linux-hwmon, Riana Tauro, Jon Ewins
On 21/09/2022 01:24, Dixit, Ashutosh wrote:
> On Tue, 13 Sep 2022 01:50:08 -0700, Tvrtko Ursulin wrote:
>>
>
> Hi Tvrtko,
>
>> On 25/08/2022 14:21, Badal Nilawar wrote:
>>> From: Dale B Stimson <dale.b.stimson@intel.com>
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>>> index 9a2d10edfce8..03d71c6869d3 100644
>>> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>>> @@ -25,3 +25,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>/energy1_input
>>> +Date: June 2022
>>> +KernelVersion: 5.19
>>
>> Date and kernel version will need updating throughout I think.
>>
>> But why I am here actually is to ask if there are plans to make
>> intel_gpu_top support this? It would be nice to have since it reports power
>> for integrated.
>
> There were no plans but now Riana has an IGT patch series which exposes a
> unified inteface for rapl/hwmon (igfx/dgfx):
>
> https://patchwork.freedesktop.org/series/108185/
>
> So perhaps we can either have intel_gpu_top use this IGT lib or if it
> doesn't, copy some functions to intel_gpu_top.
Looks promising and would be nice yeah.
On the practicalities, first thing which comes to mind that probably it
would need to be a separate library like igt_perf and igt_device_scan,
since so far we have avoided linking intel_gpu_top with the whole lib_igt.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 5/7] drm/i915/hwmon: Expose card reactive critical power
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
` (3 preceding siblings ...)
2022-08-25 13:21 ` [PATCH 4/7] drm/i915/hwmon: Show device level energy usage Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-25 13:21 ` [PATCH 6/7] drm/i915/hwmon: Expose power1_max_interval Badal Nilawar
2022-08-25 13:21 ` [PATCH 7/7] drm/i915/hwmon: Extend power/energy for XEHPSDV Badal Nilawar
6 siblings, 0 replies; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Ashutosh Dixit <ashutosh.dixit@intel.com>
Expose the card reactive critical (I1) power. I1 is exposed as
power1_crit in microwatts (typically for client products) or as
curr1_crit in milliamperes (typically for server).
v2: Add curr1_crit functionality (Ashutosh)
v3:
- Use HWMON_CHANNEL_INFO to define power1_crit, curr1_crit (Badal)
- Update date and kernel version in Documentation.
v4: Use hwm_ prefix for static functions (Ashutosh)
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 26 +++++
drivers/gpu/drm/i915/i915_hwmon.c | 95 ++++++++++++++++++-
drivers/gpu/drm/i915/i915_reg.h | 6 ++
3 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 03d71c6869d3..bb1101757154 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -26,6 +26,32 @@ Description: RO. Card default power limit (default TDP setting).
Only supported for particular Intel i915 graphics platforms.
+What: /sys/devices/.../hwmon/hwmon<i>/power1_crit
+Date: June 2022
+KernelVersion: 5.19
+Contact: dri-devel@lists.freedesktop.org
+Description: RW. Card reactive critical (I1) power limit in microwatts.
+
+ Card reactive critical (I1) power limit in microwatts is exposed
+ for client products. The power controller will throttle the
+ operating frequency if the power averaged over a window exceeds
+ this limit.
+
+ Only supported for particular Intel i915 graphics platforms.
+
+What: /sys/devices/.../hwmon/hwmon<i>/curr1_crit
+Date: June 2022
+KernelVersion: 5.19
+Contact: dri-devel@lists.freedesktop.org
+Description: RW. Card reactive critical (I1) power limit in milliamperes.
+
+ Card reactive critical (I1) power limit in milliamperes is
+ exposed for server products. The power controller will throttle
+ the operating frequency if the power averaged over a window
+ exceeds this limit.
+
+ Only supported for particular Intel i915 graphics platforms.
+
What: /sys/devices/.../hwmon/hwmon<i>/energy1_input
Date: June 2022
KernelVersion: 5.19
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index e35f125be242..e476c8a9351b 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -11,14 +11,17 @@
#include "i915_hwmon.h"
#include "i915_reg.h"
#include "intel_mchbar_regs.h"
+#include "intel_pcode.h"
#include "gt/intel_gt_regs.h"
/*
* SF_* - scale factors for particular quantities according to hwmon spec.
* - power - microwatts
+ * - curr - milliamperes
* - energy - microjoules
*/
#define SF_POWER 1000000
+#define SF_CURR 1000
#define SF_ENERGY 1000000
#define FIELD_SHIFT(__mask) \
@@ -176,11 +179,25 @@ i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy)
static const struct hwmon_channel_info *hwm_info[] = {
HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
- HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
+ HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT),
HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT),
+ HWMON_CHANNEL_INFO(curr, HWMON_C_CRIT),
NULL
};
+/* I1 is exposed as power_crit or as curr_crit depending on bit 31 */
+static int hwm_pcode_read_i1(struct drm_i915_private *i915, u32 *uval)
+{
+ return snb_pcode_read_p(&i915->uncore, PCODE_POWER_SETUP,
+ POWER_SETUP_SUBCOMMAND_READ_I1, 0, uval);
+}
+
+static int hwm_pcode_write_i1(struct drm_i915_private *i915, u32 uval)
+{
+ return snb_pcode_write_p(&i915->uncore, PCODE_POWER_SETUP,
+ POWER_SETUP_SUBCOMMAND_WRITE_I1, 0, uval);
+}
+
static umode_t
hwm_in_is_visible(const struct hwm_drvdata *ddat, u32 attr)
{
@@ -214,13 +231,18 @@ hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
static umode_t
hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
{
+ struct drm_i915_private *i915 = ddat->uncore->i915;
struct i915_hwmon *hwmon = ddat->hwmon;
+ u32 uval;
switch (attr) {
case hwmon_power_max:
return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 : 0;
case hwmon_power_rated_max:
return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
+ case hwmon_power_crit:
+ return (hwm_pcode_read_i1(i915, &uval) ||
+ !(uval & POWER_SETUP_I1_WATTS)) ? 0 : 0644;
default:
return 0;
}
@@ -230,6 +252,8 @@ static int
hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
{
struct i915_hwmon *hwmon = ddat->hwmon;
+ int ret;
+ u32 uval;
switch (attr) {
case hwmon_power_max:
@@ -248,6 +272,15 @@ hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
hwmon->scl_shift_power,
SF_POWER);
return 0;
+ case hwmon_power_crit:
+ ret = hwm_pcode_read_i1(ddat->uncore->i915, &uval);
+ if (ret)
+ return ret;
+ if (!(uval & POWER_SETUP_I1_WATTS))
+ return -ENODEV;
+ *val = mul_u64_u32_shr(REG_FIELD_GET(POWER_SETUP_I1_DATA_MASK, uval),
+ SF_POWER, POWER_SETUP_I1_SHIFT);
+ return 0;
default:
return -EOPNOTSUPP;
}
@@ -257,6 +290,7 @@ static int
hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
{
struct i915_hwmon *hwmon = ddat->hwmon;
+ u32 uval;
switch (attr) {
case hwmon_power_max:
@@ -267,6 +301,9 @@ hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
hwmon->scl_shift_power,
SF_POWER, val);
return 0;
+ case hwmon_power_crit:
+ uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_POWER);
+ return hwm_pcode_write_i1(ddat->uncore->i915, uval);
default:
return -EOPNOTSUPP;
}
@@ -298,6 +335,56 @@ hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
}
}
+static umode_t
+hwm_curr_is_visible(const struct hwm_drvdata *ddat, u32 attr)
+{
+ struct drm_i915_private *i915 = ddat->uncore->i915;
+ u32 uval;
+
+ switch (attr) {
+ case hwmon_curr_crit:
+ return (hwm_pcode_read_i1(i915, &uval) ||
+ (uval & POWER_SETUP_I1_WATTS)) ? 0 : 0644;
+ default:
+ return 0;
+ }
+}
+
+static int
+hwm_curr_read(struct hwm_drvdata *ddat, u32 attr, long *val)
+{
+ int ret;
+ u32 uval;
+
+ switch (attr) {
+ case hwmon_curr_crit:
+ ret = hwm_pcode_read_i1(ddat->uncore->i915, &uval);
+ if (ret)
+ return ret;
+ if (uval & POWER_SETUP_I1_WATTS)
+ return -ENODEV;
+ *val = mul_u64_u32_shr(REG_FIELD_GET(POWER_SETUP_I1_DATA_MASK, uval),
+ SF_CURR, POWER_SETUP_I1_SHIFT);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+hwm_curr_write(struct hwm_drvdata *ddat, u32 attr, long val)
+{
+ u32 uval;
+
+ switch (attr) {
+ case hwmon_curr_crit:
+ uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_CURR);
+ return hwm_pcode_write_i1(ddat->uncore->i915, uval);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static umode_t
hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
u32 attr, int channel)
@@ -311,6 +398,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
return hwm_power_is_visible(ddat, attr, channel);
case hwmon_energy:
return hwm_energy_is_visible(ddat, attr);
+ case hwmon_curr:
+ return hwm_curr_is_visible(ddat, attr);
default:
return 0;
}
@@ -329,6 +418,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
return hwm_power_read(ddat, attr, channel, val);
case hwmon_energy:
return hwm_energy_read(ddat, attr, val);
+ case hwmon_curr:
+ return hwm_curr_read(ddat, attr, val);
default:
return -EOPNOTSUPP;
}
@@ -343,6 +434,8 @@ hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
switch (type) {
case hwmon_power:
return hwm_power_write(ddat, attr, channel, val);
+ case hwmon_curr:
+ return hwm_curr_write(ddat, attr, val);
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fd13411a28d9..6f72e7285e64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6676,6 +6676,12 @@
#define DG1_PCODE_STATUS 0x7E
#define DG1_UNCORE_GET_INIT_STATUS 0x0
#define DG1_UNCORE_INIT_STATUS_COMPLETE 0x1
+#define PCODE_POWER_SETUP 0x7C
+#define POWER_SETUP_SUBCOMMAND_READ_I1 0x4
+#define POWER_SETUP_SUBCOMMAND_WRITE_I1 0x5
+#define POWER_SETUP_I1_WATTS REG_BIT(31)
+#define POWER_SETUP_I1_SHIFT 6 /* 10.6 fixed point format */
+#define POWER_SETUP_I1_DATA_MASK REG_GENMASK(15, 0)
#define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US 0x23
#define XEHP_PCODE_FREQUENCY_CONFIG 0x6e /* xehpsdv, pvc */
/* XEHP_PCODE_FREQUENCY_CONFIG sub-commands (param1) */
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 6/7] drm/i915/hwmon: Expose power1_max_interval
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
` (4 preceding siblings ...)
2022-08-25 13:21 ` [PATCH 5/7] drm/i915/hwmon: Expose card reactive critical power Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-25 13:21 ` [PATCH 7/7] drm/i915/hwmon: Extend power/energy for XEHPSDV Badal Nilawar
6 siblings, 0 replies; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Ashutosh Dixit <ashutosh.dixit@intel.com>
Expose power1_max_interval, that is the tau corresponding to PL1. Some bit
manipulation is needed because of the format of PKG_PWR_LIM_1_TIME in
GT0_PACKAGE_RAPL_LIMIT register (1.x * power(2,y)).
v2: Update date and kernel version in Documentation (Badal)
v3: Cleaned up hwm_power1_max_interval_store() (Badal)
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 9 ++
drivers/gpu/drm/i915/i915_hwmon.c | 114 +++++++++++++++++-
drivers/gpu/drm/i915/i915_reg.h | 4 +-
drivers/gpu/drm/i915/intel_mchbar_regs.h | 4 +
4 files changed, 128 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index bb1101757154..34668f6c2dc4 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -26,6 +26,15 @@ Description: RO. Card default power limit (default TDP setting).
Only supported for particular Intel i915 graphics platforms.
+What: /sys/devices/.../hwmon/hwmon<i>/power1_max_interval
+Date: June 2022
+KernelVersion: 5.19
+Contact: dri-devel@lists.freedesktop.org
+Description: RW. Sustained power limit interval (Tau in PL1/Tau) in
+ milliseconds over which sustained power is averaged.
+
+ Only supported for particular Intel i915 graphics platforms.
+
What: /sys/devices/.../hwmon/hwmon<i>/power1_crit
Date: June 2022
KernelVersion: 5.19
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index e476c8a9351b..b8ac52f07681 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -16,10 +16,12 @@
/*
* SF_* - scale factors for particular quantities according to hwmon spec.
+ * - time - milliseconds
* - power - microwatts
* - curr - milliamperes
* - energy - microjoules
*/
+#define SF_TIME 1000
#define SF_POWER 1000000
#define SF_CURR 1000
#define SF_ENERGY 1000000
@@ -56,6 +58,7 @@ struct i915_hwmon {
struct hwm_reg rg;
int scl_shift_power;
int scl_shift_energy;
+ int scl_shift_time;
};
static void
@@ -177,6 +180,114 @@ i915_hwmon_energy_status_get(struct drm_i915_private *i915, long *energy)
return hwm_energy(ddat, energy);
}
+static ssize_t
+hwm_power1_max_interval_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ intel_wakeref_t wakeref;
+ u32 r, x, y, x_w = 2; /* 2 bits */
+ u64 tau4, out;
+
+ with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
+ r = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
+
+ x = REG_FIELD_GET(PKG_PWR_LIM_1_TIME_X, r);
+ y = REG_FIELD_GET(PKG_PWR_LIM_1_TIME_Y, r);
+ /*
+ * tau = 1.x * power(2,y), x = bits(23:22), y = bits(21:17)
+ * = (4 | x) << (y - 2)
+ * where (y - 2) ensures a 1.x fixed point representation of 1.x
+ * However because y can be < 2, we compute
+ * tau4 = (4 | x) << y
+ * but add 2 when doing the final right shift to account for units
+ */
+ tau4 = ((1 << x_w) | x) << y;
+ /* val in hwmon interface units (millisec) */
+ out = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
+
+ return sysfs_emit(buf, "%llu\n", out);
+}
+
+static ssize_t
+hwm_power1_max_interval_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+ struct i915_hwmon *hwmon = ddat->hwmon;
+ long val, max_win, ret;
+ u32 x, y, rxy, x_w = 2; /* 2 bits */
+ u64 tau4, r;
+
+#define PKG_MAX_WIN_DEFAULT 0x12ull
+
+ ret = kstrtoul(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ /*
+ * val must be < max in hwmon interface units. The steps below are
+ * explained in i915_power1_max_interval_show()
+ */
+ r = FIELD_PREP(PKG_MAX_WIN, PKG_MAX_WIN_DEFAULT);
+ x = REG_FIELD_GET(PKG_MAX_WIN_X, r);
+ y = REG_FIELD_GET(PKG_MAX_WIN_Y, r);
+ tau4 = ((1 << x_w) | x) << y;
+ max_win = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
+
+ if (val > max_win)
+ return -EINVAL;
+
+ /* val in hw units */
+ val = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_time, SF_TIME);
+ /* Convert to 1.x * power(2,y) */
+ if (!val)
+ return -EINVAL;
+ y = ilog2(val);
+ /* x = (val - (1 << y)) >> (y - 2); */
+ x = (val - (1ul << y)) << x_w >> y;
+
+ rxy = REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_X, x) | REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_Y, y);
+
+ hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+ PKG_PWR_LIM_1_TIME, rxy);
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR(power1_max_interval, 0664,
+ hwm_power1_max_interval_show,
+ hwm_power1_max_interval_store, 0);
+
+static struct attribute *hwm_attributes[] = {
+ &sensor_dev_attr_power1_max_interval.dev_attr.attr,
+ NULL
+};
+
+static umode_t hwm_attributes_visible(struct kobject *kobj,
+ struct attribute *attr, int index)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+ struct i915_hwmon *hwmon = ddat->hwmon;
+
+ if (attr == &sensor_dev_attr_power1_max_interval.dev_attr.attr)
+ return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? attr->mode : 0;
+ else
+ return 0;
+}
+
+static const struct attribute_group hwm_attrgroup = {
+ .attrs = hwm_attributes,
+ .is_visible = hwm_attributes_visible,
+};
+
+static const struct attribute_group *hwm_groups[] = {
+ &hwm_attrgroup,
+ NULL
+};
+
static const struct hwmon_channel_info *hwm_info[] = {
HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT),
@@ -495,6 +606,7 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
hwmon->scl_shift_energy = REG_FIELD_GET(PKG_ENERGY_UNIT, val_sku_unit);
+ hwmon->scl_shift_time = REG_FIELD_GET(PKG_TIME_UNIT, val_sku_unit);
/*
* Initialize 'struct hwm_energy_info', i.e. set fields to the
@@ -533,7 +645,7 @@ void i915_hwmon_register(struct drm_i915_private *i915)
hwmon_dev = hwmon_device_register_with_info(dev, ddat->name,
ddat,
&hwm_chip_info,
- NULL);
+ hwm_groups);
if (IS_ERR(hwmon_dev)) {
mutex_destroy(&hwmon->hwmon_lock);
i915->hwmon = NULL;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6f72e7285e64..68251ba3bc53 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1879,8 +1879,8 @@
#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 PKG_MAX_WIN_X GENMASK_ULL(54, 53)
+#define PKG_MAX_WIN_Y 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 1014d0b7cc16..9331a3c15fd1 100644
--- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
+++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
@@ -206,6 +206,10 @@
#define RPE_MASK REG_GENMASK(15, 8)
#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)
+#define PKG_PWR_LIM_1_TIME_X REG_GENMASK(23, 22)
+#define PKG_PWR_LIM_1_TIME_Y REG_GENMASK(21, 17)
/* snb MCH registers for priority tuning */
#define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 7/7] drm/i915/hwmon: Extend power/energy for XEHPSDV
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
` (5 preceding siblings ...)
2022-08-25 13:21 ` [PATCH 6/7] drm/i915/hwmon: Expose power1_max_interval Badal Nilawar
@ 2022-08-25 13:21 ` Badal Nilawar
2022-08-30 5:34 ` Dixit, Ashutosh
6 siblings, 1 reply; 32+ messages in thread
From: Badal Nilawar @ 2022-08-25 13:21 UTC (permalink / raw)
To: intel-gfx
Cc: ashutosh.dixit, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
From: Dale B Stimson <dale.b.stimson@intel.com>
Extend hwmon power/energy for XEHPSDV especially per gt level energy
usage.
v2: Update to latest HWMON spec (Ashutosh)
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 7 +-
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 5 +
drivers/gpu/drm/i915/i915_hwmon.c | 120 +++++++++++++++++-
3 files changed, 128 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 34668f6c2dc4..e69bc43d4c9e 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -65,6 +65,11 @@ What: /sys/devices/.../hwmon/hwmon<i>/energy1_input
Date: June 2022
KernelVersion: 5.19
Contact: dri-devel@lists.freedesktop.org
-Description: RO. Energy input of device in microjoules.
+Description: RO. Energy input of device or gt in microjoules.
+
+ For i915 device level hwmon devices (name "i915") this
+ reflects energy input for the entire device. For gt level
+ hwmon devices (name "i915_gtN") this reflects energy input
+ for the gt.
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 5d4fbda4d326..b7b343cec2da 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1579,4 +1579,9 @@
#define GEN12_SFC_DONE(n) _MMIO(0x1cc000 + (n) * 0x1000)
+#define GT0_PACKAGE_ENERGY_STATUS _MMIO(0x250004)
+#define GT0_PACKAGE_RAPL_LIMIT _MMIO(0x250008)
+#define GT0_PACKAGE_POWER_SKU_UNIT _MMIO(0x250068)
+#define GT0_PLATFORM_ENERGY_STATUS _MMIO(0x25006c)
+
#endif /* __INTEL_GT_REGS__ */
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index b8ac52f07681..76d73216c54e 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -12,6 +12,7 @@
#include "i915_reg.h"
#include "intel_mchbar_regs.h"
#include "intel_pcode.h"
+#include "gt/intel_gt.h"
#include "gt/intel_gt_regs.h"
/*
@@ -21,7 +22,7 @@
* - curr - milliamperes
* - energy - microjoules
*/
-#define SF_TIME 1000
+#define SF_TIME 1000
#define SF_POWER 1000000
#define SF_CURR 1000
#define SF_ENERGY 1000000
@@ -37,6 +38,7 @@ struct hwm_reg {
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 hwm_energy_info {
@@ -50,10 +52,12 @@ struct hwm_drvdata {
struct device *hwmon_dev;
struct hwm_energy_info ei; /* Energy info for energy1_input */
char name[12];
+ int gt_n;
};
struct i915_hwmon {
struct hwm_drvdata ddat;
+ struct hwm_drvdata ddat_gt[I915_MAX_GT];
struct mutex hwmon_lock; /* counter overflow logic and rmw */
struct hwm_reg rg;
int scl_shift_power;
@@ -148,7 +152,10 @@ hwm_energy(struct hwm_drvdata *ddat, long *energy)
i915_reg_t rgaddr;
u32 reg_val;
- rgaddr = hwmon->rg.energy_status_all;
+ if (ddat->gt_n >= 0)
+ rgaddr = hwmon->rg.energy_status_tile;
+ else
+ rgaddr = hwmon->rg.energy_status_all;
if (!i915_mmio_reg_valid(rgaddr))
return -EOPNOTSUPP;
@@ -296,6 +303,11 @@ static const struct hwmon_channel_info *hwm_info[] = {
NULL
};
+static const struct hwmon_channel_info *hwm_gt_info[] = {
+ HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT),
+ NULL
+};
+
/* I1 is exposed as power_crit or as curr_crit depending on bit 31 */
static int hwm_pcode_read_i1(struct drm_i915_private *i915, u32 *uval)
{
@@ -428,7 +440,10 @@ hwm_energy_is_visible(const struct hwm_drvdata *ddat, u32 attr)
switch (attr) {
case hwmon_energy_input:
- rgaddr = hwmon->rg.energy_status_all;
+ if (ddat->gt_n >= 0)
+ rgaddr = hwmon->rg.energy_status_tile;
+ else
+ rgaddr = hwmon->rg.energy_status_all;
return i915_mmio_reg_valid(rgaddr) ? 0444 : 0;
default:
return 0;
@@ -563,6 +578,44 @@ static const struct hwmon_chip_info hwm_chip_info = {
.info = hwm_info,
};
+static umode_t
+hwm_gt_is_visible(const void *drvdata, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ struct hwm_drvdata *ddat = (struct hwm_drvdata *)drvdata;
+
+ switch (type) {
+ case hwmon_energy:
+ return hwm_energy_is_visible(ddat, attr);
+ default:
+ return 0;
+ }
+}
+
+static int
+hwm_gt_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long *val)
+{
+ struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+
+ switch (type) {
+ case hwmon_energy:
+ return hwm_energy_read(ddat, attr, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static const struct hwmon_ops hwm_gt_ops = {
+ .is_visible = hwm_gt_is_visible,
+ .read = hwm_gt_read,
+};
+
+static const struct hwmon_chip_info hwm_gt_chip_info = {
+ .ops = &hwm_gt_ops,
+ .info = hwm_gt_info,
+};
+
static void
hwm_get_preregistration_info(struct drm_i915_private *i915)
{
@@ -571,7 +624,9 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
struct hwm_drvdata *ddat = &hwmon->ddat;
intel_wakeref_t wakeref;
u32 val_sku_unit;
+ struct intel_gt *gt;
long energy;
+ int i;
if (IS_DG1(i915) || IS_DG2(i915)) {
hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
@@ -579,12 +634,21 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
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 if (IS_XEHPSDV(i915)) {
+ hwmon->rg.pkg_power_sku_unit = GT0_PACKAGE_POWER_SKU_UNIT;
+ hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
+ hwmon->rg.pkg_rapl_limit = GT0_PACKAGE_RAPL_LIMIT;
+ hwmon->rg.energy_status_all = GT0_PLATFORM_ENERGY_STATUS;
+ hwmon->rg.energy_status_tile = GT0_PACKAGE_ENERGY_STATUS;
+ hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
} else {
hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
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;
}
with_intel_runtime_pm(uncore->rpm, wakeref) {
@@ -614,6 +678,10 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
*/
if (i915_mmio_reg_valid(hwmon->rg.energy_status_all))
hwm_energy(ddat, &energy);
+ if (i915_mmio_reg_valid(hwmon->rg.energy_status_tile)) {
+ for_each_gt(gt, i915, i)
+ hwm_energy(&hwmon->ddat_gt[i], &energy);
+ }
}
void i915_hwmon_register(struct drm_i915_private *i915)
@@ -622,6 +690,10 @@ void i915_hwmon_register(struct drm_i915_private *i915)
struct i915_hwmon *hwmon;
struct device *hwmon_dev;
struct hwm_drvdata *ddat;
+ struct hwm_drvdata *ddat_gt;
+ struct intel_gt *gt;
+ const char *ddname;
+ int i;
/* hwmon is available only for dGfx */
if (!IS_DGFX(i915))
@@ -638,6 +710,16 @@ void i915_hwmon_register(struct drm_i915_private *i915)
ddat->hwmon = hwmon;
ddat->uncore = &i915->uncore;
snprintf(ddat->name, sizeof(ddat->name), "i915");
+ ddat->gt_n = -1;
+
+ for_each_gt(gt, i915, i) {
+ ddat_gt = hwmon->ddat_gt + i;
+
+ ddat_gt->hwmon = hwmon;
+ ddat_gt->uncore = gt->uncore;
+ snprintf(ddat_gt->name, sizeof(ddat_gt->name), "i915_gt%u", i);
+ ddat_gt->gt_n = i;
+ }
hwm_get_preregistration_info(i915);
@@ -654,18 +736,50 @@ void i915_hwmon_register(struct drm_i915_private *i915)
}
ddat->hwmon_dev = hwmon_dev;
+
+ for_each_gt(gt, i915, i) {
+ ddat_gt = hwmon->ddat_gt + i;
+ /*
+ * Create per-gt directories only if a per-gt attribute is
+ * visible. Currently this is only energy
+ */
+ if (!hwm_gt_is_visible(ddat_gt, hwmon_energy, hwmon_energy_input, 0))
+ continue;
+
+ ddname = ddat_gt->name;
+ hwmon_dev = hwmon_device_register_with_info(dev, ddname,
+ ddat_gt,
+ &hwm_gt_chip_info,
+ NULL);
+ if (!IS_ERR(hwmon_dev))
+ ddat_gt->hwmon_dev = hwmon_dev;
+ }
}
void i915_hwmon_unregister(struct drm_i915_private *i915)
{
struct i915_hwmon *hwmon;
struct hwm_drvdata *ddat;
+ struct intel_gt *gt;
+ int i;
hwmon = fetch_and_zero(&i915->hwmon);
if (!hwmon)
return;
ddat = &hwmon->ddat;
+
+ for_each_gt(gt, i915, i) {
+ struct hwm_drvdata *ddat_gt;
+
+ ddat_gt = hwmon->ddat_gt + i;
+
+ if (ddat_gt->hwmon_dev) {
+ hwmon_device_unregister(ddat_gt->hwmon_dev);
+ ddat_gt->hwmon_dev = NULL;
+ }
+ }
+
if (ddat->hwmon_dev)
hwmon_device_unregister(ddat->hwmon_dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 7/7] drm/i915/hwmon: Extend power/energy for XEHPSDV
2022-08-25 13:21 ` [PATCH 7/7] drm/i915/hwmon: Extend power/energy for XEHPSDV Badal Nilawar
@ 2022-08-30 5:34 ` Dixit, Ashutosh
0 siblings, 0 replies; 32+ messages in thread
From: Dixit, Ashutosh @ 2022-08-30 5:34 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-gfx, riana.tauro, anshuman.gupta, jon.ewins, linux-hwmon
On Thu, 25 Aug 2022 06:21:18 -0700, Badal Nilawar wrote:
>
> From: Dale B Stimson <dale.b.stimson@intel.com>
>
> Extend hwmon power/energy for XEHPSDV especially per gt level energy
> usage.
A couple of very minor nits below which need to be fixed. But otherwise
this patch is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index b8ac52f07681..76d73216c54e 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -12,6 +12,7 @@
> #include "i915_reg.h"
> #include "intel_mchbar_regs.h"
> #include "intel_pcode.h"
> +#include "gt/intel_gt.h"
> #include "gt/intel_gt_regs.h"
>
> /*
> @@ -21,7 +22,7 @@
> * - curr - milliamperes
> * - energy - microjoules
> */
> -#define SF_TIME 1000
> +#define SF_TIME 1000
This change should not be in this patch, it seems an unnecessary space has
been inserted.
> @@ -622,6 +690,10 @@ void i915_hwmon_register(struct drm_i915_private *i915)
> struct i915_hwmon *hwmon;
> struct device *hwmon_dev;
> struct hwm_drvdata *ddat;
> + struct hwm_drvdata *ddat_gt;
> + struct intel_gt *gt;
> + const char *ddname;
Variable is not needed, let's delete it and directly use ddat_gt->name.
> void i915_hwmon_unregister(struct drm_i915_private *i915)
> {
> struct i915_hwmon *hwmon;
> struct hwm_drvdata *ddat;
> + struct intel_gt *gt;
> + int i;
>
> hwmon = fetch_and_zero(&i915->hwmon);
> if (!hwmon)
> return;
>
> ddat = &hwmon->ddat;
> +
> + for_each_gt(gt, i915, i) {
> + struct hwm_drvdata *ddat_gt;
> +
> + ddat_gt = hwmon->ddat_gt + i;
> +
> + if (ddat_gt->hwmon_dev) {
> + hwmon_device_unregister(ddat_gt->hwmon_dev);
> + ddat_gt->hwmon_dev = NULL;
No need to set to NULL, we are doing 'kfree(hwmon)' below, let's just
delete this line.
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 32+ messages in thread