Re: [Mesa-dev] [PATCH v3] i965: Track last location of bo used for the batch

2017-07-07 Thread Daniel Vetter
On Fri, Jul 07, 2017 at 11:23:36AM +0100, Chris Wilson wrote:
> Borrow a trick from anv, and use the last known index for the bo to skip
> a search of the batch->exec_bo when adding a new relocation. In defence
> against the bo being used in multiple batches simultaneously, we check
> that this slot exists and points back to us.
> 
> v2: Also update brw_batch_references()
> v3: Reset bo->index on creation (Daniel)
> 
> Signed-off-by: Chris Wilson 
> Cc: Kenneth Graunke 
> Cc: Matt Turner 
> Cc: Jason Ekstrand 
> Cc: Daniel Vetter 
> ---
>  src/mesa/drivers/dri/i965/brw_bufmgr.c|  1 +
>  src/mesa/drivers/dri/i965/brw_bufmgr.h|  7 +++
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 18 --
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index da12a13152..4e43a448ae 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -353,6 +353,7 @@ retry:
> p_atomic_set(&bo->refcount, 1);
> bo->reusable = true;
> bo->cache_coherent = bufmgr->has_llc;
> +   bo->index = -1;
>  
> pthread_mutex_unlock(&bufmgr->lock);
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h 
> b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> index 4d671b6aae..27a27ca244 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> @@ -76,6 +76,13 @@ struct brw_bo {
> uint64_t offset64;
>  
> /**
> +* Index of this buffer inside the batch, -1 when not in a batch. Note
> +* that a single buffer may be in multiple batches (contexts), the index
> +* only refers to its last use and should not be trusted!
> +*/
> +   unsigned int index;
> +
> +   /**
>  * Boolean of whether the GPU is definitely not accessing the buffer.
>  *
>  * This is only valid when reusable, since non-reusable
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 62d2fe8ef3..f76ece8d71 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -515,12 +515,20 @@ throttle(struct brw_context *brw)
> }
>  }
>  
> +#define READ_ONCE(x) (*(volatile __typeof__(x) *)&(x))
> +
>  static void
>  add_exec_bo(struct intel_batchbuffer *batch, struct brw_bo *bo)
>  {
> if (bo != batch->bo) {
> -  for (int i = 0; i < batch->exec_count; i++) {
> - if (batch->exec_bos[i] == bo)
> +  unsigned int index = READ_ONCE(bo->index);
> +
> +  if (index < batch->exec_count && batch->exec_bos[index] == bo)
> + return;
> +
> +  /* May have been shared between multiple active batches */
> +  for (index = 0; index < batch->exec_count; index++) {
> + if (batch->exec_bos[index] == bo)
>  return;
>}
>  
> @@ -553,6 +561,7 @@ add_exec_bo(struct intel_batchbuffer *batch, struct 
> brw_bo *bo)
> validation_entry->rsvd1 = 0;
> validation_entry->rsvd2 = 0;
>  
> +   bo->index = batch->exec_count;
> batch->exec_bos[batch->exec_count] = bo;
> batch->exec_count++;
> batch->aperture_space += bo->size;
> @@ -597,6 +606,7 @@ execbuffer(int fd,
>struct brw_bo *bo = batch->exec_bos[i];
>  
>bo->idle = false;
> +  bo->index = -1;
>  
>/* Update brw_bo::offset64 */
>if (batch->exec_objects[i].offset != bo->offset64) {
> @@ -742,6 +752,10 @@ brw_batch_has_aperture_space(struct brw_context *brw, 
> unsigned extra_space)
>  bool
>  brw_batch_references(struct intel_batchbuffer *batch, struct brw_bo *bo)
>  {
> +   unsigned int index = READ_ONCE(bo->index);
> +   if (index < batch->exec_count && batch->exec_bos[index] == bo)
> +  return true;

Yeah that's a neat addition.

Reviewed-by: Daniel Vetter 

> +
> for (int i = 0; i < batch->exec_count; i++) {
>if (batch->exec_bos[i] == bo)
>   return true;
> -- 
> 2.13.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v3] i965: Track last location of bo used for the batch

2017-07-07 Thread Chris Wilson
Borrow a trick from anv, and use the last known index for the bo to skip
a search of the batch->exec_bo when adding a new relocation. In defence
against the bo being used in multiple batches simultaneously, we check
that this slot exists and points back to us.

v2: Also update brw_batch_references()
v3: Reset bo->index on creation (Daniel)

Signed-off-by: Chris Wilson 
Cc: Kenneth Graunke 
Cc: Matt Turner 
Cc: Jason Ekstrand 
Cc: Daniel Vetter 
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c|  1 +
 src/mesa/drivers/dri/i965/brw_bufmgr.h|  7 +++
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 18 --
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index da12a13152..4e43a448ae 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -353,6 +353,7 @@ retry:
p_atomic_set(&bo->refcount, 1);
bo->reusable = true;
bo->cache_coherent = bufmgr->has_llc;
+   bo->index = -1;
 
pthread_mutex_unlock(&bufmgr->lock);
 
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h 
b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index 4d671b6aae..27a27ca244 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -76,6 +76,13 @@ struct brw_bo {
uint64_t offset64;
 
/**
+* Index of this buffer inside the batch, -1 when not in a batch. Note
+* that a single buffer may be in multiple batches (contexts), the index
+* only refers to its last use and should not be trusted!
+*/
+   unsigned int index;
+
+   /**
 * Boolean of whether the GPU is definitely not accessing the buffer.
 *
 * This is only valid when reusable, since non-reusable
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 62d2fe8ef3..f76ece8d71 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -515,12 +515,20 @@ throttle(struct brw_context *brw)
}
 }
 
+#define READ_ONCE(x) (*(volatile __typeof__(x) *)&(x))
+
 static void
 add_exec_bo(struct intel_batchbuffer *batch, struct brw_bo *bo)
 {
if (bo != batch->bo) {
-  for (int i = 0; i < batch->exec_count; i++) {
- if (batch->exec_bos[i] == bo)
+  unsigned int index = READ_ONCE(bo->index);
+
+  if (index < batch->exec_count && batch->exec_bos[index] == bo)
+ return;
+
+  /* May have been shared between multiple active batches */
+  for (index = 0; index < batch->exec_count; index++) {
+ if (batch->exec_bos[index] == bo)
 return;
   }
 
@@ -553,6 +561,7 @@ add_exec_bo(struct intel_batchbuffer *batch, struct brw_bo 
*bo)
validation_entry->rsvd1 = 0;
validation_entry->rsvd2 = 0;
 
+   bo->index = batch->exec_count;
batch->exec_bos[batch->exec_count] = bo;
batch->exec_count++;
batch->aperture_space += bo->size;
@@ -597,6 +606,7 @@ execbuffer(int fd,
   struct brw_bo *bo = batch->exec_bos[i];
 
   bo->idle = false;
+  bo->index = -1;
 
   /* Update brw_bo::offset64 */
   if (batch->exec_objects[i].offset != bo->offset64) {
@@ -742,6 +752,10 @@ brw_batch_has_aperture_space(struct brw_context *brw, 
unsigned extra_space)
 bool
 brw_batch_references(struct intel_batchbuffer *batch, struct brw_bo *bo)
 {
+   unsigned int index = READ_ONCE(bo->index);
+   if (index < batch->exec_count && batch->exec_bos[index] == bo)
+  return true;
+
for (int i = 0; i < batch->exec_count; i++) {
   if (batch->exec_bos[i] == bo)
  return true;
-- 
2.13.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev