[Intel-gfx] [PATCH 2/6] drm/i915: Use a private interface for register access within GT

Ben Widawsky ben at bwidawsk.net
Fri Jul 12 22:25:27 CEST 2013


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 <chris at 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 <ben at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center



More information about the Intel-gfx mailing list