On Mon, Dec 01, 2014 at 02:47:06PM -0200, Paulo Zanoni wrote: > 2014-12-01 14:34 GMT-02:00 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: > > On Mon, Dec 01, 2014 at 02:29:11PM -0200, Paulo Zanoni wrote: > >> 2014-11-20 19:01 GMT-02:00 Imre Deak <imre.deak@xxxxxxxxx>: > >> > Atm, igt/gem_reset_stats can trigger the recently added WARN on > >> > left-over PM_IIR bits in gen6_enable_rps_interrupts(). There are two > >> > reasons for this: > >> > 1. we call intel_enable_gt_powersave() without a preceeding > >> > intel_disable_gt_powersave() > >> > 2. gen6_disable_rps_interrupts() doesn't mask interrupts in PM_IMR > >> > >> We don't do this, but we mask stuff through GEN6_PMINTRMSK. Shouldn't > >> this be enough to prevent IIR from changing? > >> > >> Chris? > > > > It should. We should be doing both really, use PM_IMR to treat > > IMR/IIR/IER consistently with other interrupts, and use the special > > PMINTRMASK as part of rps power tuning. > > In that case: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> Queued for -next, thanks for the patch. -Daniel > But one thing makes me wonder: we disable IER on > gen6_disable_rps_interrupts but never seem to enable it again (except > for the usual pre/post/uninstall functions)... I know it is not a > problem introduced by this patch, but shouldn't this be a problem too? We only need rps interrupts when we are not rpm suspended, so I think this should be fine. Not fully convinced, so if anyone else has doubts a new rpm subtest for the pm_rps testcase might be useful to prove this one way or the other. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
