On Tue, Jun 28, 2016 at 05:20:43PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> usleep_range is not recommended for waits shorten than 10us.
>
> Make the wait_for_us use the atomic variant for such waits.
>
> To do so we need to reimplement the _wait_for_atomic macro to
> be safe with regards to preemption and interrupts.
>
> v2: Reimplement _wait_for_atomic to be irq and preemption safe.
> (Chris Wilson and Imre Deak)
>
> v3: Fixed in_atomic check due rebase error.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 56 ++++++++++++++++++++++++++--------------
> 1 file changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3156d8df7921..265f76157876 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -69,39 +69,57 @@
> })
>
> #define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 1000)
> -#define wait_for_us(COND, US) _wait_for((COND), (US), 1)
>
> /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
> #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> -# define _WAIT_FOR_ATOMIC_CHECK WARN_ON_ONCE(!in_atomic())
> +# define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) WARN_ON_ONCE((ATOMIC) && !in_atomic())
> #else
> -# define _WAIT_FOR_ATOMIC_CHECK do { } while (0)
> +# define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) do { } while (0)
> #endif
>
> -#define _wait_for_atomic(COND, US) ({ \
> - unsigned long end__; \
> - int ret__ = 0; \
> - _WAIT_FOR_ATOMIC_CHECK; \
> +#define _wait_for_atomic(COND, US, ATOMIC) \
> +({ \
> + int cpu, ret, timeout = (US) * 1000; \
> + u64 base; \
> + _WAIT_FOR_ATOMIC_CHECK(ATOMIC); \
> BUILD_BUG_ON((US) > 50000); \
> - end__ = (local_clock() >> 10) + (US) + 1; \
> - while (!(COND)) { \
> - if (time_after((unsigned long)(local_clock() >> 10), end__)) { \
> - /* Unlike the regular wait_for(), this atomic variant \
> - * cannot be preempted (and we'll just ignore the issue\
> - * of irq interruptions) and so we know that no time \
> - * has passed since the last check of COND and can \
> - * immediately report the timeout. \
> - */ \
> - ret__ = -ETIMEDOUT; \
> + preempt_disable(); \
> + cpu = smp_processor_id(); \
> + base = local_clock(); \
> + for (;;) { \
> + u64 now = local_clock(); \
> + preempt_enable(); \
> + if (COND) { \
> + ret = 0; \
> + break; \
> + } \
> + if (now - base >= timeout) { \
> + ret = -ETIMEDOUT; \
> break; \
> } \
> cpu_relax(); \
> + preempt_disable(); \
> + if (unlikely(cpu != smp_processor_id())) { \
> + timeout -= now - base; \
> + cpu = smp_processor_id(); \
> + base = local_clock(); \
> + } \
> } \
> + ret; \
> +})
> +
> +#define wait_for_us(COND, US) \
> +({ \
> + int ret__; \
/* usleep_range() is not recommended for waits shorten than 10us. */ \
> + if ((US) > 10) \
Do we want __builtin_constant_p(US) to be sure this folds away?
Can you resend this as a new series, I'm 100% sure if CI picked it up?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
