[PATCH v2 1/5] drm/i915: Only disable execlist preemption for the duration of the request

[PATCH v2 1/5] drm/i915: Only disable execlist preemption for the duration of the request

[Chris Wilson]

We need to prevent resubmission of the context immediately following an
initial resubmit (which does a lite-restore preemption). Currently we do
this by disabling all submission whilst the context is still active, but
we can improve this by limiting the restriction to only until we
receive notification from the context-switch interrupt that the
lite-restore preemption is complete.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 18 ++++++++++++------
 drivers/gpu/drm/i915/intel_lrc.c        |  7 +++----
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fa69d72fdcb9..9d7a77ecec3d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3320,15 +3320,21 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 
 			rcu_read_lock();
 			rq = READ_ONCE(engine->execlist_port[0].request);
-			if (rq)
-				print_request(m, rq, "\t\tELSP[0] ");
-			else
+			if (rq) {
+				seq_printf(m, "\t\tELSP[0] count=%d, ",
+					   engine->execlist_port[0].count);
+				print_request(m, rq, "rq: ");
+			} else {
 				seq_printf(m, "\t\tELSP[0] idle\n");
+			}
 			rq = READ_ONCE(engine->execlist_port[1].request);
-			if (rq)
-				print_request(m, rq, "\t\tELSP[1] ");
-			else
+			if (rq) {
+				seq_printf(m, "\t\tELSP[1] count=%d, ",
+					   engine->execlist_port[1].count);
+				print_request(m, rq, "rq: ");
+			} else {
 				seq_printf(m, "\t\tELSP[1] idle\n");
+			}
 			rcu_read_unlock();
 
 			spin_lock_irq(&engine->timeline->lock);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index eceffe25c022..873c3a8a580b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -388,7 +388,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 		execlists_context_status_change(port[0].request,
 						INTEL_CONTEXT_SCHEDULE_IN);
 	desc[0] = execlists_update_context(port[0].request);
