Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space()
@ 2017-04-26  8:37 Chris Wilson
  2017-04-26  8:37 ` [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space() Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chris Wilson @ 2017-04-26  8:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

Exploit the power-of-two ring size to compute the space across the
wraparound using a mask rather than a if. Convert to unsigned integers
so the operation is well defined.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 20 ++++++++++----------
 drivers/gpu/drm/i915/intel_ringbuffer.h | 30 ++++++++++++++++--------------
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6836efb7e3d2..16739352f057 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -39,12 +39,15 @@
  */
 #define LEGACY_REQUEST_SIZE 200
 
-static int __intel_ring_space(int head, int tail, int size)
+static unsigned int __intel_ring_space(unsigned int head,
+				       unsigned int tail,
+				       unsigned int size)
 {
-	int space = head - tail;
-	if (space <= 0)
-		space += size;
-	return space - I915_RING_FREE_SPACE;
+	/* "If the Ring Buffer Head Pointer and the Tail Pointer are on the
+	 * same cacheline, the Head Pointer must not be greater than the Tail
+	 * Pointer."
+	 */
+	return (head - tail - CACHELINE_BYTES) & (size - 1);
 }
 
 void intel_ring_update_space(struct intel_ring *ring)
@@ -1619,12 +1622,9 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 	GEM_BUG_ON(!req->reserved_space);
 
 	list_for_each_entry(target, &ring->request_list, ring_link) {
-		unsigned space;
-
 		/* Would completion of this request free enough space? */
-		space = __intel_ring_space(target->postfix, ring->emit,
-					   ring->size);
-		if (space >= bytes)
+		if (bytes <= __intel_ring_space(target->postfix,
+						ring->emit, ring->size))
 			break;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 96710b616efb..1b1099b94912 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -17,17 +17,6 @@
 #define CACHELINE_BYTES 64
 #define CACHELINE_DWORDS (CACHELINE_BYTES / sizeof(uint32_t))
 
-/*
- * Gen2 BSpec "1. Programming Environment" / 1.4.4.6 "Ring Buffer Use"
- * Gen3 BSpec "vol1c Memory Interface Functions" / 2.3.4.5 "Ring Buffer Use"
- * Gen4+ BSpec "vol1c Memory Interface and Command Stream" / 5.3.4.5 "Ring Buffer Use"
- *
- * "If the Ring Buffer Head Pointer and the Tail Pointer are on the same
- * cacheline, the Head Pointer must not be greater than the Tail
- * Pointer."
- */
-#define I915_RING_FREE_SPACE 64
-
 struct intel_hw_status_page {
 	struct i915_vma *vma;
 	u32 *page_addr;
@@ -145,9 +134,9 @@ struct intel_ring {
 	u32 tail;
 	u32 emit;
 
-	int space;
-	int size;
-	int effective_size;
+	u32 space;
+	u32 size;
+	u32 effective_size;
 };
 
 struct i915_gem_context;
@@ -547,6 +536,19 @@ assert_ring_tail_valid(const struct intel_ring *ring, unsigned int tail)
 	 */
 	GEM_BUG_ON(!IS_ALIGNED(tail, 8));
 	GEM_BUG_ON(tail >= ring->size);
+
+	/* "Ring Buffer Use"
+	 *	Gen2 BSpec "1. Programming Environment" / 1.4.4.6
+	 *	Gen3 BSpec "1c Memory Interface Functions" / 2.3.4.5
+	 *	Gen4+ BSpec "1c Memory Interface and Command Stream" / 5.3.4.5
+	 * "If the Ring Buffer Head Pointer and the Tail Pointer are on the
+	 * same cacheline, the Head Pointer must not be greater than the Tail
+	 * Pointer."
+	 */
+#define cacheline(a) round_down(a, CACHELINE_BYTES)
+	GEM_BUG_ON(cacheline(tail) == cacheline(ring->head) &&
+		   tail < ring->head);
+#undef cacheline
 }
 
 static inline unsigned int
-- 
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] 9+ messages in thread

* [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space()
  2017-04-26  8:37 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
@ 2017-04-26  8:37 ` Chris Wilson
  2017-04-28  8:40   ` Joonas Lahtinen
  2017-04-26  8:37 ` [PATCH 3/3] drm/i915: Micro-optimise hotpath through intel_ring_begin() Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-04-26  8:37 UTC (permalink / raw)
  To: intel-gfx

Some callers immediately want to know the current ring->space after
calling intel_ring_update_space(), which we can freely provide via the
return parameter.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++++----
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 16739352f057..25e9a0dcc42d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -50,9 +50,14 @@ static unsigned int __intel_ring_space(unsigned int head,
 	return (head - tail - CACHELINE_BYTES) & (size - 1);
 }
 
-void intel_ring_update_space(struct intel_ring *ring)
+unsigned int intel_ring_update_space(struct intel_ring *ring)
 {
-	ring->space = __intel_ring_space(ring->head, ring->emit, ring->size);
+	unsigned int space;
+
+	space = __intel_ring_space(ring->head, ring->emit, ring->size);
+
+	ring->space = space;
+	return space;
 }
 
 static int
@@ -1606,8 +1611,7 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 
 	lockdep_assert_held(&req->i915->drm.struct_mutex);
 
-	intel_ring_update_space(ring);
-	if (ring->space >= bytes)
+	if (intel_ring_update_space(ring) >= bytes)
 		return 0;
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1b1099b94912..824f3eaa5e22 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -485,7 +485,7 @@ int intel_ring_pin(struct intel_ring *ring,
 		   struct drm_i915_private *i915,
 		   unsigned int offset_bias);
 void intel_ring_reset(struct intel_ring *ring, u32 tail);
-void intel_ring_update_space(struct intel_ring *ring);
+unsigned int intel_ring_update_space(struct intel_ring *ring);
 void intel_ring_unpin(struct intel_ring *ring);
 void intel_ring_free(struct intel_ring *ring);
 
-- 
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] 9+ messages in thread

