Quoting Kenneth Graunke (2017-09-06 16:20:00)
> On Wednesday, September 6, 2017 3:08:44 AM PDT Chris Wilson wrote:
> > Quoting Kenneth Graunke (2017-09-06 01:09:47)
> > > Previously, we would just assert fail and die in this case. The only
> > > safeguard is the "estimated max prim size" checks when starting a draw
> > > (or compute dispatch or BLORP operation)...which are woefully broken.
> > >
> > > Growing is fairly straightforward:
> > >
> > > 1. Allocate a new larger BO.
> > > 2. memcpy the existing contents over to the new buffer
> > > 3. Set the new BO to the same GTT offset as the old BO. When emitting
> > > relocations, we write the presumed GTT offset of the target BO. If
> > > we changed it, we'd have to update all the existing values (by
> > > walking the relocation list and looking at offsets), which is more
> > > expensive. With the old BO freed, ideally the kernel could simply
> > > place the new BO at that offset anyway.
> > > 4. Update the validation list to contain the new BO.
> > > 5. Update the relocation list to have the GEM handle for the new BO
> > > (which we can skip if using I915_EXEC_HANDLE_LUT).
> > > ---
> > > src/mesa/drivers/dri/i965/intel_batchbuffer.c | 104
> > > ++++++++++++++++++++++++--
> > > 1 file changed, 99 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > > b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > > index 909f56f9792..118f75c4d71 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > > @@ -43,6 +43,9 @@
> > > #define BATCH_SZ (8192*sizeof(uint32_t))
> > > #define STATE_SZ (8192*sizeof(uint32_t))
> > >
> > > +/* Don't exceed this - batchbuffers need to fit in the ring! */
> >
> > I don't understand this comment. I probably just have the wrong pov, you
> > say ring and I then think of the legacy/lrc ringbuffer in the kernel.
>
> My understanding was that the legacy Gen4-7.5 ringbuffer mode allocated a
> ringbuffer that was...128kB large? So if you exceeded that size, the
> batch would not fit in the ring at all, and execbuf would fail.
We don't copy the batch into the ring, we just stick a
MI_BATCH_BUFFER_START in there (with a flush before/after and a
breadcrumb, along with any context switch, change of mm, etc).
64k is indeed a magic limit for the state batch though, but as there's
no limit on the batch buffer size (after converting it to a pure command
stream, just the prospect of a timeout). Well... There is an implicit
assumption that you don't exceed 256KiB for a batch (as a limit for a gen2
workaround and a different gen7 workaround).
Elsewhere, I have used 256KiB batches (and then shrunk to fit) simply
because of UINT16_MAX dwords. (Which is kind of why the kernel assumes
that the upper reasonable maximum size is 256KiB for its w/a.)
> > > + brw_bo_reference(new_bo);
> > > + brw_bo_unreference(old_bo);
> >
> > > +
> > > + if (!batch->use_batch_first) {
> > > + /* We're not using I915_EXEC_HANDLE_LUT, which means we need to go
> > > + * update the relocation list entries to point at the new BO as
> > > well.
> > > + * (With newer kernels, the "handle" is an offset into the
> > > validation
> > > + * list, which remains unchanged, so we can skip this.)
> > > + */
> > > + replace_bo_in_reloc_list(&batch->batch_relocs,
> > > + old_bo->gem_handle, new_bo->gem_handle);
> > > + replace_bo_in_reloc_list(&batch->state_relocs,
> > > + old_bo->gem_handle, new_bo->gem_handle);
> > > + }
> > > +
> > > + /* Drop the *bo_ptr reference. This should free the old BO. */
> > > + brw_bo_unreference(old_bo);
> >
> > Ok, just took a double take even with the comment in place.
> >
> > > + *bo_ptr = new_bo;
> > > + *map_ptr = new_map;
> > > +}
>
> Yeah...the double unreference is kind of spooky. The alternative is to
> make the batch and state buffers not referenced by the validation list,
> which does save a few atomics, but adds a != batch_bo && != state_bo
> check in the validation list loops...so...seemed better to just do this.
It's not terrible, it just looked fishy! But I didn't like any of my
suggestions for the comment either.
-Chris
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
