On Tue, May 07, 2019 at 11:55:11AM -0700, John Harrison wrote: > 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. makes sense. Although I prefer the use of v3 with __MI_LRI only used inside the function, it seems I'm the only one... let's move with v4 then... > > John. > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
