Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-08-04 Thread Chris Wilson 
Quoting Muthukumar, Aravindan (2017-08-04 09:53:57)
> > The tip of https://cgit.freedesktop.org/~ickle/mesa/log/?h=qbo
> > i.e.
> > https://cgit.freedesktop.org/~ickle/mesa/commit/?h=qbo=b40fa6633bdac9
> > 4cef2fd5f56360dfdb5eeb3738
> 
> I tested the patch series with mesa demos which seems functionally fine. 
> Is there any specific functional/perf issue that needs to be looked at?

If you have a target workload, finding any potential regressions would
be very useful. Not that I expect there to be any, but scrutiny from an
unbiased perspective is vital.

Speaking of which, back to ezbench...
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-08-04 Thread Muthukumar, Aravindan 
Hi Chris,

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Tuesday, August 1, 2017 2:35 PM
> To: Marathe, Yogesh <yogesh.mara...@intel.com>; mesa-
> d...@lists.freedesktop.org
> Cc: Muthukumar, Aravindan <aravindan.muthuku...@intel.com>
> Subject: RE: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object 
> availability
> in add_exec_bo
> 
> Quoting Marathe, Yogesh (2017-08-01 09:21:51)
> > > > -Original Message-
> > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > > Sent: Friday, July 28, 2017 2:30 PM
> > > > To: Muthukumar, Aravindan <aravindan.muthuku...@intel.com>; mesa-
> > > > d...@lists.freedesktop.org
> > > > Cc: Marathe, Yogesh <yogesh.mara...@intel.com>; Muthukumar,
> > > > Aravindan <aravindan.muthuku...@intel.com>
> > > > Subject: Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer
> > > > object availability in add_exec_bo
> > > >
> > > > Quoting aravindan.muthuku...@intel.com (2017-07-28 09:37:01)
> > > > > From: Aravindan Muthukumar <aravindan.muthuku...@intel.com>
> > > > >
> > > > > Original logic loops over the list for every buffer object.
> > > > > Maintained a flag to identify whether bo is already there in list.
> > > >
> > > > No. brw_bo is shared between many contexts, and so you are marking
> > > > it as available in every one. May I suggest looking for issues in
> > > > https://patchwork.freedesktop.org/series/27719/ and help bring
> > > > that series to fruition.
> > >
> > > Thanks Chris, we'll get back.
> >
> > Agreed. The patch series you have pointed out covers it, patch can be
> dropped.
> > We tried applying series on master but it didn't apply cleanly, is
> > there a base commit/branch on which this was tested / expected to work?
> 
> The tip of https://cgit.freedesktop.org/~ickle/mesa/log/?h=qbo
> i.e.
> https://cgit.freedesktop.org/~ickle/mesa/commit/?h=qbo=b40fa6633bdac9
> 4cef2fd5f56360dfdb5eeb3738

I tested the patch series with mesa demos which seems functionally fine. 
Is there any specific functional/perf issue that needs to be looked at?

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

Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-08-01 Thread Chris Wilson 
Quoting Marathe, Yogesh (2017-08-01 09:21:51)
> > > -Original Message-
> > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > Sent: Friday, July 28, 2017 2:30 PM
> > > To: Muthukumar, Aravindan <aravindan.muthuku...@intel.com>; mesa-
> > > d...@lists.freedesktop.org
> > > Cc: Marathe, Yogesh <yogesh.mara...@intel.com>; Muthukumar, Aravindan
> > > <aravindan.muthuku...@intel.com>
> > > Subject: Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object
> > > availability in add_exec_bo
> > >
> > > Quoting aravindan.muthuku...@intel.com (2017-07-28 09:37:01)
> > > > From: Aravindan Muthukumar <aravindan.muthuku...@intel.com>
> > > >
> > > > Original logic loops over the list for every buffer object.
> > > > Maintained a flag to identify whether bo is already there in list.
> > >
> > > No. brw_bo is shared between many contexts, and so you are marking it
> > > as available in every one. May I suggest looking for issues in
> > > https://patchwork.freedesktop.org/series/27719/ and help bring that
> > > series to fruition.
> > 
> > Thanks Chris, we'll get back.
> 
> Agreed. The patch series you have pointed out covers it, patch can be dropped.
> We tried applying series on master but it didn't apply cleanly, is there a 
> base
> commit/branch on which this was tested / expected to work?

