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@xxxxxxxxxxxxxxx wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxxx>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
