intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests
@ 2022-11-30  5:34 Riana Tauro
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower Riana Tauro
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Riana Tauro @ 2022-11-30  5:34 UTC (permalink / raw)
  To: intel-gfx

Rename librapl library to libpower. Add hwmon support in libpower for
dgfx. 
Use libpower in selftests.

Rev2 : Update commit message
Rev3 : Remove redundant code
Rev4 : Add hwmon per-gt support
Rev5 : No functional changes. 
       Change author for last patch

Riana Tauro (2):
  drm/i915/selftests: Rename librapl library to libpower
  drm/i915/hwmon: Add helper function to obtain energy values

Tilak Tangudu (1):
  drm/i915/selftests: Add hwmon support in libpower for dgfx

 drivers/gpu/drm/i915/Makefile             |  2 +-
 drivers/gpu/drm/i915/gt/selftest_rc6.c    | 12 ++++----
 drivers/gpu/drm/i915/gt/selftest_rps.c    | 26 ++++++++---------
 drivers/gpu/drm/i915/gt/selftest_slpc.c   |  4 +--
 drivers/gpu/drm/i915/i915_hwmon.c         | 28 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_hwmon.h         |  3 ++
 drivers/gpu/drm/i915/selftests/libpower.c | 33 ++++++++++++++++++++++
 drivers/gpu/drm/i915/selftests/libpower.h | 19 +++++++++++++
 drivers/gpu/drm/i915/selftests/librapl.c  | 34 -----------------------
 drivers/gpu/drm/i915/selftests/librapl.h  | 17 ------------
 10 files changed, 105 insertions(+), 73 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/libpower.c
 create mode 100644 drivers/gpu/drm/i915/selftests/libpower.h
 delete mode 100644 drivers/gpu/drm/i915/selftests/librapl.c
 delete mode 100644 drivers/gpu/drm/i915/selftests/librapl.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower
  2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
@ 2022-11-30  5:34 ` Riana Tauro
  2022-12-02 22:13   ` Dixit, Ashutosh
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values Riana Tauro
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Riana Tauro @ 2022-11-30  5:34 UTC (permalink / raw)
  To: intel-gfx

Rename librapl files to libpower and replace librapl
with libpower prefix. No functional changes

v2: update commit message (Anshuman)

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/Makefile                   |  2 +-
 drivers/gpu/drm/i915/gt/selftest_rc6.c          | 12 ++++++------
 drivers/gpu/drm/i915/gt/selftest_rps.c          |  8 ++++----
 drivers/gpu/drm/i915/gt/selftest_slpc.c         |  2 +-
 .../i915/selftests/{librapl.c => libpower.c}    | 10 +++++-----
 drivers/gpu/drm/i915/selftests/libpower.h       | 17 +++++++++++++++++
 drivers/gpu/drm/i915/selftests/librapl.h        | 17 -----------------
 7 files changed, 34 insertions(+), 34 deletions(-)
 rename drivers/gpu/drm/i915/selftests/{librapl.c => libpower.c} (69%)
 create mode 100644 drivers/gpu/drm/i915/selftests/libpower.h
 delete mode 100644 drivers/gpu/drm/i915/selftests/librapl.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 01974b82d205..ddece53f7c9f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -342,7 +342,7 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \
 	selftests/igt_mmap.o \
 	selftests/igt_reset.o \
 	selftests/igt_spinner.o \
-	selftests/librapl.o
+	selftests/libpower.o
 
 # virtual gpu code
 i915-y += i915_vgpu.o
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 2ceeadecc639..15b84c428f66 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -11,7 +11,7 @@
 #include "selftest_rc6.h"
 
 #include "selftests/i915_random.h"
