diff mbox series

[RFC] drm/i915/selftests: add basic selftests for rc6

Message ID 20191121125747.37232-1-andi.shyti@intel.com (mailing list archive)
State New, archived
Headers show
Series [RFC] drm/i915/selftests: add basic selftests for rc6 | expand

Commit Message

Andi Shyti Nov. 21, 2019, 12:57 p.m. UTC
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 <andi.shyti@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
Hi,

in this RC6 test the live_rc6_threshold doesn't work, either
because I misinterpreted the concept, or the GPU I am using does
not support something or the code I am posting is junk. Either
way, ideas?

Thanks,
Andi

 drivers/gpu/drm/i915/gt/selftest_gt_pm.c |   3 +
 drivers/gpu/drm/i915/gt/selftest_rc6.c   | 180 +++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/selftest_rc6.h   |   3 +
 3 files changed, 186 insertions(+)

Comments

Chris Wilson Nov. 21, 2019, 1:21 p.m. UTC | #1
Quoting Andi Shyti (2019-11-21 12:57:47)
> 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 <andi.shyti@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> Hi,
> 
> in this RC6 test the live_rc6_threshold doesn't work, either
> because I misinterpreted the concept, or the GPU I am using does
> not support something or the code I am posting is junk. Either
> way, ideas?
> 
> Thanks,
> Andi
> 
>  drivers/gpu/drm/i915/gt/selftest_gt_pm.c |   3 +
>  drivers/gpu/drm/i915/gt/selftest_rc6.c   | 180 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/selftest_rc6.h   |   3 +
>  3 files changed, 186 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..b5a872affa87 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> @@ -68,7 +68,10 @@ int intel_gt_pm_late_selftests(struct drm_i915_private *i915)
>                  * They are intended to be run last in CI and the system
>                  * rebooted afterwards.
>                  */
> +               SUBTEST(live_rc6_basic),
> +               SUBTEST(live_rc6_threshold),
>                 SUBTEST(live_rc6_ctx_wa),
> +               SUBTEST(live_rc6_busy),

The newcomers should be safe, right? No reason not to put them in the
normal gt_pm_selftests rather than the dangerous dungeon?

>         };
>  
>         if (intel_gt_is_wedged(&i915->gt))
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index 67b7a6bc64f5..c80d17f3da10 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -11,6 +11,31 @@
>  #include "selftest_rc6.h"
>  
>  #include "selftests/i915_random.h"
> +#include "selftests/igt_spinner.h"
> +
> +static bool test_rc6(struct drm_i915_private *dev_priv, bool enabled)
> +{
> +       u32 ec1, ec2;
> +       u64 interval;
> +
> +       interval = I915_READ(GEN6_RC_EVALUATION_INTERVAL);

intel_uncore_read()!

> +
> +       /*
> +        * the interval is stored in steps of 1.28us
> +        */
> +       interval = interval * 128 / 100 / 1000; /* miliseconds */

interval = div_u64(mul_u32_u32(interval, 128), 100 * 1000);

or CI will complain about breaking 32b builds.

> +
> +       ec1 = I915_READ(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 = I915_READ(GEN6_GT_GFX_RC6);

Hmm, I still don't trust msleep, but this will do as a first
approximation.

> +
> +       return enabled != (ec1 >= ec2);

Ok. So sleep for a bit and check the rc6 counter is incrementing.
Assumes we are already idle, probably best to add an explicit
intel_gt_pm_wait_for_idle() at the start of the rc6 tests (or
intel_gt_pm_selftests in general).

> +}
>  
>  static const u32 *__live_rc6_ctx(struct intel_context *ce)
>  {
> @@ -144,3 +169,158 @@ int live_rc6_ctx_wa(void *arg)
>         kfree(engines);
>         return err;
>  }
> +
> +int live_rc6_basic(void *arg)
> +{
> +       struct intel_gt *gt = arg;
> +       struct intel_rc6 *rc6 = &gt->rc6;
> +       int i, err = 0;
> +
> +       if (!HAS_RC6(gt->i915))
> +               return -ENODEV;
> +
> +       /*
> +        * 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(gt->i915, rc6->enabled)) {
> +                       if (!i)
> +                               return -EINVAL;
> +
> +                       /* restore before leaving */
> +                       err = -EINVAL;
> +               }
> +
> +               if (rc6->enabled)
> +                       intel_rc6_disable(&gt->rc6);
> +               else
> +                       intel_rc6_enable(&gt->rc6);
> +       }
> +
> +       return err;
> +}
> +
> +int live_rc6_threshold(void *arg)
> +{
> +       struct intel_gt *gt = arg;
> +       struct drm_i915_private *dev_priv = gt->i915;
> +       u32 threshold, interval;
> +       u32 t_orig, i_orig;
> +       int err = 0;
> +
> +       t_orig = I915_READ(GEN6_RC6_THRESHOLD);
> +       i_orig = I915_READ(GEN6_RC_EVALUATION_INTERVAL);
> +
> +       /*
> +        * set the threshold to 50ms
> +        *
> +        * 50ms * 1000 = 50000us
> +        * 50000 / (1.28 * 100) / 100 (we don't have floating point)
> +        */
> +       threshold = 50 * 1000 / 128 * 100;
> +       I915_WRITE(GEN6_RC6_THRESHOLD, threshold);
> +
> +       /* set interval indicatively to half the threshold */
> +       interval = threshold / 2;
> +       I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, interval);
> +
> +       /* interval < threshold */
> +       if (!test_rc6(gt->i915, false)) {
> +               err = -EINVAL;
> +               pr_err("i915 mismatch: rc6 with interval < threshold\n");
> +               goto out;
> +       }
> +       /* set interval indicatively to twice the threshold */
> +       interval = threshold * 2;
> +       I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, interval);
> +
> +       /* interval > threshold */
> +       if (!test_rc6(gt->i915, true)) {
> +               err = -EINVAL;
> +               pr_err("i915 mismatch: not in rc6 with interval > threshold\n");
> +       }
> +
> +out:
> +       I915_WRITE(GEN6_RC6_THRESHOLD, t_orig);
> +       I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, i_orig);
> +
> +       return err;
> +}
> +
> +static int rc6_busy_thread(void *arg)
> +{
> +       struct intel_engine_cs *engine;
> +       struct intel_gt *gt = arg;
> +       struct igt_spinner spin;
> +       enum intel_engine_id id;
> +       struct i915_request *rq;
> +       int err = 0;
> +
> +       /* any existing engine in the current gt is good */
> +       for_each_engine(engine, gt, id)
> +               break;
> +
> +       err = igt_spinner_init(&spin, gt);
> +       if (err)
> +               return err;
> +
> +       rq = igt_spinner_create_request(&spin, engine->kernel_context, MI_NOOP);
> +       if (IS_ERR(rq)) {
> +               err = PTR_ERR(rq);
> +               goto out;
> +       }
> +
> +       i915_request_get(rq);
> +       i915_request_add(rq);
> +       igt_wait_for_spinner(&spin, rq); /* it's enough waiting */
> +       igt_spinner_end(&spin);
> +
> +       i915_request_wait(rq, 0, HZ / 5);
> +       i915_request_put(rq);
> +
> +out:
> +       igt_spinner_fini(&spin);
> +       return err;
> +}
> +
> +int live_rc6_busy(void *arg)
> +{
> +       struct intel_gt *gt = arg;
> +       struct task_struct *thread;
> +       int err = 0;
> +
> +       thread = kmalloc(sizeof(*thread), GFP_KERNEL);
> +       if (!thread)
> +               return -ENOMEM;
> +
> +       thread = kthread_run(rc6_busy_thread, arg, "rc6_busy_selftest");
> +       if (IS_ERR(thread))
> +               return PTR_ERR(thread);
> +
> +       get_task_struct(thread);
> +
> +       /* gpu is busy, we shouldn't be in rc6 */
> +       if (!test_rc6(gt->i915, false)) {
> +               err = -EINVAL;
> +               pr_err("never busy enough for having a nap\n");
> +       }

I would have used the spinner inline so you have better control over it.
As it you you don't sync with the thread before starting and whatnot.

> +
> +       err = kthread_stop(thread);
> +       if (err < 0)
> +               pr_err("fail and exit\n");
> +
> +       put_task_struct(thread);
> +
> +       intel_gt_pm_wait_for_idle(gt);
> +
> +       /* gpu is busy, we should be in rc6 */
> +       if (!test_rc6(gt->i915, true)) {
> +               err = -EINVAL;
> +               pr_err("i915 is idle but doesn't go in rc6\n");
> +       }

Aye, this tail is perhaps the most important test of them all! :)

Ok, looks like we have the makings of some very useful tests. Lets see
why they aren't behaving...
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
index 5e563b877368..b5a872affa87 100644
--- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
@@ -68,7 +68,10 @@  int intel_gt_pm_late_selftests(struct drm_i915_private *i915)
 		 * They are intended to be run last in CI and the system
 		 * rebooted afterwards.
 		 */
+		SUBTEST(live_rc6_basic),
+		SUBTEST(live_rc6_threshold),
 		SUBTEST(live_rc6_ctx_wa),
+		SUBTEST(live_rc6_busy),
 	};
 
 	if (intel_gt_is_wedged(&i915->gt))
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 67b7a6bc64f5..c80d17f3da10 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -11,6 +11,31 @@ 
 #include "selftest_rc6.h"
 
 #include "selftests/i915_random.h"
