[Intel-gfx] [PATCH v3 2/3] drm/i915/selftests: Wrap a timer into a i915_sw_fence

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Thu Oct 12 15:06:36 UTC 2017


On 12/10/2017 13:57, Chris Wilson wrote:
> For some selftests, we want to issue requests but delay them going to
> hardware. Furthermore, we don't want those requests to block
> indefinitely (or else we may hang the driver and block testing) so we
> want to employ a timeout. So naturally we want a fence that is
> automatically signaled by a timer.
> 
> v2: Add kselftests.
> v3: Limit the API available to selftests; there isn't an overwhelming
> reason to export it universally.

v4: Added support for debug objects and rewrote everything? :) More on 
stack business, when a debug feature becomes to intrusive for normal use..

> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_sw_fence.c           | 10 ++++
>   drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 42 ++++++++++++++
>   drivers/gpu/drm/i915/selftests/lib_sw_fence.c  | 78 ++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/selftests/lib_sw_fence.h  | 42 ++++++++++++++
>   4 files changed, 172 insertions(+)
>   create mode 100644 drivers/gpu/drm/i915/selftests/lib_sw_fence.c
>   create mode 100644 drivers/gpu/drm/i915/selftests/lib_sw_fence.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index 808ea4d5b962..ca33cc08cb07 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -41,6 +41,11 @@ static inline void debug_fence_init(struct i915_sw_fence *fence)
>   	debug_object_init(fence, &i915_sw_fence_debug_descr);
>   }
>   
> +static inline void debug_fence_init_onstack(struct i915_sw_fence *fence)
> +{
> +	debug_object_init_on_stack(fence, &i915_sw_fence_debug_descr);
> +}
> +
>   static inline void debug_fence_activate(struct i915_sw_fence *fence)
>   {
>   	debug_object_activate(fence, &i915_sw_fence_debug_descr);
> @@ -79,6 +84,10 @@ static inline void debug_fence_init(struct i915_sw_fence *fence)
>   {
>   }
>   
> +static inline void debug_fence_init_onstack(struct i915_sw_fence *fence)
> +{
> +}
> +
>   static inline void debug_fence_activate(struct i915_sw_fence *fence)
>   {
>   }
> @@ -507,5 +516,6 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence,
>   }
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftests/lib_sw_fence.c"
>   #include "selftests/i915_sw_fence.c"
>   #endif
> diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> index 19d145d6bf52..ea01d0fe3ace 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> @@ -24,6 +24,7 @@
>   
>   #include <linux/completion.h>
>   #include <linux/delay.h>
> +#include <linux/prime_numbers.h>
>   
>   #include "../i915_selftest.h"
>   
> @@ -565,6 +566,46 @@ static int test_ipc(void *arg)
>   	return ret;
>   }
>   
> +static int test_timer(void *arg)
> +{
> +	unsigned long target, delay;
> +	struct timed_fence tf;
> +
> +	timed_fence_init(&tf, target = jiffies);
> +	if (!i915_sw_fence_done(&tf.fence)) {
> +		pr_err("Fence with immediate expiration not signaled\n");
> +		goto err;
> +	}
> +	timed_fence_fini(&tf);
> +
> +	for_each_prime_number(delay, i915_selftest.timeout_jiffies/2) {
> +		timed_fence_init(&tf, target = jiffies + delay);
> +		if (i915_sw_fence_done(&tf.fence)) {
> +			pr_err("Fence with future expiration (%lu jiffies) already signaled\n", delay);
> +			goto err;
> +		}
> +
> +		i915_sw_fence_wait(&tf.fence);
> +		if (!i915_sw_fence_done(&tf.fence)) {
> +			pr_err("Fence not signaled after wait\n");
> +			goto err;
> +		}
> +		if (time_before(jiffies, target)) {
> +			pr_err("Fence signaled too early, target=%lu, now=%lu\n",
> +			       target, jiffies);
> +			goto err;
> +		}
> +
> +		timed_fence_fini(&tf);
> +	}
> +
> +	return 0;
> +
> +err:
> +	timed_fence_fini(&tf);
> +	return -EINVAL;
> +}
> +
>   int i915_sw_fence_mock_selftests(void)
>   {
>   	static const struct i915_subtest tests[] = {
> @@ -576,6 +617,7 @@ int i915_sw_fence_mock_selftests(void)
>   		SUBTEST(test_C_AB),
>   		SUBTEST(test_chain),
>   		SUBTEST(test_ipc),
> +		SUBTEST(test_timer),
>   	};
>   
>   	return i915_subtests(tests, NULL);
> diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> new file mode 100644
> index 000000000000..3790fdf44a1a
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> @@ -0,0 +1,78 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "lib_sw_fence.h"
> +
> +/* Small library of different fence types useful for writing tests */
> +
> +static int __i915_sw_fence_call
> +nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
> +{
> +	return NOTIFY_DONE;
> +}
> +
> +void __onstack_fence_init(struct i915_sw_fence *fence,
> +			  const char *name,
> +			  struct lock_class_key *key)
> +{
> +	debug_fence_init_onstack(fence);
> +
> +	__init_waitqueue_head(&fence->wait, name, key);
> +	atomic_set(&fence->pending, 1);
> +	fence->flags = (unsigned long)nop_fence_notify;
> +}
> +
> +void onstack_fence_fini(struct i915_sw_fence *fence)
> +{
> +	i915_sw_fence_commit(fence);
> +	i915_sw_fence_fini(fence);
> +}
> +
> +static void timed_fence_wake(unsigned long data)
> +{
> +	struct timed_fence *tf = (struct timed_fence *)data;
> +
> +	i915_sw_fence_commit(&tf->fence);
> +}
> +
> +void timed_fence_init(struct timed_fence *tf, unsigned long expires)
> +{
> +	onstack_fence_init(&tf->fence);
> +
> +	setup_timer_on_stack(&tf->timer, timed_fence_wake, (unsigned long)tf);
> +
> +	if (time_after(expires, jiffies))
> +		mod_timer(&tf->timer, expires);
> +	else
> +		i915_sw_fence_commit(&tf->fence);
> +}
> +
> +void timed_fence_fini(struct timed_fence *tf)
> +{
> +	if (del_timer_sync(&tf->timer))
> +		i915_sw_fence_commit(&tf->fence);
> +
> +	destroy_timer_on_stack(&tf->timer);
> +	i915_sw_fence_fini(&tf->fence);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.h b/drivers/gpu/drm/i915/selftests/lib_sw_fence.h
> new file mode 100644
> index 000000000000..474aafb92ae1
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.h
> @@ -0,0 +1,42 @@
> +/*
> + * lib_sw_fence.h - library routines for testing N:M synchronisation points
> + *
> + * Copyright (C) 2017 Intel Corporation
> + *
> + * This file is released under the GPLv2.
> + *
> + */
> +
> +#ifndef _LIB_SW_FENCE_H_
> +#define _LIB_SW_FENCE_H_
> +
> +#include <linux/timer.h>
> +
> +#include "../i915_sw_fence.h"
> +
> +#ifdef CONFIG_LOCKDEP
> +#define onstack_fence_init(fence)				\
> +do {								\
> +	static struct lock_class_key __key;			\
> +								\
> +	__onstack_fence_init((fence), #fence, &__key);	\
> +} while (0)
> +#else
> +#define onstack_fence_init(fence)				\
> +	__onstack_fence_init((fence), NULL, NULL)
> +#endif
> +
> +void __onstack_fence_init(struct i915_sw_fence *fence,
> +			  const char *name,
> +			  struct lock_class_key *key);
> +void onstack_fence_fini(struct i915_sw_fence *fence);
> +
> +struct timed_fence {
> +	struct i915_sw_fence fence;
> +	struct timer_list timer;
> +};
> +
> +void timed_fence_init(struct timed_fence *tf, unsigned long expires);
> +void timed_fence_fini(struct timed_fence *tf);
> +
> +#endif /* _LIB_SW_FENCE_H_ */
> 

Trusting that the lockdep onstack business builds in both cases. The 
rest looks ok to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko


More information about the Intel-gfx mailing list