Re: [Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
Andi Shyti writes:
> Hi Mika,
>
>> > +static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
>> > +{
>> > + struct intel_uncore *uncore = rc6_to_uncore(rc6);
>> > + intel_wakeref_t wakeref;
>> > + u32 ec1, ec2;
>> > + u32 interval;
>> > +
>> > + wakeref = intel_runtime_pm_get(uncore->rpm);
>> > +
>> > + interval = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
>> > +
>> > + /*
>> > + * the interval is stored in steps of 1.28us
>> > + */
>> > + interval = div_u64(mul_u32_u32(interval, 128),
>> > + 100 * 1000); /* => miliseconds */
>> > +
>>
>> s/miliseconds/milliseconds.
>
> thanks!
>
>> I have a faint memory that the interval was not always 1.28us
>> but gen dependant.
>
> 1.28 is the incremental step and I haven't seen any different
> value in the docs. Have you?
I must have been mixing this with freq bins. Sorry.
And in this level as Chris said, we dont need to care.
>
>> > + pr_info("interval:%x [%dms], threshold:%x, rc6:%x, enabled?:%s\n",
>> > + intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL),
>> > + interval,
>> > + intel_uncore_read(uncore, GEN6_RC6_THRESHOLD),
>> > + ec2 - ec1,
>> > + yesno(enabled));
>> > +
>> > + intel_runtime_pm_put(uncore->rpm, wakeref);
>> > +
>> > + return enabled != (ec1 >= ec2);
>>
>> Wrap?
>
> actually here I forgot a couple of things that went forgotten in
> my git repo.
>
> Anyway, do you mean with "wrap" to add parenthesis?
>
I meant that if you take samples between wrap
period.
-Mika
>> > + intel_rc6_unpark(rc6);
>> > +
>> > + /* interval < threshold */
>> > + if (!test_rc6(rc6, false)) {
>>
>> consider removing the assertion of 'activeness' in parameter
>> and just if (!rc6_active(rc6)). Or am I missing something in here?
>
> yes, you are right, it's misleading. I will make it more clear.
>
> The basic idea is:
>
> 1. disable rc6
> 2. check whether it's disabled test_rc6(rc6, false)
>
> or
>
> 1. enable rc6
> 2. check if it's enabled test_rc6(rc6, true)
>
> Chris was skeptical about the naming as well.
>
> Thanks!
>
> Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
Quoting Andi Shyti (2020-02-05 18:40:55) > Hi Mika, > > > I have a faint memory that the interval was not always 1.28us > > but gen dependant. > > 1.28 is the incremental step and I haven't seen any different > value in the docs. Have you? The rc6 residency counter does flip over to a different clock on vlv/bsw. But the lack of information abounds, and I haven't seen anything that suggests these registers are anything but units of 1280ns. If we get to the point where we can differentiate between 833ns or 1280ns that will be very impressive. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
Hi Mika,
> > +static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
> > +{
> > + struct intel_uncore *uncore = rc6_to_uncore(rc6);
> > + intel_wakeref_t wakeref;
> > + u32 ec1, ec2;
> > + u32 interval;
> > +
> > + wakeref = intel_runtime_pm_get(uncore->rpm);
> > +
> > + interval = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
> > +
> > + /*
> > +* the interval is stored in steps of 1.28us
> > +*/
> > + interval = div_u64(mul_u32_u32(interval, 128),
> > + 100 * 1000); /* => miliseconds */
> > +
>
> s/miliseconds/milliseconds.
thanks!
> I have a faint memory that the interval was not always 1.28us
> but gen dependant.
1.28 is the incremental step and I haven't seen any different
value in the docs. Have you?
> > + pr_info("interval:%x [%dms], threshold:%x, rc6:%x, enabled?:%s\n",
> > + intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL),
> > + interval,
> > + intel_uncore_read(uncore, GEN6_RC6_THRESHOLD),
> > + ec2 - ec1,
> > + yesno(enabled));
> > +
> > + intel_runtime_pm_put(uncore->rpm, wakeref);
> > +
> > + return enabled != (ec1 >= ec2);
>
> Wrap?
actually here I forgot a couple of things that went forgotten in
my git repo.
Anyway, do you mean with "wrap" to add parenthesis?
> > + intel_rc6_unpark(rc6);
> > +
> > + /* interval < threshold */
> > + if (!test_rc6(rc6, false)) {
>
> consider removing the assertion of 'activeness' in parameter
> and just if (!rc6_active(rc6)). Or am I missing something in here?
yes, you are right, it's misleading. I will make it more clear.
The basic idea is:
1. disable rc6
2. check whether it's disabled test_rc6(rc6, false)
or
1. enable rc6
2. check if it's enabled test_rc6(rc6, true)
Chris was skeptical about the naming as well.
Thanks!
Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
Chris Wilson writes:
> From: Andi Shyti
>
> Add three basic tests for rc6 power status:
>
> 1. live_rc6_basic - simply checks if rc6 works when it's enabled
>or stops when it's disabled.
>
> 2. live_rc6_threshold - rc6 should not work when the evaluation
>interval is less than the threshold and should work otherwise.
>
> 3. live_rc6_busy - keeps the gpu busy and then goes in idle;
>checks that we don't fall in rc6 when busy and that we do fall
>in rc6 when idling.
>
> The three tests are added as sutest of the bigger live_late_gt_pm
> selftest.
>
> The basic rc6 functionality is tested by checking the reference
> counter within the evaluation interval.
>
> Signed-off-by: Andi Shyti
> Cc: Chris Wilson
> ---
> drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 2 +
> drivers/gpu/drm/i915/gt/selftest_rc6.c | 173 +++
> drivers/gpu/drm/i915/gt/selftest_rc6.h | 2 +
> 3 files changed, 177 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> index 09ff8e4f88af..5c7b92301a14 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> @@ -52,6 +52,8 @@ int intel_gt_pm_live_selftests(struct drm_i915_private
> *i915)
> {
> static const struct i915_subtest tests[] = {
> SUBTEST(live_rc6_manual),
> + SUBTEST(live_rc6_threshold),
> + SUBTEST(live_rc6_busy),
> SUBTEST(live_gt_resume),
> };
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index 5f7e2dcf5686..92375f458845 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -11,6 +11,7 @@
> #include "selftest_rc6.h"
>
> #include "selftests/i915_random.h"
> +#include "selftests/igt_spinner.h"
>
> int live_rc6_manual(void *arg)
> {
> @@ -202,3 +203,175 @@ int live_rc6_ctx_wa(void *arg)
> kfree(engines);
> return err;
> }
> +
> +static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
> +{
> + struct intel_uncore *uncore = rc6_to_uncore(rc6);
> + intel_wakeref_t wakeref;
> + u32 ec1, ec2;
> + u32 interval;
> +
> + wakeref = intel_runtime_pm_get(uncore->rpm);
> +
> + interval = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
> +
> + /*
> + * the interval is stored in steps of 1.28us
> + */
> + interval = div_u64(mul_u32_u32(interval, 128),
> +100 * 1000); /* => miliseconds */
> +
s/miliseconds/milliseconds.
I have a faint memory that the interval was not always 1.28us
but gen dependant.
> + ec1 = intel_uncore_read(uncore, GEN6_GT_GFX_RC6);
> +
> + /*
> + * It's not important to precisely wait the interval time.
> + * I'll wait at least twice the time in order to be sure
> + * that the counting happens in the reference counter.
> + */
> + msleep(2 * interval);
> +
> + ec2 = intel_uncore_read(uncore, GEN6_GT_GFX_RC6);
> +
> + pr_info("interval:%x [%dms], threshold:%x, rc6:%x, enabled?:%s\n",
> + intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL),
> + interval,
> + intel_uncore_read(uncore, GEN6_RC6_THRESHOLD),
> + ec2 - ec1,
> + yesno(enabled));
> +
> + intel_runtime_pm_put(uncore->rpm, wakeref);
> +
> + return enabled != (ec1 >= ec2);
Wrap?
> +}
> +
> +int live_rc6_threshold(void *arg)
> +{
> + struct intel_gt *gt = arg;
> + struct intel_uncore *uncore = gt->uncore;
> + struct intel_rc6 *rc6 = >->rc6;
> + intel_wakeref_t wakeref;
> + u32 threshold, interval;
> + u32 t_orig, i_orig;
> + int err = 0;
> +
> + if (!rc6->manual) /* No interferring PCU! */
> + return 0;
> +
> + wakeref = intel_runtime_pm_get(uncore->rpm);
> +
> + __intel_rc6_disable(rc6); /* stop before adjusting thresholds */
> +
> + t_orig = intel_uncore_read(uncore, GEN6_RC6_THRESHOLD);
> + i_orig = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
> +
> + /*
> + * set the threshold to 50ms
> + *
> + * 50ms * 1000 = 5us
> + * 5 / (1.28 * 100) / 100 (we don't have floating point)
> + */
> + threshold = 50 * 1000 / 128 * 100;
> + intel_uncore_write(uncore, GEN6_RC6_THRESHOLD, threshold);
> +
> + /* set interval indicatively to half the threshold */
> + interval = threshold / 2;
> + intel_uncore_write(uncore, GEN6_RC_EVALUATION_INTERVAL, interval);
> +
> + intel_rc6_unpark(rc6);
> +
> + /* interval < threshold */
> + if (!test_rc6(rc6, false)) {
consider removing the assertion of 'activeness' in parameter
and just if (!rc6_active(rc6)). Or am I missing something in here?
-Mika
> + pr_err("i915 mismatch: rc6 with interval < threshold\n");
> + err = -EINVAL;
> + }
> +
> + __intel_rc6_disable(rc6);
> +
> + /* set in
[Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
From: Andi Shyti
Add three basic tests for rc6 power status:
1. live_rc6_basic - simply checks if rc6 works when it's enabled
or stops when it's disabled.
2. live_rc6_threshold - rc6 should not work when the evaluation
interval is less than the threshold and should work otherwise.
3. live_rc6_busy - keeps the gpu busy and then goes in idle;
checks that we don't fall in rc6 when busy and that we do fall
in rc6 when idling.
The three tests are added as sutest of the bigger live_late_gt_pm
selftest.
The basic rc6 functionality is tested by checking the reference
counter within the evaluation interval.
Signed-off-by: Andi Shyti
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 2 +
drivers/gpu/drm/i915/gt/selftest_rc6.c | 173 +++
drivers/gpu/drm/i915/gt/selftest_rc6.h | 2 +
3 files changed, 177 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
index 09ff8e4f88af..5c7b92301a14 100644
--- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
@@ -52,6 +52,8 @@ int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
{
static const struct i915_subtest tests[] = {
SUBTEST(live_rc6_manual),
+ SUBTEST(live_rc6_threshold),
+ SUBTEST(live_rc6_busy),
SUBTEST(live_gt_resume),
};
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c
b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 5f7e2dcf5686..92375f458845 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -11,6 +11,7 @@
#include "selftest_rc6.h"
#include "selftests/i915_random.h"
+#include "selftests/igt_spinner.h"
int live_rc6_manual(void *arg)
{
@@ -202,3 +203,175 @@ int live_rc6_ctx_wa(void *arg)
kfree(engines);
return err;
}
+
+static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
+{
+ struct intel_uncore *uncore = rc6_to_uncore(rc6);
+ intel_wakeref_t wakeref;
+ u32 ec1, ec2;
+ u32 interval;
+
+ wakeref = intel_runtime_pm_get(uncore->rpm);
+
+ interval = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
+
+ /*
+* the interval is stored in steps of 1.28us
+*/
+ interval = div_u64(mul_u32_u32(interval, 128),
+ 100 * 1000); /* => miliseconds */
+
+ ec1 = intel_uncore_read(uncore, GEN6_GT_GFX_RC6);
+
+ /*
+* It's not important to precisely wait the interval time.
+* I'll wait at least twice the time in order to be sure
+* that the counting happens in the reference counter.
+*/
+ msleep(2 * interval);
+
+ ec2 = intel_uncore_read(uncore, GEN6_GT_GFX_RC6);
+
+ pr_info("interval:%x [%dms], threshold:%x, rc6:%x, enabled?:%s\n",
+ intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL),
+ interval,
+ intel_uncore_read(uncore, GEN6_RC6_THRESHOLD),
+ ec2 - ec1,
+ yesno(enabled));
+
+ intel_runtime_pm_put(uncore->rpm, wakeref);
+
+ return enabled != (ec1 >= ec2);
+}
+
+int live_rc6_threshold(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_uncore *uncore = gt->uncore;
+ struct intel_rc6 *rc6 = >->rc6;
+ intel_wakeref_t wakeref;
+ u32 threshold, interval;
+ u32 t_orig, i_orig;
+ int err = 0;
+
+ if (!rc6->manual) /* No interferring PCU! */
+ return 0;
+
+ wakeref = intel_runtime_pm_get(uncore->rpm);
+
+ __intel_rc6_disable(rc6); /* stop before adjusting thresholds */
+
+ t_orig = intel_uncore_read(uncore, GEN6_RC6_THRESHOLD);
+ i_orig = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
+
+ /*
+* set the threshold to 50ms
+*
+* 50ms * 1000 = 5us
+* 5 / (1.28 * 100) / 100 (we don't have floating point)
+*/
+ threshold = 50 * 1000 / 128 * 100;
+ intel_uncore_write(uncore, GEN6_RC6_THRESHOLD, threshold);
+
+ /* set interval indicatively to half the threshold */
+ interval = threshold / 2;
+ intel_uncore_write(uncore, GEN6_RC_EVALUATION_INTERVAL, interval);
+
+ intel_rc6_unpark(rc6);
+
+ /* interval < threshold */
+ if (!test_rc6(rc6, false)) {
+ pr_err("i915 mismatch: rc6 with interval < threshold\n");
+ err = -EINVAL;
+ }
+
+ __intel_rc6_disable(rc6);
+
+ /* set interval indicatively to twice the threshold */
+ interval = threshold * 2;
+ intel_uncore_write(uncore, GEN6_RC_EVALUATION_INTERVAL, interval);
+
+ intel_rc6_unpark(rc6);
+
+ /* interval > threshold */
+ if (!test_rc6(rc6, true)) {
+ pr_err("i915 mismatch: not in rc6 with interval > threshold\n");
+ err = -EINVAL;
+ }
+
+ __intel_rc6_disable(rc6);
+
+
Re: [Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
Quoting Chris Wilson (2019-11-22 00:16:27) > From: Andi Shyti > > Add three basic tests for rc6 power status: > > 1. live_rc6_basic - simply checks if rc6 works when it's enabled >or stops when it's disabled. > > 2. live_rc6_threshold - rc6 should not work when the evaluation >interval is less than the threshold and should work otherwise. > > 3. live_rc6_busy - keeps the gpu busy and then goes in idle; >checks that we don't fall in rc6 when busy and that we do fall >in rc6 when idling. One test worth extending is checking that live_rc6_busy works after a reset, both per-engine and device. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/selftests: add basic selftests for rc6
From: Andi Shyti
Add three basic tests for rc6 power status:
1. live_rc6_basic - simply checks if rc6 works when it's enabled
or stops when it's disabled.
2. live_rc6_threshold - rc6 should not work when the evaluation
interval is less than the threshold and should work otherwise.
3. live_rc6_busy - keeps the gpu busy and then goes in idle;
checks that we don't fall in rc6 when busy and that we do fall
in rc6 when idling.
The three tests are added as sutest of the bigger live_late_gt_pm
selftest.
The basic rc6 functionality is tested by checking the reference
counter within the evaluation interval.
Signed-off-by: Andi Shyti
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 3 +
drivers/gpu/drm/i915/gt/selftest_rc6.c | 189 +++
drivers/gpu/drm/i915/gt/selftest_rc6.h | 3 +
3 files changed, 195 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
index 5e563b877368..7833ef3b252f 100644
--- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
@@ -51,6 +51,9 @@ static int live_gt_resume(void *arg)
int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
{
static const struct i915_subtest tests[] = {
+ SUBTEST(live_rc6_basic),
+ SUBTEST(live_rc6_threshold),
+ SUBTEST(live_rc6_busy),
SUBTEST(live_gt_resume),
};
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c
b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 67b7a6bc64f5..9d0862272665 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -11,6 +11,7 @@
#include "selftest_rc6.h"
#include "selftests/i915_random.h"
+#include "selftests/igt_spinner.h"
static const u32 *__live_rc6_ctx(struct intel_context *ce)
{
@@ -144,3 +145,191 @@ int live_rc6_ctx_wa(void *arg)
kfree(engines);
return err;
}
+
+static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
+{
+ struct intel_uncore *uncore = rc6_to_uncore(rc6);
+ intel_wakeref_t wakeref;
+ u32 ec1, ec2;
+ u32 interval;
+
+ wakeref = intel_runtime_pm_get(uncore->rpm);
+
+ interval = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
+
+ /*
+* the interval is stored in steps of 1.28us
+*/
+ interval = div_u64(mul_u32_u32(interval, 128),
+ 100 * 1000); /* => miliseconds */
+
+ ec1 = intel_uncore_read(uncore, GEN6_GT_GFX_RC6);
+
+ /*
+* It's not important to precisely wait the interval time.
+* I'll wait at least twice the time in order to be sure
+* that the counting happens in the reference counter.
+*/
+ msleep(2 * interval);
+
+ ec2 = intel_uncore_read(uncore, GEN6_GT_GFX_RC6);
+
+ intel_runtime_pm_put(uncore->rpm, wakeref);
+
+ return enabled != (ec1 >= ec2);
+}
+
+int live_rc6_basic(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_rc6 *rc6 = >->rc6;
+ intel_wakeref_t wakeref;
+ int i, err = 0;
+
+ if (!rc6->supported)
+ return 0;
+
+ wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+
+ /*
+* the two loops test rc6 both in case it's enabled
+* and in the case it's disabled. It restores the prvious
+* status
+*/
+ for (i = 0; i < 2; i++) {
+ if (!test_rc6(rc6, rc6->enabled)) {
+ if (!i)
+ return -EINVAL;
+
+ /* restore before leaving */
+ err = -EINVAL;
+ }
+
+ if (rc6->enabled)
+ intel_rc6_disable(>->rc6);
+ else
+ intel_rc6_enable(>->rc6);
+ }
+
+ intel_runtime_pm_put(gt->uncore->rpm, wakeref);
+ return err;
+}
+
+int live_rc6_threshold(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_uncore *uncore = gt->uncore;
+ struct intel_rc6 *rc6 = >->rc6;
+ intel_wakeref_t wakeref;
+ u32 threshold, interval;
+ u32 t_orig, i_orig;
+ int err = 0;
+
+ if (!rc6->supported)
+ return 0;
+
+ wakeref = intel_runtime_pm_get(uncore->rpm);
+
+ t_orig = intel_uncore_read(uncore, GEN6_RC6_THRESHOLD);
+ i_orig = intel_uncore_read(uncore, GEN6_RC_EVALUATION_INTERVAL);
+
+ /*
+* set the threshold to 50ms
+*
+* 50ms * 1000 = 5us
+* 5 / (1.28 * 100) / 100 (we don't have floating point)
+*/
+ threshold = 50 * 1000 / 128 * 100;
+ intel_uncore_write(uncore, GEN6_RC6_THRESHOLD, threshold);
+
+ /* set interval indicatively to half the threshold */
+ interval = threshold / 2;
+ intel_uncore_write(uncore, GEN6_RC_EVALUATION_INTERVAL, interval);
+
+ /* interval < threshold */
+ if (!test_rc6