* [PATCH 3/3] drm/i915: Micro-optimise hotpath through intel_ring_begin()
  2017-04-26  8:37 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
  2017-04-26  8:37 ` [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space() Chris Wilson
@ 2017-04-26  8:37 ` Chris Wilson
  2017-04-26  9:07   ` [PATCH v2] " Chris Wilson
  2017-04-26  8:59 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() Patchwork
  2017-04-26  9:27 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() (rev2) Patchwork
  3 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-04-26  8:37 UTC (permalink / raw)
  To: intel-gfx

Typically, there is space available within the ring and if not we have
to wait (by definition a slow path). Rearrange the code to reduce the
number of branches and stack size for the hotpath, accomodating a slight
growth for the wait.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 61 +++++++++++++++++----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 25e9a0dcc42d..c1227eff28e0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1603,7 +1603,7 @@ static int ring_request_alloc(struct drm_i915_gem_request *request)
 	return 0;
 }
 
-static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
+static noinline int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 {
 	struct intel_ring *ring = req->ring;
 	struct drm_i915_gem_request *target;
@@ -1651,49 +1651,50 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
 {
 	struct intel_ring *ring = req->ring;
-	int remain_actual = ring->size - ring->emit;
-	int remain_usable = ring->effective_size - ring->emit;
-	int bytes = num_dwords * sizeof(u32);
-	int total_bytes, wait_bytes;
-	bool need_wrap = false;
+	const unsigned int remain_usable = ring->effective_size - ring->emit;
+	const unsigned int bytes = num_dwords * sizeof(u32);
+	unsigned int need_wrap = 0;
+	unsigned int total_bytes;
 	u32 *cs;
 
 	total_bytes = bytes + req->reserved_space;
+	GEM_BUG_ON(total_bytes > remain_usable);
 
-	if (unlikely(bytes > remain_usable)) {
-		/*
-		 * Not enough space for the basic request. So need to flush
-		 * out the remainder and then wait for base + reserved.
-		 */
-		wait_bytes = remain_actual + total_bytes;
-		need_wrap = true;
-	} else if (unlikely(total_bytes > remain_usable)) {
-		/*
-		 * The base request will fit but the reserved space
-		 * falls off the end. So we don't need an immediate wrap
-		 * and only need to effectively wait for the reserved
-		 * size space from the start of ringbuffer.
-		 */
-		wait_bytes = remain_actual + req->reserved_space;
-	} else {
-		/* No wrapping required, just waiting. */
-		wait_bytes = total_bytes;
+	if (unlikely(total_bytes > remain_usable)) {
+		const int remain_actual = ring->size - ring->emit;
+
+		if (bytes > remain_usable) {
+			/* Not enough space for the basic request. So need to
+			 * flush out the remainder and then wait for
+			 * base + reserved.
+			 */
+			total_bytes += remain_actual;
+			need_wrap = remain_actual | 1;
+		} else  {
+			/* The base request will fit but the reserved space
+			 * falls off the end. So we don't need an immediate
+			 * wrap and only need to effectively wait for the
+			 * reserved size from the start of ringbuffer.
+			 */
+			total_bytes = req->reserved_space + remain_actual;
+		}
 	}
 
-	if (wait_bytes > ring->space) {
-		int ret = wait_for_space(req, wait_bytes);
+	if (unlikely(total_bytes > ring->space)) {
+		int ret = wait_for_space(req, total_bytes);
 		if (unlikely(ret))
 			return ERR_PTR(ret);
 	}
 
 	if (unlikely(need_wrap)) {
-		GEM_BUG_ON(remain_actual > ring->space);
-		GEM_BUG_ON(ring->emit + remain_actual > ring->size);
+		need_wrap &= ~1;
+		GEM_BUG_ON(need_wrap > ring->space);
+		GEM_BUG_ON(ring->emit + need_wrap > ring->size);
 
 		/* Fill the tail with MI_NOOP */
-		memset(ring->vaddr + ring->emit, 0, remain_actual);
+		memset(ring->vaddr + ring->emit, 0, need_wrap);
 		ring->emit = 0;
-		ring->space -= remain_actual;
+		ring->space -= need_wrap;
 	}
 
 	GEM_BUG_ON(ring->emit > ring->size - bytes);
-- 
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] 9+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space()
  2017-04-26  8:37 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
  2017-04-26  8:37 ` [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space() Chris Wilson
  2017-04-26  8:37 ` [PATCH 3/3] drm/i915: Micro-optimise hotpath through intel_ring_begin() Chris Wilson
@ 2017-04-26  8:59 ` Patchwork
  2017-04-26  9:03   ` Chris Wilson
  2017-04-26  9:27 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() (rev2) Patchwork
  3 siblings, 1 reply; 9+ messages in thread
From: Patchwork @ 2017-04-26  8:59 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space()
URL   : https://patchwork.freedesktop.org/series/23555/
State : failure

== Summary ==

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

Test gem_close_race:
        Subgroup basic-threads:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-ivb-3770)
                pass       -> INCOMPLETE (fi-kbl-7500u)
                pass       -> INCOMPLETE (fi-bdw-gvtdvm)
                pass       -> INCOMPLETE (fi-skl-6260u)
                pass       -> INCOMPLETE (fi-snb-2520m)
                pass       -> INCOMPLETE (fi-byt-j1900)
                pass       -> INCOMPLETE (fi-ilk-650)
                pass       -> INCOMPLETE (fi-hsw-4770r)
                pass       -> INCOMPLETE (fi-snb-2600)
                pass       -> INCOMPLETE (fi-skl-6700k)
                pass       -> INCOMPLETE (fi-byt-n2820)
                pass       -> INCOMPLETE (fi-skl-6770hq)
                pass       -> INCOMPLETE (fi-bdw-5557u)
                pass       -> INCOMPLETE (fi-hsw-4770)
                pass       -> INCOMPLETE (fi-kbl-7560u)
                pass       -> INCOMPLETE (fi-skl-gvtdvm)
                pass       -> INCOMPLETE (fi-skl-6700hq)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> INCOMPLETE (fi-bsw-n3050)
                pass       -> INCOMPLETE (fi-bxt-j4205)

fi-bdw-5557u     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-bdw-gvtdvm    total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-bsw-n3050     total:14   pass:13   dwarn:0   dfail:0   fail:0   skip:0  
fi-bxt-j4205     total:14   pass:13   dwarn:0   dfail:0   fail:0   skip:0  
fi-byt-j1900     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-byt-n2820     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-hsw-4770      total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-hsw-4770r     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-ilk-650       total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3520m     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3770      total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-7500u     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-7560u     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6260u     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6700hq    total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6700k     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6770hq    total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-gvtdvm    total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2520m     total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0  

7ffb3045557cbc7b49695b20416351e4e812179c drm-tip: 2017y-04m-25d-14h-42m-59s UTC integration manifest
ae807dd drm/i915: Micro-optimise hotpath through intel_ring_begin()
fb7970b drm/i915: Report the ring->space from intel_ring_update_space()
1b1fb2e drm/i915: Avoid the branch in computing intel_ring_space()

== Logs ==

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

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

* Re: ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space()
  2017-04-26  8:59 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() Patchwork
@ 2017-04-26  9:03   ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-04-26  9:03 UTC (permalink / raw)
  To: intel-gfx

On Wed, Apr 26, 2017 at 08:59:14AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space()
> URL   : https://patchwork.freedesktop.org/series/23555/
> State : failure
> 
> == Summary ==
> 
> Series 23555v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/23555/revisions/1/mbox/
> 
> Test gem_close_race:
>         Subgroup basic-threads:
>                 pass       -> INCOMPLETE (fi-ivb-3520m)
>                 pass       -> INCOMPLETE (fi-ivb-3770)
>                 pass       -> INCOMPLETE (fi-kbl-7500u)
>                 pass       -> INCOMPLETE (fi-bdw-gvtdvm)
>                 pass       -> INCOMPLETE (fi-skl-6260u)
>                 pass       -> INCOMPLETE (fi-snb-2520m)
>                 pass       -> INCOMPLETE (fi-byt-j1900)
>                 pass       -> INCOMPLETE (fi-ilk-650)
>                 pass       -> INCOMPLETE (fi-hsw-4770r)
>                 pass       -> INCOMPLETE (fi-snb-2600)
>                 pass       -> INCOMPLETE (fi-skl-6700k)
>                 pass       -> INCOMPLETE (fi-byt-n2820)
>                 pass       -> INCOMPLETE (fi-skl-6770hq)
>                 pass       -> INCOMPLETE (fi-bdw-5557u)
>                 pass       -> INCOMPLETE (fi-hsw-4770)
>                 pass       -> INCOMPLETE (fi-kbl-7560u)
>                 pass       -> INCOMPLETE (fi-skl-gvtdvm)
>                 pass       -> INCOMPLETE (fi-skl-6700hq)

Oops. Serves me right for tweaking before sending.
-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] 9+ messages in thread

* [PATCH v2] drm/i915: Micro-optimise hotpath through intel_ring_begin()
  2017-04-26  8:37 ` [PATCH 3/3] drm/i915: Micro-optimise hotpath through intel_ring_begin() Chris Wilson
@ 2017-04-26  9:07   ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-04-26  9:07 UTC (permalink / raw)
  To: intel-gfx

Typically, there is space available within the ring and if not we have
to wait (by definition a slow path). Rearrange the code to reduce the
number of branches and stack size for the hotpath, accomodating a slight
growth for the wait.

v2: Fix the new assert that packets are not larger than the actual ring.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 61 +++++++++++++++++----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 25e9a0dcc42d..77b49c490b0c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1603,7 +1603,7 @@ static int ring_request_alloc(struct drm_i915_gem_request *request)
 	return 0;
 }
 
