Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-08 Thread Chris Wilson
On Tue, Jul 07, 2015 at 10:03:09PM -0700, Kenneth Graunke wrote: Hi Chris, I made a genuine effort to review this patch, hoping to better understand the various changes and what you were trying to accomplish. I spent many hours reading and trying to enumerate changes - or potential changes

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-08 Thread Chris Wilson
On Wed, Jul 08, 2015 at 09:51:07AM +0100, Chris Wilson wrote: On Tue, Jul 07, 2015 at 10:03:09PM -0700, Kenneth Graunke wrote: * Gen4-5 structure changes. Did you mean brw_structs.h? diff --git a/src/mesa/drivers/dri/i965/brw_structs.h b/src/mesa/drivers/dri/i965/brw_structs.h index

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-08 Thread Chris Wilson
On Wed, Jul 08, 2015 at 10:49:24AM -0700, Kenneth Graunke wrote: On Wednesday, July 08, 2015 03:17:35 PM Chris Wilson wrote: On Wed, Jul 08, 2015 at 09:51:07AM +0100, Chris Wilson wrote: On Tue, Jul 07, 2015 at 10:03:09PM -0700, Kenneth Graunke wrote: * Gen4-5 structure changes. Did

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-08 Thread Kenneth Graunke
On Wednesday, July 08, 2015 03:17:35 PM Chris Wilson wrote: On Wed, Jul 08, 2015 at 09:51:07AM +0100, Chris Wilson wrote: On Tue, Jul 07, 2015 at 10:03:09PM -0700, Kenneth Graunke wrote: * Gen4-5 structure changes. Did you mean brw_structs.h? diff --git

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-07 Thread Chris Wilson
On Tue, Jul 07, 2015 at 01:14:53PM +0300, Abdiel Janulgue wrote: On 07/06/2015 01:33 PM, Chris Wilson wrote: @@ -600,7 +593,10 @@ brw_emit_null_surface_state(struct brw_context *brw, 1 BRW_SURFACE_WRITEDISABLE_B_SHIFT | 1 BRW_SURFACE_WRITEDISABLE_A_SHIFT);

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-07 Thread Abdiel Janulgue
On 07/06/2015 01:33 PM, Chris Wilson wrote: +/* + * Add a relocation entry for the target buffer into the current batch. + * + * This is the heart of performing fast relocations, both here and in + * the corresponding kernel relocation routines. + * + * - Instead of passing in handles

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-07 Thread Abdiel Janulgue
On 07/07/2015 01:19 PM, Chris Wilson wrote: On Tue, Jul 07, 2015 at 01:14:53PM +0300, Abdiel Janulgue wrote: On 07/06/2015 01:33 PM, Chris Wilson wrote: @@ -600,7 +593,10 @@ brw_emit_null_surface_state(struct brw_context *brw, 1 BRW_SURFACE_WRITEDISABLE_B_SHIFT |

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-07 Thread Kenneth Graunke
Hi Chris, I made a genuine effort to review this patch, hoping to better understand the various changes and what you were trying to accomplish. I spent many hours reading and trying to enumerate changes - or potential changes I needed to look hard at to convince myself whether they were correct.

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-06 Thread Chris Wilson
Just skipping to interesting comments for the moment. On Mon, Jul 06, 2015 at 12:34:00PM -0700, Kenneth Graunke wrote: While I really like this idea in principle, the current patch is rather huge, making it difficult to review; bisectability would also suffer. Would it be possible to split it

Re: [Mesa-dev] [PATCH 04/18] i965: Introduce a context-local batch manager

2015-07-06 Thread Kenneth Graunke
On Monday, July 06, 2015 11:33:09 AM Chris Wilson wrote: When submitting commands to the GPU every cycle of latency counts; mutexes, spinlocks, even atomics quickly add to substantial overhead. This batch manager acts as thread-local shim over the buffer manager (drm_intel_bufmgr_gem). As we