On Thu, Apr 12, 2012 at 06:03:30PM -0700, Ben Widawsky wrote: > On Wed, 11 Apr 2012 22:12:59 +0200 > Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > > > Now that these are properly refactored this additional indirection > > doesn't really buy us anything but confusion. Hence inline them. > > > > This duplicates the ironlake gt enable/disable code snippet, but we've > > already separate ilk from gen6+ gt irq in i915_irq.c, so I think this > > makes more sense. > > > > Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > > Bikeshed: > While doing all this, I think put/get irq is really terribly named. I > was a much bigger fan of the enable disable. Actually that can be combined with Chris' bikeshed to move the ring->put|get_irg to dev_priv->core.ring_enable/disable_irq. But I've figured that the same patch series should also move the forcewake function pointers from dev_priv->display to dev_priv->core, so this is imo a different patch series. > Also, you could use a bit of flow control to write to the correct IMR > register and not duplicate functions at all. You already do the > POSTING_READ so performance shouldn't matter. > > Something like... > > uint32_t imr = GEN(dev) >= 5 ? GTIMR: IMR; I've figured I want to ditch all IS_GEN checks from these functions. But for this case it makes some sense. Imo could be done with the follow-up though. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
