> -----Original Message-----
> From: Tauro, Riana <riana.tauro@xxxxxxxxx>
> Sent: Monday, November 21, 2022 1:00 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Tauro, Riana <riana.tauro@xxxxxxxxx>; Gupta, Anshuman
> <anshuman.gupta@xxxxxxxxx>; Dixit, Ashutosh <ashutosh.dixit@xxxxxxxxx>;
> Tangudu, Tilak <tilak.tangudu@xxxxxxxxx>; Nilawar, Badal
> <badal.nilawar@xxxxxxxxx>
> Subject: [PATCH v3 3/3] drm/i915/selftests: Add hwmon support in libpower
> for dgfx
>
> From: Tilak Tangudu <tilak.tangudu@xxxxxxxxx>
>
> 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
>
> Signed-off-by: Tilak Tangudu <tilak.tangudu@xxxxxxxxx>
> Signed-off-by: Riana Tauro <riana.tauro@xxxxxxxxx>
LGTM
Reviewed-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/selftest_rc6.c | 8 +++---
> drivers/gpu/drm/i915/gt/selftest_rps.c | 22 +++++++--------
> drivers/gpu/drm/i915/gt/selftest_slpc.c | 2 +-
> drivers/gpu/drm/i915/selftests/libpower.c | 33 ++++++++++++-----------
> drivers/gpu/drm/i915/selftests/libpower.h | 8 +++---
> 5 files changed, 38 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index 15b84c428f66..845058ed83ed 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -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->i915);
> msleep(250);
> - rc0_power = libpower_get_energy_uJ() - rc0_power;
> + rc0_power = libpower_get_energy_uJ(gt->i915) - 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->i915);
> msleep(100);
> - rc6_power = libpower_get_energy_uJ() - rc6_power;
> + rc6_power = libpower_get_energy_uJ(gt->i915) - 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..6732aa7d4792 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->i915);
> usleep_range(1000 * duration_ms, 2000 * duration_ms);
> - dE = libpower_get_energy_uJ() - dE;
> + dE = libpower_get_energy_uJ(gt->i915) - 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(>->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(>->rps, *freq);
> + return measure_power(gt, freq);
> }
>
> int live_rps_power(void *arg)
> @@ -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..c4b14f2b268c 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(>->rps);
> - *power = measure_power(>->rps, freq);
> + *power = measure_power(gt, freq);
>
> return err;
> }
> diff --git a/drivers/gpu/drm/i915/selftests/libpower.c
> b/drivers/gpu/drm/i915/selftests/libpower.c
> index c66e993c5f85..df37cba30353 100644
> --- a/drivers/gpu/drm/i915/selftests/libpower.c
> +++ b/drivers/gpu/drm/i915/selftests/libpower.c
> @@ -6,29 +6,30 @@
> #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 drm_i915_private *i915)
> {
> unsigned long long power;
> u32 units;
> + long energy_uJ = 0;
>
> - if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
> - return 0;
> + if (IS_DGFX(i915)) {
> +#if IS_REACHABLE(CONFIG_HWMON)
> + if (i915_hwmon_get_energy(i915, &energy_uJ)) #endif
> + 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..03a44611f9e9 100644
> --- a/drivers/gpu/drm/i915/selftests/libpower.h
> +++ b/drivers/gpu/drm/i915/selftests/libpower.h
> @@ -10,8 +10,10 @@
>
> struct drm_i915_private;
>
> -bool libpower_supported(const struct drm_i915_private *i915);
> -
> -u64 libpower_get_energy_uJ(void);
> +u64 libpower_get_energy_uJ(struct drm_i915_private *i915);
>
> +static inline bool libpower_supported(struct drm_i915_private *i915) {
> + return libpower_get_energy_uJ(i915);
> +}
> #endif /* SELFTEST_LIBPOWER_H */
> --
> 2.25.1
