Re: [Mesa-dev] [PATCH 1/2] i965: Move MI_BATCHBUFFER_END handling into brw_finish_batch().

2017-09-19 Thread Chris Wilson
Quoting Kenneth Graunke (2017-09-18 21:38:09)
> On Monday, September 18, 2017 11:03:37 AM PDT Chris Wilson wrote:
> > Tangential: Do we care about intel_upload_finish on batch flush any more
> > now that async access is universal?
> 
> I'm not sure.  We could certainly enable async maps for the upload BOs,
> and avoid flushing them when finishing the batch.  Then again, since the
> uploaded data is still related to the commands in the batch, it might
> make sense to "start fresh" with every batch.
> 
> I don't think it much matters, anymore.

I was hoping it would reduce one special case; the upload buffer becomes
just an unnamed BO and it would feed through the same range analysis as
named BO (eventually). Right now, just to remove one more line of magic.

The only caveat is if it is used for readback, and is then tied to later
batches before the readback is sync'ed. That seems unlikely, but I
haven't checked. On the write, tying the upload buffer to multiple
batches doesn't seem to present any problems (and offers a minor
reduction in mem/GTT pressure, although unrealistic to expect it be a
tipping point).
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965: Move MI_BATCHBUFFER_END handling into brw_finish_batch().

2017-09-18 Thread Kenneth Graunke
On Monday, September 18, 2017 11:03:37 AM PDT Chris Wilson wrote:
> Quoting Kenneth Graunke (2017-09-18 18:56:56)
> > This is, by definition, finishing the batch.
> > ---
> >  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 14 +++---
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
> > b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > index 7d5a8947e8f..dd584f533b9 100644
> > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > @@ -665,6 +665,13 @@ brw_finish_batch(struct brw_context *brw)
> >PIPE_CONTROL_CS_STALL);
> >}
> > }
> > +
> > +   /* Mark the end of the buffer. */
> > +   intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END);
> > +   if (USED_BATCH(brw->batch) & 1) {
> > +  /* Round batchbuffer usage to 2 DWORDs. */
> > +  intel_batchbuffer_emit_dword(>batch, MI_NOOP);
> > +   }
> >  }
> >  
> >  static void
> > @@ -899,13 +906,6 @@ _intel_batchbuffer_flush_fence(struct brw_context *brw,
> >  
> > brw_finish_batch(brw);
> >  
> > -   /* Mark the end of the buffer. */
> > -   intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END);
> > -   if (USED_BATCH(brw->batch) & 1) {
> > -  /* Round batchbuffer usage to 2 DWORDs. */
> > -  intel_batchbuffer_emit_dword(>batch, MI_NOOP);
> > -   }
> > -
> > intel_upload_finish(brw);
> 
> Tangential: Do we care about intel_upload_finish on batch flush any more
> now that async access is universal?
> -Chris

I'm not sure.  We could certainly enable async maps for the upload BOs,
and avoid flushing them when finishing the batch.  Then again, since the
uploaded data is still related to the commands in the batch, it might
make sense to "start fresh" with every batch.

I don't think it much matters, anymore.

--Ken

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965: Move MI_BATCHBUFFER_END handling into brw_finish_batch().

2017-09-18 Thread Chris Wilson
Quoting Kenneth Graunke (2017-09-18 18:56:56)
> This is, by definition, finishing the batch.
> ---
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 7d5a8947e8f..dd584f533b9 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -665,6 +665,13 @@ brw_finish_batch(struct brw_context *brw)
>PIPE_CONTROL_CS_STALL);
>}
> }
> +
> +   /* Mark the end of the buffer. */
> +   intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END);
> +   if (USED_BATCH(brw->batch) & 1) {
> +  /* Round batchbuffer usage to 2 DWORDs. */
> +  intel_batchbuffer_emit_dword(>batch, MI_NOOP);
> +   }
>  }
>  
>  static void
> @@ -899,13 +906,6 @@ _intel_batchbuffer_flush_fence(struct brw_context *brw,
>  
> brw_finish_batch(brw);
>  
> -   /* Mark the end of the buffer. */
> -   intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END);
> -   if (USED_BATCH(brw->batch) & 1) {
> -  /* Round batchbuffer usage to 2 DWORDs. */
> -  intel_batchbuffer_emit_dword(>batch, MI_NOOP);
> -   }
> -
> intel_upload_finish(brw);

Tangential: Do we care about intel_upload_finish on batch flush any more
now that async access is universal?
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] i965: Move MI_BATCHBUFFER_END handling into brw_finish_batch().

2017-09-18 Thread Kenneth Graunke
This is, by definition, finishing the batch.
---
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 7d5a8947e8f..dd584f533b9 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -665,6 +665,13 @@ brw_finish_batch(struct brw_context *brw)
   PIPE_CONTROL_CS_STALL);
   }
}
+
+   /* Mark the end of the buffer. */
+   intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END);
+   if (USED_BATCH(brw->batch) & 1) {
+  /* Round batchbuffer usage to 2 DWORDs. */
+  intel_batchbuffer_emit_dword(>batch, MI_NOOP);
+   }
 }
 
 static void
@@ -899,13 +906,6 @@ _intel_batchbuffer_flush_fence(struct brw_context *brw,
 
brw_finish_batch(brw);
 
-   /* Mark the end of the buffer. */
-   intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END);
-   if (USED_BATCH(brw->batch) & 1) {
-  /* Round batchbuffer usage to 2 DWORDs. */
-  intel_batchbuffer_emit_dword(>batch, MI_NOOP);
-   }
-
intel_upload_finish(brw);
 
/* Check that we didn't just wrap our batchbuffer at a bad time. */
-- 
2.14.1

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