| Message ID | 20190109213038.16754-1-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: init per-engine WAs for all engines | expand |
Quoting Daniele Ceraolo Spurio (2019-01-09 21:30:38) > commit 4a15c75c4246 ("drm/i915: Introduce per-engine workarounds") > refactored the workaround code to have functions per-engine, but didn't > call any of them from logical_xcs_ring_init. Since we do have a non-RCS > workaround for KBL (WaKBLVECSSemaphoreWaitPoll) we do need to call > intel_engine_init_workarounds for non-RCS engines. > Note that whitelist is still RCS-only. Yeah, the danger is that our selftests are only as good as the code to setup the workaround lists. Hmm, really that code shouldn't be using the wa_list built into the engine, but building them itself to double check against such failures or corruption. I would push intel_engine_init_workarounds() to logical_ring init (after intel_engine_init_common()?) and then remove the redundant call from rcs_init. Is it worth doing the same for init_whitelist... Not so sure, since the RCS nature of that whitelist is an intrinsic property that we should be aware of (so less desire for automagical init). Similar argument is that my intention is for the same code to be used to setup the legacy ringbuffer workarounds, and so should we look at doing the init as part of intel_engine_init_common() itself? -Chris
Quoting Chris Wilson (2019-01-09 21:48:29) > Quoting Daniele Ceraolo Spurio (2019-01-09 21:30:38) > > commit 4a15c75c4246 ("drm/i915: Introduce per-engine workarounds") > > refactored the workaround code to have functions per-engine, but didn't > > call any of them from logical_xcs_ring_init. Since we do have a non-RCS > > workaround for KBL (WaKBLVECSSemaphoreWaitPoll) we do need to call > > intel_engine_init_workarounds for non-RCS engines. > > Note that whitelist is still RCS-only. > > Yeah, the danger is that our selftests are only as good as the code to > setup the workaround lists. Hmm, really that code shouldn't be using the > wa_list built into the engine, but building them itself to double check > against such failures or corruption. Fwiw, I think we should first create a patch for the selftests to detect the missing initialisation. -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 6c98fb7cebf2..13c6c579ec7a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2304,9 +2304,17 @@ int logical_render_ring_init(struct intel_engine_cs *engine) int logical_xcs_ring_init(struct intel_engine_cs *engine) { + int ret; + logical_ring_setup(engine); - return logical_ring_init(engine); + ret = logical_ring_init(engine); + if (ret) + return ret; + + intel_engine_init_workarounds(engine); + + return 0; } static u32
commit 4a15c75c4246 ("drm/i915: Introduce per-engine workarounds") refactored the workaround code to have functions per-engine, but didn't call any of them from logical_xcs_ring_init. Since we do have a non-RCS workaround for KBL (WaKBLVECSSemaphoreWaitPoll) we do need to call intel_engine_init_workarounds for non-RCS engines. Note that whitelist is still RCS-only. Fixes: 4a15c75c4246 ("drm/i915: Introduce per-engine workarounds") Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)