| Message ID | 1516991485-32678-2-git-send-email-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Daniele Ceraolo Spurio (2018-01-26 18:31:25) > From: Thomas Daniel <thomas.daniel@intel.com> > > 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. > > 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 > v7: add preemption support, rename els to submit_reg (Chris) > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Signed-off-by: Thomas Daniel <thomas.daniel@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_pci.c | 3 +- > drivers/gpu/drm/i915/intel_device_info.h | 1 + > drivers/gpu/drm/i915/intel_lrc.c | 52 +++++++++++++++++++++++++------- > drivers/gpu/drm/i915/intel_lrc.h | 3 ++ > drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++-- > 6 files changed, 53 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f48a8ee..0493b4b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2743,6 +2743,8 @@ 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) > #define HAS_LOGICAL_RING_PREEMPTION(dev_priv) \ > ((dev_priv)->info.has_logical_ring_preemption) > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index f28c165..6c86cba 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -583,7 +583,8 @@ > GEN10_FEATURES, \ > .gen = 11, \ > .ddb_size = 2048, \ > - .has_csr = 0 > + .has_csr = 0, \ > + .has_logical_ring_elsq = 1 > > static const struct intel_device_info intel_icelake_11_info __initconst = { > GEN11_FEATURES, > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 9542018..dbf0f2d 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -96,6 +96,7 @@ enum intel_platform { > func(has_l3_dpf); \ > func(has_llc); \ > func(has_logical_ring_contexts); \ > + func(has_logical_ring_elsq); \ > func(has_logical_ring_preemption); \ > func(has_overlay); \ > func(has_pooled_eu); \ > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index ac78fc2..6c7b9c3 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -400,17 +400,29 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq) > return ce->lrc_desc; > } > > -static inline void elsp_write(u64 desc, u32 __iomem *elsp) > +static inline void write_desc(struct intel_engine_cs *engine, u64 desc, u32 port) > { > - writel(upper_32_bits(desc), elsp); > - writel(lower_32_bits(desc), elsp); > + if (HAS_LOGICAL_RING_ELSQ(engine->i915)) { > + writel(lower_32_bits(desc), engine->execlists.submit_reg + port * 2); > + writel(upper_32_bits(desc), engine->execlists.submit_reg + port * 2 + 1); > + } else { > + writel(upper_32_bits(desc), engine->execlists.submit_reg); > + writel(lower_32_bits(desc), engine->execlists.submit_reg); > + } > } > > static void execlists_submit_ports(struct intel_engine_cs *engine) > { > + struct drm_i915_private *dev_priv = engine->i915; > struct execlist_port *port = engine->execlists.port; > unsigned int n; > > + /* > + * ELSQ note: the submit queue is not cleared after being submitted > + * to the HW so we need to make sure we always clean it up. This is > + * currently ensured by the fact that we always write the same number > + * of elsq entries, keep this in mind before changing the loop below. > + */ > for (n = execlists_num_ports(&engine->execlists); n--; ) { > struct drm_i915_gem_request *rq; > unsigned int count; > @@ -434,8 +446,13 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) > desc = 0; > } > > - elsp_write(desc, engine->execlists.elsp); > + write_desc(engine, desc, n); > } > + > + /* we need to manually load the submit queue */ > + if (HAS_LOGICAL_RING_ELSQ(dev_priv)) > + I915_WRITE_FW(RING_EXECLIST_CONTROL(engine), EL_CTRL_LOAD); > + > execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); > } > > @@ -470,11 +487,12 @@ static void port_assign(struct execlist_port *port, > > static void inject_preempt_context(struct intel_engine_cs *engine) > { > + struct drm_i915_private *dev_priv = engine->i915; > struct intel_context *ce = > &engine->i915->preempt_context->engine[engine->id]; > unsigned int n; > > - GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID); > + GEM_BUG_ON(dev_priv->preempt_context->hw_id != PREEMPT_ID); > GEM_BUG_ON(!IS_ALIGNED(ce->ring->size, WA_TAIL_BYTES)); > > memset(ce->ring->vaddr + ce->ring->tail, 0, WA_TAIL_BYTES); > @@ -490,9 +508,14 @@ static void inject_preempt_context(struct intel_engine_cs *engine) > > GEM_TRACE("%s\n", engine->name); > for (n = execlists_num_ports(&engine->execlists); --n; ) > - elsp_write(0, engine->execlists.elsp); > + write_desc(engine, 0, n); > + > + write_desc(engine, ce->lrc_desc, n); > + > + /* we need to manually load the submit queue */ > + if (HAS_LOGICAL_RING_ELSQ(dev_priv)) > + I915_WRITE_FW(RING_EXECLIST_CONTROL(engine), EL_CTRL_LOAD); if (execlists->commit_reg) writel(EL_CTRL_LOAd, execlists->commit_reg); I would then use that same conditional for write_desc. > > - elsp_write(ce->lrc_desc, engine->execlists.elsp); > execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); > } > > @@ -762,6 +785,8 @@ static void execlists_submission_tasklet(unsigned long data) > struct intel_engine_execlists * const execlists = &engine->execlists; > struct execlist_port * const port = execlists->port; > struct drm_i915_private *dev_priv = engine->i915; > + struct intel_context *preempt_ce = > + &dev_priv->preempt_context->engine[engine->id]; > bool fw = false; > > /* We can skip acquiring intel_runtime_pm_get() here as it was taken > @@ -870,7 +895,8 @@ static void execlists_submission_tasklet(unsigned long data) > GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE); > > if (status & GEN8_CTX_STATUS_COMPLETE && > - buf[2*head + 1] == PREEMPT_ID) { > + HAS_LOGICAL_RING_PREEMPTION(dev_priv) && > + buf[2*head + 1] == upper_32_bits(preempt_ce->lrc_desc)) { buf[2*head + 1] == execlists->preempt_status_complete No need for HAS_LOGICAL_RING_PREEMPTION as you can then set to an impossible value. If you want to send that as a bug fix patch first... -Chris
<snip> >> @@ -870,7 +895,8 @@ static void execlists_submission_tasklet(unsigned long data) >> GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE); >> >> if (status & GEN8_CTX_STATUS_COMPLETE && >> - buf[2*head + 1] == PREEMPT_ID) { >> + HAS_LOGICAL_RING_PREEMPTION(dev_priv) && >> + buf[2*head + 1] == upper_32_bits(preempt_ce->lrc_desc)) { > > buf[2*head + 1] == execlists->preempt_status_complete > > No need for HAS_LOGICAL_RING_PREEMPTION as you can then set to an > impossible value. If you want to send that as a bug fix patch first... > -Chris > Unless I'm missing something this isn't a bug pre-gen11 since we always allocate the preempt context and thus we can't erroneously match PREEMPT_ID. lrc_desc is only created after pinning, so to use that the extra check was required. I'll add execlists->preempt_status_complete (and the other change for the commit_reg) to this patch. Daniele
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f48a8ee..0493b4b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2743,6 +2743,8 @@ 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) #define HAS_LOGICAL_RING_PREEMPTION(dev_priv) \ ((dev_priv)->info.has_logical_ring_preemption) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index f28c165..6c86cba 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -583,7 +583,8 @@ GEN10_FEATURES, \ .gen = 11, \ .ddb_size = 2048, \ - .has_csr = 0 + .has_csr = 0, \ + .has_logical_ring_elsq = 1 static const struct intel_device_info intel_icelake_11_info __initconst = { GEN11_FEATURES, diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 9542018..dbf0f2d 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -96,6 +96,7 @@ enum intel_platform { func(has_l3_dpf); \ func(has_llc); \ func(has_logical_ring_contexts); \ + func(has_logical_ring_elsq); \ func(has_logical_ring_preemption); \ func(has_overlay); \ func(has_pooled_eu); \ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ac78fc2..6c7b9c3 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -400,17 +400,29 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq) return ce->lrc_desc; } -static inline void elsp_write(u64 desc, u32 __iomem *elsp) +static inline void write_desc(struct intel_engine_cs *engine, u64 desc, u32 port) { - writel(upper_32_bits(desc), elsp); - writel(lower_32_bits(desc), elsp); + if (HAS_LOGICAL_RING_ELSQ(engine->i915)) { + writel(lower_32_bits(desc), engine->execlists.submit_reg + port * 2); + writel(upper_32_bits(desc), engine->execlists.submit_reg + port * 2 + 1); + } else { + writel(upper_32_bits(desc), engine->execlists.submit_reg); + writel(lower_32_bits(desc), engine->execlists.submit_reg); + } } static void execlists_submit_ports(struct intel_engine_cs *engine) { + struct drm_i915_private *dev_priv = engine->i915; struct execlist_port *port = engine->execlists.port; unsigned int n; + /* + * ELSQ note: the submit queue is not cleared after being submitted + * to the HW so we need to make sure we always clean it up. This is + * currently ensured by the fact that we always write the same number + * of elsq entries, keep this in mind before changing the loop below. + */ for (n = execlists_num_ports(&engine->execlists); n--; ) { struct drm_i915_gem_request *rq; unsigned int count; @@ -434,8 +446,13 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) desc = 0; } - elsp_write(desc, engine->execlists.elsp); + write_desc(engine, desc, n); } + + /* we need to manually load the submit queue */ + if (HAS_LOGICAL_RING_ELSQ(dev_priv)) + I915_WRITE_FW(RING_EXECLIST_CONTROL(engine), EL_CTRL_LOAD); + execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); } @@ -470,11 +487,12 @@ static void port_assign(struct execlist_port *port, static void inject_preempt_context(struct intel_engine_cs *engine) { + struct drm_i915_private *dev_priv = engine->i915; struct intel_context *ce = &engine->i915->preempt_context->engine[engine->id]; unsigned int n; - GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID); + GEM_BUG_ON(dev_priv->preempt_context->hw_id != PREEMPT_ID); GEM_BUG_ON(!IS_ALIGNED(ce->ring->size, WA_TAIL_BYTES)); memset(ce->ring->vaddr + ce->ring->tail, 0, WA_TAIL_BYTES); @@ -490,9 +508,14 @@ static void inject_preempt_context(struct intel_engine_cs *engine) GEM_TRACE("%s\n", engine->name); for (n = execlists_num_ports(&engine->execlists); --n; ) - elsp_write(0, engine->execlists.elsp); + write_desc(engine, 0, n); + + write_desc(engine, ce->lrc_desc, n); + + /* we need to manually load the submit queue */ + if (HAS_LOGICAL_RING_ELSQ(dev_priv)) + I915_WRITE_FW(RING_EXECLIST_CONTROL(engine), EL_CTRL_LOAD); - elsp_write(ce->lrc_desc, engine->execlists.elsp); execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); } @@ -762,6 +785,8 @@ static void execlists_submission_tasklet(unsigned long data) struct intel_engine_execlists * const execlists = &engine->execlists; struct execlist_port * const port = execlists->port; struct drm_i915_private *dev_priv = engine->i915; + struct intel_context *preempt_ce = + &dev_priv->preempt_context->engine[engine->id]; bool fw = false; /* We can skip acquiring intel_runtime_pm_get() here as it was taken @@ -870,7 +895,8 @@ static void execlists_submission_tasklet(unsigned long data) GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE); if (status & GEN8_CTX_STATUS_COMPLETE && - buf[2*head + 1] == PREEMPT_ID) { + HAS_LOGICAL_RING_PREEMPTION(dev_priv) && + buf[2*head + 1] == upper_32_bits(preempt_ce->lrc_desc)) { GEM_TRACE("%s preempt-idle\n", engine->name); execlists_cancel_port_requests(execlists); @@ -2005,8 +2031,12 @@ static int logical_ring_init(struct intel_engine_cs *engine) if (ret) goto error; - engine->execlists.elsp = - engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); + if (HAS_LOGICAL_RING_ELSQ(engine->i915)) + engine->execlists.submit_reg = engine->i915->regs + + i915_mmio_reg_offset(RING_EXECLIST_SQ_CONTENTS(engine)); + else + engine->execlists.submit_reg = engine->i915->regs + + i915_mmio_reg_offset(RING_ELSP(engine)); return 0; @@ -2270,7 +2300,7 @@ static void execlists_init_reg_state(u32 *regs, if (!engine->default_state) regs[CTX_CONTEXT_CONTROL + 1] |= _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT); - if (ctx->hw_id == PREEMPT_ID) + if (ctx->hw_id == PREEMPT_ID && INTEL_GEN(engine->i915) < 11) regs[CTX_CONTEXT_CONTROL + 1] |= _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT); diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 636ced4..fa5de77 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -39,6 +39,9 @@ #define CTX_CTRL_RS_CTX_ENABLE (1 << 1) #define CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT (1 << 2) #define RING_CONTEXT_STATUS_BUF_BASE(engine) _MMIO((engine)->mmio_base + 0x370) +#define RING_EXECLIST_SQ_CONTENTS(engine) _MMIO((engine)->mmio_base + 0x510) +#define RING_EXECLIST_CONTROL(engine) _MMIO((engine)->mmio_base + 0x550) +#define EL_CTRL_LOAD (1 << 0) #define RING_CONTEXT_STATUS_BUF_LO(engine, i) _MMIO((engine)->mmio_base + 0x370 + (i) * 8) #define RING_CONTEXT_STATUS_BUF_HI(engine, i) _MMIO((engine)->mmio_base + 0x370 + (i) * 8 + 4) #define RING_CONTEXT_STATUS_PTR(engine) _MMIO((engine)->mmio_base + 0x3a0) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index c5ff203..ebccc5f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -200,9 +200,11 @@ struct intel_engine_execlists { bool no_priolist; /** - * @elsp: the ExecList Submission Port register + * @submit_reg: gen-specific execlist submission register + * set to the ExecList Submission Port (elsp) register pre-Gen11 and to + * the ExecList Submission Queue Contents register array for Gen11+ */ - u32 __iomem *elsp; + u32 __iomem *submit_reg; /** * @port: execlist port states