[Intel-gfx] [PATCH 3/3] drm/i915/guc/slpc: Add SLPC selftest live_slpc_power
Tauro, Riana
riana.tauro at intel.com
Wed Sep 28 05:57:17 UTC 2022
On 9/27/2022 4:42 PM, Gupta, Anshuman wrote:
>
>
>> -----Original Message-----
>> From: Belgaumkar, Vinay <vinay.belgaumkar at intel.com>
>> Sent: Monday, September 26, 2022 9:35 PM
>> To: Tauro, Riana <riana.tauro at intel.com>; intel-gfx at lists.freedesktop.org
>> Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Dixit, Ashutosh
>> <ashutosh.dixit at 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 at intel.com>
>>
>> LGTM,
>>
>> Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar at 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;
More information about the Intel-gfx
mailing list