| Message ID | 20220923110043.789178-4-riana.tauro@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add SLPC selftest live_slpc_power | expand |
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.tauro@intel.com> LGTM, Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@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 = >->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(>->rps); > + *power = measure_power(>->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 = >->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;
> -----Original Message----- > From: Belgaumkar, Vinay <vinay.belgaumkar@intel.com> > Sent: Monday, September 26, 2022 9:35 PM > To: Tauro, Riana <riana.tauro@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Dixit, Ashutosh > <ashutosh.dixit@intel.com> > Subject: Re: [PATCH 3/3] drm/i915/guc/slpc: Add SLPC selftest live_slpc_power > > > 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.tauro@intel.com> > > LGTM, > > Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@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 = >->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(>->rps); > > + *power = measure_power(>->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 = >->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; This seems a wrong abstraction, this should a generic call should check both hwmon registration for dgfx and rapl for igfx. Br, Anshuman Gupta. > > + > > + 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;
On 9/27/2022 4:42 PM, Gupta, Anshuman wrote: > > >> -----Original Message----- >> From: Belgaumkar, Vinay <vinay.belgaumkar@intel.com> >> Sent: Monday, September 26, 2022 9:35 PM >> To: Tauro, Riana <riana.tauro@intel.com>; intel-gfx@lists.freedesktop.org >> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Dixit, Ashutosh >> <ashutosh.dixit@intel.com> >> Subject: Re: [PATCH 3/3] drm/i915/guc/slpc: Add SLPC selftest live_slpc_power >> >> >> 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.tauro@intel.com> >> >> LGTM, >> >> Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@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 = >->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(>->rps); >>> + *power = measure_power(>->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 = >->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; > This seems a wrong abstraction, this should a generic call should check both hwmon registration for dgfx and rapl for igfx. > Br, > Anshuman Gupta. The current librapl_supported has only rapl related changes. The hwmon energy is yet to be added. Will change the name with the hwmon patch Thanks Riana Tauro >>> + >>> + 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;
> -----Original Message----- > From: Tauro, Riana <riana.tauro@intel.com> > Sent: Wednesday, September 28, 2022 11:27 AM > To: Gupta, Anshuman <anshuman.gupta@intel.com>; Belgaumkar, Vinay > <vinay.belgaumkar@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Dixit, Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal > <badal.nilawar@intel.com> > Subject: Re: [PATCH 3/3] drm/i915/guc/slpc: Add SLPC selftest > live_slpc_power > > > > On 9/27/2022 4:42 PM, Gupta, Anshuman wrote: > > > > > >> -----Original Message----- > >> From: Belgaumkar, Vinay <vinay.belgaumkar@intel.com> > >> Sent: Monday, September 26, 2022 9:35 PM > >> To: Tauro, Riana <riana.tauro@intel.com>; > >> intel-gfx@lists.freedesktop.org > >> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Dixit, Ashutosh > >> <ashutosh.dixit@intel.com> > >> Subject: Re: [PATCH 3/3] drm/i915/guc/slpc: Add SLPC selftest > >> live_slpc_power > >> > >> > >> 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.tauro@intel.com> > >> > >> LGTM, > >> > >> Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@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 = >->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(>->rps); > >>> + *power = measure_power(>->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 = >->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; > > This seems a wrong abstraction, this should a generic call should > check both hwmon registration for dgfx and rapl for igfx. > > Br, > > Anshuman Gupta. > The current librapl_supported has only rapl related changes. The hwmon > energy is yet to be added. > > Will change the name with the hwmon patch HWMON series is already reviewed and ready to merge just waiting for CI results. I think we can merge this after hwmom. Br, Anshuman Gupta. > > Thanks > Riana Tauro > > >>> + > >>> + 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;
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 = >->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(>->rps); + *power = measure_power(>->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 = >->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;
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.tauro@intel.com> --- drivers/gpu/drm/i915/gt/selftest_slpc.c | 127 ++++++++++++++++++++++-- 1 file changed, 118 insertions(+), 9 deletions(-)