On Fri, Oct 31, 2014 at 11:40:36AM +0200, Ville Syrjälä wrote: > On Thu, Oct 30, 2014 at 06:12:51PM -0200, Paulo Zanoni wrote: > > While trying to check for the correctness of the lines above, I > > noticed that in __i915_enable_pipestat(), if the enable mask is > > already what we want, we won't clear/update the status bits. Is that > > correct? Why? Anyway, any problems in that function should be fixed by > > a separate patch. > > I think the current behaviour is correct. We don't want to lose > interrupts in case someone enables the same pipestat bit twice. But > obviously then disabling twice doesn't work because we don't refcount > the individual bits. So perhaps we want to print a warning if some of > the bits we're trying to set are already set? Yeah a warning in case something is disabled/enabled already might be useful. That's valid for all the various irq mask handling helpers we have in general though (i.e. gt, display ...). And I'm pretty sure it'll lead to a massive WARN backtrace fest ;-) -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