On Fri, Jul 12, 2013 at 03:02:32PM +0100, Chris Wilson wrote:
> The GT functions for enabling register access also need to occasionally
> write to and read from registers. To avoid the potential recursion as we
> modify the public interface to be stricter, introduce a private register
> access API for the GT functions.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_gt.c | 92 
> +++++++++++++++++++++++++----------------
>  1 file changed, 56 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_gt.c b/drivers/gpu/drm/i915/intel_gt.c
> index 060e256..cb3116c 100644
> --- a/drivers/gpu/drm/i915/intel_gt.c
> +++ b/drivers/gpu/drm/i915/intel_gt.c
> @@ -26,6 +26,19 @@
>  
>  #define FORCEWAKE_ACK_TIMEOUT_MS 2
>  
> +#define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + 
> (reg__))
> +#define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, 
> (dev_priv__)->regs + (reg__))
> +
> +#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + 
> (reg__))
> +#define __raw_i915_write16(dev_priv__, reg__, val__) writew(val__, 
> (dev_priv__)->regs + (reg__))
> +
> +#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + 
> (reg__))
> +#define __raw_i915_write32(dev_priv__, reg__, val__) writel(val__, 
> (dev_priv__)->regs + (reg__))

Instead of what you did, I would have preferred
#define __raw_posting_read

Also, I don't mind dev_priv as an implicit arg, but I know Daniel has
complained about such things before.

