[06/12] drm/i915: Do not lie about atomic wait granularity
diff mbox

Message ID 1454411190-15721-7-git-send-email-tvrtko.ursulin@linux.intel.com
State New
Headers show

Commit Message

Tvrtko Ursulin Feb. 2, 2016, 11:06 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Currently the wait_for_atomic_us only allows for a millisecond
granularity which is not nice towards callers requesting small
micro-second waits.

Re-implement it so micro-second granularity is really supported
and not just in the name of the macro.

v2:
  * Warn when used from non-atomic context (if possible).
  * Warn on too long atomic waits.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_drv.h | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Comments

Chris Wilson Feb. 2, 2016, 12:29 p.m. UTC | #1
On Tue, Feb 02, 2016 at 11:06:24AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Currently the wait_for_atomic_us only allows for a millisecond
> granularity which is not nice towards callers requesting small
> micro-second waits.
> 
> Re-implement it so micro-second granularity is really supported
> and not just in the name of the macro.
> 
> v2:
>   * Warn when used from non-atomic context (if possible).
>   * Warn on too long atomic waits.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

This also warns if we attempt to use a variable timeout as well, which
is a bonus.

>  drivers/gpu/drm/i915/intel_drv.h | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 779d17a9fcce..af6c1539106f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -63,11 +63,32 @@
>  	ret__;								\
>  })
>  
> +#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)

Ok, that took a little bit of digging to verify.

/* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */

> +  #define _WAIT_FOR_ATOMIC_CHECK WARN_ON_ONCE(!in_atomic())
> +#else
> +  #define _WAIT_FOR_ATOMIC_CHECK do { } while(0)
> +#endif

Hmm, not often that we do indent inside #if, but when it is, it is
more typically '# define'

> +#define _wait_for_atomic(COND, US) ({ \
> +	unsigned long end__; \
> +	int ret__ = 0; \
> +	_WAIT_FOR_ATOMIC_CHECK; \
> +	BUILD_BUG_ON((US) > 50000); \
> +	end__ = (local_clock() >> 10) + (US) + 1; \
> +	while (!(COND)) { \
> +		if (time_after((unsigned long)(local_clock() >> 10), end__)) { \
> +			ret__ = -ETIMEDOUT; \
> +			break; \
> +		} \
> +		cpu_relax(); \
> +	} \
> +	ret__; \
> +})

Ok,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 779d17a9fcce..af6c1539106f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -63,11 +63,32 @@ 
 	ret__;								\
 })
 
+#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
+  #define _WAIT_FOR_ATOMIC_CHECK WARN_ON_ONCE(!in_atomic())
+#else
+  #define _WAIT_FOR_ATOMIC_CHECK do { } while(0)
+#endif
+
+#define _wait_for_atomic(COND, US) ({ \
+	unsigned long end__; \
+	int ret__ = 0; \
+	_WAIT_FOR_ATOMIC_CHECK; \
+	BUILD_BUG_ON((US) > 50000); \
+	end__ = (local_clock() >> 10) + (US) + 1; \
+	while (!(COND)) { \
+		if (time_after((unsigned long)(local_clock() >> 10), end__)) { \
+			ret__ = -ETIMEDOUT; \
+			break; \
+		} \
+		cpu_relax(); \
+	} \
+	ret__; \
+})
+
 #define wait_for(COND, MS) _wait_for(COND, (MS) * 1000, 1000)
 #define wait_for_us(COND, US) _wait_for(COND, US, 1)
-#define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
-#define wait_for_atomic_us(COND, US) _wait_for((COND), \
-					       DIV_ROUND_UP((US), 1000), 0)
+#define wait_for_atomic(COND, MS) _wait_for_atomic(COND, (MS) * 1000)
+#define wait_for_atomic_us(COND, US) _wait_for_atomic((COND), (US))
 
 #define KHz(x) (1000 * (x))
 #define MHz(x) KHz(1000 * (x))