Re: [igt-dev] [PATCH i-g-t 2/7] lib/intel_batchbuffer: Detect and use kernel alignment capability

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Thu, 14 Oct 2021 00:25:29 -0700, Zbigniew Kempczyński wrote:
>
> On Wed, Oct 13, 2021 at 07:37:22PM -0700, Dixit, Ashutosh wrote:
> > On Thu, 07 Oct 2021 23:54:27 -0700, Zbigniew Kempczyński wrote:
> > >
> > > diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> > > index 9c26fe207..ed7c25015 100644
> > > --- a/lib/intel_batchbuffer.c
> > > +++ b/lib/intel_batchbuffer.c
> > > @@ -1335,6 +1335,7 @@ __intel_bb_create(int i915, uint32_t ctx, uint32_t size, bool do_relocs,
> > >
> > >	igt_assert(ibb);
> > >
> > > +	ibb->allows_passing_alignment = gem_allows_passing_alignment(i915);
> > >	ibb->uses_full_ppgtt = gem_uses_full_ppgtt(i915);
> > >	ibb->devid = intel_get_drm_devid(i915);
> > >	ibb->gen = intel_gen(ibb->devid);
> > > @@ -1783,6 +1784,7 @@ __add_to_cache(struct intel_bb *ibb, uint32_t handle)
> > >	igt_assert(object);
> > >
> > >	object->handle = handle;
> > > +	object->alignment = 0;
> > >	found = tsearch((void *) object, &ibb->root, __compare_objects);
> > >
> > >	if (*found == object) {
> > > @@ -1905,7 +1907,7 @@ intel_bb_add_object(struct intel_bb *ibb, uint32_t handle, uint64_t size,
> > >		   || ALIGN(offset, alignment) == offset);
> > >
> > >	object = __add_to_cache(ibb, handle);
> > > -	object->alignment = alignment ?: 4096;
> > > +	alignment = alignment ?: 4096;
> >
> > Can't we do:
> >	if (ibb->allows_passing_alignment)
> >		object->alignment = alignment ?: 4096;
> > So we don't need further changes in this function? Or it breaks something?
>
> I think it will work at this place, object->alignment is touched twice:
> 1. first on __add_to_cache() and set to 0
> 2. second on finalizing object settings after acquiring offset
>
> I added this on the bottom where object->... are filling with established
> values. I just wanted to have all those settings in one place, not
> object->alignment on the top and all the rest on the bottom. But there's
> cosmetic and personal preference.

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>