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!
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
