[v7,2/2] drm/i915/icl: Enhanced execution list support
diff mbox

Message ID 1516991485-32678-2-git-send-email-daniele.ceraolospurio@intel.com
State New
Headers show

Commit Message

Daniele Ceraolo Spurio Jan. 26, 2018, 6:31 p.m. UTC
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(-)

Comments

Chris Wilson Jan. 26, 2018, 9:47 p.m. UTC | #1
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
Daniele Ceraolo Spurio Jan. 26, 2018, 10:02 p.m. UTC | #2
<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

Patch
diff mbox

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