All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/execlists: Trim irq handler
@ 2017-03-25 20:10 Chris Wilson
  2017-03-25 20:27 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-03-27  9:10 ` [PATCH] " Tvrtko Ursulin
  0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2017-03-25 20:10 UTC (permalink / raw)
  To: intel-gfx

I noticed that gcc was spilling the CSB to the stack, so rearrange the
code to be more compact. Spilling in this function is slightly more
interesting due to the mmio reads acting as memory barriers and so
end up flushing the stack spills. Still miniscule to having to do at
least the pair of uncached reads :(

function                                     old     new   delta
intel_lrc_irq_handler                       1039     878    -161

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d45e6d13545a..e9822b0b308d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -506,7 +506,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 			dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
 		u32 __iomem *buf =
 			dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0));
-		unsigned int csb, head, tail;
+		unsigned int head, tail;
 
 		/* The write will be ordered by the uncached read (itself
 		 * a memory barrier), so we do not need another in the form
@@ -519,17 +519,14 @@ static void intel_lrc_irq_handler(unsigned long data)
 		 * is set and we do a new loop.
 		 */
 		__clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-		csb = readl(csb_mmio);
-		head = GEN8_CSB_READ_PTR(csb);
-		tail = GEN8_CSB_WRITE_PTR(csb);
-		if (head == tail)
-			break;
+		head = readl(csb_mmio);
+		tail = GEN8_CSB_WRITE_PTR(head);
+		head = GEN8_CSB_READ_PTR(head);
+		while (head != tail) {
+			unsigned int status;
 
-		if (tail < head)
-			tail += GEN8_CSB_ENTRIES;
-		do {
-			unsigned int idx = ++head % GEN8_CSB_ENTRIES;
-			unsigned int status = readl(buf + 2 * idx);
+			if (++head == GEN8_CSB_ENTRIES)
+				head = 0;
 
 			/* We are flying near dragons again.
 			 *
@@ -548,11 +545,12 @@ static void intel_lrc_irq_handler(unsigned long data)
 			 * status notifier.
 			 */
 
+			status = readl(buf + 2 * head);
 			if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK))
 				continue;
 
 			/* Check the context/desc id for this event matches */
-			GEM_DEBUG_BUG_ON(readl(buf + 2 * idx + 1) !=
+			GEM_DEBUG_BUG_ON(readl(buf + 2 * head + 1) !=
 					 port[0].context_id);
 
 			GEM_BUG_ON(port[0].count == 0);
@@ -570,10 +568,9 @@ 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),
+		writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK, head << 8),
 		       csb_mmio);
 	}
 
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/execlists: Trim irq handler
  2017-03-25 20:10 [PATCH] drm/i915/execlists: Trim irq handler Chris Wilson
@ 2017-03-25 20:27 ` Patchwork
  2017-03-27  9:10 ` [PATCH] " Tvrtko Ursulin
  1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2017-03-25 20:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/execlists: Trim irq handler
URL   : https://patchwork.freedesktop.org/series/21887/
State : success

== Summary ==

Series 21887v1 drm/i915/execlists: Trim irq handler
https://patchwork.freedesktop.org/api/1.0/series/21887/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 468s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 459s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 586s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 538s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 565s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 513s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 507s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 435s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 432s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 439s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 505s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 487s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 491s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 486s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 608s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 486s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 516s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 458s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 424s

5930ef736d17adf7f398f5b3cf01815c53798485 drm-tip: 2017y-03m-25d-13h-17m-45s UTC integration manifest
211128d drm/i915/execlists: Trim irq handler

== Logs ==

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915/execlists: Trim irq handler
  2017-03-25 20:10 [PATCH] drm/i915/execlists: Trim irq handler Chris Wilson
  2017-03-25 20:27 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-03-27  9:10 ` Tvrtko Ursulin
  2017-03-27 12:26   ` Chris Wilson
  1 sibling, 1 reply; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-03-27  9:10 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 25/03/2017 20:10, Chris Wilson wrote:
> I noticed that gcc was spilling the CSB to the stack, so rearrange the
> code to be more compact. Spilling in this function is slightly more
> interesting due to the mmio reads acting as memory barriers and so
> end up flushing the stack spills. Still miniscule to having to do at
> least the pair of uncached reads :(
>
> function                                     old     new   delta
> intel_lrc_irq_handler                       1039     878    -161
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d45e6d13545a..e9822b0b308d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -506,7 +506,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>  			dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
>  		u32 __iomem *buf =
>  			dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0));
> -		unsigned int csb, head, tail;
> +		unsigned int head, tail;
>
>  		/* The write will be ordered by the uncached read (itself
>  		 * a memory barrier), so we do not need another in the form
> @@ -519,17 +519,14 @@ static void intel_lrc_irq_handler(unsigned long data)
>  		 * is set and we do a new loop.
>  		 */
>  		__clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> -		csb = readl(csb_mmio);
> -		head = GEN8_CSB_READ_PTR(csb);
> -		tail = GEN8_CSB_WRITE_PTR(csb);
> -		if (head == tail)
> -			break;
> +		head = readl(csb_mmio);
> +		tail = GEN8_CSB_WRITE_PTR(head);
> +		head = GEN8_CSB_READ_PTR(head);
> +		while (head != tail) {
> +			unsigned int status;
>
> -		if (tail < head)
> -			tail += GEN8_CSB_ENTRIES;
> -		do {
> -			unsigned int idx = ++head % GEN8_CSB_ENTRIES;
> -			unsigned int status = readl(buf + 2 * idx);
> +			if (++head == GEN8_CSB_ENTRIES)
> +				head = 0;
>
>  			/* We are flying near dragons again.
>  			 *
> @@ -548,11 +545,12 @@ static void intel_lrc_irq_handler(unsigned long data)
>  			 * status notifier.
>  			 */
>
> +			status = readl(buf + 2 * head);
>  			if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK))
>  				continue;
>
>  			/* Check the context/desc id for this event matches */
> -			GEM_DEBUG_BUG_ON(readl(buf + 2 * idx + 1) !=
> +			GEM_DEBUG_BUG_ON(readl(buf + 2 * head + 1) !=
>  					 port[0].context_id);
>
>  			GEM_BUG_ON(port[0].count == 0);
> @@ -570,10 +568,9 @@ 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),
> +		writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK, head << 8),
>  		       csb_mmio);
>  	}
>
>

Looks correct, even I think a bit easier to understand.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915/execlists: Trim irq handler
  2017-03-27  9:10 ` [PATCH] " Tvrtko Ursulin
@ 2017-03-27 12:26   ` Chris Wilson
  0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2017-03-27 12:26 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Mon, Mar 27, 2017 at 10:10:54AM +0100, Tvrtko Ursulin wrote:
> 
> On 25/03/2017 20:10, Chris Wilson wrote:
> >I noticed that gcc was spilling the CSB to the stack, so rearrange the
> >code to be more compact. Spilling in this function is slightly more
> >interesting due to the mmio reads acting as memory barriers and so
> >end up flushing the stack spills. Still miniscule to having to do at
> >least the pair of uncached reads :(
> >
> >function                                     old     new   delta
> >intel_lrc_irq_handler                       1039     878    -161
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> Looks correct, even I think a bit easier to understand.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for the review, it does feel like a step in the right direction
with regards the dance between csb, head, tail and index. Pushed,
-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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-27 12:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-25 20:10 [PATCH] drm/i915/execlists: Trim irq handler Chris Wilson
2017-03-25 20:27 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-03-27  9:10 ` [PATCH] " Tvrtko Ursulin
2017-03-27 12:26   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.