On Wed, Aug 27, 2014 at 1:28 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Wed, Aug 27, 2014 at 12:23:47PM +0200, Daniel Vetter wrote: >> On Wed, Aug 27, 2014 at 09:51:58AM +0100, Chris Wilson wrote: >> > On Wed, Aug 27, 2014 at 10:43:37AM +0200, Daniel Vetter wrote: >> > > Now that vlv has runtime pm we kinda should check for that like on the >> > > pch split platforms. Looks like this was simply lost in the vlv rpm >> > > enabling. >> > >> > Is there a reason why setting up the pipestat prior to enabling >> > interrupts is verboten, or is it just not done today? >> >> The similar warning on pch platforms was added to catch runtime pm bugs >> where someone enabled some interrupt source, but didn't hold a runtime pm >> reference. That's just not going to work. So motivation for this patch is >> vlv/chv runtime pm really. >> >> But I also think that this check provides value for driver >> load/resume/suspend since if we change interrupt settings after the >> interrupts have been disabled or before enabling something is at least >> fishy. At least insofar as that our irq install hooks nowadays overwrite >> all of them with default values. > > That's a better blurp. But doesn't it sound a little overly generic just > to keep whether or not pipestat_enable is called after uninstall or > before install? Since we track the expected register value, could we > just semi-randomly check that the register matches that value? I guess we could do both, but I don't really see a good place. Maybe in the enable/disable functions, but I'm not even sure we track the expected state for everyone really. -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
