Quoting Daniel Vetter (2017-07-07 10:55:49)
> On Mon, Jun 19, 2017 at 11:06:47AM +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.
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Kenneth Graunke <kenn...@whitecape.org>
> > Cc: Matt Turner <matts...@gmail.com>
> > Cc: Jason Ekstrand <jason.ekstr...@intel.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_bufmgr.h | 5 +++++
> > src/mesa/drivers/dri/i965/intel_batchbuffer.c | 14 ++++++++++++--
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > index 48488bc33b..dd3a37040a 100644
> > --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > @@ -76,6 +76,11 @@ struct brw_bo {
> > uint64_t offset64;
> >
> > /**
> > + * Index of this buffer inside the batch, -1 when not in a batch.
> > + */
> > + 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..ca7d6b81b1 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;
>
> Since we check for matching backpointer I don't think we even need to
> clear this here, but it's cheap. For consistency I think we should also
> clear when allocating a bo:
At the time I was thinking about trying to make -1 mean something
useful, but was kept being stymied by the lack of per-context tracking.
The advantage is that for a single user, it speeds up the not in this
batch check. Multi-user batches has the disadvantage that the bo->index
will ping-pong and they have to keep rechecking (same as before). So I
think it does come out on top, and adding -1 to init should help even
more.
There was also another spot I noticed that used the same hunt for an
exec_bo, brw_batch_references(), that can use the same quick check.
-Chris
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