positive (I didn't check for missed conversions)
Reviewed-by: Ben Widawsky <b...@bwidawsk.net>

> +
> +#define __raw_i915_read64(dev_priv__, reg__) readq((dev_priv__)->regs + 
> (reg__))
> +#define __raw_i915_write64(dev_priv__, reg__, val__) writeq(val__, 
> (dev_priv__)->regs + (reg__))
> +
> +
>  static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
>  {
>       u32 gt_thread_status_mask;
> @@ -38,26 +51,27 @@ static void __gen6_gt_wait_for_thread_c0(struct 
> drm_i915_private *dev_priv)
>       /* w/a for a sporadic read returning 0 by waiting for the GT
>        * thread to wake up.
>        */
> -     if (wait_for_atomic_us((I915_READ_NOTRACE(GEN6_GT_THREAD_STATUS_REG) & 
> gt_thread_status_mask) == 0, 500))
> +     if (wait_for_atomic_us((__raw_i915_read32(dev_priv, 
> GEN6_GT_THREAD_STATUS_REG) & gt_thread_status_mask) == 0, 500))
>               DRM_ERROR("GT thread status wait timed out\n");
>  }
>  
>  static void __gen6_gt_force_wake_reset(struct drm_i915_private *dev_priv)
>  {
> -     I915_WRITE_NOTRACE(FORCEWAKE, 0);
> -     POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE 
> */
> +     __raw_i915_write32(dev_priv, FORCEWAKE, 0);
> +     /* something from same cacheline, but !FORCEWAKE */
> +     (void)__raw_i915_read32(dev_priv, ECOBUS);
>  }
>  
>  static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  {
> -     if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0,
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK) & 1) == 
> 0,
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for forcewake old ack to 
> clear.\n");
>  
> -     I915_WRITE_NOTRACE(FORCEWAKE, 1);
> -     POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE 
> */
> +     __raw_i915_write32(dev_priv, FORCEWAKE, 1);
> +     (void)__raw_i915_read32(dev_priv, ECOBUS); /* something from same 
> cacheline, but !FORCEWAKE */
>  
> -     if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1),
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK) & 1),
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for forcewake to ack request.\n");
>  
> @@ -67,9 +81,9 @@ static void __gen6_gt_force_wake_get(struct 
> drm_i915_private *dev_priv)
>  
>  static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
>  {
> -     I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff));
> +     __raw_i915_write32(dev_priv, FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff));
>       /* something from same cacheline, but !FORCEWAKE_MT */
> -     POSTING_READ(ECOBUS);
> +     (void)__raw_i915_read32(dev_priv, ECOBUS);
>  }
>  
>  static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> @@ -81,15 +95,16 @@ static void __gen6_gt_force_wake_mt_get(struct 
> drm_i915_private *dev_priv)
>       else
>               forcewake_ack = FORCEWAKE_MT_ACK;
>  
> -     if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 
> FORCEWAKE_KERNEL) == 0,
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, forcewake_ack) & 
> FORCEWAKE_KERNEL) == 0,
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for forcewake old ack to 
> clear.\n");
>  
> -     I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
> +     __raw_i915_write32(dev_priv, FORCEWAKE_MT,
> +                        _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
>       /* something from same cacheline, but !FORCEWAKE_MT */
> -     POSTING_READ(ECOBUS);
> +     (void)__raw_i915_read32(dev_priv, ECOBUS);
>  
> -     if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 
> FORCEWAKE_KERNEL),
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, forcewake_ack) & 
> FORCEWAKE_KERNEL),
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for forcewake to ack request.\n");
>  
> @@ -100,25 +115,27 @@ static void __gen6_gt_force_wake_mt_get(struct 
> drm_i915_private *dev_priv)
>  static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
>  {
>       u32 gtfifodbg;
> -     gtfifodbg = I915_READ_NOTRACE(GTFIFODBG);
> +
> +     gtfifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
>       if (WARN(gtfifodbg & GT_FIFO_CPU_ERROR_MASK,
>            "MMIO read or write has been dropped %x\n", gtfifodbg))
> -             I915_WRITE_NOTRACE(GTFIFODBG, GT_FIFO_CPU_ERROR_MASK);
> +             __raw_i915_write32(dev_priv, GTFIFODBG, GT_FIFO_CPU_ERROR_MASK);
>  }
>  
>  static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  {
> -     I915_WRITE_NOTRACE(FORCEWAKE, 0);
> +     __raw_i915_write32(dev_priv, FORCEWAKE, 0);
>       /* something from same cacheline, but !FORCEWAKE */
> -     POSTING_READ(ECOBUS);
> +     (void)__raw_i915_read32(dev_priv, ECOBUS);
>       gen6_gt_check_fifodbg(dev_priv);
>  }
>  
>  static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
>  {
> -     I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> +     __raw_i915_write32(dev_priv, FORCEWAKE_MT,
> +                        _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
>       /* something from same cacheline, but !FORCEWAKE_MT */
> -     POSTING_READ(ECOBUS);
> +     (void)__raw_i915_read32(dev_priv, ECOBUS);
>       gen6_gt_check_fifodbg(dev_priv);
>  }
>  
> @@ -128,10 +145,10 @@ static int __gen6_gt_wait_for_fifo(struct 
> drm_i915_private *dev_priv)
>  
>       if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
>               int loop = 500;
> -             u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> +             u32 fifo = __raw_i915_read32(dev_priv, GT_FIFO_FREE_ENTRIES);
>               while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
>                       udelay(10);
> -                     fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> +                     fifo = __raw_i915_read32(dev_priv, 
> GT_FIFO_FREE_ENTRIES);
>               }
>               if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
>                       ++ret;
> @@ -144,26 +161,28 @@ static int __gen6_gt_wait_for_fifo(struct 
> drm_i915_private *dev_priv)
>  
>  static void vlv_force_wake_reset(struct drm_i915_private *dev_priv)
>  {
> -     I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(0xffff));
> +     __raw_i915_write32(dev_priv, FORCEWAKE_VLV,
> +                        _MASKED_BIT_DISABLE(0xffff));
>       /* something from same cacheline, but !FORCEWAKE_VLV */
> -     POSTING_READ(FORCEWAKE_ACK_VLV);
> +     (void)__raw_i915_read32(dev_priv, FORCEWAKE_ACK_VLV);
>  }
>  
>  static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
>  {
> -     if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 
> FORCEWAKE_KERNEL) == 0,
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK_VLV) & 
> FORCEWAKE_KERNEL) == 0,
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for forcewake old ack to 
> clear.\n");
>  
> -     I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
> -     I915_WRITE_NOTRACE(FORCEWAKE_MEDIA_VLV,
> +     __raw_i915_write32(dev_priv, FORCEWAKE_VLV,
> +                        _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
> +     __raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
>                          _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
>  
> -     if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 
> FORCEWAKE_KERNEL),
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK_VLV) & 
> FORCEWAKE_KERNEL),
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for GT to ack forcewake 
> request.\n");
>  
> -     if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK_MEDIA_VLV) &
> +     if (wait_for_atomic((__raw_i915_read32(dev_priv, 
> FORCEWAKE_ACK_MEDIA_VLV) &
>                            FORCEWAKE_KERNEL),
>                           FORCEWAKE_ACK_TIMEOUT_MS))
>               DRM_ERROR("Timed out waiting for media to ack forcewake 
> request.\n");
> @@ -174,8 +193,9 @@ static void vlv_force_wake_get(struct drm_i915_private 
> *dev_priv)
>  
>  static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
>  {
> -     I915_WRITE_NOTRACE(FORCEWAKE_VLV, 
> _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> -     I915_WRITE_NOTRACE(FORCEWAKE_MEDIA_VLV,
> +     __raw_i915_write32(dev_priv, FORCEWAKE_VLV,
> +                        _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> +     __raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
>                          _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
>       /* The below doubles as a POSTING_READ */
>       gen6_gt_check_fifodbg(dev_priv);
> @@ -209,7 +229,7 @@ void intel_gt_init(struct drm_device *dev)
>                */
>               mutex_lock(&dev->struct_mutex);
>               __gen6_gt_force_wake_mt_get(dev_priv);
> -             ecobus = I915_READ_NOTRACE(ECOBUS);
> +             ecobus = __raw_i915_read32(dev_priv, ECOBUS);
>               __gen6_gt_force_wake_mt_put(dev_priv);
>               mutex_unlock(&dev->struct_mutex);
>  
> @@ -285,17 +305,17 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
>       /* WaIssueDummyWriteToWakeupFromRC6:ilk Issue a dummy write to wake up
>        * the chip from rc6 before touching it for real. MI_MODE is masked,
>        * hence harmless to write 0 into. */
> -     I915_WRITE_NOTRACE(MI_MODE, 0);
> +     __raw_i915_write32(dev_priv, MI_MODE, 0);
>  }
>  
>  static void
>  hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
>  {
>       if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
> -         (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
> +         (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
>               DRM_ERROR("Unknown unclaimed register before writing to %x\n",
>                         reg);
> -             I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +             __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
>       }
>  }
>  
> @@ -303,9 +323,9 @@ static void
>  hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
>  {
>       if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
> -         (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
> +         (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
>               DRM_ERROR("Unclaimed write to %x\n", reg);
> -             I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> +             __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
>       }
>  }
>  
> -- 
> 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

Reply via email to