Message ID | 1386681892-31586-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Dec 10, 2013 at 03:24:52PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Not all registers need forcewake even if they're not shadowed. > Add the missing NEEDS_FORCE_WAKE() check to gen8_writeX() to > avoid needless forcewake usage when writing eg. display > registers. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > I didn't test this at all, so be warned. > > drivers/gpu/drm/i915/intel_uncore.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index e63658e..78fe8d6 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -616,7 +616,8 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg) > #define __gen8_write(x) \ > static void \ > gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \ > - bool __needs_put = !is_gen8_shadowed(dev_priv, reg); \ > + bool __needs_put = NEEDS_FORCE_WAKE(dev_priv, reg) && \ > + !is_gen8_shadowed(dev_priv, reg); \ > REG_WRITE_HEADER; \ > if (__needs_put) { \ > dev_priv->uncore.funcs.force_wake_get(dev_priv, \ I would prefer if this was simply checking the display offset, as opposed to NEEDS_FORCE_WAKE - since if someone adds to the macro we run the risk of a potentially hard to track down bug. In any case, the patch is functionally correct, and I'd prefer the change, but up to you: Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Dec 10, 2013 at 11:47:17AM -0800, Ben Widawsky wrote: > On Tue, Dec 10, 2013 at 03:24:52PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Not all registers need forcewake even if they're not shadowed. > > Add the missing NEEDS_FORCE_WAKE() check to gen8_writeX() to > > avoid needless forcewake usage when writing eg. display > > registers. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > I didn't test this at all, so be warned. > > > > drivers/gpu/drm/i915/intel_uncore.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index e63658e..78fe8d6 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -616,7 +616,8 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg) > > #define __gen8_write(x) \ > > static void \ > > gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \ > > - bool __needs_put = !is_gen8_shadowed(dev_priv, reg); \ > > + bool __needs_put = NEEDS_FORCE_WAKE(dev_priv, reg) && \ > > + !is_gen8_shadowed(dev_priv, reg); \ > > REG_WRITE_HEADER; \ > > if (__needs_put) { \ > > dev_priv->uncore.funcs.force_wake_get(dev_priv, \ > > I would prefer if this was simply checking the display offset, as > opposed to NEEDS_FORCE_WAKE - since if someone adds to the macro we run > the risk of a potentially hard to track down bug. Yeah, I agree that an explicit check would be better since the forcewake stuff works a bit different on gen8+ -Daniel > > In any case, the patch is functionally correct, and I'd prefer the > change, but up to you: > Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > > > -- > > 1.8.3.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ben Widawsky, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index e63658e..78fe8d6 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -616,7 +616,8 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg) #define __gen8_write(x) \ static void \ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \ - bool __needs_put = !is_gen8_shadowed(dev_priv, reg); \ + bool __needs_put = NEEDS_FORCE_WAKE(dev_priv, reg) && \ + !is_gen8_shadowed(dev_priv, reg); \ REG_WRITE_HEADER; \ if (__needs_put) { \ dev_priv->uncore.funcs.force_wake_get(dev_priv, \