[igt-dev] [PATCH i-g-t 2/3] tests/i915/gem_softpin: Add safe-start test

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Thu Jan 6 09:55:50 UTC 2022


On Wed, Jan 05, 2022 at 06:24:14PM -0800, Dixit, Ashutosh wrote:
> On Tue, 04 Jan 2022 22:49:44 -0800, Zbigniew Kempczyński wrote:
> >
> > +	offset1 = gem_detect_min_start_offset_for_region(i915, region1);
> > +	offset2 = gem_detect_min_start_offset_for_region(i915, region2);
> > +	alignment = gem_detect_safe_alignment(i915);
> > +	igt_debug("safe alignment: %llx\n", (long long) alignment);
> > +	igt_debug("safe start offset: %llx\n",
> > +		  (long long) gem_detect_safe_start_offset(i915));
> > +	igt_debug("minimum object1 start offset: %llx\n", (long long) offset1);
> > +	igt_debug("minimum object2 start offset: %llx\n", (long long) offset2);
> 
> Maybe also add print for the min_alignment value here?

You want to print minimum alignment for same region?

> 
> > +	igt_describe("Check start offset and alignment detection");
> > +	igt_subtest("safe-start")
> > +		safe_start(fd);
> 
> I think "safe-start" is a really ambiguous name so we should change the
> name to something like "safe-align" or 'safe-offset", no?

Already got that name to safe-alignment on first version of patch but
I got suggestion to change to safe-start. Anyway we exercise start offset
as well as alignment here.

But my preference is also safe-align so there 2:1 voice. 

> 
> Otherwise this is:
> 
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>

Thanks for the review, I'm going to wait for you comment regarding
min alignment. 

--
Zbigniew


More information about the igt-dev mailing list