[Intel-gfx] [PATCH v2 2/8] drm/i915: Add more control to wait_for routines

[Chris Wilson]

Quoting Sean Paul (2017-12-01 17:48:17)
> On Fri, Dec 1, 2017 at 12:44 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > Quoting Sean Paul (2017-12-01 17:20:24)
> >>  /**
> >> - * _wait_for - magic (register) wait macro
> >> + * __wait_for - magic wait macro
> >>   *
> >> - * Does the right thing for modeset paths when run under kdgb or similar atomic
> >> - * contexts. Note that it's important that we check the condition again after
> >> + * Macro to help avoid open coding check/wait/timeout patterns, will do the
> >> + * right think wrt to choosing msleep vs usleep_range based on how long the wait
> >> + * interval is. Note that it's important that we check the condition again after
> >>   * having timed out, since the timeout could be due to preemption or similar and
> >>   * we've never had a chance to check the condition before the timeout.
> >>   */
> >> -#define _wait_for(COND, US, W) ({ \
> >> +#define __wait_for(OP, COND, US, W) ({ \
> >>         unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;   \
> >>         int ret__;                                                      \
> >>         might_sleep();                                                  \
> >>         for (;;) {                                                      \
> >>                 bool expired__ = time_after(jiffies, timeout__);        \
> >> +               OP;                                                     \
> >>                 if (COND) {                                             \
> >>                         ret__ = 0;                                      \
> >>                         break;                                          \
> >> @@ -62,11 +64,16 @@
> >>                         ret__ = -ETIMEDOUT;                             \
> >>                         break;                                          \
> >>                 }                                                       \
> >> -               usleep_range((W), (W) * 2);                             \
> >> +               if (W > (20 * 1000))                                    \
> >> +                       msleep(W / 1000);                               \
> >> +               else                                                    \
> >> +                       usleep_range((W), (W) * 2);                     \
> >
> > The current wait_for() is a little more complicated nowadays (variable
> > W).
> >
> 
> Hmm, am I based off the wrong tree? I'm using drm-intel-next.

drm-intel-next is what was sent as a PR; drm-intel-next-queued is always
current. To be sure CI, doesn't complain about merge conflicts, base on
drm-tip.

> > Are ms intervals going to be that common? Using a state-machine springs
> > to mind, but you could argue that msleep() is just a yield. Using msleep
> > though is going to leave D processes visible and a bump in load :|
> >
> 
> Probably uncommon, but at the very least, I need one. I wouldn't feel
> comfortable handling such a large wait using usleep_range.

We can use the constants to remove the predicate for when it never will
be needed, so it's not an issue -- either wait will produce a long
uninterruptible sleep.  It's just if that we were to frequently hit that
long sleep, there would be some push back against using wait_for() on that
path as no one likes an "idle" system with load 1.
-Chris