[PATCH 1/7] drm/i915: Prevent using uninitialized MMIO funcs

[PATCH 1/7] drm/i915: Prevent using uninitialized MMIO funcs

[Ben Widawsky]

For upcoming patches which will have GEN specific MMIO functions, we'll
need to initialize the uncore data structure earlier than we do today.

If we do not do this, the following will be problematic:

intel_uncore_sanitize
	intel_disable_gt_powersave
		gen6_disable_rps
			I915_WRITE(GEN6_RC_CONTROL, 0); <--- MMIO
intel_uncore_init // initializes MMIO

By initializing the function pointers first, we should be safe.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f221631..0a84cd5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1621,8 +1621,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_irq_init(dev);
 	intel_pm_init(dev);
-	intel_uncore_sanitize(dev);
 	intel_uncore_init(dev);
+	intel_uncore_sanitize(dev);
 
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev);
-- 
1.8.4

[PATCH 2/7] drm/i915: Move edram detection early_sanitize

[Ben Widawsky]

In order to be able to have virtual functions for the MMIO, we need to
use the raw access function. To keep things simple, just move this to
our early_sanitize code in uncore.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c     | 10 ----------
 drivers/gpu/drm/i915/intel_uncore.c | 11 +++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0a84cd5..ad55c02 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1549,16 +1549,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	intel_uncore_early_sanitize(dev);
 
-	if (IS_HASWELL(dev) && (I915_READ(HSW_EDRAM_PRESENT) == 1)) {
-		/* The docs do not explain exactly how the calculation can be
-		 * made. It is somewhat guessable, but for now, it's always
-		 * 128MB.
-		 * NB: We can't write IDICR yet because we do not have gt funcs
-		 * set up */
-		dev_priv->ellc_size = 128;
-		DRM_INFO("Found %zuMB of eLLC\n", dev_priv->ellc_size);
-	}
-
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
 		goto out_regs;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 288a3a6..e9034b8 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -222,6 +222,17 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
 
 	if (HAS_FPGA_DBG_UNCLAIMED(dev))
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+
+	if (IS_HASWELL(dev) &&
+	    (__raw_i915_read32(dev_priv, HSW_EDRAM_PRESENT) == 1)) {
+		/* The docs do not explain exactly how the calculation can be
+		 * made. It is somewhat guessable, but for now, it's always
+		 * 128MB.
+		 * NB: We can't write IDICR yet because we do not have gt funcs
+		 * set up */
+		dev_priv->ellc_size = 128;
+		DRM_INFO("Found %zuMB of eLLC\n", dev_priv->ellc_size);
+	}
 }
 
 void intel_uncore_init(struct drm_device *dev)
-- 
1.8.4

[PATCH 3/7] drm/i915: Create MMIO virtual functions

[Ben Widawsky]

In preparation for having per GEN MMIO functions, create, and start
using MMIO functions in our uncore data structure. This simply makes the
transition easier by allowing us to just plug in the per GEN stuff
later.

For simplicity, I moved the intel_uncore_init() function down since
those rely on static functions defined lower in the file. This is most
of the churn in this patch.

I made one unrelated change here by using off_t datatype for the offset
of the register to write. I like the clarity that this brings to the
code. If I did it as a separate patch, I am pretty certain it would get
bikeshedded to oblivion.

Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h     |  60 ++++++++--------
 drivers/gpu/drm/i915/intel_uncore.c | 139 +++++++++++++++++++-----------------
 2 files changed, 104 insertions(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ed8653f..a054b30 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -399,6 +399,20 @@ struct drm_i915_display_funcs {
 struct intel_uncore_funcs {
 	void (*force_wake_get)(struct drm_i915_private *dev_priv);
 	void (*force_wake_put)(struct drm_i915_private *dev_priv);
+
+	uint8_t  (*mmio_readb)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
+	uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
+	uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
+	uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
+
+	void (*mmio_writeb)(struct drm_i915_private *dev_priv, off_t offset,
+				uint8_t val, bool trace);
+	void (*mmio_writew)(struct drm_i915_private *dev_priv, off_t offset,
+				uint16_t val, bool trace);
+	void (*mmio_writel)(struct drm_i915_private *dev_priv, off_t offset,
+				uint32_t val, bool trace);
+	void (*mmio_writeq)(struct drm_i915_private *dev_priv, off_t offset,
+				uint64_t val, bool trace);
 };
 
 struct intel_uncore {
@@ -2338,37 +2352,21 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 int vlv_gpu_freq(int ddr_freq, int val);
 int vlv_freq_opcode(int ddr_freq, int val);
 
-#define __i915_read(x) \
-	u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool trace);
-__i915_read(8)
-__i915_read(16)
-__i915_read(32)
-__i915_read(64)
-#undef __i915_read
-
-#define __i915_write(x) \
-	void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool trace);
-__i915_write(8)
-__i915_write(16)
-__i915_write(32)
-__i915_write(64)
-#undef __i915_write
-
-#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), 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), 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), true)
-#define I915_READ64(reg)	i915_read64(dev_priv, (reg), true)
+#define I915_READ8(reg)		dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
+#define I915_WRITE8(reg, val)	dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)
+
+#define I915_READ16(reg)	dev_priv->uncore.funcs.mmio_readw(dev_priv, (reg), true)
+#define I915_WRITE16(reg, val)	dev_priv->uncore.funcs.mmio_writew(dev_priv, (reg), (val), true)
+#define I915_READ16_NOTRACE(reg)	dev_priv->uncore.funcs.mmio_readw(dev_priv, (reg), false)
+#define I915_WRITE16_NOTRACE(reg, val)	dev_priv->uncore.funcs.mmio_writew(dev_priv, (reg), (val), false)
+
+#define I915_READ(reg)		dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), true)
+#define I915_WRITE(reg, val)	dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), true)
+#define I915_READ_NOTRACE(reg)		dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), false)
+#define I915_WRITE_NOTRACE(reg, val)	dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), false)
+
+#define I915_WRITE64(reg, val)	dev_priv->uncore.funcs.mmio_writeq(dev_priv, (reg), (val), true)
+#define I915_READ64(reg)	dev_priv->uncore.funcs.mmio_readq(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_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index e9034b8..6098480 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -235,68 +235,6 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
 	}
 }
 
-void intel_uncore_init(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	INIT_DELAYED_WORK(&dev_priv->uncore.force_wake_work,
-			  gen6_force_wake_work);
-
-	if (IS_VALLEYVIEW(dev)) {
-		dev_priv->uncore.funcs.force_wake_get = vlv_force_wake_get;
-		dev_priv->uncore.funcs.force_wake_put = vlv_force_wake_put;
-	} else if (IS_HASWELL(dev)) {
-		dev_priv->uncore.funcs.force_wake_get = __gen6_gt_force_wake_mt_get;
-		dev_priv->uncore.funcs.force_wake_put = __gen6_gt_force_wake_mt_put;
-	} else if (IS_IVYBRIDGE(dev)) {
-		u32 ecobus;
-
-		/* IVB configs may use multi-threaded forcewake */
-
-		/* A small trick here - if the bios hasn't configured
-		 * MT forcewake, and if the device is in RC6, then
-		 * force_wake_mt_get will not wake the device and the
-		 * ECOBUS read will return zero. Which will be
-		 * (correctly) interpreted by the test below as MT
-		 * forcewake being disabled.
-		 */
-		mutex_lock(&dev->struct_mutex);
-		__gen6_gt_force_wake_mt_get(dev_priv);
-		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
-		__gen6_gt_force_wake_mt_put(dev_priv);
-		mutex_unlock(&dev->struct_mutex);
-
-		if (ecobus & FORCEWAKE_MT_ENABLE) {
-			dev_priv->uncore.funcs.force_wake_get =
-				__gen6_gt_force_wake_mt_get;
-			dev_priv->uncore.funcs.force_wake_put =
-				__gen6_gt_force_wake_mt_put;
-		} else {
-			DRM_INFO("No MT forcewake available on Ivybridge, this can result in issues\n");
-			DRM_INFO("when using vblank-synced partial screen updates.\n");
-			dev_priv->uncore.funcs.force_wake_get =
-				__gen6_gt_force_wake_get;
-			dev_priv->uncore.funcs.force_wake_put =
-				__gen6_gt_force_wake_put;
-		}
-	} else if (IS_GEN6(dev)) {
-		dev_priv->uncore.funcs.force_wake_get =
-			__gen6_gt_force_wake_get;
-		dev_priv->uncore.funcs.force_wake_put =
-			__gen6_gt_force_wake_put;
-	}
-}
-
-void intel_uncore_fini(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	flush_delayed_work(&dev_priv->uncore.force_wake_work);
-
-	/* Paranoia: make sure we have disabled everything before we exit. */
-	intel_uncore_sanitize(dev);
-}
-
 static void intel_uncore_forcewake_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -404,7 +342,8 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 }
 
 #define __i915_read(x) \
-u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool trace) { \
+static u##x \
+i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	unsigned long irqflags; \
 	u##x val = 0; \
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
@@ -431,7 +370,8 @@ __i915_read(64)
 #undef __i915_read
 
 #define __i915_write(x) \
-void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool trace) { \
+static void \
+i915_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
 	unsigned long irqflags; \
 	u32 __fifo_ret = 0; \
 	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
@@ -455,6 +395,77 @@ __i915_write(32)
 __i915_write(64)
 #undef __i915_write
 
+void intel_uncore_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	INIT_DELAYED_WORK(&dev_priv->uncore.force_wake_work,
+			  gen6_force_wake_work);
+
+	if (IS_VALLEYVIEW(dev)) {
+		dev_priv->uncore.funcs.force_wake_get = vlv_force_wake_get;
+		dev_priv->uncore.funcs.force_wake_put = vlv_force_wake_put;
+	} else if (IS_HASWELL(dev)) {
+		dev_priv->uncore.funcs.force_wake_get = __gen6_gt_force_wake_mt_get;
+		dev_priv->uncore.funcs.force_wake_put = __gen6_gt_force_wake_mt_put;
+	} else if (IS_IVYBRIDGE(dev)) {
+		u32 ecobus;
+
+		/* IVB configs may use multi-threaded forcewake */
+
+		/* A small trick here - if the bios hasn't configured
+		 * MT forcewake, and if the device is in RC6, then
+		 * force_wake_mt_get will not wake the device and the
+		 * ECOBUS read will return zero. Which will be
+		 * (correctly) interpreted by the test below as MT
+		 * forcewake being disabled.
+		 */
+		mutex_lock(&dev->struct_mutex);
+		__gen6_gt_force_wake_mt_get(dev_priv);
+		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
+		__gen6_gt_force_wake_mt_put(dev_priv);
+		mutex_unlock(&dev->struct_mutex);
+
+		if (ecobus & FORCEWAKE_MT_ENABLE) {
+			dev_priv->uncore.funcs.force_wake_get =
+				__gen6_gt_force_wake_mt_get;
+			dev_priv->uncore.funcs.force_wake_put =
+				__gen6_gt_force_wake_mt_put;
+		} else {
+			DRM_INFO("No MT forcewake available on Ivybridge, this can result in issues\n");
+			DRM_INFO("when using vblank-synced partial screen updates.\n");
+			dev_priv->uncore.funcs.force_wake_get =
+				__gen6_gt_force_wake_get;
+			dev_priv->uncore.funcs.force_wake_put =
+				__gen6_gt_force_wake_put;
+		}
+	} else if (IS_GEN6(dev)) {
+		dev_priv->uncore.funcs.force_wake_get =
+			__gen6_gt_force_wake_get;
+		dev_priv->uncore.funcs.force_wake_put =
+			__gen6_gt_force_wake_put;
+	}
+
+	dev_priv->uncore.funcs.mmio_readb  = i915_read8;
+	dev_priv->uncore.funcs.mmio_readw  = i915_read16;
+	dev_priv->uncore.funcs.mmio_readl  = i915_read32;
+	dev_priv->uncore.funcs.mmio_readq  = i915_read64;
+	dev_priv->uncore.funcs.mmio_writeb = i915_write8;
+	dev_priv->uncore.funcs.mmio_writew = i915_write16;
+	dev_priv->uncore.funcs.mmio_writel = i915_write32;
+	dev_priv->uncore.funcs.mmio_writeq = i915_write64;
+}
+
+void intel_uncore_fini(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	flush_delayed_work(&dev_priv->uncore.force_wake_work);
+
+	/* Paranoia: make sure we have disabled everything before we exit. */
+	intel_uncore_sanitize(dev);
+}
+
 static const struct register_whitelist {
 	uint64_t offset;
 	uint32_t size;
-- 
1.8.4

[PATCH 4/7] drm/i915: Extra common MMIO lines

[Ben Widawsky]

Just to make the churn and code duplication in upcoming patches a bit
less, turn code which is common to all GEN MMIO functions into a macro.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 6098480..01a3bec 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -341,12 +341,20 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 	}
 }
 
+#define REG_READ_HEADER(x) \
+	unsigned long irqflags; \
+	u##x val = 0; \
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
+
+#define REG_READ_FOOTER \
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
+	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
+	return val
+
 #define __i915_read(x) \
 static u##x \
 i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
-	unsigned long irqflags; \
-	u##x val = 0; \
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
+	REG_READ_HEADER(x); \
 	if (dev_priv->info->gen == 5) \
 		ilk_dummy_write(dev_priv); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
@@ -358,9 +366,7 @@ i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	} else { \
 		val = __raw_i915_read##x(dev_priv, reg); \
 	} \
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
-	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
-	return val; \
+	REG_READ_FOOTER; \
 }
 
 __i915_read(8)
@@ -368,14 +374,19 @@ __i915_read(16)
 __i915_read(32)
 __i915_read(64)
 #undef __i915_read
+#undef REG_READ_FOOTER
+#undef REG_READ_HEADER
+
+#define REG_WRITE_HEADER \
+	unsigned long irqflags; \
+	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
 
 #define __i915_write(x) \
 static void \
 i915_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
-	unsigned long irqflags; \
 	u32 __fifo_ret = 0; \
-	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
+	REG_WRITE_HEADER; \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
@@ -394,6 +405,7 @@ __i915_write(16)
 __i915_write(32)
 __i915_write(64)
 #undef __i915_write
+#undef REG_WRITE_HEADER
 
 void intel_uncore_init(struct drm_device *dev)
 {
-- 
1.8.4

[PATCH 4/7] [v2] drm/i915: Extract common MMIO lines

[Ben Widawsky]

Just to make the churn and code duplication in upcoming patches a bit
less, turn code which is common to all GEN MMIO functions into a macro.

v2: Fix typo in subject

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 6098480..01a3bec 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -341,12 +341,20 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 	}
 }
 
+#define REG_READ_HEADER(x) \
+	unsigned long irqflags; \
+	u##x val = 0; \
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
+
+#define REG_READ_FOOTER \
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
+	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
+	return val
+
 #define __i915_read(x) \
 static u##x \
 i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
-	unsigned long irqflags; \
-	u##x val = 0; \
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
+	REG_READ_HEADER(x); \
 	if (dev_priv->info->gen == 5) \
 		ilk_dummy_write(dev_priv); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
@@ -358,9 +366,7 @@ i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	} else { \
 		val = __raw_i915_read##x(dev_priv, reg); \
 	} \
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
-	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
-	return val; \
+	REG_READ_FOOTER; \
 }
 
 __i915_read(8)
@@ -368,14 +374,19 @@ __i915_read(16)
 __i915_read(32)
 __i915_read(64)
 #undef __i915_read
+#undef REG_READ_FOOTER
+#undef REG_READ_HEADER
+
+#define REG_WRITE_HEADER \
+	unsigned long irqflags; \
+	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
 
 #define __i915_write(x) \
 static void \
 i915_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
-	unsigned long irqflags; \
 	u32 __fifo_ret = 0; \
-	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
+	REG_WRITE_HEADER; \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
@@ -394,6 +405,7 @@ __i915_write(16)
 __i915_write(32)
 __i915_write(64)
 #undef __i915_write
+#undef REG_WRITE_HEADER
 
 void intel_uncore_init(struct drm_device *dev)
 {
-- 
1.8.4

[PATCH 5/7] drm/i915: Create GEN specific read MMIO

[Ben Widawsky]

Extracting the MMIO read functionality makes per gen handling a bit
simpler, and the overall function a lot easier to read. The increasing
complexity of reads doesn't get too bad as the generation number
increases:

gen[2-4]: Nothing special
gen5: ILK dummy write workaround
gen6+: forcewake shenanigans

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_uncore.c | 72 ++++++++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 01a3bec..bcb4cfd 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -351,12 +351,27 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
 	return val
 
-#define __i915_read(x) \
+#define __gen4_read(x) \
 static u##x \
-i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
+gen4_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
+	REG_READ_HEADER(x); \
+	val = __raw_i915_read##x(dev_priv, reg); \
+	REG_READ_FOOTER; \
+}
+
+#define __gen5_read(x) \
+static u##x \
+gen5_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
+	REG_READ_HEADER(x); \
+	ilk_dummy_write(dev_priv); \
+	val = __raw_i915_read##x(dev_priv, reg); \
+	REG_READ_FOOTER; \
+}
+
+#define __gen6_read(x) \
+static u##x \
+gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	REG_READ_HEADER(x); \
-	if (dev_priv->info->gen == 5) \
-		ilk_dummy_write(dev_priv); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		if (dev_priv->uncore.forcewake_count == 0) \
 			dev_priv->uncore.funcs.force_wake_get(dev_priv); \
@@ -369,11 +384,22 @@ i915_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	REG_READ_FOOTER; \
 }
 
-__i915_read(8)
-__i915_read(16)
-__i915_read(32)
-__i915_read(64)
-#undef __i915_read
+__gen6_read(8)
+__gen6_read(16)
+__gen6_read(32)
+__gen6_read(64)
+__gen5_read(8)
+__gen5_read(16)
+__gen5_read(32)
+__gen5_read(64)
+__gen4_read(8)
+__gen4_read(16)
+__gen4_read(32)
+__gen4_read(64)
+
+#undef __gen6_read
+#undef __gen5_read
+#undef __gen4_read
 #undef REG_READ_FOOTER
 #undef REG_READ_HEADER
 
@@ -400,6 +426,7 @@ i915_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
 	hsw_unclaimed_reg_check(dev_priv, reg); \
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
 }
+
 __i915_write(8)
 __i915_write(16)
 __i915_write(32)
@@ -458,10 +485,29 @@ void intel_uncore_init(struct drm_device *dev)
 			__gen6_gt_force_wake_put;
 	}
 
-	dev_priv->uncore.funcs.mmio_readb  = i915_read8;
-	dev_priv->uncore.funcs.mmio_readw  = i915_read16;
-	dev_priv->uncore.funcs.mmio_readl  = i915_read32;
-	dev_priv->uncore.funcs.mmio_readq  = i915_read64;
+	switch (INTEL_INFO(dev)->gen) {
+	case 7:
+	case 6:
+		dev_priv->uncore.funcs.mmio_readb  = gen6_read8;
+		dev_priv->uncore.funcs.mmio_readw  = gen6_read16;
+		dev_priv->uncore.funcs.mmio_readl  = gen6_read32;
+		dev_priv->uncore.funcs.mmio_readq  = gen6_read64;
+		break;
+	case 5:
+		dev_priv->uncore.funcs.mmio_readb  = gen5_read8;
+		dev_priv->uncore.funcs.mmio_readw  = gen5_read16;
+		dev_priv->uncore.funcs.mmio_readl  = gen5_read32;
+		dev_priv->uncore.funcs.mmio_readq  = gen5_read64;
+		break;
+	case 4:
+	case 3:
+	case 2:
+		dev_priv->uncore.funcs.mmio_readb  = gen4_read8;
+		dev_priv->uncore.funcs.mmio_readw  = gen4_read16;
+		dev_priv->uncore.funcs.mmio_readl  = gen4_read32;
+		dev_priv->uncore.funcs.mmio_readq  = gen4_read64;
+		break;
+	}
 	dev_priv->uncore.funcs.mmio_writeb = i915_write8;
 	dev_priv->uncore.funcs.mmio_writew = i915_write16;
 	dev_priv->uncore.funcs.mmio_writel = i915_write32;
-- 
1.8.4

[PATCH 6/7] drm/i915: Create GEN specific write MMIO

[Ben Widawsky]

Similar to the previous patch which implemented GEN specific reads; this
patch does the same for writes. Writes have a bit of adding complexity
due to the FPGA_DBG feature of HSW plus:

gen[2-4]: nothing special
gen5: ILK dummy write
gen[6-7]: forcewake shenanigans
gen[HSW}: forcewake shenanigans + FPGA_DBG

I was a bit torn about whether or not to combine 6-HSW as one function,
since the FPGA_DBG is cleanly separated, and it wouldn't make the 6-7
MMIO too messy. In the end, I chose the clearest possible solution which
splits out HSW.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_uncore.c | 87 +++++++++++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bcb4cfd..cd56212 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -408,16 +408,46 @@ __gen4_read(64)
 	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
 
-#define __i915_write(x) \
+#define __gen4_write(x) \
 static void \
-i915_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
+gen4_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
+	REG_WRITE_HEADER; \
+	__raw_i915_write##x(dev_priv, reg, val); \
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
+}
+
+#define __gen5_write(x) \
+static void \
+gen5_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
+	REG_WRITE_HEADER; \
+	ilk_dummy_write(dev_priv); \
+	__raw_i915_write##x(dev_priv, reg, val); \
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
+}
+
+#define __gen6_write(x) \
+static void \
+gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
+	u32 __fifo_ret = 0; \
+	REG_WRITE_HEADER; \
+	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
+		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
+	} \
+	__raw_i915_write##x(dev_priv, reg, val); \
+	if (unlikely(__fifo_ret)) { \
+		gen6_gt_check_fifodbg(dev_priv); \
+	} \
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
+}
+
+#define __hsw_write(x) \
+static void \
+hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
 	u32 __fifo_ret = 0; \
 	REG_WRITE_HEADER; \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
-	if (dev_priv->info->gen == 5) \
-		ilk_dummy_write(dev_priv); \
 	hsw_unclaimed_reg_clear(dev_priv, reg); \
 	__raw_i915_write##x(dev_priv, reg, val); \
 	if (unlikely(__fifo_ret)) { \
@@ -427,11 +457,27 @@ i915_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
 }
 
-__i915_write(8)
-__i915_write(16)
-__i915_write(32)
-__i915_write(64)
-#undef __i915_write
+__hsw_write(8)
+__hsw_write(16)
+__hsw_write(32)
+__hsw_write(64)
+__gen6_write(8)
+__gen6_write(16)
+__gen6_write(32)
+__gen6_write(64)
+__gen5_write(8)
+__gen5_write(16)
+__gen5_write(32)
+__gen5_write(64)
+__gen4_write(8)
+__gen4_write(16)
+__gen4_write(32)
+__gen4_write(64)
+
+#undef __hsw_write
+#undef __gen6_write
+#undef __gen5_write
+#undef __gen4_write
 #undef REG_WRITE_HEADER
 
 void intel_uncore_init(struct drm_device *dev)
@@ -488,12 +534,27 @@ void intel_uncore_init(struct drm_device *dev)
 	switch (INTEL_INFO(dev)->gen) {
 	case 7:
 	case 6:
+		if (IS_HASWELL(dev)) {
+			dev_priv->uncore.funcs.mmio_writeb  = hsw_write8;
+			dev_priv->uncore.funcs.mmio_writew  = hsw_write16;
+			dev_priv->uncore.funcs.mmio_writel  = hsw_write32;
+			dev_priv->uncore.funcs.mmio_writeq  = hsw_write64;
+		} else {
+			dev_priv->uncore.funcs.mmio_writeb  = gen6_write8;
+			dev_priv->uncore.funcs.mmio_writew  = gen6_write16;
+			dev_priv->uncore.funcs.mmio_writel  = gen6_write32;
+			dev_priv->uncore.funcs.mmio_writeq  = gen6_write64;
+		}
 		dev_priv->uncore.funcs.mmio_readb  = gen6_read8;
 		dev_priv->uncore.funcs.mmio_readw  = gen6_read16;
 		dev_priv->uncore.funcs.mmio_readl  = gen6_read32;
 		dev_priv->uncore.funcs.mmio_readq  = gen6_read64;
 		break;
 	case 5:
+		dev_priv->uncore.funcs.mmio_writeb  = gen5_write8;
+		dev_priv->uncore.funcs.mmio_writew  = gen5_write16;
+		dev_priv->uncore.funcs.mmio_writel  = gen5_write32;
+		dev_priv->uncore.funcs.mmio_writeq  = gen5_write64;
 		dev_priv->uncore.funcs.mmio_readb  = gen5_read8;
 		dev_priv->uncore.funcs.mmio_readw  = gen5_read16;
 		dev_priv->uncore.funcs.mmio_readl  = gen5_read32;
@@ -502,16 +563,16 @@ void intel_uncore_init(struct drm_device *dev)
 	case 4:
 	case 3:
 	case 2:
+		dev_priv->uncore.funcs.mmio_writeb  = gen4_write8;
+		dev_priv->uncore.funcs.mmio_writew  = gen4_write16;
+		dev_priv->uncore.funcs.mmio_writel  = gen4_write32;
+		dev_priv->uncore.funcs.mmio_writeq  = gen4_write64;
 		dev_priv->uncore.funcs.mmio_readb  = gen4_read8;
 		dev_priv->uncore.funcs.mmio_readw  = gen4_read16;
 		dev_priv->uncore.funcs.mmio_readl  = gen4_read32;
 		dev_priv->uncore.funcs.mmio_readq  = gen4_read64;
 		break;
 	}
-	dev_priv->uncore.funcs.mmio_writeb = i915_write8;
-	dev_priv->uncore.funcs.mmio_writew = i915_write16;
-	dev_priv->uncore.funcs.mmio_writel = i915_write32;
-	dev_priv->uncore.funcs.mmio_writeq = i915_write64;
 }
 
 void intel_uncore_fini(struct drm_device *dev)
-- 
1.8.4

[PATCH 7/7] drm/i915: Mark gen specific conditions 'likely'

[Ben Widawsky]

Now that MMIO has been split up into gen specific functions it is
obvious when HAS_FPGA_DBG_UNCLAIMED, HAS_FORCE_WAKE are needed.

I'm a bit on the fence whether or not to even have the checks at all.
For now I am leaving them there because I think they are valuable for
debug, and early silicon bringup. In these cases, one could hardcode the
appropriate values, and not have forcewake, and fpga_dbg.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_uncore.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index cd56212..30fbee0 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -307,7 +307,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 
 /* We give fast paths for the really cool registers */
 #define NEEDS_FORCE_WAKE(dev_priv, reg) \
-	((HAS_FORCE_WAKE((dev_priv)->dev)) && \
+	((likely(HAS_FORCE_WAKE((dev_priv)->dev))) && \
 	 ((reg) < 0x40000) &&            \
 	 ((reg) != FORCEWAKE))
 
@@ -323,7 +323,7 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
 static void
 hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
 {
-	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
+	if (likely(HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev)) &&
 	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
 		DRM_ERROR("Unknown unclaimed register before writing to %x\n",
 			  reg);
@@ -334,7 +334,7 @@ hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
 static void
 hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 {
-	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
+	if (likely(HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev)) &&
 	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
 		DRM_ERROR("Unclaimed write to %x\n", reg);
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
-- 
1.8.4

Re: [PATCH 7/7] drm/i915: Mark gen specific conditions 'likely'

[Daniel Vetter]

On Fri, Oct 04, 2013 at 09:22:55PM -0700, Ben Widawsky wrote:
> Now that MMIO has been split up into gen specific functions it is
> obvious when HAS_FPGA_DBG_UNCLAIMED, HAS_FORCE_WAKE are needed.
> 
> I'm a bit on the fence whether or not to even have the checks at all.
> For now I am leaving them there because I think they are valuable for
> debug, and early silicon bringup. In these cases, one could hardcode the
> appropriate values, and not have forcewake, and fpga_dbg.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Can't we just completely fold away the gen/platform checks now?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index cd56212..30fbee0 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -307,7 +307,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  
>  /* We give fast paths for the really cool registers */
>  #define NEEDS_FORCE_WAKE(dev_priv, reg) \
> -	((HAS_FORCE_WAKE((dev_priv)->dev)) && \
> +	((likely(HAS_FORCE_WAKE((dev_priv)->dev))) && \
>  	 ((reg) < 0x40000) &&            \
>  	 ((reg) != FORCEWAKE))
>  
> @@ -323,7 +323,7 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
>  static void
>  hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
>  {
> -	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
> +	if (likely(HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev)) &&
>  	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
>  		DRM_ERROR("Unknown unclaimed register before writing to %x\n",
>  			  reg);
> @@ -334,7 +334,7 @@ hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
>  static void
>  hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
>  {
> -	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
> +	if (likely(HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev)) &&
>  	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
>  		DRM_ERROR("Unclaimed write to %x\n", reg);
>  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 7/7] drm/i915: Remove gen specific checks in MMIO

[Ben Widawsky]

Now that MMIO has been split up into gen specific functions it is
obvious when HAS_FPGA_DBG_UNCLAIMED, HAS_FORCE_WAKE are needed. As such,
we can remove this extraneous condition.

As a result of this, as well as previously existing function pointers
for forcewake, we no longer need the has_force_wake member in the device
specific data structure.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.c         |  5 +----
 drivers/gpu/drm/i915/i915_drv.h         |  3 ---
 drivers/gpu/drm/i915/intel_ringbuffer.c |  6 ++----
 drivers/gpu/drm/i915/intel_uncore.c     | 16 +++++++++-------
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0fc9658..8ac8e81 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -257,7 +257,6 @@ static const struct intel_device_info intel_sandybridge_d_info = {
 	.has_bsd_ring = 1,
 	.has_blt_ring = 1,
 	.has_llc = 1,
-	.has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_sandybridge_m_info = {
@@ -267,7 +266,6 @@ static const struct intel_device_info intel_sandybridge_m_info = {
 	.has_bsd_ring = 1,
 	.has_blt_ring = 1,
 	.has_llc = 1,
-	.has_force_wake = 1,
 };
 
 #define GEN7_FEATURES  \
@@ -275,8 +273,7 @@ static const struct intel_device_info intel_sandybridge_m_info = {
 	.need_gfx_hws = 1, .has_hotplug = 1, \
 	.has_bsd_ring = 1, \
 	.has_blt_ring = 1, \
-	.has_llc = 1, \
-	.has_force_wake = 1
+	.has_llc = 1
 
 static const struct intel_device_info intel_ivybridge_d_info = {
 	GEN7_FEATURES,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a054b30..ee9cbe2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -441,7 +441,6 @@ struct intel_uncore {
 	func(is_valleyview) sep \
 	func(is_haswell) sep \
 	func(is_preliminary) sep \
-	func(has_force_wake) sep \
 	func(has_fbc) sep \
 	func(has_pipe_cxsr) sep \
 	func(has_hotplug) sep \
@@ -1729,8 +1728,6 @@ struct drm_i915_file_private {
 #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
 #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE)
 
-#define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake)
-
 /* DPF == dynamic parity feature */
 #define HAS_L3_DPF(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
 #define NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_DPF(dev))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b67104a..4e108fc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -395,8 +395,7 @@ static int init_ring_common(struct intel_ring_buffer *ring)
 	int ret = 0;
 	u32 head;
 
-	if (HAS_FORCE_WAKE(dev))
-		gen6_gt_force_wake_get(dev_priv);
+	gen6_gt_force_wake_get(dev_priv);
 
 	if (I915_NEED_GFX_HWS(dev))
 		intel_ring_setup_status_page(ring);
@@ -469,8 +468,7 @@ static int init_ring_common(struct intel_ring_buffer *ring)
 	memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
 
 out:
-	if (HAS_FORCE_WAKE(dev))
-		gen6_gt_force_wake_put(dev_priv);
+	gen6_gt_force_wake_put(dev_priv);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index cd56212..446288e 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -282,6 +282,9 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 {
 	unsigned long irqflags;
 
+	if (!dev_priv->uncore.funcs.force_wake_get)
+		return;
+
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 	if (dev_priv->uncore.forcewake_count++ == 0)
 		dev_priv->uncore.funcs.force_wake_get(dev_priv);
@@ -295,6 +298,9 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
 	unsigned long irqflags;
 
+	if (!dev_priv->uncore.funcs.force_wake_put)
+		return;
+
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 	if (--dev_priv->uncore.forcewake_count == 0) {
 		dev_priv->uncore.forcewake_count++;
@@ -307,9 +313,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 
 /* We give fast paths for the really cool registers */
 #define NEEDS_FORCE_WAKE(dev_priv, reg) \
-	((HAS_FORCE_WAKE((dev_priv)->dev)) && \
-	 ((reg) < 0x40000) &&            \
-	 ((reg) != FORCEWAKE))
+	 ((reg) < 0x40000 && (reg) != FORCEWAKE)
 
 static void
 ilk_dummy_write(struct drm_i915_private *dev_priv)
@@ -323,8 +327,7 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
 static void
 hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
 {
-	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
-	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
+	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
 		DRM_ERROR("Unknown unclaimed register before writing to %x\n",
 			  reg);
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
@@ -334,8 +337,7 @@ hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
 static void
 hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 {
-	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv->dev) &&
-	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
+	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
 		DRM_ERROR("Unclaimed write to %x\n", reg);
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
 	}
-- 
1.8.4

Re: [PATCH 1/7] drm/i915: Prevent using uninitialized MMIO funcs

[Damien Lespiau]

On Fri, Oct 04, 2013 at 09:22:49PM -0700, Ben Widawsky wrote:
> For upcoming patches which will have GEN specific MMIO functions, we'll
> need to initialize the uncore data structure earlier than we do today.
> 
> If we do not do this, the following will be problematic:
> 
> intel_uncore_sanitize
> 	intel_disable_gt_powersave
> 		gen6_disable_rps
> 			I915_WRITE(GEN6_RC_CONTROL, 0); <--- MMIO
> intel_uncore_init // initializes MMIO
> 
> By initializing the function pointers first, we should be safe.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

For the whole series:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f221631..0a84cd5 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1621,8 +1621,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	intel_irq_init(dev);
>  	intel_pm_init(dev);
> -	intel_uncore_sanitize(dev);
>  	intel_uncore_init(dev);
> +	intel_uncore_sanitize(dev);
>  
>  	/* Try to make sure MCHBAR is enabled before poking at it */
>  	intel_setup_mchbar(dev);
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915: Prevent using uninitialized MMIO funcs

[Daniel Vetter]

On Tue, Oct 08, 2013 at 02:38:22PM +0100, Damien Lespiau wrote:
> On Fri, Oct 04, 2013 at 09:22:49PM -0700, Ben Widawsky wrote:
> > For upcoming patches which will have GEN specific MMIO functions, we'll
> > need to initialize the uncore data structure earlier than we do today.
> > 
> > If we do not do this, the following will be problematic:
> > 
> > intel_uncore_sanitize
> > 	intel_disable_gt_powersave
> > 		gen6_disable_rps
> > 			I915_WRITE(GEN6_RC_CONTROL, 0); <--- MMIO
> > intel_uncore_init // initializes MMIO
> > 
> > By initializing the function pointers first, we should be safe.
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> For the whole series:
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

All merged, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch