Re: [igt-dev] [PATCH i-g-t 1/3] tests/gem_ctx_shared: Adopt to use allocator

["Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>]

On Thu, Aug 19, 2021 at 10:37:00PM -0700, Dixit, Ashutosh wrote:
> On Thu, 19 Aug 2021 21:46:14 -0700, Zbigniew Kempczyński wrote:
> >
> > On Thu, Aug 19, 2021 at 02:16:13PM -0700, Dixit, Ashutosh wrote:
> > > On Wed, 18 Aug 2021 21:49:39 -0700, Zbigniew Kempczyński wrote:
> > > >
> > > > @@ -518,12 +533,24 @@ static void store_dword(int i915, const intel_ctx_t *ctx, unsigned ring,
> > > >
> > > >	memset(obj, 0, sizeof(obj));
> > > >	obj[0].handle = cork;
> > > > -	obj[0].offset = cork << 20;
> > > >	obj[1].handle = target;
> > > > -	obj[1].offset = target << 20;
> > > >	obj[2].handle = gem_create(i915, 4096);
> > > > -	obj[2].offset = 256 << 10;
> > > > -	obj[2].offset += (random() % 128) << 12;
> > > > +	if (ahnd) {
> > > > +		obj[0].offset = cork_offset;
> > > > +		obj[0].flags |= EXEC_OBJECT_PINNED;
> > > > +		obj[1].offset = target_offset;
> > > > +		obj[1].flags |= EXEC_OBJECT_PINNED;
> > > > +		if (write_domain)
> > > > +			obj[1].flags |= EXEC_OBJECT_WRITE;
> > > > +		obj[2].offset = get_offset(ahnd, obj[2].handle, 4096, 0x0);
> > > > +		obj[2].flags |= EXEC_OBJECT_PINNED;
> > > > +		execbuf.flags |= I915_EXEC_NO_RELOC;
> > >
> > > I915_EXEC_NO_RELOC needed (since I think we weren't doing this earlier)?
> >
> > That's minor optimization for softpinning. We got relocation_count == 0 for
> > each gem object, so eb->relocs is empty list. We also don't expect offset
> > change on execbuf return.
> 
> I thought I915_EXEC_NO_RELOC is only needed if we are using relocations, if
> we are softpinning it's not needed?
> 

For softpinning using that flag just ensures you will skip some code
which does nothing (but for example is iterating over buffer list).

Take a look to execbuffer beginning:

	if (DBG_FORCE_RELOC || !(args->flags & I915_EXEC_NO_RELOC))
		args->flags |= __EXEC_HAS_RELOC;

then see two contitionals:
	if (eb->args->flags & __EXEC_HAS_RELOC) {
		...
	}

That code is not executed when NO_RELOC is set giving some small
performance improvement (otherwise it is iterating over buffers to update
offsets if necessary).

--
Zbigniew