Re: [igt-dev] [PATCH i-g-t v3 18/52] tests/gem_exec_async: Adopt to use allocator

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

On Mon, 26 Jul 2021 12:59:52 -0700, Zbigniew Kempczyński wrote:
>
> For newer gens we're not able to rely on relocations. Adopt to use
> offsets acquired from the allocator.

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

But a couple of questions/comments below.

> +static void store_dword(int fd, int id, const intel_ctx_t *ctx,
> +			 unsigned ring, uint32_t target, uint64_t target_offset,
> +			 uint32_t offset, uint32_t value)
>  {
>	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>	struct drm_i915_gem_exec_object2 obj[2];
> @@ -50,6 +53,15 @@ static void store_dword(int fd, const intel_ctx_t *ctx, unsigned ring,
>	obj[0].flags = EXEC_OBJECT_ASYNC;
>	obj[1].handle = gem_create(fd, 4096);
>
> +	if (id) {
> +		obj[0].offset = target_offset;
> +		obj[0].flags |= EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
> +				EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +		obj[1].offset = (id + 1) * SZ_1M;

So this is where I think we are assigning new offsets to successive batches
to avoid stalls, correct? Though I don't know why we don't get these
offsets from the allocator (though I guess this will work since we have a
4K scratch buffer and whatever the spin needs)?

Maybe we can add a one line comment above, something like:

/* Assign new offsets to successive batches to prevent stalls */

> @@ -89,6 +101,8 @@ static void one(int fd, const intel_ctx_t *ctx,
>	uint32_t scratch = gem_create(fd, 4096);
>	igt_spin_t *spin;
>	uint32_t *result;
> +	uint64_t ahnd = get_simple_l2h_ahnd(fd, ctx->id);

Is there a particular reason for using simple rather than the reloc
allocator here?