[igt-dev] [PATCH i-g-t 2/2] tests/api_intel_allocator: Add default-alignment test

[Zbigniew Kempczyński]

On Thu, Feb 24, 2022 at 12:16:38PM -0800, Dixit, Ashutosh wrote:
> On Wed, 23 Feb 2022 23:54:15 -0800, Zbigniew Kempczyński wrote:
> >
> > diff --git a/tests/i915/api_intel_allocator.c b/tests/i915/api_intel_allocator.c
> > index 7ae4a00b7..32d1aad70 100644
> > --- a/tests/i915/api_intel_allocator.c
> > +++ b/tests/i915/api_intel_allocator.c
> > @@ -120,6 +120,34 @@ static void reserve(int fd, uint8_t type)
> >	igt_assert_eq(intel_allocator_close(ahnd), true);
> >  }
> >
> > +static void default_alignment(int fd)
> > +{
> > +	struct test_obj obj[3];
> > +	uint64_t ahnd, default_alignment = 0x4000;
> > +
> > +	ahnd = intel_allocator_open_full(fd, 0, 0, 0, INTEL_ALLOCATOR_SIMPLE,
> > +					 ALLOC_STRATEGY_LOW_TO_HIGH,
> > +					 default_alignment);
> > +
> > +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
> > +		obj[i].handle = gem_handle_gen();
> > +		obj[i].offset = intel_allocator_alloc(ahnd, obj[i].handle, 4096,
> > +				i == 2 ? 4096 : 0);
> > +		igt_debug("obj[%d].offset: %llx, handle: %u\n", i,
> > +			 (long long) obj[i].offset, obj[i].handle);
> > +	}
> > +
> > +	igt_assert_eq(obj[1].offset - obj[0].offset, default_alignment);
> 
> What would happen here if we had not specified default_alignment (i.e. let
> the allocator choose safe_alignment silently)? Do we need a
> "get_alignment()" API? Or clients know that the default default_alignment
> is safe_alignment?
> 
> In any case, this patch is fine so:
> 
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>

I think caller shouldn't do any assumptions regarding default alignment 
setting if it passes 0. At the moment I haven't seen any reason to
export this value - this is simple change and it can be done if someone
will require it.

Test above exercises possibility to explicitly set alignment we want
and objects will be placed according to our alignment requirements
in alloc(). BTW take a look to api_intel_allocator.c:reuse(). We use 0x40 
as default alignment there.

Thanks for the review.
--
Zbigniew