-static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
+static noinline int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 {
 	struct intel_ring *ring = req->ring;
 	struct drm_i915_gem_request *target;
@@ -1651,49 +1651,50 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 u32 *intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
 {
 	struct intel_ring *ring = req->ring;
-	int remain_actual = ring->size - ring->emit;
-	int remain_usable = ring->effective_size - ring->emit;
-	int bytes = num_dwords * sizeof(u32);
-	int total_bytes, wait_bytes;
-	bool need_wrap = false;
+	const unsigned int remain_usable = ring->effective_size - ring->emit;
+	const unsigned int bytes = num_dwords * sizeof(u32);
+	unsigned int need_wrap = 0;
+	unsigned int total_bytes;
 	u32 *cs;
 
 	total_bytes = bytes + req->reserved_space;
+	GEM_BUG_ON(total_bytes > ring->effective_size);
 
-	if (unlikely(bytes > remain_usable)) {
-		/*
-		 * Not enough space for the basic request. So need to flush
-		 * out the remainder and then wait for base + reserved.
-		 */
-		wait_bytes = remain_actual + total_bytes;
-		need_wrap = true;
-	} else if (unlikely(total_bytes > remain_usable)) {
-		/*
-		 * The base request will fit but the reserved space
-		 * falls off the end. So we don't need an immediate wrap
-		 * and only need to effectively wait for the reserved
-		 * size space from the start of ringbuffer.
-		 */
-		wait_bytes = remain_actual + req->reserved_space;
-	} else {
-		/* No wrapping required, just waiting. */
-		wait_bytes = total_bytes;
+	if (unlikely(total_bytes > remain_usable)) {
+		const int remain_actual = ring->size - ring->emit;
+
+		if (bytes > remain_usable) {
+			/* Not enough space for the basic request. So need to
+			 * flush out the remainder and then wait for
+			 * base + reserved.
+			 */
+			total_bytes += remain_actual;
+			need_wrap = remain_actual | 1;
+		} else  {
+			/* The base request will fit but the reserved space
+			 * falls off the end. So we don't need an immediate
+			 * wrap and only need to effectively wait for the
+			 * reserved size from the start of ringbuffer.
+			 */
+			total_bytes = req->reserved_space + remain_actual;
+		}
 	}
 
-	if (wait_bytes > ring->space) {
-		int ret = wait_for_space(req, wait_bytes);
+	if (unlikely(total_bytes > ring->space)) {
+		int ret = wait_for_space(req, total_bytes);
 		if (unlikely(ret))
 			return ERR_PTR(ret);
 	}
 
 	if (unlikely(need_wrap)) {
-		GEM_BUG_ON(remain_actual > ring->space);
-		GEM_BUG_ON(ring->emit + remain_actual > ring->size);
+		need_wrap &= ~1;
+		GEM_BUG_ON(need_wrap > ring->space);
+		GEM_BUG_ON(ring->emit + need_wrap > ring->size);
 
 		/* Fill the tail with MI_NOOP */
-		memset(ring->vaddr + ring->emit, 0, remain_actual);
+		memset(ring->vaddr + ring->emit, 0, need_wrap);
 		ring->emit = 0;
-		ring->space -= remain_actual;
+		ring->space -= need_wrap;
 	}
 
 	GEM_BUG_ON(ring->emit > ring->size - bytes);
-- 
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] 9+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() (rev2)
  2017-04-26  8:37 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
                   ` (2 preceding siblings ...)
  2017-04-26  8:59 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() Patchwork
@ 2017-04-26  9:27 ` Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-04-26  9:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() (rev2)
URL   : https://patchwork.freedesktop.org/series/23555/
State : success

== Summary ==

Series 23555v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/23555/revisions/2/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:418s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:428s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:524s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:476s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:555s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:440s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:550s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:445s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:475s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:403s
fi-skl-gvtdvm failed to connect after reboot

7ffb3045557cbc7b49695b20416351e4e812179c drm-tip: 2017y-04m-25d-14h-42m-59s UTC integration manifest
dac888b drm/i915: Micro-optimise hotpath through intel_ring_begin()
f29e6ba drm/i915: Report the ring->space from intel_ring_update_space()
7145680 drm/i915: Avoid the branch in computing intel_ring_space()

== Logs ==

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

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

* Re: [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space()
  2017-04-26  8:37 ` [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space() Chris Wilson
@ 2017-04-28  8:40   ` Joonas Lahtinen
  0 siblings, 0 replies; 9+ messages in thread
From: Joonas Lahtinen @ 2017-04-28  8:40 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On ke, 2017-04-26 at 09:37 +0100, Chris Wilson wrote:
> Some callers immediately want to know the current ring->space after
> calling intel_ring_update_space(), which we can freely provide via the
> return parameter.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space()
  2017-05-03 10:06 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
@ 2017-05-03 10:06 ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-05-03 10:06 UTC (permalink / raw)
  To: intel-gfx

Some callers immediately want to know the current ring->space after
calling intel_ring_update_space(), which we can freely provide via the
return parameter.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++++----
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5947bba20c4a..c46e5439d379 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -51,9 +51,14 @@ static unsigned int __intel_ring_space(unsigned int head,
 	return (head - tail - CACHELINE_BYTES) & (size - 1);
 }
 
-void intel_ring_update_space(struct intel_ring *ring)
+unsigned int intel_ring_update_space(struct intel_ring *ring)
 {
-	ring->space = __intel_ring_space(ring->head, ring->emit, ring->size);
+	unsigned int space;
+
+	space = __intel_ring_space(ring->head, ring->emit, ring->size);
+
+	ring->space = space;
+	return space;
 }
 
 static int
@@ -1657,8 +1662,7 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 
 	lockdep_assert_held(&req->i915->drm.struct_mutex);
 
-	intel_ring_update_space(ring);
-	if (ring->space >= bytes)
+	if (intel_ring_update_space(ring) >= bytes)
 		return 0;
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 64584d134681..4a599e3480a9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -486,7 +486,7 @@ int intel_ring_pin(struct intel_ring *ring,
 		   struct drm_i915_private *i915,
 		   unsigned int offset_bias);
 void intel_ring_reset(struct intel_ring *ring, u32 tail);
-void intel_ring_update_space(struct intel_ring *ring);
+unsigned int intel_ring_update_space(struct intel_ring *ring);
 void intel_ring_unpin(struct intel_ring *ring);
 void intel_ring_free(struct intel_ring *ring);
 
-- 
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] 9+ messages in thread

end of thread, other threads:[~2017-05-03 10:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-26  8:37 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
2017-04-26  8:37 ` [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space() Chris Wilson
2017-04-28  8:40   ` Joonas Lahtinen
2017-04-26  8:37 ` [PATCH 3/3] drm/i915: Micro-optimise hotpath through intel_ring_begin() Chris Wilson
2017-04-26  9:07   ` [PATCH v2] " Chris Wilson
2017-04-26  8:59 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() Patchwork
2017-04-26  9:03   ` Chris Wilson
2017-04-26  9:27 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Avoid the branch in computing intel_ring_space() (rev2) Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-05-03 10:06 [PATCH 1/3] drm/i915: Avoid the branch in computing intel_ring_space() Chris Wilson
2017-05-03 10:06 ` [PATCH 2/3] drm/i915: Report the ring->space from intel_ring_update_space() Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox