On 5/6/2019 14:36, Rodrigo Vivi wrote:
On Tue, Apr 23, 2019 at 06:50:13PM -0700, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
With virtual engines, it is no longer possible to know which specific
physical engine a given request will be executed on at the time that
request is generated. This means that the request itself must be engine
agnostic - any direct register writes must be relative to the engine
and not absolute addresses.
The LRI command has support for engine relative addressing. However,
the mechanism is not transparent to the driver. The scheme for Gen11
(MI_LRI_ADD_CS_MMIO_START) requires the LRI address to have no
absolute engine base component. The hardware then adds on the correct
engine offset at execution time.
Due to the non-trivial and differing schemes on different hardware, it
is not possible to simply update the code that creates the LRI
commands to set a remap flag and let the hardware get on with it.
Instead, this patch adds function wrappers for generating the LRI
command itself and then for constructing the correct address to use
with the LRI.
v2: Fix build break in GVT. Remove flags parameter [review feedback
from Chris W].
v3: Fix build break in selftest. Rebase to newer base tree and fix
merge conflict.
v4: More rebasing. Rmoved relative addressing support from Gen7-9 only
code paths [review feedback from Chris W].
First of all, would you have a rebased version after gt/ ?
I have just done the rebase. Was planning to resend shortly. Although if
there is more discussion about the best direction to take then I would
rather hold off posting until a consensus is reached.
So, from my point of view v3 was better than this because this spread
the __MI_LOAD_REGISTER_IMM everywhere.
Maybe I just disagree with Chris and I'd prefer a single place
like v3, but anyway we could probably arrive in an intermediate
solution like: Couldn't we do in a way that we keep the MI_LRI without
'__' and use this new function only on the paths needed?
and maybe name this function gen11_get_lri_cmd? to make it clear
that gen11+ needs to use this path.
The intention was to make it clear that no new code should be directly
writing MI_LRI. Everything should go through the helper function. Hence
renaming to add the '__' to make it obvious. Otherwise, someone might
use the old one by accident and we won't notice until some random and
hard to track down failure related to virtual engines.
Not sure I would say that the __MI_LRI is spreading 'everywhere'. There
are only 8 instances versus double that of the get_lri_cmd version. Note
also that it is not only Gen11+ specific paths. There are multiple
places that are gen agnostic. So, unless you want to split those into
pre/post Gen11 versions as well, you would end up with Gen7 calling a
Gen11 labelled function.
John.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