-	engine->preempt_wa = port[0].count++; /* bdw only? fixed on skl? */
+	port[0].count++;
 
 	if (port[1].request) {
 		GEM_BUG_ON(port[1].count);
@@ -558,7 +558,8 @@ static bool execlists_elsp_ready(struct intel_engine_cs *engine)
 	int port;
 
 	port = 1; /* wait for a free slot */
-	if (engine->disable_lite_restore_wa || engine->preempt_wa)
+	if (engine->disable_lite_restore_wa ||
+	    engine->execlist_port[0].count > 1)
 		port = 0; /* wait for GPU to be idle before continuing */
 
 	return !engine->execlist_port[port].request;
@@ -609,8 +610,6 @@ static void intel_lrc_irq_handler(unsigned long data)
 				i915_gem_request_put(port[0].request);
 				port[0] = port[1];
 				memset(&port[1], 0, sizeof(port[1]));
-
-				engine->preempt_wa = false;
 			}
 
 			GEM_BUG_ON(port[0].count == 0 &&
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 79c2b8d72322..34cdbb6350a8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -381,7 +381,6 @@ struct intel_engine_cs {
 	struct rb_node *execlist_first;
 	unsigned int fw_domains;
 	bool disable_lite_restore_wa;
-	bool preempt_wa;
 	u32 ctx_desc_template;
 
 	/* Contexts are pinned whilst they are active on the GPU. The last
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/5] drm/i915: Only run execlist context-switch handler after an interrupt

[Chris Wilson]

Mark when we run the execlist tasklet following the interrupt, so we
don't probe a potentially initialised register when submitting the
contexts multiple times.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c         | 7 +++++--
 drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6fefc34ef602..42be116dd33d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1349,8 +1349,11 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir, int test_shift)
 {
 	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift))
 		notify_ring(engine);
-	if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift))
-		tasklet_schedule(&engine->irq_tasklet);
+
+	if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift)) {
+		set_bit(IRQ_CTX_SWITCH, &engine->irq_tasklet.state);
+		tasklet_hi_schedule(&engine->irq_tasklet);
+	}
 }
 
 static irqreturn_t gen8_gt_irq_ack(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 873c3a8a580b..d2adfc77260a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -577,7 +577,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 
 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
 
-	if (!execlists_elsp_idle(engine)) {
+	while (test_and_clear_bit(IRQ_CTX_SWITCH, &engine->irq_tasklet.state)) {
 		u32 __iomem *csb_mmio =
 			dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
 		u32 __iomem *buf =
@@ -1346,6 +1346,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
 	DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
 
 	/* After a GPU reset, we may have requests to replay */
+	clear_bit(IRQ_CTX_SWITCH, &engine->irq_tasklet.state);
 	if (!execlists_elsp_idle(engine)) {
 		engine->execlist_port[0].count = 0;
 		engine->execlist_port[1].count = 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 34cdbb6350a8..c63f39d47fdd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -373,6 +373,7 @@ struct intel_engine_cs {
 
 	/* Execlists */
 	struct tasklet_struct irq_tasklet;
+#define IRQ_CTX_SWITCH (BITS_PER_LONG - 1) /* TASKLET_STATE_USER */
 	struct execlist_port {
 		struct drm_i915_gem_request *request;
 		unsigned int count;
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915: Only run execlist context-switch handler after an interrupt

[Chris Wilson]

On Mon, Jan 23, 2017 at 12:14:26PM +0000, Chris Wilson wrote:
> Mark when we run the execlist tasklet following the interrupt, so we
> don't probe a potentially initialised register when submitting the

s/initialised/uninitialised/!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915: Only run execlist context-switch handler after an interrupt

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Mark when we run the execlist tasklet following the interrupt, so we
> don't probe a potentially initialised register when submitting the
> contexts multiple times.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_irq.c         | 7 +++++--
>  drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6fefc34ef602..42be116dd33d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1349,8 +1349,11 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir, int test_shift)
>  {
>  	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift))
>  		notify_ring(engine);
> -	if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift))
> -		tasklet_schedule(&engine->irq_tasklet);
> +
> +	if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift)) {
> +		set_bit(IRQ_CTX_SWITCH, &engine->irq_tasklet.state);
> +		tasklet_hi_schedule(&engine->irq_tasklet);
> +	}
>  }
>  
>  static irqreturn_t gen8_gt_irq_ack(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 873c3a8a580b..d2adfc77260a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -577,7 +577,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>  
>  	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
>  
> -	if (!execlists_elsp_idle(engine)) {
> +	while (test_and_clear_bit(IRQ_CTX_SWITCH, &engine->irq_tasklet.state)) {
>  		u32 __iomem *csb_mmio =
>  			dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
>  		u32 __iomem *buf =
> @@ -1346,6 +1346,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
>  	DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
>  
>  	/* After a GPU reset, we may have requests to replay */
> +	clear_bit(IRQ_CTX_SWITCH, &engine->irq_tasklet.state);
>  	if (!execlists_elsp_idle(engine)) {
>  		engine->execlist_port[0].count = 0;
>  		engine->execlist_port[1].count = 0;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 34cdbb6350a8..c63f39d47fdd 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -373,6 +373,7 @@ struct intel_engine_cs {
>  
>  	/* Execlists */
>  	struct tasklet_struct irq_tasklet;
> +#define IRQ_CTX_SWITCH (BITS_PER_LONG - 1) /* TASKLET_STATE_USER */

Looking at the tasklet code, TASKLET_STATE_USER would be a
good idea. However until we have one, I think we need to
avoid piggybacking on tasklet state.

-Mika

>  	struct execlist_port {
>  		struct drm_i915_gem_request *request;
>  		unsigned int count;
> -- 
> 2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/5] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched

[Chris Wilson]

If the CSB head/tail pointers are unchanged, we can skip the update of
the CSB register afterwards.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d2adfc77260a..419a874491c6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -587,9 +587,12 @@ static void intel_lrc_irq_handler(unsigned long data)
 		csb = readl(csb_mmio);
 		head = GEN8_CSB_READ_PTR(csb);
 		tail = GEN8_CSB_WRITE_PTR(csb);
+		if (head == tail)
+			break;
+
 		if (tail < head)
 			tail += GEN8_CSB_ENTRIES;
-		while (head < tail) {
+		do {
 			unsigned int idx = ++head % GEN8_CSB_ENTRIES;
 			unsigned int status = readl(buf + 2 * idx);
 
@@ -614,7 +617,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 
 			GEM_BUG_ON(port[0].count == 0 &&
 				   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
-		}
+		} while (head < tail);
 
 		writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK,
 				     GEN8_CSB_WRITE_PTR(csb) << 8),
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/5] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched

[Chris Wilson]

On Mon, Jan 23, 2017 at 12:14:27PM +0000, Chris Wilson wrote:
> If the CSB head/tail pointers are unchanged, we can skip the update of
> the CSB register afterwards.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/5] drm/i915: Dequeue execlists on a new request if any port is available

[Chris Wilson]

If the second ELSP port is available, schedule the execlists tasklet to
see if the new request can occupy it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 419a874491c6..2bb732e0aafb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -666,7 +666,7 @@ static void execlists_submit_request(struct drm_i915_gem_request *request)
 
 	if (insert_request(&request->priotree, &engine->execlist_queue))
 		engine->execlist_first = &request->priotree.node;
-	if (execlists_elsp_idle(engine))
+	if (execlists_elsp_ready(engine))
 		tasklet_hi_schedule(&engine->irq_tasklet);
 
 	spin_unlock_irqrestore(&engine->timeline->lock, flags);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/5] drm/i915: Emit dma-fence (and execlists submit) first from signaler

[Chris Wilson]

When introduced, I thought that reducing client latency from the
signaler was the priority. Since its inception the signaler has become
responsible for keeping the execlists full, via the dma-fence. As this
is very important to minimise overall execution time, signal the
dma-fence first and then signal any waiting clients.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index fcfa423d08bd..c1f3bd2e053b 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -459,16 +459,16 @@ static int intel_breadcrumbs_signaler(void *arg)
 		 */
 		request = READ_ONCE(b->first_signal);
 		if (signal_complete(request)) {
+			local_bh_disable();
+			dma_fence_signal(&request->fence);
+			local_bh_enable(); /* kick start the tasklets */
+
 			/* Wake up all other completed waiters and select the
 			 * next bottom-half for the next user interrupt.
 			 */
 			intel_engine_remove_wait(engine,
 						 &request->signaling.wait);
 
-			local_bh_disable();
-			dma_fence_signal(&request->fence);
-			local_bh_enable(); /* kick start the tasklets */
-
 			/* Find the next oldest signal. Note that as we have
 			 * not been holding the lock, another client may
 			 * have installed an even older signal than the one
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915: Only disable execlist preemption for the duration of the request

[Chris Wilson]

On Mon, Jan 23, 2017 at 12:14:25PM +0000, Chris Wilson wrote:
> @@ -558,7 +558,8 @@ static bool execlists_elsp_ready(struct intel_engine_cs *engine)
>  	int port;
>  
>  	port = 1; /* wait for a free slot */
> -	if (engine->disable_lite_restore_wa || engine->preempt_wa)
> +	if (engine->disable_lite_restore_wa ||
> +	    engine->execlist_port[0].count > 1)
>  		port = 0; /* wait for GPU to be idle before continuing */

The disable_lite_restore_wa is a preproduction wa, right? Do we still
want to keep it?

If not this could be rewritten as

return port[0].count + port[1].count < 2;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915: Only disable execlist preemption for the duration of the request

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Mon, Jan 23, 2017 at 12:14:25PM +0000, Chris Wilson wrote:
>> @@ -558,7 +558,8 @@ static bool execlists_elsp_ready(struct intel_engine_cs *engine)
>>  	int port;
>>  
>>  	port = 1; /* wait for a free slot */
>> -	if (engine->disable_lite_restore_wa || engine->preempt_wa)
>> +	if (engine->disable_lite_restore_wa ||
>> +	    engine->execlist_port[0].count > 1)
>>  		port = 0; /* wait for GPU to be idle before continuing */
>
> The disable_lite_restore_wa is a preproduction wa, right? Do we still
> want to keep it?
>

I was about to suggest to getting rid of it. I remeber doing it
but patches are lost somewhere.

The concern with the preempt_wa change is that does it work? =)

-Mika

> If not this could be rewritten as
>
> return port[0].count + port[1].count < 2;
> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915: Only disable execlist preemption for the duration of the request

[Chris Wilson]

On Mon, Jan 23, 2017 at 02:33:20PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Mon, Jan 23, 2017 at 12:14:25PM +0000, Chris Wilson wrote:
> >> @@ -558,7 +558,8 @@ static bool execlists_elsp_ready(struct intel_engine_cs *engine)
> >>  	int port;
> >>  
> >>  	port = 1; /* wait for a free slot */
> >> -	if (engine->disable_lite_restore_wa || engine->preempt_wa)
> >> +	if (engine->disable_lite_restore_wa ||
> >> +	    engine->execlist_port[0].count > 1)
> >>  		port = 0; /* wait for GPU to be idle before continuing */
> >
> > The disable_lite_restore_wa is a preproduction wa, right? Do we still
> > want to keep it?
> >
> 
> I was about to suggest to getting rid of it. I remeber doing it
> but patches are lost somewhere.
> 
> The concern with the preempt_wa change is that does it work? =)

engine->preempt_wa is equivalent to engine->execlist_port[0].count == 2
so that identity is preserved here.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/5] drm/i915: Only disable execlist preemption for the duration of the request

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/5] drm/i915: Only disable execlist preemption for the duration of the request
URL   : https://patchwork.freedesktop.org/series/18403/
State : success

== Summary ==

Series 18403v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/18403/revisions/1/mbox/


fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

5c31c0247c4b947cd0af336ddfc5798c74402863 drm-tip: 2017y-01m-23d-12h-27m-17s UTC integration manifest
2006ca5 drm/i915: Emit dma-fence (and execlists submit) first from signaler
1e80b70 drm/i915: Dequeue execlists on a new request if any port is available
1302ce5 drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched
92b27ca drm/i915: Only run execlist context-switch handler after an interrupt
78ef036 drm/i915: Only disable execlist preemption for the duration of the request

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3581/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915: Only disable execlist preemption for the duration of the request

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> We need to prevent resubmission of the context immediately following an
> initial resubmit (which does a lite-restore preemption). Currently we do
> this by disabling all submission whilst the context is still active, but
> we can improve this by limiting the restriction to only until we
> receive notification from the context-switch interrupt that the
> lite-restore preemption is complete.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

With some handholding in irc, I can now associate the past, the
code and the commit desc and it holds true.

I also tested it lightly with kbl.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 18 ++++++++++++------
>  drivers/gpu/drm/i915/intel_lrc.c        |  7 +++----
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
>  3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index fa69d72fdcb9..9d7a77ecec3d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3320,15 +3320,21 @@ static int i915_engine_info(struct seq_file *m, void *unused)
>  
>  			rcu_read_lock();
>  			rq = READ_ONCE(engine->execlist_port[0].request);
> -			if (rq)
> -				print_request(m, rq, "\t\tELSP[0] ");
> -			else
> +			if (rq) {
> +				seq_printf(m, "\t\tELSP[0] count=%d, ",
> +					   engine->execlist_port[0].count);
> +				print_request(m, rq, "rq: ");
> +			} else {
>  				seq_printf(m, "\t\tELSP[0] idle\n");
> +			}
>  			rq = READ_ONCE(engine->execlist_port[1].request);
> -			if (rq)
> -				print_request(m, rq, "\t\tELSP[1] ");
> -			else
> +			if (rq) {
> +				seq_printf(m, "\t\tELSP[1] count=%d, ",
> +					   engine->execlist_port[1].count);
> +				print_request(m, rq, "rq: ");
> +			} else {
>  				seq_printf(m, "\t\tELSP[1] idle\n");
> +			}
>  			rcu_read_unlock();
>  
>  			spin_lock_irq(&engine->timeline->lock);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index eceffe25c022..873c3a8a580b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -388,7 +388,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
>  		execlists_context_status_change(port[0].request,
>  						INTEL_CONTEXT_SCHEDULE_IN);
>  	desc[0] = execlists_update_context(port[0].request);
> -	engine->preempt_wa = port[0].count++; /* bdw only? fixed on skl? */
> +	port[0].count++;
>  
>  	if (port[1].request) {
>  		GEM_BUG_ON(port[1].count);
> @@ -558,7 +558,8 @@ static bool execlists_elsp_ready(struct intel_engine_cs *engine)
>  	int port;
>  
>  	port = 1; /* wait for a free slot */
> -	if (engine->disable_lite_restore_wa || engine->preempt_wa)
> +	if (engine->disable_lite_restore_wa ||
> +	    engine->execlist_port[0].count > 1)
>  		port = 0; /* wait for GPU to be idle before continuing */
>  
>  	return !engine->execlist_port[port].request;
> @@ -609,8 +610,6 @@ static void intel_lrc_irq_handler(unsigned long data)
>  				i915_gem_request_put(port[0].request);
>  				port[0] = port[1];
>  				memset(&port[1], 0, sizeof(port[1]));
> -
> -				engine->preempt_wa = false;
>  			}
>  
>  			GEM_BUG_ON(port[0].count == 0 &&
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 79c2b8d72322..34cdbb6350a8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -381,7 +381,6 @@ struct intel_engine_cs {
>  	struct rb_node *execlist_first;
>  	unsigned int fw_domains;
>  	bool disable_lite_restore_wa;
> -	bool preempt_wa;
>  	u32 ctx_desc_template;
>  
>  	/* Contexts are pinned whilst they are active on the GPU. The last
> -- 
> 2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx