Re: [PATCH 18/19] drm/i915: save some time when waiting the eDP timings

Ben Widawsky
2013-11-25T22:18:00Z

On Thu, Nov 21, 2013 at 04:00:17PM +0000, Chris Wilson wrote:
> On Thu, Nov 21, 2013 at 01:47:32PM -0200, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > 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 happens 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.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > 2 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index b438e76..3a1ca80 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1051,12 +1051,41 @@ static void ironlake_wait_panel_off(struct intel_dp *intel_dp)
> > ironlake_wait_panel_status(intel_dp, IDLE_OFF_MASK, IDLE_OFF_VALUE);
> > }
> >
> > +static void ironlake_wait_jiffies_delay(unsigned long timestamp,
> > + int to_wait_ms)
> This is not hw specific, so just
> intel_wait_until_after(timestamp_jiffies, to_wait_ms)

Can't we do this with our existing wait_for, and get all the other junk
we've crammed in there?
wait_for(false, timestamp + to_wait_ms)

Or do I have this all wrong?

>
> > +{
> > + unsigned long target = timestamp + msecs_to_jiffies(to_wait_ms);
> > + unsigned long diff;
> > +
> > + if (time_after(target, jiffies)) {
> > + diff = (long)target - (long)jiffies;
> > + msleep(diff);
>
> msleep() expects a duration in ms, diff is in jiffies.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ben Widawsky, Intel Open Source Technology Center