On 30/11/2018 11:38, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-11-30 11:31:56)
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Since we now have all the GT workarounds in a table, by adding a simple
shared helper function we can now verify that their values are still
applied after some interesting events in the lifetime of the driver.
At this stage these are the driver initialization and engine reset.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 4 +++
drivers/gpu/drm/i915/i915_gem.c | 3 ++
drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
4 files changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2f3dc1cf83a6..14d019c9455b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -53,6 +53,7 @@
#include "i915_vgpu.h"
#include "intel_drv.h"
#include "intel_uc.h"
+#include "intel_workarounds.h"
static struct drm_driver driver;
@@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
goto out;
}
+ /* Catch GT workarounds affected by engine reset. */
+ intel_gt_workarounds_verify(engine->i915, engine->name);
I'd rather, quite strongly, not have this inside i915_reset_engine()
itself. i915_reset_engine() is [designed to be] called from atomic
context. Adding I915_READ() here is questionable. My preference is to
have all this inside selftests/intel_workarounds where we can cross
check the gt/all-engines after device reset and other engine reset.
I wasn't sure myself how much value checking it here adds, but regarding
I915_READs, they are happening during engine->init_hw anyway. So I don't
think a few more would harm. But yes, as said, since I was unsure myself
I am happy to drop this hunk.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
