On Tue, Aug 06, 2013 at 04:33:53PM -0300, Paulo Zanoni wrote: > 2013/8/5 Daniel Vetter <daniel@xxxxxxxx>: > > On Mon, Aug 05, 2013 at 10:42:08AM -0300, Paulo Zanoni wrote: > >> 2013/8/5 Daniel Vetter <daniel@xxxxxxxx>: > >> > 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. 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. 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
