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 <chris at chris-wilson.co.uk>
> ---
> 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 at bwidawsk.net>
--
Ben Widawsky, Intel Open Source Technology Center
