Quoting Oscar Mateo (2018-04-13 18:04:16) > > > On 4/13/2018 9:54 AM, Chris Wilson wrote: > > Quoting Oscar Mateo (2018-04-13 17:46:42) > >> > >> On 4/12/2018 8:21 AM, Chris Wilson wrote: > >>> Add a selftest to ensure that we restore the whitelisted registers after > >>> rewrite the registers everytime they might be scrubbed, e.g. module > >>> load, reset and resume. For the other volatile workaround registers, we > >>> export their presence via debugfs and check in igt/gem_workarounds. > >>> However, we don't export the whitelist and rather than do so, let's test > >>> them directly in the kernel. > >> I guess my question is... why? what was the problem with exporting the > >> list of whitelist registers in debugfs? > > We don't... (There's no RING_NONPRIV checking currently) > > There is no checking, but we were showing the full list in debugfs. Ok, > I guess it wasn't that useful without the corresponding igt... Oh no we weren't. wa_ring_whitelist_reg() isn't storing the reg in the wa list, just that we have used the RING_NONPRIV slot. At one point, I think the intention was there but that seems to disappeared and now I removed the notion entirely ;) > > I wasn't fond > > of the igt for it is checking kernel implementation rather than behaviour. > > The kernel gives it a checklist which it dutifully follows... Now that > > we have selftests, we don't need to write what I think should be unit > > tests in igt anymore. > > Ah, so I take the plan is to also check the other WAs in selftests? > Somehow I thought you wanted to treat whitelisting differently. Right, the long term goal will be to move all the workaround testing here. It just so happens that I wrote a buggy patch that CI was happy with that alerted me to the lack of testing ;) The only fly in the ointment is doing S3/S4 testing, as doing suspend/resume from inside the kernel tricky (trickier than even getting it right from userspace). So I think we may just have to be a little more creative, and do something like call i915_drv_suspend() directly. Hmm, now that's an idea. (i915_drv_suspend(); scribble over state; i915_drv_resume()). -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
