On Fri, Dec 20, 2013 at 11:30:15AM +0200, Jani Nikula wrote:
> On Thu, 19 Dec 2013, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> >
> > The eDP spec defines some points where after you do action A, you have
> > to wait some time before action B. The thing is that in our driver
> > action B does not happen exactly after action A, but we still use
> > msleep() calls directly. What this patch does is that we record the
> > timestamp of when action A happened, then, just before action B, we
> > look at how much time has passed and only sleep the remaining amount
> > needed.
> >
> > With this change, I am able to save about 5-20ms (out of the total
> > 200ms) of the backlight_off delay and completely skip the 1ms
> > backlight_on delay. The 600ms vdd_off delay doesn't happen during
> > normal usage anymore due to a previous patch.
> >
> > v2: - Rename ironlake_wait_jiffies_delay to intel_wait_until_after and
> > move it to intel_display.c
> > - Fix the msleep call: diff is in jiffies
> > v3: - Use "tmp_jiffies" so we don't need to worry about the value of
> > "jiffies" advancing while we're doing the math.
> > v4: - Rename function again.
> > - Move function to i915_drv.h.
> > - Store last_power_cycle at edp_panel_off too.
> > - Use msecs_to_jiffies_timeout, then replace the msleep with an
> > open-coded version that avoids the extra +1 jiffy.
> > - Try to add units to every variable name so we don't confuse
> > jiffies with milliseconds.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 29 +++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > 3 files changed, 56 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index cc8afff..7e9b436 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2636,4 +2636,33 @@ timespec_to_jiffies_timeout(const struct timespec *value)
> > return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> > }
> >
> > +/*
> > + * If you need to wait X milliseconds between events A and B, but event B
> > + * doesn't happen exactly after event A, you record the timestamp (jiffies) of
> > + * when event A happened, then just before event B you call this function and
> > + * pass the timestamp as the first argument, and X as the second argument.
> > + */
> > +static inline void
> > +wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
> > +{
> > + unsigned long target_jiffies, tmp_jiffies;
> > + unsigned int remaining_ms;
> > +
> > + /*
> > + * Don't re-read the value of "jiffies" every time since it may change
> > + * behind our back and break the math.
> > + */
> > + tmp_jiffies = jiffies;
> > + target_jiffies = timestamp_jiffies +
> > + msecs_to_jiffies_timeout(to_wait_ms);
>
> Is it possible this gets called before the last_* timestamp fields are
> initialized, resulting to timestamp_jiffies being 0? Do we need to
> special case that? If not, perhaps add a WARN_ON(timestamp_jiffies == 0)
> so we maintain this precondition.
afaik jiffies are allowed to wrap around to 0 ...
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