-#include "selftests/librapl.h"
+#include "selftests/libpower.h"
 
 static u64 rc6_residency(struct intel_rc6 *rc6)
 {
@@ -51,7 +51,7 @@ int live_rc6_manual(void *arg)
 	if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
 		return 0;
 
-	has_power = librapl_supported(gt->i915);
+	has_power = libpower_supported(gt->i915);
 	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
 
 	/* Force RC6 off for starters */
@@ -61,9 +61,9 @@ int live_rc6_manual(void *arg)
 	res[0] = rc6_residency(rc6);
 
 	dt = ktime_get();
-	rc0_power = librapl_energy_uJ();
+	rc0_power = libpower_get_energy_uJ();
 	msleep(250);
-	rc0_power = librapl_energy_uJ() - rc0_power;
+	rc0_power = libpower_get_energy_uJ() - rc0_power;
 	dt = ktime_sub(ktime_get(), dt);
 	res[1] = rc6_residency(rc6);
 	if ((res[1] - res[0]) >> 10) {
@@ -89,9 +89,9 @@ int live_rc6_manual(void *arg)
 	res[0] = rc6_residency(rc6);
 	intel_uncore_forcewake_flush(rc6_to_uncore(rc6), FORCEWAKE_ALL);
 	dt = ktime_get();
-	rc6_power = librapl_energy_uJ();
+	rc6_power = libpower_get_energy_uJ();
 	msleep(100);
-	rc6_power = librapl_energy_uJ() - rc6_power;
+	rc6_power = libpower_get_energy_uJ() - rc6_power;
 	dt = ktime_sub(ktime_get(), dt);
 	res[1] = rc6_residency(rc6);
 	if (res[1] == res[0]) {
diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
index 39f1b7564170..b8b0b0c7617e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
@@ -19,7 +19,7 @@
 #include "selftest_rps.h"
 #include "selftests/igt_flush_test.h"
 #include "selftests/igt_spinner.h"
-#include "selftests/librapl.h"
+#include "selftests/libpower.h"
 
 /* Try to isolate the impact of cstates from determing frequency response */
 #define CPU_LATENCY 0 /* -1 to disable pm_qos, 0 to disable cstates */
@@ -1095,9 +1095,9 @@ static u64 __measure_power(int duration_ms)
 	u64 dE, dt;
 
 	dt = ktime_get();
-	dE = librapl_energy_uJ();
+	dE = libpower_get_energy_uJ();
 	usleep_range(1000 * duration_ms, 2000 * duration_ms);
-	dE = librapl_energy_uJ() - dE;
+	dE = libpower_get_energy_uJ() - dE;
 	dt = ktime_get() - dt;
 
 	return div64_u64(1000 * 1000 * dE, dt);
@@ -1143,7 +1143,7 @@ int live_rps_power(void *arg)
 	if (!intel_rps_is_enabled(rps) || GRAPHICS_VER(gt->i915) < 6)
 		return 0;
 
-	if (!librapl_supported(gt->i915))
+	if (!libpower_supported(gt->i915))
 		return 0;
 
 	if (igt_spinner_init(&spin, gt))
diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index bd44ce73a504..fc1cdda82ec6 100644
--- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
@@ -169,7 +169,7 @@ static int slpc_power(struct intel_gt *gt, struct intel_engine_cs *engine)
 	 * actually saves power. Let's see if our RAPL measurement supports
 	 * that theory.
 	 */
-	if (!librapl_supported(gt->i915))
+	if (!libpower_supported(gt->i915))
 		return 0;
 
 	min.freq = slpc->min_freq;
diff --git a/drivers/gpu/drm/i915/selftests/librapl.c b/drivers/gpu/drm/i915/selftests/libpower.c
similarity index 69%
rename from drivers/gpu/drm/i915/selftests/librapl.c
rename to drivers/gpu/drm/i915/selftests/libpower.c
index eb03b5b28bad..c66e993c5f85 100644
--- a/drivers/gpu/drm/i915/selftests/librapl.c
+++ b/drivers/gpu/drm/i915/selftests/libpower.c
@@ -1,23 +1,23 @@
 // SPDX-License-Identifier: MIT
 /*
- * Copyright © 2020 Intel Corporation
+ * Copyright © 2022 Intel Corporation
  */
 
 #include <asm/msr.h>
 
 #include "i915_drv.h"
-#include "librapl.h"
+#include "libpower.h"
 
-bool librapl_supported(const struct drm_i915_private *i915)
+bool libpower_supported(const struct drm_i915_private *i915)
 {
 	/* Discrete cards require hwmon integration */
 	if (IS_DGFX(i915))
 		return false;
 
-	return librapl_energy_uJ();
+	return libpower_get_energy_uJ();
 }
 
-u64 librapl_energy_uJ(void)
+u64 libpower_get_energy_uJ(void)
 {
 	unsigned long long power;
 	u32 units;
diff --git a/drivers/gpu/drm/i915/selftests/libpower.h b/drivers/gpu/drm/i915/selftests/libpower.h
new file mode 100644
index 000000000000..5352981eb946
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/libpower.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef SELFTEST_LIBPOWER_H
+#define SELFTEST_LIBPOWER_H
+
+#include <linux/types.h>
+
+struct drm_i915_private;
+
+bool libpower_supported(const struct drm_i915_private *i915);
+
+u64 libpower_get_energy_uJ(void);
+
+#endif /* SELFTEST_LIBPOWER_H */
diff --git a/drivers/gpu/drm/i915/selftests/librapl.h b/drivers/gpu/drm/i915/selftests/librapl.h
deleted file mode 100644
index e3b24fad0a7a..000000000000
--- a/drivers/gpu/drm/i915/selftests/librapl.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/* SPDX-License-Identifier: MIT */
-/*
- * Copyright © 2020 Intel Corporation
- */
-
-#ifndef SELFTEST_LIBRAPL_H
-#define SELFTEST_LIBRAPL_H
-
-#include <linux/types.h>
-
-struct drm_i915_private;
-
-bool librapl_supported(const struct drm_i915_private *i915);
-
-u64 librapl_energy_uJ(void);
-
-#endif /* SELFTEST_LIBRAPL_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values
  2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower Riana Tauro
@ 2022-11-30  5:34 ` Riana Tauro
  2022-12-02 22:12   ` Dixit, Ashutosh
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx Riana Tauro
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Riana Tauro @ 2022-11-30  5:34 UTC (permalink / raw)
  To: intel-gfx

Add an interface to obtain hwmon energy values. The function returns
per-gt energy if register is valid else returns the device level
energy. This is used by selftest to verify power consumption

v2 : use i915_hwmon prefix (Anshuman)
v3 : re-use is_visible function of energy to remove
     redundant code (Anshuman)
v4 : fix kernel-doc (Anshuman)
     add per-gt hwmon support (Ashutosh)

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/i915_hwmon.c | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_hwmon.h |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index c588a17f97e9..57d4e96d5c72 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -442,6 +442,34 @@ hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
 	}
 }
 
+/**
+ * i915_hwmon_get_energy - obtains energy value
+ * @gt: intel_gt structure
+ * @energy: pointer to store energy in uJ
+ *
+ * This function checks for the validity of the underlying energy
+ * hardware register and obtains the per-gt/package level energy
+ * values.
+ *
+ * Return: 0 on success, -EOPNOTSUPP if register is invalid
+ */
+int
+i915_hwmon_get_energy(struct intel_gt *gt, long *energy)
+{
+	struct i915_hwmon *hwmon = gt->i915->hwmon;
+	struct hwm_drvdata *ddat = &hwmon->ddat;
+	struct hwm_drvdata *ddat_gt = hwmon->ddat_gt + gt->info.id;
+
+	if (hwm_energy_is_visible(ddat_gt, hwmon_energy_input))
+		hwm_energy(ddat_gt, energy);
+	else if (hwm_energy_is_visible(ddat, hwmon_energy_input))
+		hwm_energy(ddat, energy);
+	else
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
 static umode_t
 hwm_curr_is_visible(const struct hwm_drvdata *ddat, u32 attr)
 {
diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
index 7ca9cf2c34c9..1c38cfdbb7e9 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.h
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -8,13 +8,16 @@
 #define __I915_HWMON_H__
 
 struct drm_i915_private;
+struct intel_gt;
 
 #if IS_REACHABLE(CONFIG_HWMON)
 void i915_hwmon_register(struct drm_i915_private *i915);
 void i915_hwmon_unregister(struct drm_i915_private *i915);
+int i915_hwmon_get_energy(struct intel_gt *gt, long *energy);
 #else
 static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
 static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
+static inline int i915_hwmon_get_energy(struct intel_gt *gt, long *energy) { return -EOPNOTSUPP; }
 #endif
 
 #endif /* __I915_HWMON_H__ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v5 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx
  2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower Riana Tauro
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values Riana Tauro
@ 2022-11-30  5:34 ` Riana Tauro
  2022-12-02 22:15   ` Dixit, Ashutosh
  2022-11-30  5:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add hwmon support for dgfx selftests (rev5) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Riana Tauro @ 2022-11-30  5:34 UTC (permalink / raw)
  To: intel-gfx

From: Tilak Tangudu <tilak.tangudu@intel.com>

hwmon provides an interface to read energy values for discrete graphics.
add hwmon support to the existing libpower library so that it can verify
power consumption values in different selftests.

Changed prototype of libpower_get_energy_uJ

v2: add per-gt hwmon support (Ashutosh)

Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
Co-developed-by: Riana Tauro <riana.tauro@intel.com>
Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_rc6.c    | 10 ++++----
 drivers/gpu/drm/i915/gt/selftest_rps.c    | 24 +++++++++---------
 drivers/gpu/drm/i915/gt/selftest_slpc.c   |  4 +--
 drivers/gpu/drm/i915/selftests/libpower.c | 31 +++++++++++------------
 drivers/gpu/drm/i915/selftests/libpower.h | 10 +++++---
 5 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 15b84c428f66..682f2fe67b3a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -51,7 +51,7 @@ int live_rc6_manual(void *arg)
 	if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
 		return 0;
 
-	has_power = libpower_supported(gt->i915);
+	has_power = libpower_supported(gt);
 	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
 
 	/* Force RC6 off for starters */
@@ -61,9 +61,9 @@ int live_rc6_manual(void *arg)
 	res[0] = rc6_residency(rc6);
 
 	dt = ktime_get();
-	rc0_power = libpower_get_energy_uJ();
+	rc0_power = libpower_get_energy_uJ(gt);
 	msleep(250);
-	rc0_power = libpower_get_energy_uJ() - rc0_power;
+	rc0_power = libpower_get_energy_uJ(gt) - rc0_power;
 	dt = ktime_sub(ktime_get(), dt);
 	res[1] = rc6_residency(rc6);
 	if ((res[1] - res[0]) >> 10) {
@@ -89,9 +89,9 @@ int live_rc6_manual(void *arg)
 	res[0] = rc6_residency(rc6);
 	intel_uncore_forcewake_flush(rc6_to_uncore(rc6), FORCEWAKE_ALL);
 	dt = ktime_get();
-	rc6_power = libpower_get_energy_uJ();
+	rc6_power = libpower_get_energy_uJ(gt);
 	msleep(100);
-	rc6_power = libpower_get_energy_uJ() - rc6_power;
+	rc6_power = libpower_get_energy_uJ(gt) - rc6_power;
 	dt = ktime_sub(ktime_get(), dt);
 	res[1] = rc6_residency(rc6);
 	if (res[1] == res[0]) {
diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
index b8b0b0c7617e..d5e9f57cca55 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
@@ -1090,38 +1090,38 @@ int live_rps_interrupt(void *arg)
 	return err;
 }
 
-static u64 __measure_power(int duration_ms)
+static u64 __measure_power(struct intel_gt *gt, int duration_ms)
 {
 	u64 dE, dt;
 
 	dt = ktime_get();
-	dE = libpower_get_energy_uJ();
+	dE = libpower_get_energy_uJ(gt);
 	usleep_range(1000 * duration_ms, 2000 * duration_ms);
-	dE = libpower_get_energy_uJ() - dE;
+	dE = libpower_get_energy_uJ(gt) - dE;
 	dt = ktime_get() - dt;
 
 	return div64_u64(1000 * 1000 * dE, dt);
 }
 
-static u64 measure_power(struct intel_rps *rps, int *freq)
+static u64 measure_power(struct intel_gt *gt, int *freq)
 {
 	u64 x[5];
 	int i;
 
 	for (i = 0; i < 5; i++)
-		x[i] = __measure_power(5);
+		x[i] = __measure_power(gt, 5);
 
-	*freq = (*freq + intel_rps_read_actual_frequency(rps)) / 2;
+	*freq = (*freq + intel_rps_read_actual_frequency(&gt->rps)) / 2;
 
 	/* A simple triangle filter for better result stability */
 	sort(x, 5, sizeof(*x), cmp_u64, NULL);
 	return div_u64(x[1] + 2 * x[2] + x[3], 4);
 }
 
-static u64 measure_power_at(struct intel_rps *rps, int *freq)
+static u64 measure_power_at(struct intel_gt *gt, int *freq)
 {
-	*freq = rps_set_check(rps, *freq);
-	return measure_power(rps, freq);
+	*freq = rps_set_check(&gt->rps, *freq);
+	return measure_power(gt, freq);
 }
 
 int live_rps_power(void *arg)
@@ -1143,7 +1143,7 @@ int live_rps_power(void *arg)
 	if (!intel_rps_is_enabled(rps) || GRAPHICS_VER(gt->i915) < 6)
 		return 0;
 
-	if (!libpower_supported(gt->i915))
+	if (!libpower_supported(gt))
 		return 0;
 
 	if (igt_spinner_init(&spin, gt))
@@ -1187,10 +1187,10 @@ int live_rps_power(void *arg)
 		}
 
 		max.freq = rps->max_freq;
-		max.power = measure_power_at(rps, &max.freq);
+		max.power = measure_power_at(gt, &max.freq);
 
 		min.freq = rps->min_freq;
-		min.power = measure_power_at(rps, &min.freq);
+		min.power = measure_power_at(gt, &min.freq);
 
 		igt_spinner_end(&spin);
 		st_engine_heartbeat_enable(engine);
diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index fc1cdda82ec6..20b9e05d3d16 100644
--- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
@@ -78,7 +78,7 @@ static u64 measure_power_at_freq(struct intel_gt *gt, int *freq, u64 *power)
 	if (err)
 		return err;
 	*freq = intel_rps_read_actual_frequency(&gt->rps);
-	*power = measure_power(&gt->rps, freq);
+	*power = measure_power(gt, freq);
 
 	return err;
 }
@@ -169,7 +169,7 @@ static int slpc_power(struct intel_gt *gt, struct intel_engine_cs *engine)
 	 * actually saves power. Let's see if our RAPL measurement supports
 	 * that theory.
 	 */
-	if (!libpower_supported(gt->i915))
+	if (!libpower_supported(gt))
 		return 0;
 
 	min.freq = slpc->min_freq;
diff --git a/drivers/gpu/drm/i915/selftests/libpower.c b/drivers/gpu/drm/i915/selftests/libpower.c
index c66e993c5f85..3d4d2dc74a54 100644
--- a/drivers/gpu/drm/i915/selftests/libpower.c
+++ b/drivers/gpu/drm/i915/selftests/libpower.c
@@ -6,29 +6,28 @@
 #include <asm/msr.h>
 
 #include "i915_drv.h"
+#include "i915_hwmon.h"
 #include "libpower.h"
 
-bool libpower_supported(const struct drm_i915_private *i915)
-{
-	/* Discrete cards require hwmon integration */
-	if (IS_DGFX(i915))
-		return false;
-
-	return libpower_get_energy_uJ();
-}
-
-u64 libpower_get_energy_uJ(void)
+u64 libpower_get_energy_uJ(struct intel_gt *gt)
 {
 	unsigned long long power;
 	u32 units;
+	long energy_uJ = 0;
 
-	if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
-		return 0;
+	if (IS_DGFX(gt->i915)) {
+		if (i915_hwmon_get_energy(gt, &energy_uJ))
+			return 0;
+	} else {
+		if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
+			return 0;
 
-	units = (power & 0x1f00) >> 8;
+		units = (power & 0x1f00) >> 8;
 
-	if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power))
-		return 0;
+		if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power))
+			return 0;
 
-	return (1000000 * power) >> units; /* convert to uJ */
+		energy_uJ = (1000000 * power) >> units; /* convert to uJ */
+	}
+	return energy_uJ;
 }
diff --git a/drivers/gpu/drm/i915/selftests/libpower.h b/drivers/gpu/drm/i915/selftests/libpower.h
index 5352981eb946..e4410a886654 100644
--- a/drivers/gpu/drm/i915/selftests/libpower.h
+++ b/drivers/gpu/drm/i915/selftests/libpower.h
@@ -8,10 +8,12 @@
 
 #include <linux/types.h>
 
-struct drm_i915_private;
+struct intel_gt;
 
-bool libpower_supported(const struct drm_i915_private *i915);
-
-u64 libpower_get_energy_uJ(void);
+u64 libpower_get_energy_uJ(struct intel_gt *gt);
 
+static inline bool libpower_supported(struct intel_gt *gt)
+{
+	return libpower_get_energy_uJ(gt);
+}
 #endif /* SELFTEST_LIBPOWER_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add hwmon support for dgfx selftests (rev5)
  2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
                   ` (2 preceding siblings ...)
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx Riana Tauro
@ 2022-11-30  5:51 ` Patchwork
  2022-11-30  5:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
  2022-11-30  6:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-11-30  5:51 UTC (permalink / raw)
  To: Riana Tauro; +Cc: intel-gfx

== Series Details ==

Series: Add hwmon support for dgfx selftests (rev5)
URL   : https://patchwork.freedesktop.org/series/109850/
State : warning

== Summary ==

Error: dim checkpatch failed
d9cc1aa76ff4 drm/i915/selftests: Rename librapl library to libpower
Traceback (most recent call last):
  File "scripts/spdxcheck.py", line 11, in <module>
    import git
ModuleNotFoundError: No module named 'git'
-:122: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#122: 
rename from drivers/gpu/drm/i915/selftests/librapl.c

total: 0 errors, 1 warnings, 0 checks, 126 lines checked
47b65bdabdb1 drm/i915/hwmon: Add helper function to obtain energy values
03bf1a9c90bb drm/i915/selftests: Add hwmon support in libpower for dgfx



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Add hwmon support for dgfx selftests (rev5)
  2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
                   ` (3 preceding siblings ...)
  2022-11-30  5:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add hwmon support for dgfx selftests (rev5) Patchwork
@ 2022-11-30  5:51 ` Patchwork
  2022-11-30  6:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-11-30  5:51 UTC (permalink / raw)
  To: Riana Tauro; +Cc: intel-gfx

== Series Details ==

Series: Add hwmon support for dgfx selftests (rev5)
URL   : https://patchwork.freedesktop.org/series/109850/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:180:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:180:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:182:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:186:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:186:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:188:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:188:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:192:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:192:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:195:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:237:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:237:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:239:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:239:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:55:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:55:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:57:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:57:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:58:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:58:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:15: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:60:15: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:73:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:73:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:75:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:75:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:76:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:76:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:77:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:77:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:79:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:79:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:79:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:80:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:80:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:80:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:93:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:93:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:95:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:95:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:96:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:96:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:97:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:97:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:99:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:99:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:21: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:99:21: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/instrumented-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:112:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:112:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:115:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:115:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:127:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:127:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:130:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:130:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:139:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:139:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:142:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:142:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:26:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:26:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:42:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:42:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:58:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:58:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:97:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:97:1: warning: unreplaced symbol 'return'



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for Add hwmon support for dgfx selftests (rev5)
  2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
                   ` (4 preceding siblings ...)
  2022-11-30  5:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2022-11-30  6:18 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-11-30  6:18 UTC (permalink / raw)
  To: Riana Tauro; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 15358 bytes --]

== Series Details ==

Series: Add hwmon support for dgfx selftests (rev5)
URL   : https://patchwork.freedesktop.org/series/109850/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12450 -> Patchwork_109850v5
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_109850v5 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_109850v5, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/index.html

Participating hosts (32 -> 37)
------------------------------

  Additional (5): bat-kbl-2 bat-dg1-5 bat-adlp-4 bat-atsm-1 bat-jsl-3 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_109850v5:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gt_pm:
    - bat-dg1-5:          NOTRUN -> [DMESG-FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@i915_selftest@live@gt_pm.html
    - bat-dg1-6:          [PASS][2] -> [DMESG-FAIL][3] +1 similar issue
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-dg1-6/igt@i915_selftest@live@gt_pm.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-6/igt@i915_selftest@live@gt_pm.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-atsm-1}:       NOTRUN -> [INCOMPLETE][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-atsm-1/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@i915_selftest@live@gt_pm:
    - {bat-dg1-7}:        [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-dg1-7/igt@i915_selftest@live@gt_pm.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-7/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@requests:
    - {bat-rplp-1}:       [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-rplp-1/igt@i915_selftest@live@requests.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-rplp-1/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - {bat-atsm-1}:       NOTRUN -> [DMESG-FAIL][9] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-atsm-1/igt@i915_selftest@live@slpc.html
    - {bat-dg2-9}:        [PASS][10] -> [DMESG-FAIL][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-dg2-9/igt@i915_selftest@live@slpc.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg2-9/igt@i915_selftest@live@slpc.html
    - {bat-dg2-11}:       [PASS][12] -> [DMESG-FAIL][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-dg2-11/igt@i915_selftest@live@slpc.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg2-11/igt@i915_selftest@live@slpc.html
    - {bat-dg2-8}:        [PASS][14] -> [DMESG-FAIL][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-dg2-8/igt@i915_selftest@live@slpc.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg2-8/igt@i915_selftest@live@slpc.html

  
Known issues
------------

  Here are the changes found in Patchwork_109850v5 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-adlp-4:         NOTRUN -> [SKIP][16] ([i915#7456])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@debugfs_test@basic-hwmon.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-adlp-4:         NOTRUN -> [SKIP][17] ([i915#4613]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_mmap@basic:
    - bat-dg1-5:          NOTRUN -> [SKIP][18] ([i915#4083])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@gem_mmap@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg1-5:          NOTRUN -> [SKIP][19] ([i915#4077]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-dg1-5:          NOTRUN -> [SKIP][20] ([i915#4079]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@gem_tiled_pread_basic.html
    - bat-adlp-4:         NOTRUN -> [SKIP][21] ([i915#3282])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-dg1-5:          NOTRUN -> [SKIP][22] ([i915#7561])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg1-5:          NOTRUN -> [SKIP][23] ([i915#6621])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@i915_pm_rps@basic-api.html
    - bat-adlp-4:         NOTRUN -> [SKIP][24] ([i915#6621])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@i915_pm_rps@basic-api.html

  * igt@kms_addfb_basic@basic-x-tiled-legacy:
    - bat-dg1-5:          NOTRUN -> [SKIP][25] ([i915#4212]) +7 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_addfb_basic@basic-x-tiled-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg1-5:          NOTRUN -> [SKIP][26] ([i915#4215])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_chamelium@dp-crc-fast:
    - bat-adlp-4:         NOTRUN -> [SKIP][27] ([fdo#111827]) +8 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - bat-dg1-5:          NOTRUN -> [SKIP][28] ([fdo#111827]) +8 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - bat-adlp-4:         NOTRUN -> [SKIP][29] ([i915#4103])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
    - bat-dg1-5:          NOTRUN -> [SKIP][30] ([i915#4103] / [i915#4213])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-dg1-5:          NOTRUN -> [SKIP][31] ([fdo#109285])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-adlp-4:         NOTRUN -> [SKIP][32] ([i915#4093]) +3 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-adlp-4:         NOTRUN -> [SKIP][33] ([i915#3546])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@kms_pipe_crc_basic@suspend-read-crc.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-dg1-5:          NOTRUN -> [SKIP][34] ([i915#1072] / [i915#4078]) +3 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg1-5:          NOTRUN -> [SKIP][35] ([i915#3555])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-adlp-4:         NOTRUN -> [SKIP][36] ([i915#3555] / [i915#4579])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-read:
    - bat-dg1-5:          NOTRUN -> [SKIP][37] ([i915#3708]) +3 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-gtt:
    - bat-dg1-5:          NOTRUN -> [SKIP][38] ([i915#3708] / [i915#4077]) +1 similar issue
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-userptr:
    - bat-adlp-4:         NOTRUN -> [SKIP][39] ([fdo#109295] / [i915#3301] / [i915#3708])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@prime_vgem@basic-userptr.html
    - bat-dg1-5:          NOTRUN -> [SKIP][40] ([i915#3708] / [i915#4873])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-dg1-5/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@basic-write:
    - bat-adlp-4:         NOTRUN -> [SKIP][41] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-4/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@fbdev@read:
    - {bat-rpls-2}:       [SKIP][42] ([i915#2582]) -> [PASS][43] +4 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-rpls-2/igt@fbdev@read.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-rpls-2/igt@fbdev@read.html

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-rpls-1}:       [DMESG-WARN][44] ([i915#7625]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@i915_selftest@live@migrate:
    - {bat-adlp-6}:       [INCOMPLETE][46] ([i915#7348]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-adlp-6/igt@i915_selftest@live@migrate.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-adlp-6/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@slpc:
    - {bat-rpls-1}:       [DMESG-FAIL][48] ([i915#6367]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/bat-rpls-1/igt@i915_selftest@live@slpc.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [FAIL][50] ([i915#6298]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12450/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3003]: https://gitlab.freedesktop.org/drm/intel/issues/3003
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4093]: https://gitlab.freedesktop.org/drm/intel/issues/4093
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6077]: https://gitlab.freedesktop.org/drm/intel/issues/6077
  [i915#6078]: https://gitlab.freedesktop.org/drm/intel/issues/6078
  [i915#6093]: https://gitlab.freedesktop.org/drm/intel/issues/6093
  [i915#6094]: https://gitlab.freedesktop.org/drm/intel/issues/6094
  [i915#6166]: https://gitlab.freedesktop.org/drm/intel/issues/6166
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7348]: https://gitlab.freedesktop.org/drm/intel/issues/7348
  [i915#7357]: https://gitlab.freedesktop.org/drm/intel/issues/7357
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7625]: https://gitlab.freedesktop.org/drm/intel/issues/7625


Build changes
-------------

  * Linux: CI_DRM_12450 -> Patchwork_109850v5

  CI-20190529: 20190529
  CI_DRM_12450: 612c6df988b03d07ce7d938947a6f9c35947b417 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7075: ae0ad0ffa147445cdf9d33c96a018f5a8b21a5ee @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_109850v5: 612c6df988b03d07ce7d938947a6f9c35947b417 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

88580d70009e drm/i915/selftests: Add hwmon support in libpower for dgfx
b26d4a68f24b drm/i915/hwmon: Add helper function to obtain energy values
3c1d88f9f394 drm/i915/selftests: Rename librapl library to libpower

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109850v5/index.html

[-- Attachment #2: Type: text/html, Size: 16625 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values Riana Tauro
@ 2022-12-02 22:12   ` Dixit, Ashutosh
  2022-12-05  7:44     ` Tauro, Riana
  0 siblings, 1 reply; 12+ messages in thread
From: Dixit, Ashutosh @ 2022-12-02 22:12 UTC (permalink / raw)
  To: Riana Tauro; +Cc: intel-gfx

On Tue, 29 Nov 2022 21:34:26 -0800, Riana Tauro wrote:
>

Hi Riana,

Mostly looks good but I have a little nit below.

> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index c588a17f97e9..57d4e96d5c72 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -442,6 +442,34 @@ hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
>	}
>  }
>
> +/**
> + * i915_hwmon_get_energy - obtains energy value
> + * @gt: intel_gt structure
> + * @energy: pointer to store energy in uJ
> + *
> + * This function checks for the validity of the underlying energy
> + * hardware register and obtains the per-gt/package level energy

Do we every use this function to find real package level energy? I don't
see it. I think what we mean here is that package level energy if there's
only one gt and gt level energy is not available, correct?

So I think we should make this explicit in the code below. Also change the
comment above to say only per-gt level energy.

> + * values.
> + *
> + * Return: 0 on success, -EOPNOTSUPP if register is invalid
> + */
> +int
> +i915_hwmon_get_energy(struct intel_gt *gt, long *energy)
> +{
> +	struct i915_hwmon *hwmon = gt->i915->hwmon;
> +	struct hwm_drvdata *ddat = &hwmon->ddat;
> +	struct hwm_drvdata *ddat_gt = hwmon->ddat_gt + gt->info.id;
> +
> +	if (hwm_energy_is_visible(ddat_gt, hwmon_energy_input))
> +		hwm_energy(ddat_gt, energy);
> +	else if (hwm_energy_is_visible(ddat, hwmon_energy_input))

So if we get here and we are finding gt level energy there must be only one
gt, correct?

So probably we need to do something like (maybe in intel_gt.h?):

static inline int intel_num_gt(struct drm_i915_private *i915)
{
	struct intel_gt *gt;
	int num_gt = 0, i;

	for_each_gt(gt, i915, i)
		num_gt++;

	return num_gt;
}

And then the above check becomes:

	else if (intel_num_gt() == 1 &&
		 hwm_energy_is_visible(ddat, hwmon_energy_input))

So this way we are basically always returning gt level energy from
i915_hwmon_get_energy.

If ever we need package level energy in the future we can add a new
function which takes a 'struct drm_i915_private *i915' arg (and uses
i915->hwmon->ddat).

Thanks.
--
Ashutosh


> +		hwm_energy(ddat, energy);
> +	else
> +		return -EOPNOTSUPP;
> +
> +	return 0;
> +}
> +
>  static umode_t
>  hwm_curr_is_visible(const struct hwm_drvdata *ddat, u32 attr)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> index 7ca9cf2c34c9..1c38cfdbb7e9 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.h
> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> @@ -8,13 +8,16 @@
>  #define __I915_HWMON_H__
>
>  struct drm_i915_private;
> +struct intel_gt;
>
>  #if IS_REACHABLE(CONFIG_HWMON)
>  void i915_hwmon_register(struct drm_i915_private *i915);
>  void i915_hwmon_unregister(struct drm_i915_private *i915);
> +int i915_hwmon_get_energy(struct intel_gt *gt, long *energy);
>  #else
>  static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
>  static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
> +static inline int i915_hwmon_get_energy(struct intel_gt *gt, long *energy) { return -EOPNOTSUPP; }
>  #endif
>
>  #endif /* __I915_HWMON_H__ */
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower Riana Tauro
@ 2022-12-02 22:13   ` Dixit, Ashutosh
  0 siblings, 0 replies; 12+ messages in thread
From: Dixit, Ashutosh @ 2022-12-02 22:13 UTC (permalink / raw)
  To: Riana Tauro; +Cc: intel-gfx

On Tue, 29 Nov 2022 21:34:25 -0800, Riana Tauro wrote:
>
> Rename librapl files to libpower and replace librapl
> with libpower prefix. No functional changes
>
> v2: update commit message (Anshuman)
>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v5 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx
  2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx Riana Tauro
@ 2022-12-02 22:15   ` Dixit, Ashutosh
  0 siblings, 0 replies; 12+ messages in thread
From: Dixit, Ashutosh @ 2022-12-02 22:15 UTC (permalink / raw)
  To: Riana Tauro; +Cc: intel-gfx

On Tue, 29 Nov 2022 21:34:27 -0800, Riana Tauro wrote:
>
> diff --git a/drivers/gpu/drm/i915/selftests/libpower.c b/drivers/gpu/drm/i915/selftests/libpower.c
> index c66e993c5f85..3d4d2dc74a54 100644
> --- a/drivers/gpu/drm/i915/selftests/libpower.c
> +++ b/drivers/gpu/drm/i915/selftests/libpower.c
> @@ -6,29 +6,28 @@
>  #include <asm/msr.h>
>
>  #include "i915_drv.h"
> +#include "i915_hwmon.h"
>  #include "libpower.h"
>
> -bool libpower_supported(const struct drm_i915_private *i915)
> -{
> -	/* Discrete cards require hwmon integration */
> -	if (IS_DGFX(i915))
> -		return false;
> -
> -	return libpower_get_energy_uJ();
> -}
> -
> -u64 libpower_get_energy_uJ(void)
> +u64 libpower_get_energy_uJ(struct intel_gt *gt)
>  {
>	unsigned long long power;
>	u32 units;
> +	long energy_uJ = 0;
>
> -	if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
> -		return 0;
> +	if (IS_DGFX(gt->i915)) {
> +		if (i915_hwmon_get_energy(gt, &energy_uJ))
> +			return 0;
> +	} else {
> +		if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
> +			return 0;
>
> -	units = (power & 0x1f00) >> 8;
> +		units = (power & 0x1f00) >> 8;
>
> -	if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power))
> -		return 0;
> +		if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power))
> +			return 0;
>
> -	return (1000000 * power) >> units; /* convert to uJ */
> +		energy_uJ = (1000000 * power) >> units; /* convert to uJ */
> +	}
> +	return energy_uJ;
>  }
> diff --git a/drivers/gpu/drm/i915/selftests/libpower.h b/drivers/gpu/drm/i915/selftests/libpower.h
> index 5352981eb946..e4410a886654 100644
> --- a/drivers/gpu/drm/i915/selftests/libpower.h
> +++ b/drivers/gpu/drm/i915/selftests/libpower.h
> @@ -8,10 +8,12 @@
>
>  #include <linux/types.h>
>
> -struct drm_i915_private;
> +struct intel_gt;
>
> -bool libpower_supported(const struct drm_i915_private *i915);
> -
> -u64 libpower_get_energy_uJ(void);
> +u64 libpower_get_energy_uJ(struct intel_gt *gt);
>
> +static inline bool libpower_supported(struct intel_gt *gt)
> +{
> +	return libpower_get_energy_uJ(gt);
> +}

I think it might have been preferable to make the libpower.* changes as
part of Patch 2 (so Patch 3 would be trivial) but probably doesn't matter
so this is also:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values
  2022-12-02 22:12   ` Dixit, Ashutosh
