Re: [PATCH i-g-t v5 3/4] tests/gem_exec_reloc: Calculate offsets from minimum GTT alignment

From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
To: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Subject: Re: [PATCH i-g-t v5 3/4] tests/gem_exec_reloc: Calculate offsets from minimum GTT alignment
Date: Mon, 4 Nov 2019 12:48:48 -0800	[thread overview]
Message-ID: <20191104204848.qukbrs6gohryuqbo@vrkonda-desk.ra.intel.com> (raw)
In-Reply-To: <20191104171330.24821-4-janusz.krzysztofik@linux.intel.com>

On Mon, Nov 04, 2019 at 06:13:29PM +0100, Janusz Krzysztofik wrote:
>The basic-range subtest assumes 4kB alignment while calculating softpin
>offsets.  On future backends with possibly larger minimum page sizes
>the test will fail as a half of calculated offsets to be tested will be
>incorrectly aligned.
>
>Replace hardcoded constants corresponding to the assumed 4kB page size
>with variables initialized with actual minimum GTT alignment size and
>order.
>
>v2: Simplify the code by reversing the size->order conversion,
>  - drop irrelevant modifications of requested object sizes.
>v3: Reword commit message after removal of patch "Don't filter out
>    addresses on full PPGTT" from the series,
>  - initialize page size order with an actual minimum returned by a new
>    helper (inspired by Chris).
>v4: Update the helper name, use the term 'minimum GTT alignment' across
>    the change, adjust variable names,
>  - refresh the commit message on top of the reintroduced patch that
>    fixes invalid offsets incorrectly assumed as occupied.
>
>Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
>Cc: Katarzyna Dec <katarzyna.dec@intel.com>
>Cc: Stuart Summers <stuart.summers@intel.com>
>Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> tests/i915/gem_exec_reloc.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
>index e46a4df7..a2a04343 100644
>--- a/tests/i915/gem_exec_reloc.c
>+++ b/tests/i915/gem_exec_reloc.c
>@@ -521,14 +521,16 @@ static void basic_range(int fd, unsigned flags)
> 	uint64_t gtt_size = gem_aperture_size(fd);
> 	const uint32_t bbe = MI_BATCH_BUFFER_END;
> 	igt_spin_t *spin = NULL;
>+	int alignment_order = gem_gtt_min_alignment_order(fd);
>+	uint64_t alignment = 1ull << alignment_order;
> 	int count, n, err;
>
> 	igt_require(gem_has_softpin(fd));
>
>-	for (count = 12; gtt_size >> (count + 1); count++)
>+	for (count = alignment_order; gtt_size >> (count + 1); count++)
> 		;
>
>-	count -= 12;
>+	count -= alignment_order;
>
> 	memset(obj, 0, sizeof(obj));
> 	memset(reloc, 0, sizeof(reloc));
>@@ -537,7 +539,7 @@ static void basic_range(int fd, unsigned flags)
> 	n = 0;
> 	for (int i = 0; i <= count; i++) {
> 		obj[n].handle = gem_create(fd, 4096);
>-		obj[n].offset = (1ull << (i + 12)) - 4096;
>+		obj[n].offset = (1ull << (i + alignment_order)) - alignment;
> 		obj[n].offset = gen8_canonical_address(obj[n].offset);
> 		obj[n].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> 		err = gem_gtt_validate_object(fd, &obj[n]);
>@@ -558,7 +560,7 @@ static void basic_range(int fd, unsigned flags)
> 	}
> 	for (int i = 1; i < count; i++) {
> 		obj[n].handle = gem_create(fd, 4096);
>-		obj[n].offset = 1ull << (i + 12);
>+		obj[n].offset = 1ull << (i + alignment_order);
> 		obj[n].offset = gen8_canonical_address(obj[n].offset);
> 		obj[n].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> 		err = gem_gtt_validate_object(fd, &obj[n]);
>-- 
>2.21.0

Reviewed-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx