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...
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.
The test we use is to read the registers back from the CS (this helps us
be sure that the registers will be valid for MI_LRI etc). In order to
generate the expected list, we split intel_whitelist_workarounds_emit
into two phases, the first to build the list and the second to apply.
Inside the test, we only build the list and then check that list against
the hw.
v2: Filter out pre-gen8 as they do not have RING_NONPRIV.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
+static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
+ struct whitelist *w)
+{
+ struct drm_i915_private *i915 = engine->i915;
+
+ GEM_BUG_ON(engine->id != RCS);
+
+ w->count = 0;
+ w->nopid = i915_mmio_reg_offset(RING_NOPID(engine->mmio_base));
:)
+
+ if (INTEL_GEN(i915) < 8)
+ return NULL;
+ else if (IS_BROADWELL(i915))
+ bdw_whitelist_build(engine, w);
Is it worth passing the engine around? Even of we end up with
whitelisted register in engines != RCS, we will need more changes than this.
No idea, it was easy enough to pass around, so I did just in case it was
useful in future (pulling out i915 or whatever).
-Chris
I'd say let's cross that bridge when we get to it, but with or without it:
Reviewed-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