@ 2022-12-05  7:44     ` Tauro, Riana
  2022-12-05  8:46       ` Dixit, Ashutosh
  0 siblings, 1 reply; 12+ messages in thread
From: Tauro, Riana @ 2022-12-05  7:44 UTC (permalink / raw)
  To: Dixit, Ashutosh, intel-gfx, tilak.tangudu, anshuman.gupta, badal.nilawar



On 12/3/2022 3:42 AM, Dixit, Ashutosh wrote:
> On Tue, 29 Nov 2022 21:34:26 -0800, Riana Tauro wrote:
>>
> 
> Hi Riana,
> 
> Mostly looks good but I have a little nit below.
> 
>> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
>> index c588a17f97e9..57d4e96d5c72 100644
>> --- a/drivers/gpu/drm/i915/i915_hwmon.c
>> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
>> @@ -442,6 +442,34 @@ hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
>> 	}
>>   }
>>
>> +/**
>> + * i915_hwmon_get_energy - obtains energy value
>> + * @gt: intel_gt structure
>> + * @energy: pointer to store energy in uJ
>> + *
>> + * This function checks for the validity of the underlying energy
>> + * hardware register and obtains the per-gt/package level energy
> 
> Do we every use this function to find real package level energy? I don't
> see it. I think what we mean here is that package level energy if there's
> only one gt and gt level energy is not available, correct?
Yes. When gt level energy is not available function should return the 
package energy.
> 
> So I think we should make this explicit in the code below. Also change the
> comment above to say only per-gt level energy.
> 
Will change this.
>> + * values.
>> + *
>> + * Return: 0 on success, -EOPNOTSUPP if register is invalid
>> + */
>> +int
>> +i915_hwmon_get_energy(struct intel_gt *gt, long *energy)
>> +{
>> +	struct i915_hwmon *hwmon = gt->i915->hwmon;
>> +	struct hwm_drvdata *ddat = &hwmon->ddat;
>> +	struct hwm_drvdata *ddat_gt = hwmon->ddat_gt + gt->info.id;
>> +
>> +	if (hwm_energy_is_visible(ddat_gt, hwmon_energy_input))
>> +		hwm_energy(ddat_gt, energy);
>> +	else if (hwm_energy_is_visible(ddat, hwmon_energy_input))
> 
> So if we get here and we are finding gt level energy there must be only one
> gt, correct?
> 
> So probably we need to do something like (maybe in intel_gt.h?):
> 
> static inline int intel_num_gt(struct drm_i915_private *i915)
> {
> 	struct intel_gt *gt;
> 	int num_gt = 0, i;
> 
> 	for_each_gt(gt, i915, i)
> 		num_gt++;
> 
> 	return num_gt;
> }
> 
> And then the above check becomes:
> 
> 	else if (intel_num_gt() == 1 &&
> 		 hwm_energy_is_visible(ddat, hwmon_energy_input))
> 
> So this way we are basically always returning gt level energy from
> i915_hwmon_get_energy.

Is it okay to use this macro instead of adding a new function?

if (!HAS_EXTRA_GT_LIST(gt->i915) && hwm_energy_is_visible(ddat, 
hwmon_energy_input))

Thanks
Riana
> 
> If ever we need package level energy in the future we can add a new
> function which takes a 'struct drm_i915_private *i915' arg (and uses
> i915->hwmon->ddat).
> 
> Thanks.
> --
> Ashutosh
> 
> 
>> +		hwm_energy(ddat, energy);
>> +	else
>> +		return -EOPNOTSUPP;
>> +
>> +	return 0;
>> +}
>> +
>>   static umode_t
>>   hwm_curr_is_visible(const struct hwm_drvdata *ddat, u32 attr)
>>   {
>> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
>> index 7ca9cf2c34c9..1c38cfdbb7e9 100644
>> --- a/drivers/gpu/drm/i915/i915_hwmon.h
>> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
>> @@ -8,13 +8,16 @@
>>   #define __I915_HWMON_H__
>>
>>   struct drm_i915_private;
>> +struct intel_gt;
>>
>>   #if IS_REACHABLE(CONFIG_HWMON)
>>   void i915_hwmon_register(struct drm_i915_private *i915);
>>   void i915_hwmon_unregister(struct drm_i915_private *i915);
>> +int i915_hwmon_get_energy(struct intel_gt *gt, long *energy);
>>   #else
>>   static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
>>   static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
>> +static inline int i915_hwmon_get_energy(struct intel_gt *gt, long *energy) { return -EOPNOTSUPP; }
>>   #endif
>>
>>   #endif /* __I915_HWMON_H__ */
>> --
>> 2.25.1
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values
  2022-12-05  7:44     ` Tauro, Riana
@ 2022-12-05  8:46       ` Dixit, Ashutosh
  0 siblings, 0 replies; 12+ messages in thread
From: Dixit, Ashutosh @ 2022-12-05  8:46 UTC (permalink / raw)
  To: Tauro, Riana; +Cc: intel-gfx

On Sun, 04 Dec 2022 23:44:57 -0800, Tauro, Riana wrote:
>
> On 12/3/2022 3:42 AM, Dixit, Ashutosh wrote:
> > On Tue, 29 Nov 2022 21:34:26 -0800, Riana Tauro wrote:
> >>
> >
> > Hi Riana,
> >
> > Mostly looks good but I have a little nit below.
> >
> >> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> >> index c588a17f97e9..57d4e96d5c72 100644
> >> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> >> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> >> @@ -442,6 +442,34 @@ hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> >>	}
> >>   }
> >>
> >> +/**
> >> + * i915_hwmon_get_energy - obtains energy value
> >> + * @gt: intel_gt structure
> >> + * @energy: pointer to store energy in uJ
> >> + *
> >> + * This function checks for the validity of the underlying energy
> >> + * hardware register and obtains the per-gt/package level energy
> >
> > Do we every use this function to find real package level energy? I don't
> > see it. I think what we mean here is that package level energy if there's
> > only one gt and gt level energy is not available, correct?
> Yes. When gt level energy is not available function should return the
> package energy.
> >
> > So I think we should make this explicit in the code below. Also change the
> > comment above to say only per-gt level energy.
> >
> Will change this.
>
> >> + * values.
> >> + *
> >> + * Return: 0 on success, -EOPNOTSUPP if register is invalid
> >> + */
> >> +int
> >> +i915_hwmon_get_energy(struct intel_gt *gt, long *energy)
> >> +{
> >> +	struct i915_hwmon *hwmon = gt->i915->hwmon;
> >> +	struct hwm_drvdata *ddat = &hwmon->ddat;
> >> +	struct hwm_drvdata *ddat_gt = hwmon->ddat_gt + gt->info.id;
> >> +
> >> +	if (hwm_energy_is_visible(ddat_gt, hwmon_energy_input))
> >> +		hwm_energy(ddat_gt, energy);
> >> +	else if (hwm_energy_is_visible(ddat, hwmon_energy_input))
> >
> > So if we get here and we are finding gt level energy there must be only one
> > gt, correct?
> >
> > So probably we need to do something like (maybe in intel_gt.h?):
> >
> > static inline int intel_num_gt(struct drm_i915_private *i915)
> > {
> >	struct intel_gt *gt;
> >	int num_gt = 0, i;
> >
> >	for_each_gt(gt, i915, i)
> >		num_gt++;
> >
> >	return num_gt;
> > }
> >
> > And then the above check becomes:
> >
> >	else if (intel_num_gt() == 1 &&
> >		 hwm_energy_is_visible(ddat, hwmon_energy_input))
> >
> > So this way we are basically always returning gt level energy from
> > i915_hwmon_get_energy.
>
> Is it okay to use this macro instead of adding a new function?
>
> if (!HAS_EXTRA_GT_LIST(gt->i915) && hwm_energy_is_visible(ddat,
> hwmon_energy_input))

Yes, good find, looks like that's what this is. Thanks.

>
> Thanks
> Riana
> >
> > If ever we need package level energy in the future we can add a new
> > function which takes a 'struct drm_i915_private *i915' arg (and uses
> > i915->hwmon->ddat).
> >
> > Thanks.
> > --
> > Ashutosh
> >
> >
> >> +		hwm_energy(ddat, energy);
> >> +	else
> >> +		return -EOPNOTSUPP;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>   static umode_t
> >>   hwm_curr_is_visible(const struct hwm_drvdata *ddat, u32 attr)
> >>   {
> >> diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h
> >> index 7ca9cf2c34c9..1c38cfdbb7e9 100644
> >> --- a/drivers/gpu/drm/i915/i915_hwmon.h
> >> +++ b/drivers/gpu/drm/i915/i915_hwmon.h
> >> @@ -8,13 +8,16 @@
> >>   #define __I915_HWMON_H__
> >>
> >>   struct drm_i915_private;
> >> +struct intel_gt;
> >>
> >>   #if IS_REACHABLE(CONFIG_HWMON)
> >>   void i915_hwmon_register(struct drm_i915_private *i915);
> >>   void i915_hwmon_unregister(struct drm_i915_private *i915);
> >> +int i915_hwmon_get_energy(struct intel_gt *gt, long *energy);
> >>   #else
> >>   static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
> >>   static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { };
> >> +static inline int i915_hwmon_get_energy(struct intel_gt *gt, long *energy) { return -EOPNOTSUPP; }
> >>   #endif
> >>
> >>   #endif /* __I915_HWMON_H__ */
> >> --
> >> 2.25.1
> >>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-12-05  8:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-30  5:34 [Intel-gfx] [PATCH v5 0/3] Add hwmon support for dgfx selftests Riana Tauro
2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 1/3] drm/i915/selftests: Rename librapl library to libpower Riana Tauro
2022-12-02 22:13   ` Dixit, Ashutosh
2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 2/3] drm/i915/hwmon: Add helper function to obtain energy values Riana Tauro
2022-12-02 22:12   ` Dixit, Ashutosh
2022-12-05  7:44     ` Tauro, Riana
2022-12-05  8:46       ` Dixit, Ashutosh
2022-11-30  5:34 ` [Intel-gfx] [PATCH v5 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx Riana Tauro
2022-12-02 22:15   ` Dixit, Ashutosh
2022-11-30  5:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add hwmon support for dgfx selftests (rev5) Patchwork
2022-11-30  5:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-11-30  6:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).