Re: [PATCH] drm/i915: Flush the RPS bottom-half when the GPU idles

Chris Wilson
2015-12-09T20:52:56Z

On Wed, Dec 09, 2015 at 07:47:29PM +0200, Imre Deak wrote:
> >  void gen6_rps_idle(struct drm_i915_private *dev_priv)
> >  {
> > - struct drm_device *dev = dev_priv->dev;
> > + /* Flush our bottom-half so that it does not race with us
> > +  * setting the idle frequency and so that it is bounded by
> > +  * our rpm wakeref.
> > +  */
> > + flush_work(&dev_priv->rps.work);
>
> A (spurious) RPS interrupt could still reschedule the work, so could we
> also explicitly disable the interrupts? Meaning to use
> gen6_{disable,enable}_rps_interrupts() in gen6_rps_{idle,busy} and
> making sure vlv_set_rps_idle(), gen6_set_rps() would not re-enable the
> interrupts.

Yes, we can do that.

> That would also make it possible to
> remove gen6_{disable,enable}_rps_interrupts() from the
> suspend/resume path.

A while back we discussed this, and I've been running with

http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=11ff1e6deceb33a5db7be31830abb46c1450755e

which disables the RPS interrupt at idle time (and kills the then superflous
suspend path). It works but for a few spurious interrupt warnings.
Though I missed the flush_work(&rps.work) caught in this patch, which
may just account for the errors.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre