On Fri, 2014-11-07 at 18:42 -0200, Paulo Zanoni wrote: > 2014-11-05 16:48 GMT-02:00 Imre Deak <imre.deak@xxxxxxxxx>: > > 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@xxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