The tip of https://cgit.freedesktop.org/~ickle/mesa/log/?h=qbo
i.e.  
https://cgit.freedesktop.org/~ickle/mesa/commit/?h=qbo=b40fa6633bdac94cef2fd5f56360dfdb5eeb3738
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-08-01 Thread Marathe, Yogesh 
> > -Original Message-
> > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > Sent: Friday, July 28, 2017 2:30 PM
> > To: Muthukumar, Aravindan <aravindan.muthuku...@intel.com>; mesa-
> > d...@lists.freedesktop.org
> > Cc: Marathe, Yogesh <yogesh.mara...@intel.com>; Muthukumar, Aravindan
> > <aravindan.muthuku...@intel.com>
> > Subject: Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object
> > availability in add_exec_bo
> >
> > Quoting aravindan.muthuku...@intel.com (2017-07-28 09:37:01)
> > > From: Aravindan Muthukumar <aravindan.muthuku...@intel.com>
> > >
> > > Original logic loops over the list for every buffer object.
> > > Maintained a flag to identify whether bo is already there in list.
> >
> > No. brw_bo is shared between many contexts, and so you are marking it
> > as available in every one. May I suggest looking for issues in
> > https://patchwork.freedesktop.org/series/27719/ and help bring that
> > series to fruition.
> 
> Thanks Chris, we'll get back.

Agreed. The patch series you have pointed out covers it, patch can be dropped.
We tried applying series on master but it didn't apply cleanly, is there a base
commit/branch on which this was tested / expected to work?

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

Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-07-28 Thread Marathe, Yogesh 
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Friday, July 28, 2017 2:30 PM
> To: Muthukumar, Aravindan <aravindan.muthuku...@intel.com>; mesa-
> d...@lists.freedesktop.org
> Cc: Marathe, Yogesh <yogesh.mara...@intel.com>; Muthukumar, Aravindan
> <aravindan.muthuku...@intel.com>
> Subject: Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object 
> availability
> in add_exec_bo
> 
> Quoting aravindan.muthuku...@intel.com (2017-07-28 09:37:01)
> > From: Aravindan Muthukumar <aravindan.muthuku...@intel.com>
> >
> > Original logic loops over the list for every buffer object. Maintained
> > a flag to identify whether bo is already there in list.
> 
> No. brw_bo is shared between many contexts, and so you are marking it as
> available in every one. May I suggest looking for issues in
> https://patchwork.freedesktop.org/series/27719/ and help bring that series to
> fruition.

Thanks Chris, we'll get back.

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

Re: [Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-07-28 Thread Chris Wilson 
Quoting aravindan.muthuku...@intel.com (2017-07-28 09:37:01)
> From: Aravindan Muthukumar 
> 
> Original logic loops over the list for every buffer object. Maintained
> a flag to identify whether bo is already there in list.

No. brw_bo is shared between many contexts, and so you are marking it as
available in every one. May I suggest looking for issues in
https://patchwork.freedesktop.org/series/27719/ and help bring that
series to fruition.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965: Avoids loop for buffer object availability in add_exec_bo

2017-07-28 Thread aravindan . muthukumar 
From: Aravindan Muthukumar 

Original logic loops over the list for every buffer object. Maintained
a flag to identify whether bo is already there in list.

Improves performance - 3DMark by 2%
Tested with piglit

Signed-off-by: Aravindan Muthukumar 
Signed-off-by: Yogesh Marathe 
---
 src/mesa/drivers/dri/i965/brw_bufmgr.h|  5 +
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 24 +++-
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h 
b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index 6a6051b..912ffb0 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -132,6 +132,11 @@ struct brw_bo {
 * Boolean of whether this buffer is cache coherent
 */
bool cache_coherent;
+
+   /**
+ * Boolean to check whether bo is available in exec buffer objects list
+ */
+   bool bo_available;
 };
 
 #define BO_ALLOC_FOR_RENDER (1<<0)
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index e2f208a..0814e8b 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -513,10 +513,8 @@ 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)
-return;
-  }
+  if(brw_batch_references(batch,bo) == true)
+ return;
 
   brw_bo_reference(bo);
}
@@ -548,6 +546,12 @@ add_exec_bo(struct intel_batchbuffer *batch, struct brw_bo 
*bo)
validation_entry->rsvd2 = 0;
 
batch->exec_bos[batch->exec_count] = bo;
+
+   /* Marking the current bo as true since this
+* is added to the exec_bos list
+*/
+   bo->bo_available = true;
+
batch->exec_count++;
batch->aperture_space += bo->size;
 }
@@ -592,6 +596,12 @@ execbuffer(int fd,
 
   bo->idle = false;
 
+  /* Marking the flags as false for all the bo's
+   * in the list to ensure it is added in the next
+   * list of exec buffers
+   */
+  bo->bo_available = false;
+
   /* Update brw_bo::offset64 */
   if (batch->validation_list[i].offset != bo->offset64) {
  DBG("BO %d migrated: 0x%" PRIx64 " -> 0x%llx\n",
@@ -736,11 +746,7 @@ brw_batch_has_aperture_space(struct brw_context *brw, 
unsigned extra_space)
 bool
 brw_batch_references(struct intel_batchbuffer *batch, struct brw_bo *bo)
 {
-   for (int i = 0; i < batch->exec_count; i++) {
-  if (batch->exec_bos[i] == bo)
- return true;
-   }
-   return false;
+   return (bo->bo_available) ? true : false;
 }
 
 /*  This is the only way buffers get added to the validate list.
-- 
2.7.4

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