[Intel-gfx] [PATCH 6/9] drm/i915/selftests: Force bonded submission to overlap

[Tvrtko Ursulin]

On 20/11/2019 09:32, Chris Wilson wrote:
> Bonded request submission is designed to allow requests to execute in
> parallel as laid out by the user. If the master request is already
> finished before its bonded pair is submitted, the pair were not destined
> to run in parallel and we lose the information about the master engine
> to dictate selection of the secondary. If the second request was
> required to be run on a particular engine in a virtual set, that should
> have been specified, rather than left to the whims of a random
> unconnected requests!
> 
> In the selftest, I made the mistake of not ensuring the master would
> overlap with its bonded pairs, meaning that it could indeed complete
> before we submitted the bonds. Those bonds were then free to select any
> available engine in their virtual set, and not the one expected by the
> test.

There is a submit await which ensures master is not runnable before 
bonded pairs are submitted. Why was that not enough? Are the sporadic 
test failures?

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_lrc.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 16ebe4d2308e..f3b0610d1f10 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -3036,15 +3036,21 @@ static int bond_virtual_engine(struct intel_gt *gt,
>   	struct i915_gem_context *ctx;
>   	struct i915_request *rq[16];
>   	enum intel_engine_id id;
> +	struct igt_spinner spin;
>   	unsigned long n;
>   	int err;
>   
>   	GEM_BUG_ON(nsibling >= ARRAY_SIZE(rq) - 1);
>   
> -	ctx = kernel_context(gt->i915);
> -	if (!ctx)
> +	if (igt_spinner_init(&spin, gt))
>   		return -ENOMEM;
>   
> +	ctx = kernel_context(gt->i915);
> +	if (!ctx) {
> +		err = -ENOMEM;
> +		goto err_spin;
> +	}
> +
>   	err = 0;
>   	rq[0] = ERR_PTR(-ENOMEM);
>   	for_each_engine(master, gt, id) {
> @@ -3055,7 +3061,7 @@ static int bond_virtual_engine(struct intel_gt *gt,
>   
>   		memset_p((void *)rq, ERR_PTR(-EINVAL), ARRAY_SIZE(rq));
>   
> -		rq[0] = igt_request_alloc(ctx, master);
> +		rq[0] = spinner_create_request(&spin, ctx, master, MI_NOOP);
>   		if (IS_ERR(rq[0])) {
>   			err = PTR_ERR(rq[0]);
>   			goto out;
> @@ -3068,10 +3074,17 @@ static int bond_virtual_engine(struct intel_gt *gt,
>   							       &fence,
>   							       GFP_KERNEL);
>   		}
> +
>   		i915_request_add(rq[0]);
>   		if (err < 0)
>   			goto out;
>   
> +		if (!(flags & BOND_SCHEDULE) &&
> +		    !igt_wait_for_spinner(&spin, rq[0])) {
> +			err = -EIO;
> +			goto out;
> +		}
> +
>   		for (n = 0; n < nsibling; n++) {
>   			struct intel_context *ve;
>   
> @@ -3119,6 +3132,8 @@ static int bond_virtual_engine(struct intel_gt *gt,
>   			}
>   		}
>   		onstack_fence_fini(&fence);
> +		intel_engine_flush_submission(master);
> +		igt_spinner_end(&spin);
>   
>   		if (i915_request_wait(rq[0], 0, HZ / 10) < 0) {
>   			pr_err("Master request did not execute (on %s)!\n",
> @@ -3156,6 +3171,8 @@ static int bond_virtual_engine(struct intel_gt *gt,
>   		err = -EIO;
>   
>   	kernel_context_close(ctx);
> +err_spin:
> +	igt_spinner_fini(&spin);
>   	return err;
>   }
>   
>