[Mesa-dev] [PATCH 06/10] i965: Add virtual memory allocator infrastructure to brw_bufmgr.

[Kenneth Graunke]

On Friday, May 4, 2018 2:27:27 PM PDT Chris Wilson wrote:
> Quoting Kenneth Graunke (2018-05-04 02:12:36)
> > +   if (brw_using_softpin(bufmgr) && bo->gtt_offset == 0ull) {
> > +      bo->gtt_offset = vma_alloc(bufmgr, memzone, bo->size, 1);
> > +
> > +      if (bo->gtt_offset == 0ull)
> > +         goto err_free;
> > +   }
> > +
> >     bo->name = name;
> >     p_atomic_set(&bo->refcount, 1);
> >     bo->reusable = true;
> > @@ -545,6 +792,9 @@ brw_bo_gem_create_from_name(struct brw_bufmgr *bufmgr,
> >     bo->external = true;
> >     bo->kflags = bufmgr->initial_kflags;
> >  
> > +   if (brw_using_softpin(bufmgr))
> > +      bo->gtt_offset = vma_alloc(bufmgr, BRW_MEMZONE_OTHER, bo->size, 1);
> 
> At this point, I think you want bo_using_softpoin() and pull it from the
> kflags. Not any different today, but I think more defensive, especially
> on the free paths.

I can get behind that.  I've continued using brw_using_softpin(bufmgr)
for places that initialize arrays/heaps, but switched to bo->kflags &
EXEC_OBJECT_PINNED for the BO alloc/free paths.

> On exec I think you want something like
> 
> assert(!bo_using_softpin(bo) ||
>        bo->gtt_offset == execobj[bo->index].gtt_iffset);
> 
> to document and check that the kernel isn't moving the objects and
> breaking the vma manager.

Yeah, definitely.  I had originally thought to skip the loop over the
BOs entirely, but we already loop to set bo->idle = false...so I'd just
skipped updating them.  I changed patch 7 to simply assert(!(bo->kflags
& EXEC_OBJECT_PINNED)) if the GTT offsets don't match.

> Overall impression: very, very neat.
> -Chris

Thank you!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180507/87e4510d/attachment.sig>