[Intel-gfx] [RFCv2 01/19] drm/i915: Provide a hook for selftests

[Chris Wilson]

On Wed, Jan 11, 2017 at 06:17:48PM +0000, Tvrtko Ursulin wrote:
> On 20/12/2016 13:07, Chris Wilson wrote:
> >@@ -522,6 +534,11 @@ static struct pci_driver i915_pci_driver = {
> > static int __init i915_init(void)
> > {
> > 	bool use_kms = true;
> >+	int err;
> >+
> >+	err = i915_mock_selftests();
> >+	if (err)
> >+		return err > 0 ? 0 : err;
> 
> Am I again confused by the return codes? :) Module param of -1 will
> result in i915_mock_selftests returning 1, which here translates to
> 0 so it won't abort the load like it should.

I had to give up on that for silent passing and do the remove from
userspace on success instead. Returning anything other than 0 causes noise
in dmesg. That I can live with after an error during the selftest, since
dmesg should also contain more details on the test failure.

If i915.mock_selftests=-1 then we run the tests and stop. We just leave
the module loaded even though it hasn't bound to any pci devices. :|
igt/drv_selftest and kselftests/gpu/i915.sh then unload the module.

> >+static void set_default_test_all(struct selftest *st, unsigned long count)
> >+{
> >+	unsigned long i;
> >+
> >+	for (i = 0; i < count; i++)
> >+		if (st[i].enabled)
> >+			return;
> >+
> >+	for (i = 0; i < count; i++)
> >+		st[i].enabled = true;
> >+}
> 
> unsigned int should be enough for everyone! :) (i & count)

Such shortsightedness!

> >+static int run_selftests(const char *name,
> >+			 struct selftest *st,
> >+			 unsigned long count,
> >+			 void *data)
> >+{
> >+	int err = 0;
> >+
> 
> If I got it right:
> 
> /* Make sure both live and mock run with the same seed if ran one
> after another. */

Yes, choose the seed once, run every selected test with the same seed.
 
> ? just not sure what happens if user sets zero.

I wasn't such if 0 was a valid seed, so I wasn't caring too much if the
user did i915.st_random_seed=0. They will see the pr_info() and go
wtf, and hopefully don't do that again.

> >+	while (!i915_selftest.random_seed)
> >+		i915_selftest.random_seed = get_random_int();
> >+
> >+	i915_selftest.timeout_jiffies =
> >+		i915_selftest.timeout_ms ?
> >+		msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
> >+		MAX_SCHEDULE_TIMEOUT;
> >+
> >+	set_default_test_all(st, count);
> >+
> >+	pr_info("i915: Performing %s selftests with st_random_seed=%x and st_timeout=%u\n",
> >+		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
> >+
> >+	/* Tests are listed in order in i915_*_selftests.h */
> >+	for (; count--; st++) {
> >+		if (!st->enabled)
> >+			continue;
> >+
> >+		cond_resched();
> >+		if (signal_pending(current))
> >+			return -EINTR;
> >+
> >+		pr_debug("i915: Running %s\n", st->name);
> >+		if (data)
> >+			err = st->live(data);
> >+		else
> >+			err = st->mock();
> >+		if (err)
> >+			break;
> >+	}
> >+
> >+	if (WARN(err > 0 || err == -ENOTTY,
> >+		 "%s returned %d, conflicting with selftest's magic values!\n",
> >+		 st->name, err))
> >+		err = -1;
> >+
> >+	rcu_barrier();
> 
> Why this?

Paranoia for the tests aborting without the barrier, as we can't rely on
module_unload providing it since we may go on to load the driver as
normal.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre