[Intel-gfx] [PATCH 0/8] sanitize RPS interrupt enabling/disabling

[Imre Deak]

On Fri, 2014-11-07 at 18:42 -0200, Paulo Zanoni wrote:
> 2014-11-05 16:48 GMT-02:00 Imre Deak <imre.deak at intel.com>:
> > While fixing [1] I noticed that we can simplify a couple of things in
> > the RPS enabling/disabling code. So I did that and also fixed one WARN
> > that we can hit with some of the pm_rpm subtests. Hopefully these
> > changes also makes it clearer how we avoid the race during RPS interrupt
> > disabling and makes it less fragile (see the comment in patch 7/8).
> 
> For patches 1-4: Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> But for patch 4, can't we try to create a new intel_rps.c and move
> stuff there, instead of keeping them in i915_irq.c? I kinda like the
> idea of files containing single features... Anyway, we can do this in
> yet-another patch.

intel_rps.c sounds like a good idea. For now I moved the RPS interrupt
handling to i915_irq.c, since half of it is already there
(snb_update_pm_irq() which pokes at the same registers) as well as the
RPS work itself and its scheduling. Ville also had the idea to further
simplify things since the PM IRQ clearing, masking, enabling is very
similar to how this is done for other interrupt sources and i915_irq.c
would be a good staging area for such future work. So I'd also suggest
refactoring thing into intel_rps.c as a follow-up.

> For patches 5-6: the addition of gen9_disable_rps()  and
> gen9_enable_rps() kinda broke these patches...

Good catch, I didn't consider the GEN9 patches. We don't yet support RPS
there at all, but still call gen9_enable/disable_rps(), since the same
function deals with RC6 too. I assume that the GEN9 RPS support will be
added soonish though so for now I'd solve this by simply skipping
gen6_reset/enable/disable_rps_interrupts() for GEN9.

> 
> >
> > [1] https://bugs.freedesktop.org/show_bug.cgi?id=82939
> >
> > Imre Deak (8):
> >   drm/i915: unify gen6/gen8 pm irq helpers
> >   drm/i915: unify gen6/gen8 rps irq handler
> >   drm/i915: unify gen6/gen8 rps irq enable/disable
> >   drm/i915: move rps irq enable/disable to i915_irq.c
> >   drm/i915: move rps irq disable one level up
> >   drm/i915: sanitize rps irq enabling
> >   drm/i915: sanitize rps irq disabling
> >   drm/i915: disable rps irqs earlier during suspend/unload
> >
> >  drivers/gpu/drm/i915/i915_drv.c      |   9 +--
> >  drivers/gpu/drm/i915/i915_drv.h      |   6 +-
> >  drivers/gpu/drm/i915/i915_irq.c      | 122 +++++++++++++++++++----------------
> >  drivers/gpu/drm/i915/intel_display.c |   6 +-
> >  drivers/gpu/drm/i915/intel_drv.h     |   5 +-
> >  drivers/gpu/drm/i915/intel_pm.c      |  90 +++-----------------------
> >  6 files changed, 90 insertions(+), 148 deletions(-)
> >
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
>