Re: [PATCH] drm/i915: Convert the forcewake worker into a timer func

Chris Wilson
2014-03-03T07:05:21Z

On Sun, Mar 02, 2014 at 05:01:26PM -0800, Ben Widawsky wrote:
> On Fri, Feb 28, 2014 at 06:44:03PM +0000, Chris Wilson wrote:
> > We don't want to suffer scheduling delay when turning off the GPU after
> > waking it up to touch registers. Ideally, we only want to keep the GPU
> > awake for the register access sequence, with a single forcewake dance on
> > the first access and release immediately after the last. We set a timer
> > on the first access so that we only dance once and on the next scheduler
> > tick, we drop the forcewake again.
> >
> > This moves the cleanup routine from the common i915 workqueue to a timer
> > func so that we don't anger powertop, and drop the forcewake again
> > quicker.
> >
> > v2: Enable the deferred force_wake_put for regular register reads as
> > well.
>
> So I went to test this patch today. Oddly this isn't showing up in any
> of my profiles on the latest drm-intel-nightly. I just tested today, and
> Daniel made it a 3.14-rc4 based thing today. It was still present on
> 3.13.0. Actually, i915_gem_retire_work_handler is the only one even
> showing up in powertop, which is how it should be IMO.
>
> Therefore I'm not sure what we should do with this.

>From one perspective, that sounds promising - if it doesn't cause
powertop to start a panic. This patch is what I wanted for the
forcewake-off delay in the first place, so it is worth
investigating as it is more likely to have an effect. There are still
the regular dropped mmio writes being reported in downstream bugzillas.
:|
-Chris

--
Chris Wilson, Intel Open Source Technology Centre