[Intel-gfx] [PATCH i-g-t] tests: add pc8

[Daniel Vetter]

On Tue, Aug 06, 2013 at 05:43:23PM -0300, Paulo Zanoni wrote:
> 2013/8/6 Daniel Vetter <daniel at ffwll.ch>:
> > On Tue, Aug 06, 2013 at 04:33:53PM -0300, Paulo Zanoni wrote:
> >> 2013/8/5 Daniel Vetter <daniel at ffwll.ch>:
> >> > On Mon, Aug 05, 2013 at 10:42:08AM -0300, Paulo Zanoni wrote:
> >> >> 2013/8/5 Daniel Vetter <daniel at ffwll.ch>:
> >> >> > On Mon, Jul 29, 2013 at 05:53:27PM -0300, Paulo Zanoni wrote:
> >> >> The problem is: let's imagine there's some important register that we
> >> >> initialize when starting the driver, but then we don't touch it
> >> >> anymore. And this register is one of the registers that get reset when
> >> >> we enter PC8, but our code doesn't fix it after leaving PC8. So if you
> >> >> boot your machine, do something to allow PC8+ and then disallow it,
> >> >> the register will go back to the "reset" state (which isn't guaranteed
> >> >> to be 0x0, especially on display registers). Then you run tests/pc8:
> >> >> it reads the register (which contains the "reset" value instead of the
> >> >> real value set by our driver when it got loaded, because we already
> >> >> entered PC8+ once), enters PC8+, leaves PC8+, reads the register
> >> >> again, notices the value is the same and then gives us a "PASS". On
> >> >> the other hand, if you didn't reach PC8+ before running tests/pc8,
> >> >> then it would have noticed the value of the register changes.
> >> >>
> >> >> In other words: if some register gets initialized by our driver to a
> >> >> non-default value, but this value gets lost after we enter/leave PC8+,
> >> >> we will *only* be able to notice the difference when comparing a state
> >> >> where we *never* entered PC8+ against a state where we "already
> >> >> entered PC8+ at least once", because after the first time you
> >> >> enter/leave PC8+ you'll already have reset the register, so you'll be
> >> >> comparing bugged state against bugged state.
> >> >
> >> > Ok, I see your point. But imo igt testcases shouldn't (at least with the
> >> > testcases run by default) have such detailed knowledge of what the kernel
> >> > actually does with the registers. So a depency like "this test needs to be
> >> > run after a fresh boot when we've never entered pc8+ yet" isn't good since
> >> > it'll make the testcase fragile.
> >> >
> >> > Instead the test should check whether everything still works after we've
> >> > been in pc8+ for a bit. One exception could be w/a bits if we have a
> >> > common igt testcase to check for all of them. Then we could just rerun
> >> > that testcase.
> >> >
> >> > But if e.g. the swizzling settings get lost over pc8+ it's better to add
> >> > an explicit functional swizzle test here instead of checking registers.
> >>
> >> So instead of doing this, why not just make sure the very first i-g-t
> >> test gets us to PC8+, ant then run the full i-g-t suite after that?
> >> This should cover all the stuff that breaks due to us not restoring
> >> registers properly. Then we only have to worry about the things that
> >> run while PC8 is enabled, we can do that in another test.
> >
> > It would be nice if we could do this (e.g. also for re-running igt after
> > gpu hangs or after a suspend/resume). But I think atm QA runs testcases
> > essentially in a random order (or not in one we can control) so doing that
> > is pretty hard.
> 
> Can't we just tell them we need this one first?

Not really since such testcase would be really fragile. If you really
need/want a reliable starting point you could simply suspend/resume the
machine. lib/drmtest.c has a little helper for that, see
tests/gem_suspend.c

> > Since the major usecase here would be to sanity-check some registers (for
> > w/a and sane settings, e.g. the ddi transalation buffer settings) and we
> > don't yet really have a good integration of the existing wa setting
> > checker tool I think we can just punt here. I'll add a note to my igt
> > wishlist about this. And of course pls keep the special test mode in your
> > testcase so that developers could manually check that we correctly restore
> > registers.
> 
> Well, the ddi buffer translation problem leads to big screen
> corruption on testdisplay.

There are always limits to what we can reliably test, but such issues
could be caught by QA's QR code reader support in testdisplay.
Unfortunately that support isn't readily available as a little drmtest.c
helper function, mostly since I don't have the setup myself and so can't
test it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch