Re: [PATCH] drm/i915: Delay the relase of the forcewake by a jiffie

Chris Wilson
2014-02-24T08:30:29Z

On Sun, Feb 23, 2014 at 12:12:25PM -0800, Ben Widawsky wrote:
> On Mon, Aug 26, 2013 at 12:06:43PM +0100, Chris Wilson wrote:
> > Obtaining the forcwake requires expensive and time consuming
> > serialisation. And we often try to obtain the forcewake multiple times
> > in very quick succession. We can reduce the overhead of these sequences
> > by delaying the forcewake release, and so not hammer the hw quite so
> > hard.
> >
> > I was hoping this would help with the spurious
> > [drm:__gen6_gt_force_wake_mt_get] *ERROR* Timed out waiting for forcewake old ack to clear.
> > found on Haswell. Alas not.
> >
> > v2: Fix teardown ordering - unmap the regs after turning off forcewake,
> > and make sure we do turn off forcewake - both found by Ville.
> >
> > Note: I have no claims for improved performance, stablity or power
> > comsumption for this patch. We should not be hitting the registers often
> > enough for this to improve benchmarks, but given the nature of our hw it
> > is likely to improve long term stability.
>
> I don't understand how or why but from casual powertop observation, this
> workqueue uses between 4x and 50x or the nearest other i915 workqueue
> (i915_gem_retire_work_handler). On my x240...

What does that mean? We expect this to be frequently hit since we use it
so often, but retire_work_handler is only run every couple of seconds to
trim our lists (in case of userspace failure).

The idea is to have the forcewake reset run during the next scheduler event,
so using a workqueue was probably the wrong choice and perhaps we should have
used a timer instead?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre