Quoting Tvrtko Ursulin (2019-08-01 16:29:53) > > On 01/08/2019 12:13, Chris Wilson wrote: > > Quoting Chris Wilson (2019-08-01 11:57:06) > >> Quoting Tvrtko Ursulin (2019-08-01 09:53:15) > >>> We could store it in ce then. We already have well defined control > >>> points for when vm changes when all are updated. > >> > >> We are storing it in ce; it's not like we recompute it all that often, > >> and when we do it's because we have rebound the vma. > >> > >>> If done like this then it looks like assigning ctx->hw_id could also do > >>> the default_desc update, so that we can avoid even more work done at pin > >>> time. > >> > >> What ctx->hw_id? You are imagining things again :-p > >> > >> Remember that we only do this on first pin from idle, not every pin. > > > > Fwiw, I quickly looked at only doing it if the vma is rebound, but > > that's move branches just to save a couple. The low frequency at which > > we have to actually compute this (walk a few more branches inside an > > already branchy contxt_pin) doesn't seem to justify the extra storage for > > me. It's not like we are recomputing lrc_desc on every submit as it once > > was. > > On every submit if last request got retired in the meantime, no, for > instance bursty loads? Yeah it is very inconsequential but at some point > we made an effort to cache as much as possible what is invariant so it > saddens me a bit to remove that. Once we have hw_id out of the way, we only need to set the bottom 32b here. > For instance Icelake engine dependent stuff sneaked into > intel_lrc.c/lrc_desriptors at some point, which is also against the > spirit of caching. If we were to move the cached value in ce then we > would be able to remove that and have it once again minimal in there. Well we can set all bits but hw_id/lrca at init time. How about if I run that past you? > Not only just minimal, but not separated in two separate places. I guess > this patch improves things in that respect - consolidates the lrc_desc > computation once again. > > I did not get the part about VMA re-binding. I did not suggest to move > the lrca offset into cache as well. I was just thinking about the gen, > engine and vm dependent bits could naturally go into > i915_gem_context.c/default_desc_template. Just need to take (engine, > hw_id, vm). I'm just thinking about the bit that changes inside ce->lrc_desc. > And virtual engine would have to re-compute it when moving engines. Hm.. > we don't seem to do that? Only when pinning we set it up based on > sibling[0] so how it all works? We don't re-pin when moving engine I > thought. No. We don't. Whoops. Good job clearly nothing uses that then. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
