[Intel-gfx] [PATCH v3 3/3] drm/i915/selftests: Add hwmon support in libpower for dgfx
Dixit, Ashutosh
ashutosh.dixit at intel.com
Tue Nov 22 02:40:00 UTC 2022
On Sun, 20 Nov 2022 23:29:46 -0800, Riana Tauro wrote:
>
> 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;
So:
* arg for live_rps_power and live_rc6_manual is gt
* freq's are per gt
* rc6 residency is per gt
But the power/energy we are using is per device (gt->i915)? And we expect
device level power to be low when only one gt might be in rc6?
Shouldn't all these functions be per gt? Specially when we might have
multiple gt's soon.
Or if per gt functions don't make in all cases have both device and gt
level functions?
i915 hwmon provides both gt and device/package/card level energy but iGFX
till now reads a MSR which does not need gt or i915. So per gt I think
should work for now.
> 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);
Hmm looks like this whole live_rps_power stuff is only for host turbo, not
slpc. Anyway that's a future patch.
> + 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
No IS_REACHABLE stuff here. Declare i915_hwmon_get_energy similar to other
functions in i915_hwmon.h.
Thanks.
--
Ashutosh
> + 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
>
More information about the Intel-gfx
mailing list