[Intel-gfx] [PATCH] drm/i915: Use usleep_range() in wait_for()

[Daniel Vetter]

On Mon, Mar 23, 2015 at 11:39:12AM +0200, Ville Syrjälä wrote:
> On Fri, Mar 20, 2015 at 09:09:51PM +0000, Chris Wilson wrote:
> > On Fri, Mar 20, 2015 at 09:28:08PM +0200, ville.syrjala at linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > msleep() can sleep for way too long, so switch wait_for() to use
> > > usleep_range() instead. Following a totally unscientific method
> > > I just picked the range as W-2W.
> > > 
> > > This cuts the i915 init time on my BSW to almost half:
> > > - initcall i915_init+0x0/0xa8 [i915] returned 0 after 419977 usecs
> > > + initcall i915_init+0x0/0xa8 [i915] returned 0 after 238419 usecs
> > > 
> > > Note that I didn't perform any other benchmarks on this so far.
> > > 
> > > Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> > 
> > Hmm, I think we can improve further with a more variable sleep. The
> > maximum we pass to wait_for() is usually plucked from somewhere in the
> > spec (or is just a safety factor). Either way, it is a good guide as to
> > how to actually sleep for - if say we try to only sample 1000 times up
> > to the maximum:
> > 
> > if (do_sleep && drm_can_sleep()) {
> > 	usleep_range((MS), 10*(MS));
> > }
> > 
> > So whilst you have a situation where we clearly sleep too long between
> > sampling (a register), it would be beneficial to start adding some debug
> > infrastructure. Or even better if usleep_range already does it for us...
> 
> Yeah, it would be nice to have some more information about how long we
> sleep typically in each case.
> 
> We could perhaps then even micro optimize the 'set knob -> poll for knob
> to become active' pattern by doing a preemptive sleep between the set
> and poll steps in the hopes that we don't have to check the condition
> more than once. Not sure that would be worth the effort though.

Something like the below might be worth a quick shot. Maybe with 3 instead
of 2, but then we need to make sure we don't drop down to 0 ever.
-Daniel

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d2a4de0e4f4a..b796e4c47da5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -58,6 +58,7 @@
 		}							\
 		if ((W) && drm_can_sleep()) {				\
 			usleep_range((W)*1000, (W)*2000);		\
+			W = ROUND_UP(W, 2);				\
 		} else {						\
 			cpu_relax();					\
 		}							\
@@ -65,7 +66,7 @@
 	ret__;								\
 })
 
-#define wait_for(COND, MS) _wait_for(COND, MS, 1)
+#define wait_for(COND, MS) _wait_for(COND, MS, ROUND_UP(COND, 2))
 #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)
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch