[Intel-gfx] [PATCH] drm/i915: clear up I915_(READ|WRITE)_NOTRACE confusion

Daniel Vetter daniel.vetter at ffwll.ch
Thu Dec 22 01:28:36 CET 2011


Half of the users actually don't want just no tracing, but need to
avoid the forcewake dance for correctness. So add new variants
__I915_READ and __I915_WRITE for that.

Also improve the _NOTRACE variants to do the forcewake dance.
Currently not required because the only user is the i2c code, which is
in the display block and hence not suspect to forcewake issues, but
nice to avoid confusion while debuggin.

Finally add a comment explaining what's going on with ECOBUS. This one
is shamelessly inspired by a patch from Keith Packard.

Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c      |   30 ++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_drv.h      |   34 ++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_display.c |    7 ++++++-
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3a712cf..112f7f5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -333,14 +333,14 @@ void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 	int count;
 
 	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1))
+	while (count++ < 50 && (__I915_READ(FORCEWAKE_ACK) & 1))
 		udelay(10);
 
-	I915_WRITE_NOTRACE(FORCEWAKE, 1);
+	__I915_WRITE(FORCEWAKE, 1);
 	POSTING_READ(FORCEWAKE);
 
 	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0)
+	while (count++ < 50 && (__I915_READ(FORCEWAKE_ACK) & 1) == 0)
 		udelay(10);
 }
 
@@ -349,14 +349,14 @@ void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
 	int count;
 
 	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1))
+	while (count++ < 50 && (__I915_READ(FORCEWAKE_MT_ACK) & 1))
 		udelay(10);
 
-	I915_WRITE_NOTRACE(FORCEWAKE_MT, (1<<16) | 1);
+	__I915_WRITE(FORCEWAKE_MT, (1<<16) | 1);
 	POSTING_READ(FORCEWAKE_MT);
 
 	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0)
+	while (count++ < 50 && (__I915_READ(FORCEWAKE_MT_ACK) & 1) == 0)
 		udelay(10);
 }
 
@@ -378,13 +378,13 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 
 void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
-	I915_WRITE_NOTRACE(FORCEWAKE, 0);
+	__I915_WRITE(FORCEWAKE, 0);
 	POSTING_READ(FORCEWAKE);
 }
 
 void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
 {
-	I915_WRITE_NOTRACE(FORCEWAKE_MT, (1<<16) | 0);
+	__I915_WRITE(FORCEWAKE_MT, (1<<16) | 0);
 	POSTING_READ(FORCEWAKE_MT);
 }
 
@@ -405,10 +405,10 @@ void __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 = __I915_READ(GT_FIFO_FREE_ENTRIES);
 		while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
 			udelay(10);
-			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
+			fifo = __I915_READ(GT_FIFO_FREE_ENTRIES);
 		}
 		WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES);
 		dev_priv->gt_fifo_count = fifo;
@@ -934,7 +934,7 @@ MODULE_LICENSE("GPL and additional rights");
 	 ((reg) < 0x40000))		 \
 
 #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 (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		gen6_gt_force_wake_get(dev_priv); \
@@ -943,7 +943,8 @@ 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; \
 }
 
@@ -954,8 +955,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) { \
-	trace_i915_reg_rw(true, reg, val, sizeof(val)); \
+void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool trace) { \
+	if (trace) \
+		trace_i915_reg_rw(true, reg, val, sizeof(val)); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__gen6_gt_wait_for_fifo(dev_priv); \
 	} \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d8f442f..39debc9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1358,7 +1358,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv);
 void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
 
 #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)
@@ -1367,7 +1367,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)
@@ -1375,24 +1375,26 @@ __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)		readw(dev_priv->regs + (reg))
+#define __I915_WRITE16(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_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)		readl(dev_priv->regs + (reg))
+#define __I915_WRITE(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)
+#define POSTING_READ(reg)		(void)__I915_READ(reg)
+#define POSTING_READ16(reg)		(void)__I915_READ16(reg)
 
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48b479c..c4f3451 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8587,8 +8587,13 @@ static void intel_init_display(struct drm_device *dev)
 			u32	ecobus;
 
 			mutex_lock(&dev->struct_mutex);
+			/* The ECOBUS reg is actually in the gt power well, but
+			 * if the gt is powered down and mt forcewake not
+			 * enabled we'll just read a bogus 0 leading to the
+			 * correct conclusion that mt forcewake is indeed not
+			 * enabled. */
 			__gen6_gt_force_wake_mt_get(dev_priv);
-			ecobus = I915_READ_NOTRACE(ECOBUS);
+			ecobus = __I915_READ(ECOBUS);
 			__gen6_gt_force_wake_mt_put(dev_priv);
 			mutex_unlock(&dev->struct_mutex);
 
-- 
1.7.7.3




More information about the Intel-gfx mailing list