From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 3/6] drm/i915: Use the common register access functions for NOTRACE variants Date: Fri, 12 Jul 2013 13:34:09 -0700 Message-ID: <20130712203408.GH15384@bwidawsk.net> References: <1373637756-11852-1-git-send-email-chris@chris-wilson.co.uk> <1373637756-11852-3-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from shiva.localdomain (unknown [209.20.75.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 46226E5EE4 for ; Fri, 12 Jul 2013 13:34:17 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1373637756-11852-3-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jul 12, 2013 at 03:02:33PM +0100, Chris Wilson wrote: > Detangle the confusion that NOTRACE variants of the register read/write > routines were directly using the raw register access. We need for those > routines to reuse the common code for serializing register access and > ensuring the correct register power states. This is only possible now > that the only routines that required raw access use their own API. > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_drv.h | 28 ++++++++++++++-------------- > drivers/gpu/drm/i915/intel_gt.c | 8 ++++---- > 2 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f140b04..1ab6d4e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2082,7 +2082,7 @@ int vlv_gpu_freq(int ddr_freq, int val); > int vlv_freq_opcode(int ddr_freq, int val); > > #define __i915_read(x, y) \ > - u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg); > + u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool trace); > > __i915_read(8, b) > __i915_read(16, w) > @@ -2091,7 +2091,7 @@ __i915_read(64, q) > #undef __i915_read > > #define __i915_write(x, y) \ > - void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val); > + void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool trace); > > __i915_write(8, b) > __i915_write(16, w) > @@ -2099,21 +2099,21 @@ __i915_write(32, l) > __i915_write(64, q) > #undef __i915_write > > -#define I915_READ8(reg) i915_read8(dev_priv, (reg)) > -#define I915_WRITE8(reg, val) i915_write8(dev_priv, (reg), (val)) > +#define I915_READ8(reg) i915_read8(dev_priv, (reg), true) > +#define I915_WRITE8(reg, val) i915_write8(dev_priv, (reg), (val), true) > > -#define I915_READ16(reg) i915_read16(dev_priv, (reg)) > -#define I915_WRITE16(reg, val) i915_write16(dev_priv, (reg), (val)) > -#define I915_READ16_NOTRACE(reg) readw(dev_priv->regs + (reg)) > -#define I915_WRITE16_NOTRACE(reg, val) writew(val, dev_priv->regs + (reg)) > +#define I915_READ16(reg) i915_read16(dev_priv, (reg), true) > +#define I915_WRITE16(reg, val) i915_write16(dev_priv, (reg), (val), true) > +#define I915_READ16_NOTRACE(reg) i915_read16(dev_priv, (reg), false) > +#define I915_WRITE16_NOTRACE(reg, val) i915_write16(dev_priv, (reg), (val), false) > > -#define I915_READ(reg) i915_read32(dev_priv, (reg)) > -#define I915_WRITE(reg, val) i915_write32(dev_priv, (reg), (val)) > -#define I915_READ_NOTRACE(reg) readl(dev_priv->regs + (reg)) > -#define I915_WRITE_NOTRACE(reg, val) writel(val, dev_priv->regs + (reg)) > +#define I915_READ(reg) i915_read32(dev_priv, (reg), true) > +#define I915_WRITE(reg, val) i915_write32(dev_priv, (reg), (val), true) > +#define I915_READ_NOTRACE(reg) i915_read32(dev_priv, (reg), false) > +#define I915_WRITE_NOTRACE(reg, val) i915_write32(dev_priv, (reg), (val), false) > > -#define I915_WRITE64(reg, val) i915_write64(dev_priv, (reg), (val)) > -#define I915_READ64(reg) i915_read64(dev_priv, (reg)) > +#define I915_WRITE64(reg, val) i915_write64(dev_priv, (reg), (val), true) > +#define I915_READ64(reg) i915_read64(dev_priv, (reg), true) > > #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) > #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) > diff --git a/drivers/gpu/drm/i915/intel_gt.c b/drivers/gpu/drm/i915/intel_gt.c > index cb3116c..d4bc7f4 100644 > --- a/drivers/gpu/drm/i915/intel_gt.c > +++ b/drivers/gpu/drm/i915/intel_gt.c > @@ -330,7 +330,7 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) > } > > #define __i915_read(x, y) \ > -u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ > +u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool trace) { \ > u##x val = 0; \ > if (IS_GEN5(dev_priv->dev)) \ > ilk_dummy_write(dev_priv); \ > @@ -346,7 +346,7 @@ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ > } else { \ > val = read##y(dev_priv->regs + reg); \ > } \ > - trace_i915_reg_rw(false, reg, val, sizeof(val)); \ > + if (trace) trace_i915_reg_rw(false, reg, val, sizeof(val)); \ > return val; \ > } > > @@ -357,9 +357,9 @@ __i915_read(64, q) > #undef __i915_read > > #define __i915_write(x, y) \ > -void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ > +void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool trace) { \ > u32 __fifo_ret = 0; \ > - trace_i915_reg_rw(true, reg, val, sizeof(val)); \ > + if (trace) trace_i915_reg_rw(true, reg, val, sizeof(val)); \ > if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ > __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ > } \ if (unlikely(trace))? taking a hit on the tracing case seems like what you want... but I never know the status of such compiler flags. Reviewed-by: Ben Widawsky -- Ben Widawsky, Intel Open Source Technology Center