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@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> 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 = >->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(>->rc6);
> + else
> + intel_rc6_enable(>->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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
