On Wed, 2018-03-28 at 20:13 +0100, Chris Wilson wrote: > Quoting Pandiyan, Dhinakaran (2018-03-28 20:01:40) > > > > On Wed, 2018-03-28 at 18:53 +0100, Chris Wilson wrote: > > > As intel_wait_for_register_fw() may use, and if successful only use, a > > > busy-wait loop, the might_sleep() warning is a little over-zealous. > > > Restrict it to a might_sleep_if() a slow timeout is specified (and so > > > the caller authorises use of a usleep). > > > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/intel_uncore.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > > index f37ecfc69e49..44c4654443ba 100644 > > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > > @@ -1996,7 +1996,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv, > > > u32 reg_value; > > > int ret; > > > > > > - might_sleep(); > > > + might_sleep_if(slow_timeout_ms); > > > > __wait_for() already has a might_sleep(), why is this needed? > > To document that this routine is a sleeper, irrespective of the > implementation. Sometimes it is implicit on the implementation and so > should only be at the low level, sometimes we want to call out the > requirements explicitly and clearly. We have "wait" in the name so > shouting out that this may indeed sleep rather than busyspin seems to > be in order. > -Chris Fair enough. There seems to be a side effect from the second hunk that your commit message does not explicitly state. > - if (ret) > + if (ret && slow_timeout_ms) This results in a different return value if condition == true and slow_timeout_ms == 0 after fast_timeout_us was exceeded. Previously, __intel_wait_for_register would have passed along the 0 from __wait_for(), now it returns -ETIMEDOUT. Which means this change should have been a separate patch. As the patch itself is sensible, Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
