Re: [igt-dev] [PATCH i-g-t v2 2/4] lib/intel_batchbuffer: add bb reset

Chris Wilson
2020-06-05T14:18:03Z

Quoting Zbigniew Kempczyński (2020-06-05 06:40:50)
> On Thu, Jun 04, 2020 at 04:07:49PM +0100, Chris Wilson wrote:
> > Quoting Zbigniew Kempczyński (2020-06-04 11:03:51)
> > > +/**
> > > + * intel_bb_object_offset_to_buf:
> > > + * @ibb: pointer to intel_bb
> > > + * @buf: buffer we want to store last exec offset and context id
> > > + *
> > > + * Copy object offset used in the batch to intel_buf to allow caller prepare
> > > + * other batch likely without relocations.
> > > + */
> > > +bool intel_bb_object_offset_to_buf(struct intel_bb *ibb, struct intel_buf *buf)
> > > +{
> > > + struct drm_i915_gem_exec_object2 object = { .handle = buf->handle };
> > > + struct drm_i915_gem_exec_object2 **found;
> > > +
> > > + igt_assert(ibb);
> > > + igt_assert(buf);
> > > +
> > > + found = tfind((void *) &object, &ibb->root, __compare_objects);
> > > + if (!found) {
> > > + buf->addr.offset = 0;
> > > + buf->addr.ctx = 0;
> > > +
> > > + return false;
> > > + }
> > > +
> > > + buf->addr.offset = (*found)->offset;
> > > + buf->addr.ctx = ibb->ctx;
> >
> > Ok, so it's tracking ctx so that we can return the right offset when the
> > buffer is used by multiple contexts. But ctx is never compared in the
> > lookup?
>
> Do you think should I add such multictx tracking to the intel_bb? It will
> require additional collecting tree (current one is an index tree over
> objects allocated, so for caching ctx/address new one is necessary).
> I assumed intel_bb_objects_offset_to_buf() is enough to provide the user
> ability to collect the addresses on its own (if it really requires that).

Ok, so this is an export. I was thinking it was an internal lookup.

My main concern was that something was using the ctx, and that appears
to be satisfied. I may suggest that it is backwards and the caller
should be telling the batch which context it was constructed for, but
time will tell :)
-Chris