Re: [PATCH v2] drm/i915/selftests: Wait for the dma-fence timeout

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 17/01/2018 13:11, Chris Wilson wrote:
> When testing that the timeout fired, we need to be sure we have waited
> just long enough for the timeout to have occurred and for the softirq
> (on another cpu) to have completed. Sleeping for an arbitrary amount is
> prone to error, so wait for the timeout instead and complain if it was
> too late.
> 
> v2: Use wait_event_timeout to provide an upper bound
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104670
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 27 +++++---------------------
>   1 file changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> index 4fb51deb81a1..301f0ce4cd3c 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> @@ -661,7 +661,6 @@ static int test_dma_fence(void *arg)
>   {
>   	struct i915_sw_fence *timeout = NULL, *not = NULL;
>   	unsigned long delay = i915_selftest.timeout_jiffies;
> -	unsigned long end, sleep;
>   	struct dma_fence *dma;
>   	int err;
>   
> @@ -688,36 +687,20 @@ static int test_dma_fence(void *arg)
>   	}
>   
>   	/* We round the timeout for the fence up to the next second */
> -	end = round_jiffies_up(jiffies + delay);
> +	delay = round_jiffies_up_relative(delay);
>   
> -	sleep = jiffies_to_usecs(delay) / 3;
> -	usleep_range(sleep, 2 * sleep);
> -	if (time_after(jiffies, end)) {
> -		pr_debug("Slept too long, delay=%lu, skipping!\n", delay);
> -		goto skip;
> -	}
> -
> -	if (i915_sw_fence_done(timeout) || i915_sw_fence_done(not)) {
> -		pr_err("Fences signaled too early\n");

This check still holds, no? Or there is another test looking at that?

> +	if (wait_event_timeout(timeout->wait,
> +			       i915_sw_fence_done(timeout),
> +			       2 * delay)) {
> +		pr_err("Timeout fence unsignaled!\n");
>   		goto err;

Doh!

Regards,

Tvrtko

>   	}
>   
> -	do {
> -		sleep = jiffies_to_usecs(end - jiffies + 1);
> -		usleep_range(sleep, 2 * sleep);
> -	} while (!time_after(jiffies, end));
> -
>   	if (i915_sw_fence_done(not)) {
>   		pr_err("No timeout fence signaled!\n");
>   		goto err;
>   	}
>   
> -	if (!i915_sw_fence_done(timeout)) {
> -		pr_err("Timeout fence unsignaled!\n");
> -		goto err;
> -	}
> -
> -skip:
>   	dma_fence_signal(dma);
>   
>   	if (!i915_sw_fence_done(timeout) || !i915_sw_fence_done(not)) {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx