Re: [PATCH i-g-t] tests: add pc8

[Paulo Zanoni <przanoni@gmail.com>]

2013/8/6 Daniel Vetter <daniel@ffwll.ch>:
> On Tue, Aug 06, 2013 at 04:33:53PM -0300, Paulo Zanoni wrote:
>> 2013/8/5 Daniel Vetter <daniel@ffwll.ch>:
>> > On Mon, Aug 05, 2013 at 10:42:08AM -0300, Paulo Zanoni wrote:
>> >> 2013/8/5 Daniel Vetter <daniel@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?

>
> 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.

>
> Imo a big issue with making the w/a checker work is that it'll be a giant
> pain to keep it in sync with the kernel sources. But maybe we can extend
> the existing w/a sourcecode comment grabber in a clever way. Damien, any
> ideas?
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni