Re: [Intel-gfx] [PATCH] drm/i915: init per-engine WAs for all engines

2019-01-09 Thread Chris Wilson
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: init per-engine WAs for all engines

2019-01-09 Thread Chris Wilson
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx