Re: [Intel-gfx] [PATCH 3/3] drm/i915/guc/slpc: Add SLPC selftest live_slpc_power

[Belgaumkar, Vinay]

On 9/23/2022 4:00 AM, Riana Tauro wrote:

A fundamental assumption is that at lower frequencies,
not only do we run slower, but we save power compared to
higher frequencies.
live_slpc_power checks if running at low frequency saves power

v2: re-use code to measure power
     fixed cosmetic review comments (Vinay)

Signed-off-by: Riana Tauro <riana.ta...@intel.com>

LGTM,

Reviewed-by: Vinay Belgaumkar <vinay.belgaum...@intel.com>

---
  drivers/gpu/drm/i915/gt/selftest_slpc.c | 127 ++++++++++++++++++++++--
  1 file changed, 118 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c 
b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index 928f74718881..4c6e9257e593 100644
--- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
@@ -11,7 +11,8 @@
  enum test_type {
        VARY_MIN,
        VARY_MAX,
-       MAX_GRANTED
+       MAX_GRANTED,
+       SLPC_POWER,
  };

static int slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 freq)

@@ -41,6 +42,39 @@ static int slpc_set_max_freq(struct intel_guc_slpc *slpc, 
u32 freq)
        return ret;
  }

+static int slpc_set_freq(struct intel_gt *gt, u32 freq)

+{
+       int err;
+       struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
+
+       err = slpc_set_max_freq(slpc, freq);
+       if (err) {
+               pr_err("Unable to update max freq");
+               return err;
+       }
+
+       err = slpc_set_min_freq(slpc, freq);
+       if (err) {
+               pr_err("Unable to update min freq");
+               return err;
+       }
+
+       return err;
+}
+
+static u64 measure_power_at_freq(struct intel_gt *gt, int *freq, u64 *power)
+{
+       int err = 0;
+
+       err = slpc_set_freq(gt, *freq);
+       if (err)
+               return err;
+       *freq = intel_rps_read_actual_frequency(&gt->rps);
+       *power = measure_power(&gt->rps, freq);
+
+       return err;
+}
+
  static int vary_max_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
                         u32 *max_act_freq)
  {
@@ -113,6 +147,58 @@ static int vary_min_freq(struct intel_guc_slpc *slpc, 
struct intel_rps *rps,
        return err;
  }

+static int slpc_power(struct intel_gt *gt, struct intel_engine_cs *engine)

+{
+       struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
+       struct {
+               u64 power;
+               int freq;
+       } min, max;
+       int err = 0;
+
+       /*
+        * Our fundamental assumption is that running at lower frequency
+        * actually saves power. Let's see if our RAPL measurement supports
+        * that theory.
+        */
+       if (!librapl_supported(gt->i915))
+               return 0;
+
+       min.freq = slpc->min_freq;
+       err = measure_power_at_freq(gt, &min.freq, &min.power);
+
+       if (err)
+               return err;
+
+       max.freq = slpc->rp0_freq;
+       err = measure_power_at_freq(gt, &max.freq, &max.power);
+
+       if (err)
+               return err;
+
+       pr_info("%s: min:%llumW @ %uMHz, max:%llumW @ %uMHz\n",
+               engine->name,
+               min.power, min.freq,
+               max.power, max.freq);
+
+       if (10 * min.freq >= 9 * max.freq) {
+               pr_notice("Could not control frequency, ran at [%uMHz, 
%uMhz]\n",
+                         min.freq, max.freq);
+       }
+
+       if (11 * min.power > 10 * max.power) {
+               pr_err("%s: did not conserve power when setting lower 
frequency!\n",
+                      engine->name);
+               err = -EINVAL;
+       }
+
+       /* Restore min/max frequencies */
+       slpc_set_max_freq(slpc, slpc->rp0_freq);
+       slpc_set_min_freq(slpc, slpc->min_freq);
+
+       return err;
+}
+
  static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps 
*rps, u32 *max_act_freq)
  {
        struct intel_gt *gt = rps_to_gt(rps);
@@ -233,17 +319,23 @@ static int run_test(struct intel_gt *gt, int test_type)

err = max_granted_freq(slpc, rps, &max_act_freq);

                        break;
+
+               case SLPC_POWER:
+                       err = slpc_power(gt, engine);
+                       break;
                }

- pr_info("Max actual frequency for %s was %d\n",

-                       engine->name, max_act_freq);
+               if (test_type != SLPC_POWER) {
+                       pr_info("Max actual frequency for %s was %d\n",
+                               engine->name, max_act_freq);

- /* Actual frequency should rise above min */

-               if (max_act_freq <= slpc_min_freq) {
-                       pr_err("Actual freq did not rise above min\n");
-                       pr_err("Perf Limit Reasons: 0x%x\n",
-                              intel_uncore_read(gt->uncore, 
GT0_PERF_LIMIT_REASONS));
-                       err = -EINVAL;
+                       /* Actual frequency should rise above min */
+                       if (max_act_freq <= slpc_min_freq) {
+                               pr_err("Actual freq did not rise above min\n");
+                               pr_err("Perf Limit Reasons: 0x%x\n",
+                                      intel_uncore_read(gt->uncore, 
GT0_PERF_LIMIT_REASONS));
+                               err = -EINVAL;
+                       }
                }

igt_spinner_end(&spin);

@@ -316,12 +408,29 @@ static int live_slpc_max_granted(void *arg)
        return ret;
  }

+static int live_slpc_power(void *arg)

+{
+       struct drm_i915_private *i915 = arg;
+       struct intel_gt *gt;
+       unsigned int i;
+       int ret;
+
+       for_each_gt(gt, i915, i) {
+               ret = run_test(gt, SLPC_POWER);
+               if (ret)
+                       return ret;
+       }
+
+       return ret;
+}
+
  int intel_slpc_live_selftests(struct drm_i915_private *i915)
  {
        static const struct i915_subtest tests[] = {
                SUBTEST(live_slpc_vary_max),
                SUBTEST(live_slpc_vary_min),
                SUBTEST(live_slpc_max_granted),
+               SUBTEST(live_slpc_power),
        };

struct intel_gt *gt;