[Intel-gfx] [CI 08/14] drm/i915/selftests: Force a rewind if at first we don't succeed

[Tvrtko Ursulin]

On 02/02/2021 15:14, Chris Wilson wrote:
> live_timeslice_rewind assumes a particular traversal and reordering
> after the first timeslice yield. However, the outcome can be either
> (A1, A2, B1) or (A1, B2, A2) depending on the path taken through the
> dependency graph. So if we do not get the outcome we need at first, give
> it a priority kick to force a rewind.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/selftest_execlists.c | 21 +++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> index 951e2bf867e1..68e1398704a4 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> @@ -1107,6 +1107,7 @@ static int live_timeslice_rewind(void *arg)
>   		struct i915_request *rq[3] = {};
>   		struct intel_context *ce;
>   		unsigned long timeslice;
> +		unsigned long timeout;
>   		int i, err = 0;
>   		u32 *slot;
>   
> @@ -1173,11 +1174,29 @@ static int live_timeslice_rewind(void *arg)
>   
>   		/* ELSP[] = { { A:rq1, A:rq2 }, { B:rq1 } } */
>   		ENGINE_TRACE(engine, "forcing tasklet for rewind\n");
> -		while (i915_request_is_active(rq[A2])) { /* semaphore yield! */
> +		i = 0;
> +		timeout = jiffies + HZ;
> +		while (i915_request_is_active(rq[A2]) &&
> +		       time_before(jiffies, timeout)) { /* semaphore yield! */
>   			/* Wait for the timeslice to kick in */
>   			del_timer(&engine->execlists.timer);
>   			tasklet_hi_schedule(&engine->execlists.tasklet);
>   			intel_engine_flush_submission(engine);
> +
> +			/*
> +			 * Unfortunately this assumes that during the
> +			 * search of the wait tree it sees the requests
> +			 * in a particular order. That order is not
> +			 * strictly determined and it may pick either
> +			 * A2 or B1 to immediately follow A1.
> +			 *
> +			 * Break the tie with a set-priority. This defeats
> +			 * the goal of trying to cause a rewind with a
> +			 * timeslice, but alas, a rewind is better than
> +			 * none.
> +			 */
> +			if (i++)
> +				i915_request_set_priority(rq[B1], 1);
>   		}
>   		/* -> ELSP[] = { { A:rq1 }, { B:rq1 } } */
>   		GEM_BUG_ON(!i915_request_is_active(rq[A1]));
> 

Didn't fully get the intricacies of the test, but, how about not messing 
with priorities but just kicking it for longer until it eventually 
re-orders to the desired sequence? Surely if it keeps insisting of the 
same order which is making no progress there is a flaw in timeslicing 
anyway? Or if it fails skip the test.

Regards,

Tvrtko