+#include "selftests/igt_spinner.h"
+
+static bool test_rc6(struct drm_i915_private *dev_priv, bool enabled)
+{
+	u32 ec1, ec2;
+	u64 interval;
+
+	interval = I915_READ(GEN6_RC_EVALUATION_INTERVAL);
+
+	/*
+	 * the interval is stored in steps of 1.28us
+	 */
+	interval = interval * 128 / 100 / 1000; /* miliseconds */
+
+	ec1 = I915_READ(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 = I915_READ(GEN6_GT_GFX_RC6);
+
+	return enabled != (ec1 >= ec2);
+}
 
 static const u32 *__live_rc6_ctx(struct intel_context *ce)
 {
@@ -144,3 +169,158 @@  int live_rc6_ctx_wa(void *arg)
 	kfree(engines);
 	return err;
 }
+
+int live_rc6_basic(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_rc6 *rc6 = &gt->rc6;
+	int i, err = 0;
+
+	if (!HAS_RC6(gt->i915))
+		return -ENODEV;
+
+	/*
+	 * 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(gt->i915, rc6->enabled)) {
+			if (!i)
+				return -EINVAL;
+
+			/* restore before leaving */
+			err = -EINVAL;
+		}
+
+		if (rc6->enabled)
+			intel_rc6_disable(&gt->rc6);
+		else
+			intel_rc6_enable(&gt->rc6);
+	}
+
+	return err;
+}
+
+int live_rc6_threshold(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct drm_i915_private *dev_priv = gt->i915;
+	u32 threshold, interval;
+	u32 t_orig, i_orig;
+	int err = 0;
+
+	t_orig = I915_READ(GEN6_RC6_THRESHOLD);
+	i_orig = I915_READ(GEN6_RC_EVALUATION_INTERVAL);
+
+	/*
+	 * set the threshold to 50ms
+	 *
+	 * 50ms * 1000 = 50000us
+	 * 50000 / (1.28 * 100) / 100 (we don't have floating point)
+	 */
+	threshold = 50 * 1000 / 128 * 100;
+	I915_WRITE(GEN6_RC6_THRESHOLD, threshold);
+
+	/* set interval indicatively to half the threshold */
+	interval = threshold / 2;
+	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, interval);
+
+	/* interval < threshold */
+	if (!test_rc6(gt->i915, false)) {
+		err = -EINVAL;
+		pr_err("i915 mismatch: rc6 with interval < threshold\n");
+		goto out;
+	}
+
+	/* set interval indicatively to twice the threshold */
+	interval = threshold * 2;
+	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, interval);
+
+	/* interval > threshold */
+	if (!test_rc6(gt->i915, true)) {
+		err = -EINVAL;
+		pr_err("i915 mismatch: not in rc6 with interval > threshold\n");
+	}
+
+out:
+	I915_WRITE(GEN6_RC6_THRESHOLD, t_orig);
+	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, i_orig);
+
+	return err;
+}
+
+static int rc6_busy_thread(void *arg)
+{
+	struct intel_engine_cs *engine;
+	struct intel_gt *gt = arg;
+	struct igt_spinner spin;
+	enum intel_engine_id id;
+	struct i915_request *rq;
+	int err = 0;
+
+	/* any existing engine in the current gt is good */
+	for_each_engine(engine, gt, id)
+		break;
+
+	err = igt_spinner_init(&spin, gt);
+	if (err)
+		return err;
+
+	rq = igt_spinner_create_request(&spin, engine->kernel_context, MI_NOOP);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto out;
+	}
+
+	i915_request_get(rq);
+	i915_request_add(rq);
+	igt_wait_for_spinner(&spin, rq); /* it's enough waiting */
+	igt_spinner_end(&spin);
+
+	i915_request_wait(rq, 0, HZ / 5);
+	i915_request_put(rq);
+
+out:
+	igt_spinner_fini(&spin);
+	return err;
+}
+
+int live_rc6_busy(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct task_struct *thread;
+	int err = 0;
+
+	thread = kmalloc(sizeof(*thread), GFP_KERNEL);
+	if (!thread)
+		return -ENOMEM;
+
+	thread = kthread_run(rc6_busy_thread, arg, "rc6_busy_selftest");
+	if (IS_ERR(thread))
+		return PTR_ERR(thread);
+
+	get_task_struct(thread);
+
+	/* gpu is busy, we shouldn't be in rc6 */
+	if (!test_rc6(gt->i915, false)) {
+		err = -EINVAL;
+		pr_err("never busy enough for having a nap\n");
+	}
+
+	err = kthread_stop(thread);
+	if (err < 0)
+		pr_err("fail and exit\n");
+
+	put_task_struct(thread);
+
+	intel_gt_pm_wait_for_idle(gt);
+
+	/* gpu is busy, we should be in rc6 */
+	if (!test_rc6(gt->i915, true)) {
+		err = -EINVAL;
+		pr_err("i915 is idle but doesn't go in rc6\n");
+	}
+
+	return err;
+}
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.h b/drivers/gpu/drm/i915/gt/selftest_rc6.h
index f907e7b035ab..23e7945e9eed 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.h
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.h
@@ -7,6 +7,9 @@ 
 #ifndef SELFTEST_RC6_H
 #define SELFTEST_RC6_H
 
+int live_rc6_basic(void *arg);
+int live_rc6_threshold(void *arg);
+int live_rc6_busy(void *arg);
 int live_rc6_ctx_wa(void *arg);
 
 #endif /* SELFTEST_RC6_H */