[Intel-gfx] [PATCH 2/5] drm/i915: Expose idle delays to Kconfig

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-08-02 15:33:33)
> 
> On 28/07/2018 17:46, Chris Wilson wrote:
> > We want to expose the parameters for controlling how long it takes for
> > us to notice and park the GPU after a stream of requests in order to try
> > and tune the optimal power-efficiency vs latency of a mostly idle system.
> 
> Who do we expect to tweak these and what kind of gains have they reported?

These are more targeted at S0ix. Which is a can of worms that I am
surprised hasn't been opened yet. Could it be that it all works better
than expected? Or more likely they just have been talking to us.

So these are definitely more optimistically opportunistic than
practical.

> > v2: retire_work has two scheduling points, update them both
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > ---
> >   drivers/gpu/drm/i915/Kconfig.profile | 23 +++++++++++++++++++++++
> >   drivers/gpu/drm/i915/i915_gem.c      |  8 +++++---
> >   drivers/gpu/drm/i915/i915_gem.h      | 13 +++++++++++++
> >   3 files changed, 41 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
> > index 8a230eeb98df..63cb744d920d 100644
> > --- a/drivers/gpu/drm/i915/Kconfig.profile
> > +++ b/drivers/gpu/drm/i915/Kconfig.profile
> > @@ -24,3 +24,26 @@ config DRM_I915_SPIN_REQUEST_CS
> >         spin prior to sleeping again.
> >   
> >         May be 0 to disable spinning after being woken.
> > +
> > +config DRM_I915_GEM_RETIRE_DELAY
> > +     int
> > +     default 1000 # milliseconds
> > +     help
> > +       We maintain a background job whose purpose is to keep cleaning up
> > +       after userspace, and may be the first to spot an idle system. This
> 
> "user space" since this is help text?
> 
> > +       parameter determines the interval between execution of the worker.
> > +
> > +       To reduce the number of timer wakeups, large delays (of greater than
> > +       a second) are rounded to the next walltime second to allow coalescing
> 
> Wake ups, wall time or with dashes?
> 
> > +       of multiple system timers into a single wakeup.
> 
> Do you perhaps want to omit the bit about rounding to next wall time 
> second and just say it may not be exact, so to reserve some internal 
> freedom?

Sure, but it's help text, even less accurate than comments ;)
I think it's prudent to say that it is inexact.
> 
> > +
> > +config DRM_I915_GEM_PARK_DELAY
> > +     int
> > +     default 100 # milliseconds
> > +     help
> > +       Before parking the engines and the GPU after the final request is
> > +       retired, we may wait for a small delay to reduce the frequecy of
> 
> typo in frequency
> 
> > +       having to park/unpark and so the latency in executing a new request.
> > +
> > +       May be 0 to immediately start parking the engines after the last
> > +       request.
> 
> I'd add so it says "the last request is retired." so there is even more 
> hint that it is not after the request has actually completed but when we 
> went to retire it.

Ok.
-Chris