[Intel-gfx] [PATCH v2] drm/i915: Keep the forcewake timer alive for 1ms past the most recent use

[Chris Wilson]

On Mon, May 15, 2017 at 11:14:32AM +0100, Tvrtko Ursulin wrote:
> 
> On 12/05/2017 23:16, Chris Wilson wrote:
> >Currently the timer is armed for 1ms after the first use and is killed
> >immediately, dropping the forcewake as early as possible. However, for
> 
> Correct for implicit grabs, but for explicit it is 1-2ms after the last use.

>From the put of the first, we don't rearm the timer on later puts.
 
> >very frequent operations the forcewake dance has a large impact on
> >latency and keeping the timer alive until we are idle is preferred. To
> 
> What workloads see the difference and by how much?

The issue I have is that we can't submit nops fast enough using
lite-restore. A large part of that was from the rpm get/put in the
tasklet, but I suspect ultimately it is the extra mmio/lite-restore that
is slowing us down. Now, I'm happy that the lite-restore to keep port[1]
accessible for the next context is a benefit, so I'm looking at how we
can improve the continual resubmission.
 
> At the time I've fixed the auto-release to go from 0-1 jiffies to
> 1-2ms, we talked about this conundrum - whether to consider the
> first grab or last put for the timer. But we decided thorough
> testing is needed to see if this would make a difference and what
> power side effects it might have.

In the above scenario, it never goes off so we are the paying the worst
price of a useless dance. It's the periodic 1ms poll on an idle system
that will suffer most, but in pratice this will delay turning off by an
extra 1ms - and that may be the difference between 0.1% and 50% in power
consumption :|
 
> >achieve this, if we call intel_uncore_forcewake_get whilst the timer is
> >alive (repeated use), then set a flag to restart the timer on expiry
> >rather than drop the forcewake usage count. The timer is racy, the
> >consequence of the race is to expire the timer earlier than is now
> >desired but does not impact on correct behaviour. The offset the race
> >slightly, we set the active flag again on intel_uncore_forcewake_put.
> 
> Using the hrtimer API to modify the timer was too expensive?

In the past it has been unsuitable for frequent adjustment, and we may
be using I915_READ every few instructions.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre