Re: [PATCH 2/2] drm/i915: Use atomic waits for short non-atomic ones

Chris Wilson
2016-06-28T12:19:44Z

On Tue, Jun 28, 2016 at 12:51:50PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> 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 disable the !in_atomic warning for such uses
> and also disable preemption since the macro is written in a way
> to only be safe to be used in atomic context (local_clock() and
> no second COND check after the timeout).
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3156d8df7921..e21bf6e6f119 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -69,20 +69,21 @@
> })
>
> #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) ({ \
> +#define _wait_for_atomic(COND, US, ATOMIC) ({ \

__busywait_for
__busyspin_for

(Thinking __usleep_until and maybe __busyspin_until)

> unsigned long end__; \
> int ret__ = 0; \
> - _WAIT_FOR_ATOMIC_CHECK; \
> - BUILD_BUG_ON((US) > 50000); \
> + _WAIT_FOR_ATOMIC_CHECK(ATOMIC); \
> + BUILD_BUG_ON((ATOMIC) && (US) > 50000); \
> + if (!(ATOMIC)) \
> + preempt_disable(); \

> end__ = (local_clock() >> 10) + (US) + 1; \
> while (!(COND)) { \
> if (time_after((unsigned long)(local_clock() >> 10), end__)) { \
> @@ -97,11 +98,23 @@
> } \

if (!ATOMIC && need_resched())
break;

?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre