Quoting Daniele Ceraolo Spurio (2018-01-26 00:10:09) > > > On 24/01/18 09:46, Chris Wilson wrote: > > Quoting Daniele Ceraolo Spurio (2018-01-24 17:30:07) > >> From: Thomas Daniel <thomas.daniel@xxxxxxxxx> > >> > >> Enhanced Execlists is an upgraded version of execlists which supports > >> up to 8 ports. The lrcs to be submitted are written to a submit queue > >> (the ExecLists Submission Queue - ELSQ), which is then loaded on the > >> HW. When writing to the ELSP register, the lrcs are written cyclically > >> in the queue from position 0 to position 7. Alternatively, it is > >> possible to write directly in the individual positions of the queue > >> using the ELSQC registers. To be able to re-use all the existing code > >> we're using the latter method and we're currently limiting ourself to > >> only using 2 elements. > >> > >> The preemption flow is sligthly different with enhanced execlists, so > >> this patch turns preemption off temporarily for platforms with ELSQ > >> while we wait for the new mechanism to land. > >> > >> v2: Rebase. > >> v3: Switch from !IS_GEN11 to GEN < 11 (Daniele Ceraolo Spurio). > >> v4: Use the elsq registers instead of elsp. (Daniele Ceraolo Spurio) > >> v5: Reword commit, rename regs to be closer to specs, turn off > >> preemption (Daniele), reuse engine->execlists.elsp (Chris) > >> v6: use has_logical_ring_elsq to differentiate the new paths > >> > >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> > >> Signed-off-by: Thomas Daniel <thomas.daniel@xxxxxxxxx> > >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 7 ++++++- > >> drivers/gpu/drm/i915/i915_pci.c | 3 ++- > >> drivers/gpu/drm/i915/intel_device_info.h | 1 + > >> drivers/gpu/drm/i915/intel_lrc.c | 35 +++++++++++++++++++++++++++----- > >> drivers/gpu/drm/i915/intel_lrc.h | 3 +++ > >> drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++-- > >> 6 files changed, 46 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index 8333692..346209a 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -2741,8 +2741,13 @@ static inline unsigned int i915_sg_segment_size(void) > >> > >> #define HAS_LOGICAL_RING_CONTEXTS(dev_priv) \ > >> ((dev_priv)->info.has_logical_ring_contexts) > >> +#define HAS_LOGICAL_RING_ELSQ(dev_priv) \ > >> + ((dev_priv)->info.has_logical_ring_elsq) > >> + > >> +/* XXX: Preemption disabled for ELSQ until support for new flow lands */ > >> #define HAS_LOGICAL_RING_PREEMPTION(dev_priv) \ > >> - ((dev_priv)->info.has_logical_ring_preemption) > >> + ((dev_priv)->info.has_logical_ring_preemption && \ > >> + !HAS_LOGICAL_RING_ELSQ(dev_priv)) > > > > It's in the intel_device_info for a reason. I knew I should not have let > > Michal turn this into a macro. > > > > You mean setting has_logical_ring_preemption to zero directly? I thought > the policy was to avoid setting things in device_info to values that > don't reflect real HW capabilities and to do the hacks elsewhere. No, data driven code. intel_device_info was introduced to remove having heavy predicates so that we could see what will be enabled and what not in one place. > > I still do not see any reason why you don't just make the current > > preemption work (it will) and then you can refine it if you prove it > > worthwhile. > > > > Just didn't see the worth of it ;). It's not a lot of code but it's in > an hot path and we're most likely going to get rid of it soon as the new > stuff is simpler. I'll put the change together and send it out so we can > evaluate that and see what works better with code at hand. Is the new stuff going to be any simpler? You still need a preemption point, so a special submission followed by detecting that in the CS handler to do the unwind. And whilst I am here, els is awful. Either stick with elsp and note that they changed the name (+layout) on icl, or replace it with a generic name. Spelling it out completely as execlists->execlists_submission is still better than els, but submit[_reg] (or submit_hw) would be clearer. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
