[PATCH 00/10] Haswell force wake/rps v3

[PATCH 00/10] Haswell force wake/rps v3

[Eugeni Dodonov]

Hi,

Addressing bike color suggestions in the previous series:
 - simplify RPS function to reduce the number of changes. With this, we still
   write the RC6p/RC6pp bits, but those should be ignored on Haswell and
   experimental results have demonstrated no issues with it so far.
 - *really* use the wait_for_atomic for waiting
 - it is still possible (although unlikely) to use non-mt force wake on
   Haswell, so account for that as well.
 - split the rc6 enabling by default into a separate patch to help with
   bisection in the unthinkable case that rc6 will haunt our lives once again
 - move all those changes into intel_pm module to leave i915_drv cleaner

Eugeni

Chris Wilson (2):
  drm/i915: Group the GT routines together in both code and vtable
  drm/i915: Implement w/a for sporadic read failures on waking from rc6

Eugeni Dodonov (8):
  drm/i915: support Haswell force waking
  drm/i915: add RPS configuration for Haswell
  drm/i915: slightly improve gt enable/disable routines
  drm/i915: enable RC6 by default on Haswell
  drm/i915: disable RC6 when disabling rps
  drm/i915: introduce haswell_init_clock_gating
  drm/i915: enable RC6 workaround on Haswell
  drm/i915: move force wake support into intel_pm

 drivers/gpu/drm/i915/i915_dma.c      |   2 +-
 drivers/gpu/drm/i915/i915_drv.c      | 138 +--------------
 drivers/gpu/drm/i915/i915_drv.h      |  17 +-
 drivers/gpu/drm/i915/i915_reg.h      |  11 ++
 drivers/gpu/drm/i915/intel_display.c |   3 -
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 drivers/gpu/drm/i915/intel_pm.c      | 335 ++++++++++++++++++++++++++++++-----
 7 files changed, 312 insertions(+), 195 deletions(-)

-- 
1.7.11.1

[PATCH 01/10] drm/i915: Group the GT routines together in both code and vtable

[Eugeni Dodonov]

From: Chris Wilson <chris@chris-wilson.co.uk>

Tidy up the routines for interacting with the GT (in particular the
forcewake dance) which are scattered throughout the code in a single
structure.

v2: use wait_for_atomic for polling.

v3: *really* use wait_for_atomic for polling.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |   2 +-
 drivers/gpu/drm/i915/i915_drv.c      | 101 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_drv.h      |  17 +++---
 drivers/gpu/drm/i915/intel_display.c |   3 --
 drivers/gpu/drm/i915/intel_pm.c      |  30 -----------
 5 files changed, 73 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2166519..4dc7697 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1548,6 +1548,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	}
 
 	intel_irq_init(dev);
+	intel_gt_init(dev);
 
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev);
@@ -1580,7 +1581,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (!IS_I945G(dev) && !IS_I945GM(dev))
 		pci_enable_msi(dev->pdev);
 
-	spin_lock_init(&dev_priv->gt_lock);
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->error_lock);
 	spin_lock_init(&dev_priv->rps_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 79be879..928b667 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -32,6 +32,7 @@
 #include "drm.h"
 #include "i915_drm.h"
 #include "i915_drv.h"
+#include "i915_trace.h"
 #include "intel_drv.h"
 
 #include <linux/console.h>
@@ -432,36 +433,26 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 	return 1;
 }
 
-void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
+static 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))
-		udelay(10);
+	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0, 500))
+		DRM_ERROR("Force wake wait timed out\n");
 
 	I915_WRITE_NOTRACE(FORCEWAKE, 1);
-	POSTING_READ(FORCEWAKE);
 
-	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0)
-		udelay(10);
+	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1), 500))
+		DRM_ERROR("Force wake wait timed out\n");
 }
 
-void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
+static 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))
-		udelay(10);
+	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0, 500))
+		DRM_ERROR("Force wake wait timed out\n");
 
 	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
-	POSTING_READ(FORCEWAKE_MT);
 
-	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0)
-		udelay(10);
+	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1), 500))
+		DRM_ERROR("Force wake wait timed out\n");
 }
 
 /*
@@ -476,7 +467,7 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 
 	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
 	if (dev_priv->forcewake_count++ == 0)
-		dev_priv->display.force_wake_get(dev_priv);
+		dev_priv->gt.force_wake_get(dev_priv);
 	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
 }
 
@@ -489,14 +480,14 @@ static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
 		I915_WRITE_NOTRACE(GTFIFODBG, GT_FIFO_CPU_ERROR_MASK);
 }
 
-void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
+static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE, 0);
 	/* The below doubles as a POSTING_READ */
 	gen6_gt_check_fifodbg(dev_priv);
 }
 
-void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
+static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(1));
 	/* The below doubles as a POSTING_READ */
@@ -512,7 +503,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 
 	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
 	if (--dev_priv->forcewake_count == 0)
-		dev_priv->display.force_wake_put(dev_priv);
+		dev_priv->gt.force_wake_put(dev_priv);
 	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
 }
 
@@ -536,12 +527,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-void vlv_force_wake_get(struct drm_i915_private *dev_priv)
+static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
 {
-	int count;
-
-	count = 0;
-
 	/* Already awake? */
 	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
 		return;
@@ -549,18 +536,58 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv)
 	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
 	POSTING_READ(FORCEWAKE_VLV);
 
-	count = 0;
-	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1) == 0)
-		udelay(10);
+	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
+		DRM_ERROR("Force wake wait timed out\n");
 }
 
-void vlv_force_wake_put(struct drm_i915_private *dev_priv)
+static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
 	/* FIXME: confirm VLV behavior with Punit folks */
 	POSTING_READ(FORCEWAKE_VLV);
 }
 
+void intel_gt_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	spin_lock_init(&dev_priv->gt_lock);
+
+	if (IS_VALLEYVIEW(dev)) {
+		dev_priv->gt.force_wake_get = vlv_force_wake_get;
+		dev_priv->gt.force_wake_put = vlv_force_wake_put;
+	} else if (INTEL_INFO(dev)->gen >= 6) {
+		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
+		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
+
+		/* IVB configs may use multi-threaded forcewake */
+		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
+			u32 ecobus;
+
+			/* 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 = I915_READ_NOTRACE(ECOBUS);
+			__gen6_gt_force_wake_mt_put(dev_priv);
+			mutex_unlock(&dev->struct_mutex);
+
+			if (ecobus & FORCEWAKE_MT_ENABLE) {
+				DRM_DEBUG_KMS("Using MT version of forcewake\n");
+				dev_priv->gt.force_wake_get =
+					__gen6_gt_force_wake_mt_get;
+				dev_priv->gt.force_wake_put =
+					__gen6_gt_force_wake_mt_put;
+			}
+		}
+	}
+}
+
 static int i915_drm_freeze(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -797,9 +824,9 @@ static int gen6_do_reset(struct drm_device *dev)
 
 	/* If reset with a user forcewake, try to restore, otherwise turn it off */
 	if (dev_priv->forcewake_count)
-		dev_priv->display.force_wake_get(dev_priv);
+		dev_priv->gt.force_wake_get(dev_priv);
 	else
-		dev_priv->display.force_wake_put(dev_priv);
+		dev_priv->gt.force_wake_put(dev_priv);
 
 	/* Restore fifo count */
 	dev_priv->gt_fifo_count = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
@@ -1248,10 +1275,10 @@ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
 		unsigned long irqflags; \
 		spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
 		if (dev_priv->forcewake_count == 0) \
-			dev_priv->display.force_wake_get(dev_priv); \
+			dev_priv->gt.force_wake_get(dev_priv); \
 		val = read##y(dev_priv->regs + reg); \
 		if (dev_priv->forcewake_count == 0) \
-			dev_priv->display.force_wake_put(dev_priv); \
+			dev_priv->gt.force_wake_put(dev_priv); \
 		spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
 	} else if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
 		val = read##y(dev_priv->regs + reg + 0x180000);		\
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0c15ab..60f6974 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -262,8 +262,6 @@ struct drm_i915_display_funcs {
 			  struct drm_i915_gem_object *obj);
 	int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			    int x, int y);
-	void (*force_wake_get)(struct drm_i915_private *dev_priv);
-	void (*force_wake_put)(struct drm_i915_private *dev_priv);
 	/* clock updates for mode set */
 	/* cursor updates */
 	/* render clock increase/decrease */
@@ -271,6 +269,11 @@ struct drm_i915_display_funcs {
 	/* pll clock increase/decrease */
 };
 
+struct drm_i915_gt_funcs {
+	void (*force_wake_get)(struct drm_i915_private *dev_priv);
+	void (*force_wake_put)(struct drm_i915_private *dev_priv);
+};
+
 struct intel_device_info {
 	u8 gen;
 	u8 is_mobile:1;
@@ -362,6 +365,8 @@ typedef struct drm_i915_private {
 	int relative_constants_mode;
 
 	void __iomem *regs;
+
+	struct drm_i915_gt_funcs gt;
 	/** gt_fifo_count and the subsequent register write are synchronized
 	 * with dev->struct_mutex. */
 	unsigned gt_fifo_count;
@@ -1200,6 +1205,7 @@ void i915_hangcheck_elapsed(unsigned long data);
 void i915_handle_error(struct drm_device *dev, bool wedged);
 
 extern void intel_irq_init(struct drm_device *dev);
+extern void intel_gt_init(struct drm_device *dev);
 
 void i915_error_state_free(struct kref *error_ref);
 
@@ -1517,13 +1523,6 @@ extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
 extern int intel_enable_rc6(const struct drm_device *dev);
 
 extern bool i915_semaphore_is_enabled(struct drm_device *dev);
-extern void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv);
-extern void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv);
-extern void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv);
-extern void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv);
-
-extern void vlv_force_wake_get(struct drm_i915_private *dev_priv);
-extern void vlv_force_wake_put(struct drm_i915_private *dev_priv);
 
 /* overlay */
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0438a10..3ca91e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7121,9 +7121,6 @@ static void intel_init_display(struct drm_device *dev)
 			dev_priv->display.write_eld = ironlake_write_eld;
 		} else
 			dev_priv->display.update_wm = NULL;
-	} else if (IS_VALLEYVIEW(dev)) {
-		dev_priv->display.force_wake_get = vlv_force_wake_get;
-		dev_priv->display.force_wake_put = vlv_force_wake_put;
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
 	}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a1d3da7..29720d2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3763,34 +3763,6 @@ void intel_init_pm(struct drm_device *dev)
 
 	/* For FIFO watermark updates */
 	if (HAS_PCH_SPLIT(dev)) {
-		dev_priv->display.force_wake_get = __gen6_gt_force_wake_get;
-		dev_priv->display.force_wake_put = __gen6_gt_force_wake_put;
-
-		/* IVB configs may use multi-threaded forcewake */
-		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-			u32	ecobus;
-
-			/* 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 = I915_READ_NOTRACE(ECOBUS);
-			__gen6_gt_force_wake_mt_put(dev_priv);
-			mutex_unlock(&dev->struct_mutex);
-
-			if (ecobus & FORCEWAKE_MT_ENABLE) {
-				DRM_DEBUG_KMS("Using MT version of forcewake\n");
-				dev_priv->display.force_wake_get =
-					__gen6_gt_force_wake_mt_get;
-				dev_priv->display.force_wake_put =
-					__gen6_gt_force_wake_mt_put;
-			}
-		}
-
 		if (HAS_PCH_IBX(dev))
 			dev_priv->display.init_pch_clock_gating = ibx_init_clock_gating;
 		else if (HAS_PCH_CPT(dev))
@@ -3846,8 +3818,6 @@ void intel_init_pm(struct drm_device *dev)
 		dev_priv->display.update_wm = valleyview_update_wm;
 		dev_priv->display.init_clock_gating =
 			valleyview_init_clock_gating;
-		dev_priv->display.force_wake_get = vlv_force_wake_get;
-		dev_priv->display.force_wake_put = vlv_force_wake_put;
 	} else if (IS_PINEVIEW(dev)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
 					    dev_priv->is_ddr3,
-- 
1.7.11.1

Re: [PATCH 01/10] drm/i915: Group the GT routines together in both code and vtable

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:02 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Tidy up the routines for interacting with the GT (in particular the
> forcewake dance) which are scattered throughout the code in a single
> structure.
> 
> v2: use wait_for_atomic for polling.
> 
> v3: *really* use wait_for_atomic for polling.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

comments below, but still it's
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |   2 +-
>  drivers/gpu/drm/i915/i915_drv.c      | 101 ++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/i915_drv.h      |  17 +++---
>  drivers/gpu/drm/i915/intel_display.c |   3 --
>  drivers/gpu/drm/i915/intel_pm.c      |  30 -----------
>  5 files changed, 73 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2166519..4dc7697 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1548,6 +1548,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	}
>  
>  	intel_irq_init(dev);
> +	intel_gt_init(dev);
>  
>  	/* Try to make sure MCHBAR is enabled before poking at it */
>  	intel_setup_mchbar(dev);
> @@ -1580,7 +1581,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	if (!IS_I945G(dev) && !IS_I945GM(dev))
>  		pci_enable_msi(dev->pdev);
>  
> -	spin_lock_init(&dev_priv->gt_lock);
>  	spin_lock_init(&dev_priv->irq_lock);
>  	spin_lock_init(&dev_priv->error_lock);
>  	spin_lock_init(&dev_priv->rps_lock);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 79be879..928b667 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -32,6 +32,7 @@
>  #include "drm.h"
>  #include "i915_drm.h"
>  #include "i915_drv.h"
> +#include "i915_trace.h"
>  #include "intel_drv.h"
>  
>  #include <linux/console.h>
> @@ -432,36 +433,26 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  	return 1;
>  }
>  
> -void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
> +static 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))
> -		udelay(10);
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0, 500))
> +		DRM_ERROR("Force wake wait timed out\n");

(Applies to all instances)
I'd really prefer a WARN_ON to get a backtrace for such cases. It just
dawned on me that since we moved to separate locking, does it make sense
to do a wedge check here also?

>  
>  	I915_WRITE_NOTRACE(FORCEWAKE, 1);
> -	POSTING_READ(FORCEWAKE);

This is an unnecessary optimization than only makes things a bit more
difficult to read IMO.

>  
> -	count = 0;
> -	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0)
> -		udelay(10);
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1), 500))
> +		DRM_ERROR("Force wake wait timed out\n");
>  }
>  
> -void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> +static 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))
> -		udelay(10);
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0, 500))
> +		DRM_ERROR("Force wake wait timed out\n");
>  
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
> -	POSTING_READ(FORCEWAKE_MT);
>  
> -	count = 0;
> -	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0)
> -		udelay(10);
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1), 500))
> +		DRM_ERROR("Force wake wait timed out\n");
>  }
>  
>  /*
> @@ -476,7 +467,7 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  
>  	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
>  	if (dev_priv->forcewake_count++ == 0)
> -		dev_priv->display.force_wake_get(dev_priv);
> +		dev_priv->gt.force_wake_get(dev_priv);
>  	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
>  }
>  
> @@ -489,14 +480,14 @@ static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
>  		I915_WRITE_NOTRACE(GTFIFODBG, GT_FIFO_CPU_ERROR_MASK);
>  }
>  
> -void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> +static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE, 0);
>  	/* The below doubles as a POSTING_READ */
>  	gen6_gt_check_fifodbg(dev_priv);
>  }
>  
> -void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
> +static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(1));
>  	/* The below doubles as a POSTING_READ */
> @@ -512,7 +503,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  
>  	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
>  	if (--dev_priv->forcewake_count == 0)
> -		dev_priv->display.force_wake_put(dev_priv);
> +		dev_priv->gt.force_wake_put(dev_priv);
>  	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
>  }
>  
> @@ -536,12 +527,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>  	return ret;
>  }
>  
> -void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> +static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
>  {
> -	int count;
> -
> -	count = 0;
> -
>  	/* Already awake? */
>  	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
>  		return;
> @@ -549,18 +536,58 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv)
>  	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
>  	POSTING_READ(FORCEWAKE_VLV);
>  
> -	count = 0;
> -	while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1) == 0)
> -		udelay(10);
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
> +		DRM_ERROR("Force wake wait timed out\n");
>  }
>  
> -void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> +static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
>  	/* FIXME: confirm VLV behavior with Punit folks */
>  	POSTING_READ(FORCEWAKE_VLV);
>  }
>  
> +void intel_gt_init(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	spin_lock_init(&dev_priv->gt_lock);
> +
> +	if (IS_VALLEYVIEW(dev)) {
> +		dev_priv->gt.force_wake_get = vlv_force_wake_get;
> +		dev_priv->gt.force_wake_put = vlv_force_wake_put;
> +	} else if (INTEL_INFO(dev)->gen >= 6) {
> +		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
> +		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
> +
> +		/* IVB configs may use multi-threaded forcewake */
> +		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> +			u32 ecobus;
> +
> +			/* 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 = I915_READ_NOTRACE(ECOBUS);
> +			__gen6_gt_force_wake_mt_put(dev_priv);
> +			mutex_unlock(&dev->struct_mutex);
> +
> +			if (ecobus & FORCEWAKE_MT_ENABLE) {
> +				DRM_DEBUG_KMS("Using MT version of forcewake\n");
> +				dev_priv->gt.force_wake_get =
> +					__gen6_gt_force_wake_mt_get;
> +				dev_priv->gt.force_wake_put =
> +					__gen6_gt_force_wake_mt_put;
> +			}
> +		}
> +	}
> +}
> +
>  static int i915_drm_freeze(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -797,9 +824,9 @@ static int gen6_do_reset(struct drm_device *dev)
>  
>  	/* If reset with a user forcewake, try to restore, otherwise turn it off */
>  	if (dev_priv->forcewake_count)
> -		dev_priv->display.force_wake_get(dev_priv);
> +		dev_priv->gt.force_wake_get(dev_priv);
>  	else
> -		dev_priv->display.force_wake_put(dev_priv);
> +		dev_priv->gt.force_wake_put(dev_priv);
>  
>  	/* Restore fifo count */
>  	dev_priv->gt_fifo_count = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> @@ -1248,10 +1275,10 @@ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
>  		unsigned long irqflags; \
>  		spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
>  		if (dev_priv->forcewake_count == 0) \
> -			dev_priv->display.force_wake_get(dev_priv); \
> +			dev_priv->gt.force_wake_get(dev_priv); \
>  		val = read##y(dev_priv->regs + reg); \
>  		if (dev_priv->forcewake_count == 0) \
> -			dev_priv->display.force_wake_put(dev_priv); \
> +			dev_priv->gt.force_wake_put(dev_priv); \
>  		spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
>  	} else if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
>  		val = read##y(dev_priv->regs + reg + 0x180000);		\
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0c15ab..60f6974 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -262,8 +262,6 @@ struct drm_i915_display_funcs {
>  			  struct drm_i915_gem_object *obj);
>  	int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  			    int x, int y);
> -	void (*force_wake_get)(struct drm_i915_private *dev_priv);
> -	void (*force_wake_put)(struct drm_i915_private *dev_priv);
>  	/* clock updates for mode set */
>  	/* cursor updates */
>  	/* render clock increase/decrease */
> @@ -271,6 +269,11 @@ struct drm_i915_display_funcs {
>  	/* pll clock increase/decrease */
>  };
>  
> +struct drm_i915_gt_funcs {
> +	void (*force_wake_get)(struct drm_i915_private *dev_priv);
> +	void (*force_wake_put)(struct drm_i915_private *dev_priv);
> +};
> +
>  struct intel_device_info {
>  	u8 gen;
>  	u8 is_mobile:1;
> @@ -362,6 +365,8 @@ typedef struct drm_i915_private {
>  	int relative_constants_mode;
>  
>  	void __iomem *regs;
> +
> +	struct drm_i915_gt_funcs gt;
>  	/** gt_fifo_count and the subsequent register write are synchronized
>  	 * with dev->struct_mutex. */
>  	unsigned gt_fifo_count;
> @@ -1200,6 +1205,7 @@ void i915_hangcheck_elapsed(unsigned long data);
>  void i915_handle_error(struct drm_device *dev, bool wedged);
>  
>  extern void intel_irq_init(struct drm_device *dev);
> +extern void intel_gt_init(struct drm_device *dev);
>  
>  void i915_error_state_free(struct kref *error_ref);
>  
> @@ -1517,13 +1523,6 @@ extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
>  extern int intel_enable_rc6(const struct drm_device *dev);
>  
>  extern bool i915_semaphore_is_enabled(struct drm_device *dev);
> -extern void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv);
> -extern void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv);
> -extern void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv);
> -extern void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv);
> -
> -extern void vlv_force_wake_get(struct drm_i915_private *dev_priv);
> -extern void vlv_force_wake_put(struct drm_i915_private *dev_priv);
>  
>  /* overlay */
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0438a10..3ca91e2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7121,9 +7121,6 @@ static void intel_init_display(struct drm_device *dev)
>  			dev_priv->display.write_eld = ironlake_write_eld;
>  		} else
>  			dev_priv->display.update_wm = NULL;
> -	} else if (IS_VALLEYVIEW(dev)) {
> -		dev_priv->display.force_wake_get = vlv_force_wake_get;
> -		dev_priv->display.force_wake_put = vlv_force_wake_put;
>  	} else if (IS_G4X(dev)) {
>  		dev_priv->display.write_eld = g4x_write_eld;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a1d3da7..29720d2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3763,34 +3763,6 @@ void intel_init_pm(struct drm_device *dev)
>  
>  	/* For FIFO watermark updates */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		dev_priv->display.force_wake_get = __gen6_gt_force_wake_get;
> -		dev_priv->display.force_wake_put = __gen6_gt_force_wake_put;
> -
> -		/* IVB configs may use multi-threaded forcewake */
> -		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> -			u32	ecobus;
> -
> -			/* 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 = I915_READ_NOTRACE(ECOBUS);
> -			__gen6_gt_force_wake_mt_put(dev_priv);
> -			mutex_unlock(&dev->struct_mutex);
> -
> -			if (ecobus & FORCEWAKE_MT_ENABLE) {
> -				DRM_DEBUG_KMS("Using MT version of forcewake\n");
> -				dev_priv->display.force_wake_get =
> -					__gen6_gt_force_wake_mt_get;
> -				dev_priv->display.force_wake_put =
> -					__gen6_gt_force_wake_mt_put;
> -			}
> -		}
> -
>  		if (HAS_PCH_IBX(dev))
>  			dev_priv->display.init_pch_clock_gating = ibx_init_clock_gating;
>  		else if (HAS_PCH_CPT(dev))
> @@ -3846,8 +3818,6 @@ void intel_init_pm(struct drm_device *dev)
>  		dev_priv->display.update_wm = valleyview_update_wm;
>  		dev_priv->display.init_clock_gating =
>  			valleyview_init_clock_gating;
> -		dev_priv->display.force_wake_get = vlv_force_wake_get;
> -		dev_priv->display.force_wake_put = vlv_force_wake_put;
>  	} else if (IS_PINEVIEW(dev)) {
>  		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
>  					    dev_priv->is_ddr3,



-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 02/10] drm/i915: Implement w/a for sporadic read failures on waking from rc6

[Eugeni Dodonov]

From: Chris Wilson <chris@chris-wilson.co.uk>

As a w/a to prevent reads sporadically returning 0, we need to wait for
the GT thread to return to TC0 before proceeding to read the registers.

v2: adapt for Haswell changes (Eugeni).

v3: use wait_for_atomic_us for thread status polling.

v3: *really* use wait_for_atomic for polling.

References: https://bugs.freedesktop.org/show_bug.cgi?id=50243
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 928b667..a4ea4a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -433,6 +433,22 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 	return 1;
 }
 
+static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
+{
+	u32 gt_thread_status_mask;
+
+	if (IS_HASWELL(dev_priv->dev))
+		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
+	else
+		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
+
+	/* 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))
+		DRM_ERROR("GT thread status wait timed out\n");
+}
+
 static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 {
 	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0, 500))
@@ -442,6 +458,8 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 
 	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1), 500))
 		DRM_ERROR("Force wake wait timed out\n");
+
+	__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
 static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
@@ -453,6 +471,8 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
 
 	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1), 500))
 		DRM_ERROR("Force wake wait timed out\n");
+
+	__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
 /*
@@ -538,6 +558,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
 
 	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
 		DRM_ERROR("Force wake wait timed out\n");
+
+	__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
 static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d8f516b..20f7f0d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1458,6 +1458,10 @@
 #define DDRMPLL1		0X12c20
 #define PEG_BAND_GAP_DATA	0x14d68
 
+#define GEN6_GT_THREAD_STATUS_REG 0x13805c
+#define GEN6_GT_THREAD_STATUS_CORE_MASK 0x7
+#define GEN6_GT_THREAD_STATUS_CORE_MASK_HSW (0x7 | (0x07 << 16))
+
 #define GEN6_GT_PERF_STATUS	0x145948
 #define GEN6_RP_STATE_LIMITS	0x145994
 #define GEN6_RP_STATE_CAP	0x145998
-- 
1.7.11.1

Re: [PATCH 02/10] drm/i915: Implement w/a for sporadic read failures on waking from rc6

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:03 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> As a w/a to prevent reads sporadically returning 0, we need to wait for
> the GT thread to return to TC0 before proceeding to read the registers.
> 
> v2: adapt for Haswell changes (Eugeni).
> 
> v3: use wait_for_atomic_us for thread status polling.
> 
> v3: *really* use wait_for_atomic for polling.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=50243
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

meh, I cannot find the register defs, but since I've heard about this
through the grapevine:
Acked-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h |  4 ++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 928b667..a4ea4a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -433,6 +433,22 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  	return 1;
>  }
>  
> +static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
> +{
> +	u32 gt_thread_status_mask;
> +
> +	if (IS_HASWELL(dev_priv->dev))
> +		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
> +	else
> +		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
> +
> +	/* 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))
> +		DRM_ERROR("GT thread status wait timed out\n");
> +}
> +
>  static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  {
>  	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0, 500))
> @@ -442,6 +458,8 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  
>  	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1), 500))
>  		DRM_ERROR("Force wake wait timed out\n");
> +
> +	__gen6_gt_wait_for_thread_c0(dev_priv);
>  }
>  
>  static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> @@ -453,6 +471,8 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
>  
>  	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1), 500))
>  		DRM_ERROR("Force wake wait timed out\n");
> +
> +	__gen6_gt_wait_for_thread_c0(dev_priv);
>  }
>  
>  /*
> @@ -538,6 +558,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
>  
>  	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
>  		DRM_ERROR("Force wake wait timed out\n");
> +
> +	__gen6_gt_wait_for_thread_c0(dev_priv);
>  }
>  
>  static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d8f516b..20f7f0d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1458,6 +1458,10 @@
>  #define DDRMPLL1		0X12c20
>  #define PEG_BAND_GAP_DATA	0x14d68
>  
> +#define GEN6_GT_THREAD_STATUS_REG 0x13805c
> +#define GEN6_GT_THREAD_STATUS_CORE_MASK 0x7
> +#define GEN6_GT_THREAD_STATUS_CORE_MASK_HSW (0x7 | (0x07 << 16))
> +
>  #define GEN6_GT_PERF_STATUS	0x145948
>  #define GEN6_RP_STATE_LIMITS	0x145994
>  #define GEN6_RP_STATE_CAP	0x145998



-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 03/10] drm/i915: support Haswell force waking

[Eugeni Dodonov]

There is a different ACK register for force wake on Haswell, so account
for that.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 22 ++++++++++++++++++----
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a4ea4a9..3ac414f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -451,12 +451,19 @@ static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
 
 static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 {
-	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0, 500))
+	u32 forcewake_ack;
+
+	if (IS_HASWELL(dev_priv->dev))
+		forcewake_ack = FORCEWAKE_ACK_HSW;
+	else
+		forcewake_ack = FORCEWAKE_ACK;
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
 		DRM_ERROR("Force wake wait timed out\n");
 
 	I915_WRITE_NOTRACE(FORCEWAKE, 1);
 
-	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1), 500))
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
 		DRM_ERROR("Force wake wait timed out\n");
 
 	__gen6_gt_wait_for_thread_c0(dev_priv);
@@ -464,12 +471,19 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 
 static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
 {
-	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0, 500))
+	u32 forcewake_ack;
+
+	if (IS_HASWELL(dev_priv->dev))
+		forcewake_ack = FORCEWAKE_ACK_HSW;
+	else
+		forcewake_ack = FORCEWAKE_MT_ACK;
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
 		DRM_ERROR("Force wake wait timed out\n");
 
 	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
 
-	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1), 500))
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
 		DRM_ERROR("Force wake wait timed out\n");
 
 	__gen6_gt_wait_for_thread_c0(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 20f7f0d..f17de3d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4095,6 +4095,7 @@
 #define  FORCEWAKE				0xA18C
 #define  FORCEWAKE_VLV				0x1300b0
 #define  FORCEWAKE_ACK_VLV			0x1300b4
+#define  FORCEWAKE_ACK_HSW			0x130044
 #define  FORCEWAKE_ACK				0x130090
 #define  FORCEWAKE_MT				0xa188 /* multi-threaded */
 #define  FORCEWAKE_MT_ACK			0x130040
-- 
1.7.11.1

Re: [PATCH 03/10] drm/i915: support Haswell force waking

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:04 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> There is a different ACK register for force wake on Haswell, so account
> for that.

Well I guess that HSW bit in patch 2 that I just acked didn't belong
there yet ;-)


Also, since you've now put conditional registers in get/put, why not go
a step further and combine mt, and non-mt?

> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 22 ++++++++++++++++++----
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a4ea4a9..3ac414f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -451,12 +451,19 @@ static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
>  
>  static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  {
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0, 500))
> +	u32 forcewake_ack;
> +
> +	if (IS_HASWELL(dev_priv->dev))
> +		forcewake_ack = FORCEWAKE_ACK_HSW;
> +	else
> +		forcewake_ack = FORCEWAKE_ACK;
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
>  		DRM_ERROR("Force wake wait timed out\n");
>  
>  	I915_WRITE_NOTRACE(FORCEWAKE, 1);
>  
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1), 500))
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
>  		DRM_ERROR("Force wake wait timed out\n");
>  
>  	__gen6_gt_wait_for_thread_c0(dev_priv);
> @@ -464,12 +471,19 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  
>  static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
>  {
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1) == 0, 500))
> +	u32 forcewake_ack;
> +
> +	if (IS_HASWELL(dev_priv->dev))
> +		forcewake_ack = FORCEWAKE_ACK_HSW;
> +	else
> +		forcewake_ack = FORCEWAKE_MT_ACK;
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
>  		DRM_ERROR("Force wake wait timed out\n");
>  
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
>  
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_MT_ACK) & 1), 500))
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
>  		DRM_ERROR("Force wake wait timed out\n");
>  
>  	__gen6_gt_wait_for_thread_c0(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 20f7f0d..f17de3d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4095,6 +4095,7 @@
>  #define  FORCEWAKE				0xA18C
>  #define  FORCEWAKE_VLV				0x1300b0
>  #define  FORCEWAKE_ACK_VLV			0x1300b4
> +#define  FORCEWAKE_ACK_HSW			0x130044
>  #define  FORCEWAKE_ACK				0x130090
>  #define  FORCEWAKE_MT				0xa188 /* multi-threaded */
>  #define  FORCEWAKE_MT_ACK			0x130040



-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 04/10] drm/i915: add RPS configuration for Haswell

[Eugeni Dodonov]

Most of the RPS and RC6 enabling functionality is similar to what we had
on Gen6/Gen7, so we preserve most of the registers.

Note that Haswell only has RC6, so account for that as well. As suggested
by Daniel Vetter, to reduce the amount of changes in the patch, we still
write the RC6p/RC6pp thresholds, but those are ignored on Haswell.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++------------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f17de3d..9d5bf06 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4156,6 +4156,7 @@
 #define   GEN6_RP_UP_IDLE_MIN			(0x1<<3)
 #define   GEN6_RP_UP_BUSY_AVG			(0x2<<3)
 #define   GEN6_RP_UP_BUSY_CONT			(0x4<<3)
+#define   GEN7_RP_DOWN_IDLE_AVG			(0x2<<0)
 #define   GEN6_RP_DOWN_IDLE_CONT		(0x1<<0)
 #define GEN6_RP_UP_THRESHOLD			0xA02C
 #define GEN6_RP_DOWN_THRESHOLD			0xA030
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 29720d2..a3ee1b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2402,20 +2402,24 @@ static void gen6_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
 	I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
 
+	/* Check if we are enabling RC6 */
 	rc6_mode = intel_enable_rc6(dev_priv->dev);
 	if (rc6_mode & INTEL_RC6_ENABLE)
 		rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
 
-	if (rc6_mode & INTEL_RC6p_ENABLE)
-		rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
+	/* We don't use those on Haswell */
+	if (!IS_HASWELL(dev)) {
+		if (rc6_mode & INTEL_RC6p_ENABLE)
+			rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
 
-	if (rc6_mode & INTEL_RC6pp_ENABLE)
-		rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
+		if (rc6_mode & INTEL_RC6pp_ENABLE)
+			rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
+	}
 
 	DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
-			(rc6_mode & INTEL_RC6_ENABLE) ? "on" : "off",
-			(rc6_mode & INTEL_RC6p_ENABLE) ? "on" : "off",
-			(rc6_mode & INTEL_RC6pp_ENABLE) ? "on" : "off");
+			(rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
+			(rc6_mask & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
+			(rc6_mask & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
 
 	I915_WRITE(GEN6_RC_CONTROL,
 		   rc6_mask |
@@ -2433,10 +2437,19 @@ static void gen6_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
 		   dev_priv->max_delay << 24 |
 		   dev_priv->min_delay << 16);
-	I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000);
-	I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000);
-	I915_WRITE(GEN6_RP_UP_EI, 100000);
-	I915_WRITE(GEN6_RP_DOWN_EI, 5000000);
+
+	if (IS_HASWELL(dev)) {
+		I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
+		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000);
+		I915_WRITE(GEN6_RP_UP_EI, 66000);
+		I915_WRITE(GEN6_RP_DOWN_EI, 350000);
+	} else {
+		I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000);
+		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000);
+		I915_WRITE(GEN6_RP_UP_EI, 100000);
+		I915_WRITE(GEN6_RP_DOWN_EI, 5000000);
+	}
+
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 	I915_WRITE(GEN6_RP_CONTROL,
 		   GEN6_RP_MEDIA_TURBO |
@@ -2444,7 +2457,7 @@ static void gen6_enable_rps(struct drm_device *dev)
 		   GEN6_RP_MEDIA_IS_GFX |
 		   GEN6_RP_ENABLE |
 		   GEN6_RP_UP_BUSY_AVG |
-		   GEN6_RP_DOWN_IDLE_CONT);
+		   (IS_HASWELL(dev)) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT);
 
 	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
 		     500))
-- 
1.7.11.1

Re: [PATCH 04/10] drm/i915: add RPS configuration for Haswell

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:05 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> Most of the RPS and RC6 enabling functionality is similar to what we had
> on Gen6/Gen7, so we preserve most of the registers.
> 
> Note that Haswell only has RC6, so account for that as well. As suggested
> by Daniel Vetter, to reduce the amount of changes in the patch, we still
> write the RC6p/RC6pp thresholds, but those are ignored on Haswell.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++------------
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f17de3d..9d5bf06 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4156,6 +4156,7 @@
>  #define   GEN6_RP_UP_IDLE_MIN			(0x1<<3)
>  #define   GEN6_RP_UP_BUSY_AVG			(0x2<<3)
>  #define   GEN6_RP_UP_BUSY_CONT			(0x4<<3)
> +#define   GEN7_RP_DOWN_IDLE_AVG			(0x2<<0)
>  #define   GEN6_RP_DOWN_IDLE_CONT		(0x1<<0)
>  #define GEN6_RP_UP_THRESHOLD			0xA02C
>  #define GEN6_RP_DOWN_THRESHOLD			0xA030
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 29720d2..a3ee1b1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2402,20 +2402,24 @@ static void gen6_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
>  	I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
>  
> +	/* Check if we are enabling RC6 */
>  	rc6_mode = intel_enable_rc6(dev_priv->dev);
>  	if (rc6_mode & INTEL_RC6_ENABLE)
>  		rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
>  
> -	if (rc6_mode & INTEL_RC6p_ENABLE)
> -		rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
> +	/* We don't use those on Haswell */
> +	if (!IS_HASWELL(dev)) {
> +		if (rc6_mode & INTEL_RC6p_ENABLE)
> +			rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
>  
> -	if (rc6_mode & INTEL_RC6pp_ENABLE)
> -		rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
> +		if (rc6_mode & INTEL_RC6pp_ENABLE)
> +			rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
> +	}
>  
>  	DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
> -			(rc6_mode & INTEL_RC6_ENABLE) ? "on" : "off",
> -			(rc6_mode & INTEL_RC6p_ENABLE) ? "on" : "off",
> -			(rc6_mode & INTEL_RC6pp_ENABLE) ? "on" : "off");
> +			(rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> +			(rc6_mask & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
> +			(rc6_mask & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");

Belongs in a separate patch, but meh.

>  
>  	I915_WRITE(GEN6_RC_CONTROL,
>  		   rc6_mask |
> @@ -2433,10 +2437,19 @@ static void gen6_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
>  		   dev_priv->max_delay << 24 |
>  		   dev_priv->min_delay << 16);
> -	I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000);
> -	I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000);
> -	I915_WRITE(GEN6_RP_UP_EI, 100000);
> -	I915_WRITE(GEN6_RP_DOWN_EI, 5000000);
> +
> +	if (IS_HASWELL(dev)) {
> +		I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
> +		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000);
> +		I915_WRITE(GEN6_RP_UP_EI, 66000);
> +		I915_WRITE(GEN6_RP_DOWN_EI, 350000);
> +	} else {
> +		I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000);
> +		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000);
> +		I915_WRITE(GEN6_RP_UP_EI, 100000);
> +		I915_WRITE(GEN6_RP_DOWN_EI, 5000000);
> +	}
> +

I can't really comment much on this other than what I said below. It'd
be nice if the policy didn't change between platforms unless we
know/understand why.

>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_TURBO |
> @@ -2444,7 +2457,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>  		   GEN6_RP_MEDIA_IS_GFX |
>  		   GEN6_RP_ENABLE |
>  		   GEN6_RP_UP_BUSY_AVG |
> -		   GEN6_RP_DOWN_IDLE_CONT);
> +		   (IS_HASWELL(dev)) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT);

I think this warrants an explanation. I think we would ideally like to
avoid different algorithms for different platforms.

>  
>  	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
>  		     500))



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 04/10] drm/i915: add RPS configuration for Haswell

[Eugeni Dodonov]

On 07/02/2012 02:49 PM, Ben Widawsky wrote:
> On Mon,  2 Jul 2012 11:51:05 -0300
> Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:
> 
>> Most of the RPS and RC6 enabling functionality is similar to what we had
>> on Gen6/Gen7, so we preserve most of the registers.
>>
>> Note that Haswell only has RC6, so account for that as well. As suggested
>> by Daniel Vetter, to reduce the amount of changes in the patch, we still
>> write the RC6p/RC6pp thresholds, but those are ignored on Haswell.
>>
>> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Daniel, to please Ben Widawsky, you could add an extra paragraph in my
message, saying:

'On Haswell, the suggested values for the RP UP and RP DOWN interrupts
have changed in a way to have lower thresholds for all the operations.
However, the suggested values for pre-Haswell machines stay the same. In
order to follow what the hardware designers suggest, we program those as
suggested for each generation now.

Also, Haswell introduces a new scheduling bit for the RP DOWN
scheduling, which we now use by default'.

Eugeni

Re: [PATCH 04/10] drm/i915: add RPS configuration for Haswell

[Ben Widawsky]

On Mon, 02 Jul 2012 17:02:59 -0300
Eugeni Dodonov <eugeni.dodonov@linux.intel.com> wrote:

> On 07/02/2012 02:49 PM, Ben Widawsky wrote:
> > On Mon,  2 Jul 2012 11:51:05 -0300
> > Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:
> > 
> >> Most of the RPS and RC6 enabling functionality is similar to what we had
> >> on Gen6/Gen7, so we preserve most of the registers.
> >>
> >> Note that Haswell only has RC6, so account for that as well. As suggested
> >> by Daniel Vetter, to reduce the amount of changes in the patch, we still
> >> write the RC6p/RC6pp thresholds, but those are ignored on Haswell.
> >>
> >> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> > Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> 
> Daniel, to please Ben Widawsky, you could add an extra paragraph in my
> message, saying:
> 
> 'On Haswell, the suggested values for the RP UP and RP DOWN interrupts
> have changed in a way to have lower thresholds for all the operations.
> However, the suggested values for pre-Haswell machines stay the same. In
> order to follow what the hardware designers suggest, we program those as
> suggested for each generation now.
> 
> Also, Haswell introduces a new scheduling bit for the RP DOWN
> scheduling, which we now use by default'.
> 
> Eugeni

The code itself states the above perfectly well. I was really trying
to understand the, "why."


-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 04/10] drm/i915: add RPS configuration for Haswell

[Chris Wilson]

On Mon,  2 Jul 2012 11:51:05 -0300, Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_TURBO |
> @@ -2444,7 +2457,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>  		   GEN6_RP_MEDIA_IS_GFX |
>  		   GEN6_RP_ENABLE |
>  		   GEN6_RP_UP_BUSY_AVG |
> -		   GEN6_RP_DOWN_IDLE_CONT);
> +		   (IS_HASWELL(dev)) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT);

And with one small typo does everything break...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 05/10] drm/i915: slightly improve gt enable/disable routines

[Eugeni Dodonov]

Just a cosmetic change to simplify the if statement.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a3ee1b1..64cd97e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3238,7 +3238,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 {
 	if (IS_IRONLAKE_M(dev))
 		ironlake_disable_drps(dev);
-	if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
+	else if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
 		gen6_disable_rps(dev);
 }
 
@@ -3248,9 +3248,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
 		ironlake_enable_drps(dev);
 		ironlake_enable_rc6(dev);
 		intel_init_emon(dev);
-	}
-
-	if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
+	} else if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
 		gen6_enable_rps(dev);
 		gen6_update_ring_freq(dev);
 	}
-- 
1.7.11.1

Re: [PATCH 05/10] drm/i915: slightly improve gt enable/disable routines

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:06 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> Just a cosmetic change to simplify the if statement.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a3ee1b1..64cd97e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3238,7 +3238,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  {
>  	if (IS_IRONLAKE_M(dev))
>  		ironlake_disable_drps(dev);
> -	if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
> +	else if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
>  		gen6_disable_rps(dev);
>  }
>  
> @@ -3248,9 +3248,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
>  		ironlake_enable_drps(dev);
>  		ironlake_enable_rc6(dev);
>  		intel_init_emon(dev);
> -	}
> -
> -	if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
> +	} else if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
>  		gen6_enable_rps(dev);
>  		gen6_update_ring_freq(dev);
>  	}



-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 06/10] drm/i915: enable RC6 by default on Haswell

[Eugeni Dodonov]

It should be working so let's turn it on by default and catch any possible
issues faster.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 64cd97e..b00df1f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2332,9 +2332,11 @@ int intel_enable_rc6(const struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen == 5)
 		return 0;
 
-	/* Sorry Haswell, no RC6 for you for now. */
+	/* On Haswell, only RC6 is available. So let's enable it by default to
+	 * provide better testing and coverage since the beginning.
+	 */
 	if (IS_HASWELL(dev))
-		return 0;
+		return INTEL_RC6_ENABLE;
 
 	/*
 	 * Disable rc6 on Sandybridge
-- 
1.7.11.1

Re: [PATCH 06/10] drm/i915: enable RC6 by default on Haswell

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:07 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> It should be working so let's turn it on by default and catch any possible
> issues faster.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 64cd97e..b00df1f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2332,9 +2332,11 @@ int intel_enable_rc6(const struct drm_device *dev)
>  	if (INTEL_INFO(dev)->gen == 5)
>  		return 0;
>  
> -	/* Sorry Haswell, no RC6 for you for now. */
> +	/* On Haswell, only RC6 is available. So let's enable it by default to
> +	 * provide better testing and coverage since the beginning.
> +	 */
>  	if (IS_HASWELL(dev))
> -		return 0;
> +		return INTEL_RC6_ENABLE;
>  
>  	/*
>  	 * Disable rc6 on Sandybridge

I think you can/should fold this in with the existing case.

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 07/10] drm/i915: disable RC6 when disabling rps

[Eugeni Dodonov]

We weren't disabling RC6 bits when bringing down RPS.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b00df1f..5ea8319 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2303,6 +2303,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	I915_WRITE(GEN6_RC_CONTROL, 0);
 	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
 	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
 	I915_WRITE(GEN6_PMIER, 0);
-- 
1.7.11.1

Re: [PATCH 07/10] drm/i915: disable RC6 when disabling rps

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:08 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> We weren't disabling RC6 bits when bringing down RPS.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b00df1f..5ea8319 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2303,6 +2303,7 @@ static void gen6_disable_rps(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	I915_WRITE(GEN6_RC_CONTROL, 0);
>  	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
>  	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
>  	I915_WRITE(GEN6_PMIER, 0);

I think we really only need to clear bit 31 to disable (and enable
already sets it to 0) but this should be fine.

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 08/10] drm/i915: introduce haswell_init_clock_gating

[Eugeni Dodonov]

This is based on Ivy Bridge clock gating for now, but is subject to
changes in the future.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 54 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5ea8319..f54196f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3415,6 +3415,58 @@ static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN7_FF_THREAD_MODE, reg);
 }
 
+static void haswell_init_clock_gating(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe;
+	uint32_t dspclk_gate = VRHUNIT_CLOCK_GATE_DISABLE;
+
+	I915_WRITE(PCH_DSPCLK_GATE_D, dspclk_gate);
+
+	I915_WRITE(WM3_LP_ILK, 0);
+	I915_WRITE(WM2_LP_ILK, 0);
+	I915_WRITE(WM1_LP_ILK, 0);
+
+	/* According to the spec, bit 13 (RCZUNIT) must be set on IVB.
+	 * This implements the WaDisableRCZUnitClockGating workaround.
+	 */
+	I915_WRITE(GEN6_UCGCTL2, GEN6_RCZUNIT_CLOCK_GATE_DISABLE);
+
+	I915_WRITE(ILK_DSPCLK_GATE, IVB_VRHUNIT_CLK_GATE);
+
+	I915_WRITE(IVB_CHICKEN3,
+		   CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE |
+		   CHICKEN3_DGMG_DONE_FIX_DISABLE);
+
+	/* Apply the WaDisableRHWOOptimizationForRenderHang workaround. */
+	I915_WRITE(GEN7_COMMON_SLICE_CHICKEN1,
+		   GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC);
+
+	/* WaApplyL3ControlAndL3ChickenMode requires those two on Ivy Bridge */
+	I915_WRITE(GEN7_L3CNTLREG1,
+			GEN7_WA_FOR_GEN7_L3_CONTROL);
+	I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER,
+			GEN7_WA_L3_CHICKEN_MODE);
+
+	/* This is required by WaCatErrorRejectionIssue */
+	I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
+			I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) |
+			GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
+
+	for_each_pipe(pipe) {
+		I915_WRITE(DSPCNTR(pipe),
+			   I915_READ(DSPCNTR(pipe)) |
+			   DISPPLANE_TRICKLE_FEED_DISABLE);
+		intel_flush_display_plane(dev_priv, pipe);
+	}
+
+	gen7_setup_fixed_func_scheduler(dev_priv);
+
+	/* WaDisable4x2SubspanOptimization */
+	I915_WRITE(CACHE_MODE_1,
+		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
+}
+
 static void ivybridge_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3824,7 +3876,7 @@ void intel_init_pm(struct drm_device *dev)
 					      "Disable CxSR\n");
 				dev_priv->display.update_wm = NULL;
 			}
-			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
+			dev_priv->display.init_clock_gating = haswell_init_clock_gating;
 			dev_priv->display.sanitize_pm = gen6_sanitize_pm;
 		} else
 			dev_priv->display.update_wm = NULL;
-- 
1.7.11.1

Re: [PATCH 08/10] drm/i915: introduce haswell_init_clock_gating

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:09 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> This is based on Ivy Bridge clock gating for now, but is subject to
> changes in the future.

I am a fan of not including this until it's actually needed. I only took
a peek, but I didn't see a need in the remaining patches.

> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 54 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5ea8319..f54196f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3415,6 +3415,58 @@ static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)
>  	I915_WRITE(GEN7_FF_THREAD_MODE, reg);
>  }
>  
> +static void haswell_init_clock_gating(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe;
> +	uint32_t dspclk_gate = VRHUNIT_CLOCK_GATE_DISABLE;
> +
> +	I915_WRITE(PCH_DSPCLK_GATE_D, dspclk_gate);
> +
> +	I915_WRITE(WM3_LP_ILK, 0);
> +	I915_WRITE(WM2_LP_ILK, 0);
> +	I915_WRITE(WM1_LP_ILK, 0);
> +
> +	/* According to the spec, bit 13 (RCZUNIT) must be set on IVB.
> +	 * This implements the WaDisableRCZUnitClockGating workaround.
> +	 */
> +	I915_WRITE(GEN6_UCGCTL2, GEN6_RCZUNIT_CLOCK_GATE_DISABLE);
> +
> +	I915_WRITE(ILK_DSPCLK_GATE, IVB_VRHUNIT_CLK_GATE);
> +
> +	I915_WRITE(IVB_CHICKEN3,
> +		   CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE |
> +		   CHICKEN3_DGMG_DONE_FIX_DISABLE);
> +
> +	/* Apply the WaDisableRHWOOptimizationForRenderHang workaround. */
> +	I915_WRITE(GEN7_COMMON_SLICE_CHICKEN1,
> +		   GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC);
> +
> +	/* WaApplyL3ControlAndL3ChickenMode requires those two on Ivy Bridge */
> +	I915_WRITE(GEN7_L3CNTLREG1,
> +			GEN7_WA_FOR_GEN7_L3_CONTROL);
> +	I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER,
> +			GEN7_WA_L3_CHICKEN_MODE);
> +
> +	/* This is required by WaCatErrorRejectionIssue */
> +	I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
> +			I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) |
> +			GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
> +
> +	for_each_pipe(pipe) {
> +		I915_WRITE(DSPCNTR(pipe),
> +			   I915_READ(DSPCNTR(pipe)) |
> +			   DISPPLANE_TRICKLE_FEED_DISABLE);
> +		intel_flush_display_plane(dev_priv, pipe);
> +	}
> +
> +	gen7_setup_fixed_func_scheduler(dev_priv);
> +
> +	/* WaDisable4x2SubspanOptimization */
> +	I915_WRITE(CACHE_MODE_1,
> +		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
> +}
> +
>  static void ivybridge_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3824,7 +3876,7 @@ void intel_init_pm(struct drm_device *dev)
>  					      "Disable CxSR\n");
>  				dev_priv->display.update_wm = NULL;
>  			}
> -			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
> +			dev_priv->display.init_clock_gating = haswell_init_clock_gating;
>  			dev_priv->display.sanitize_pm = gen6_sanitize_pm;
>  		} else
>  			dev_priv->display.update_wm = NULL;



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 08/10] drm/i915: introduce haswell_init_clock_gating

[Daniel Vetter]

On Mon, Jul 02, 2012 at 11:51:09AM -0300, Eugeni Dodonov wrote:
> This is based on Ivy Bridge clock gating for now, but is subject to
> changes in the future.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

This copy of presumeably the ivb clock gate code is missing the changes
introduce in:

commit 208482232de3590cee4757dfabe5d8cee8c6e626
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Fri May 4 18:58:59 2012 -0700

    drm/i915: set IDICOS to medium uncore resources

I guess that's not quite intentional ...

All the previous patches up to here are queued for -next, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 54 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5ea8319..f54196f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3415,6 +3415,58 @@ static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)
>  	I915_WRITE(GEN7_FF_THREAD_MODE, reg);
>  }
>  
> +static void haswell_init_clock_gating(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe;
> +	uint32_t dspclk_gate = VRHUNIT_CLOCK_GATE_DISABLE;
> +
> +	I915_WRITE(PCH_DSPCLK_GATE_D, dspclk_gate);
> +
> +	I915_WRITE(WM3_LP_ILK, 0);
> +	I915_WRITE(WM2_LP_ILK, 0);
> +	I915_WRITE(WM1_LP_ILK, 0);
> +
> +	/* According to the spec, bit 13 (RCZUNIT) must be set on IVB.
> +	 * This implements the WaDisableRCZUnitClockGating workaround.
> +	 */
> +	I915_WRITE(GEN6_UCGCTL2, GEN6_RCZUNIT_CLOCK_GATE_DISABLE);
> +
> +	I915_WRITE(ILK_DSPCLK_GATE, IVB_VRHUNIT_CLK_GATE);
> +
> +	I915_WRITE(IVB_CHICKEN3,
> +		   CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE |
> +		   CHICKEN3_DGMG_DONE_FIX_DISABLE);
> +
> +	/* Apply the WaDisableRHWOOptimizationForRenderHang workaround. */
> +	I915_WRITE(GEN7_COMMON_SLICE_CHICKEN1,
> +		   GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC);
> +
> +	/* WaApplyL3ControlAndL3ChickenMode requires those two on Ivy Bridge */
> +	I915_WRITE(GEN7_L3CNTLREG1,
> +			GEN7_WA_FOR_GEN7_L3_CONTROL);
> +	I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER,
> +			GEN7_WA_L3_CHICKEN_MODE);
> +
> +	/* This is required by WaCatErrorRejectionIssue */
> +	I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
> +			I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) |
> +			GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
> +
> +	for_each_pipe(pipe) {
> +		I915_WRITE(DSPCNTR(pipe),
> +			   I915_READ(DSPCNTR(pipe)) |
> +			   DISPPLANE_TRICKLE_FEED_DISABLE);
> +		intel_flush_display_plane(dev_priv, pipe);
> +	}
> +
> +	gen7_setup_fixed_func_scheduler(dev_priv);
> +
> +	/* WaDisable4x2SubspanOptimization */
> +	I915_WRITE(CACHE_MODE_1,
> +		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
> +}
> +
>  static void ivybridge_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3824,7 +3876,7 @@ void intel_init_pm(struct drm_device *dev)
>  					      "Disable CxSR\n");
>  				dev_priv->display.update_wm = NULL;
>  			}
> -			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
> +			dev_priv->display.init_clock_gating = haswell_init_clock_gating;
>  			dev_priv->display.sanitize_pm = gen6_sanitize_pm;
>  		} else
>  			dev_priv->display.update_wm = NULL;
> -- 
> 1.7.11.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 08/10] drm/i915: introduce haswell_init_clock_gating

[Eugeni Dodonov]

On 07/03/12 17:24, Daniel Vetter wrote:
> On Mon, Jul 02, 2012 at 11:51:09AM -0300, Eugeni Dodonov wrote:
>> This is based on Ivy Bridge clock gating for now, but is subject to
>> changes in the future.
>>
>> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> 
> This copy of presumeably the ivb clock gate code is missing the changes
> introduce in:
> 
> commit 208482232de3590cee4757dfabe5d8cee8c6e626
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Fri May 4 18:58:59 2012 -0700
> 
>     drm/i915: set IDICOS to medium uncore resources
> 
> I guess that's not quite intentional ...
> 
> All the previous patches up to here are queued for -next, thanks.

I thought that this one was specific for Ivy Bridge, so I just skipped it...

I have not tried setting these settings on Haswell, so I don't know if
it is useful here as well. I'll try later this week to see if there are
any visible effects.

Eugeni

Re: [PATCH 08/10] drm/i915: introduce haswell_init_clock_gating

[Daniel Vetter]

On Tue, Jul 03, 2012 at 09:03:16PM -0300, Eugeni Dodonov wrote:
> On 07/03/12 17:24, Daniel Vetter wrote:
> > On Mon, Jul 02, 2012 at 11:51:09AM -0300, Eugeni Dodonov wrote:
> >> This is based on Ivy Bridge clock gating for now, but is subject to
> >> changes in the future.
> >>
> >> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> > 
> > This copy of presumeably the ivb clock gate code is missing the changes
> > introduce in:
> > 
> > commit 208482232de3590cee4757dfabe5d8cee8c6e626
> > Author: Ben Widawsky <ben@bwidawsk.net>
> > Date:   Fri May 4 18:58:59 2012 -0700
> > 
> >     drm/i915: set IDICOS to medium uncore resources
> > 
> > I guess that's not quite intentional ...
> > 
> > All the previous patches up to here are queued for -next, thanks.
> 
> I thought that this one was specific for Ivy Bridge, so I just skipped it...
> 
> I have not tried setting these settings on Haswell, so I don't know if
> it is useful here as well. I'll try later this week to see if there are
> any visible effects.

Ok, I've merged the patch and took a note that you volunteered for some
benchmarking. But the commit message really should have mentioned why this
was dropped compared to the ivb clock gating function it was copy&pasted
from. I've added a note to that effect.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH 09/10] drm/i915: enable RC6 workaround on Haswell

[Eugeni Dodonov]

For Haswell, on some of the early hardware revisions, it is possible to
run into issues when RC6 state is enabled and when pipes change state.

v2: add comment saying that this is for early revisions only.

v3: beautify as suggested by Daniel Vetter.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9d5bf06..793a172 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4516,4 +4516,9 @@
 #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
 #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
 
+#define WM_DBG				0x45280
+#define  WM_DBG_DISALLOW_MULTIPLE_LP	(1<<0)
+#define  WM_DBG_DISALLOW_MAXFIFO	(1<<1)
+#define  WM_DBG_DISALLOW_SPRITE		(1<<2)
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f54196f..604f9bf 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3465,6 +3465,16 @@ static void haswell_init_clock_gating(struct drm_device *dev)
 	/* WaDisable4x2SubspanOptimization */
 	I915_WRITE(CACHE_MODE_1,
 		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
+
+	/* XXX: This is a workaround for early silicon revisions and should be
+	 * removed later.
+	 */
+	I915_WRITE(WM_DBG,
+			I915_READ(WM_DBG) |
+			WM_DBG_DISALLOW_MULTIPLE_LP |
+			WM_DBG_DISALLOW_SPRITE |
+			WM_DBG_DISALLOW_MAXFIFO);
+
 }
 
 static void ivybridge_init_clock_gating(struct drm_device *dev)
-- 
1.7.11.1

Re: [PATCH 09/10] drm/i915: enable RC6 workaround on Haswell

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:10 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> For Haswell, on some of the early hardware revisions, it is possible to
> run into issues when RC6 state is enabled and when pipes change state.
> 
> v2: add comment saying that this is for early revisions only.
> 
> v3: beautify as suggested by Daniel Vetter.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  5 +++++
>  drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9d5bf06..793a172 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4516,4 +4516,9 @@
>  #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
>  #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
>  
> +#define WM_DBG				0x45280
> +#define  WM_DBG_DISALLOW_MULTIPLE_LP	(1<<0)
> +#define  WM_DBG_DISALLOW_MAXFIFO	(1<<1)
> +#define  WM_DBG_DISALLOW_SPRITE		(1<<2)
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f54196f..604f9bf 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3465,6 +3465,16 @@ static void haswell_init_clock_gating(struct drm_device *dev)
>  	/* WaDisable4x2SubspanOptimization */
>  	I915_WRITE(CACHE_MODE_1,
>  		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
> +
> +	/* XXX: This is a workaround for early silicon revisions and should be
> +	 * removed later.
> +	 */
> +	I915_WRITE(WM_DBG,
> +			I915_READ(WM_DBG) |
> +			WM_DBG_DISALLOW_MULTIPLE_LP |
> +			WM_DBG_DISALLOW_SPRITE |
> +			WM_DBG_DISALLOW_MAXFIFO);
> +
>  }
>  
>  static void ivybridge_init_clock_gating(struct drm_device *dev)



-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 10/10] drm/i915: move force wake support into intel_pm

[Eugeni Dodonov]

This commit moves force wake support routines into intel_pm modules, and
exports the gen6_gt_check_fifodbg routine (used in I915_READ).

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 191 ---------------------------------------
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 drivers/gpu/drm/i915/intel_pm.c  | 191 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 192 insertions(+), 191 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3ac414f..c7e76e0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -433,197 +433,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 	return 1;
 }
 
-static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
-{
-	u32 gt_thread_status_mask;
-
-	if (IS_HASWELL(dev_priv->dev))
-		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
-	else
-		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
-
-	/* 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))
-		DRM_ERROR("GT thread status wait timed out\n");
-}
-
-static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
-{
-	u32 forcewake_ack;
-
-	if (IS_HASWELL(dev_priv->dev))
-		forcewake_ack = FORCEWAKE_ACK_HSW;
-	else
-		forcewake_ack = FORCEWAKE_ACK;
-
-	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
-		DRM_ERROR("Force wake wait timed out\n");
-
-	I915_WRITE_NOTRACE(FORCEWAKE, 1);
-
-	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
-		DRM_ERROR("Force wake wait timed out\n");
-
-	__gen6_gt_wait_for_thread_c0(dev_priv);
-}
-
-static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
-{
-	u32 forcewake_ack;
-
-	if (IS_HASWELL(dev_priv->dev))
-		forcewake_ack = FORCEWAKE_ACK_HSW;
-	else
-		forcewake_ack = FORCEWAKE_MT_ACK;
-
-	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
-		DRM_ERROR("Force wake wait timed out\n");
-
-	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
-
-	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
-		DRM_ERROR("Force wake wait timed out\n");
-
-	__gen6_gt_wait_for_thread_c0(dev_priv);
-}
-
-/*
- * Generally this is called implicitly by the register read function. However,
- * if some sequence requires the GT to not power down then this function should
- * be called at the beginning of the sequence followed by a call to
- * gen6_gt_force_wake_put() at the end of the sequence.
- */
-void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
-{
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
-	if (dev_priv->forcewake_count++ == 0)
-		dev_priv->gt.force_wake_get(dev_priv);
-	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
-}
-
-static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
-{
-	u32 gtfifodbg;
-	gtfifodbg = I915_READ_NOTRACE(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);
-}
-
-static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
-{
-	I915_WRITE_NOTRACE(FORCEWAKE, 0);
-	/* The below doubles as a POSTING_READ */
-	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(1));
-	/* The below doubles as a POSTING_READ */
-	gen6_gt_check_fifodbg(dev_priv);
-}
-
-/*
- * see gen6_gt_force_wake_get()
- */
-void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
-{
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
-	if (--dev_priv->forcewake_count == 0)
-		dev_priv->gt.force_wake_put(dev_priv);
-	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
-}
-
-int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
-{
-	int ret = 0;
-
-	if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
-		int loop = 500;
-		u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
-		while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
-			udelay(10);
-			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
-		}
-		if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
-			++ret;
-		dev_priv->gt_fifo_count = fifo;
-	}
-	dev_priv->gt_fifo_count--;
-
-	return ret;
-}
-
-static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
-{
-	/* Already awake? */
-	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
-		return;
-
-	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
-	POSTING_READ(FORCEWAKE_VLV);
-
-	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
-		DRM_ERROR("Force wake wait timed out\n");
-
-	__gen6_gt_wait_for_thread_c0(dev_priv);
-}
-
-static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
-{
-	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
-	/* FIXME: confirm VLV behavior with Punit folks */
-	POSTING_READ(FORCEWAKE_VLV);
-}
-
-void intel_gt_init(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	spin_lock_init(&dev_priv->gt_lock);
-
-	if (IS_VALLEYVIEW(dev)) {
-		dev_priv->gt.force_wake_get = vlv_force_wake_get;
-		dev_priv->gt.force_wake_put = vlv_force_wake_put;
-	} else if (INTEL_INFO(dev)->gen >= 6) {
-		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
-		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
-
-		/* IVB configs may use multi-threaded forcewake */
-		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-			u32 ecobus;
-
-			/* 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 = I915_READ_NOTRACE(ECOBUS);
-			__gen6_gt_force_wake_mt_put(dev_priv);
-			mutex_unlock(&dev->struct_mutex);
-
-			if (ecobus & FORCEWAKE_MT_ENABLE) {
-				DRM_DEBUG_KMS("Using MT version of forcewake\n");
-				dev_priv->gt.force_wake_get =
-					__gen6_gt_force_wake_mt_get;
-				dev_priv->gt.force_wake_put =
-					__gen6_gt_force_wake_mt_put;
-			}
-		}
-	}
-}
-
 static int i915_drm_freeze(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a741a02..6dd25e9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -533,6 +533,7 @@ extern void intel_gpu_ips_teardown(void);
 
 extern void intel_enable_gt_powersave(struct drm_device *dev);
 extern void intel_disable_gt_powersave(struct drm_device *dev);
+extern void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv);
 
 extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode);
 extern void intel_ddi_mode_set(struct drm_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 604f9bf..425e7d4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3946,3 +3946,194 @@ void intel_init_pm(struct drm_device *dev)
 	intel_init_power_wells(dev);
 }
 
+static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
+{
+	u32 gt_thread_status_mask;
+
+	if (IS_HASWELL(dev_priv->dev))
+		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
+	else
+		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
+
+	/* 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))
+		DRM_ERROR("GT thread status wait timed out\n");
+}
+
+static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
+{
+	u32 forcewake_ack;
+
+	if (IS_HASWELL(dev_priv->dev))
+		forcewake_ack = FORCEWAKE_ACK_HSW;
+	else
+		forcewake_ack = FORCEWAKE_ACK;
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
+		DRM_ERROR("Force wake wait timed out\n");
+
+	I915_WRITE_NOTRACE(FORCEWAKE, 1);
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
+		DRM_ERROR("Force wake wait timed out\n");
+
+	__gen6_gt_wait_for_thread_c0(dev_priv);
+}
+
+static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
+{
+	u32 forcewake_ack;
+
+	if (IS_HASWELL(dev_priv->dev))
+		forcewake_ack = FORCEWAKE_ACK_HSW;
+	else
+		forcewake_ack = FORCEWAKE_MT_ACK;
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
+		DRM_ERROR("Force wake wait timed out\n");
+
+	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
+		DRM_ERROR("Force wake wait timed out\n");
+
+	__gen6_gt_wait_for_thread_c0(dev_priv);
+}
+
+/*
+ * Generally this is called implicitly by the register read function. However,
+ * if some sequence requires the GT to not power down then this function should
+ * be called at the beginning of the sequence followed by a call to
+ * gen6_gt_force_wake_put() at the end of the sequence.
+ */
+void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+	if (dev_priv->forcewake_count++ == 0)
+		dev_priv->gt.force_wake_get(dev_priv);
+	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
+}
+
+void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
+{
+	u32 gtfifodbg;
+	gtfifodbg = I915_READ_NOTRACE(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);
+}
+
+static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
+{
+	I915_WRITE_NOTRACE(FORCEWAKE, 0);
+	/* The below doubles as a POSTING_READ */
+	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(1));
+	/* The below doubles as a POSTING_READ */
+	gen6_gt_check_fifodbg(dev_priv);
+}
+
+/*
+ * see gen6_gt_force_wake_get()
+ */
+void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+	if (--dev_priv->forcewake_count == 0)
+		dev_priv->gt.force_wake_put(dev_priv);
+	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
+}
+
+int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
+{
+	int ret = 0;
+
+	if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
+		int loop = 500;
+		u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
+		while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
+			udelay(10);
+			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
+		}
+		if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
+			++ret;
+		dev_priv->gt_fifo_count = fifo;
+	}
+	dev_priv->gt_fifo_count--;
+
+	return ret;
+}
+
+static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
+{
+	/* Already awake? */
+	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
+		return;
+
+	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
+	POSTING_READ(FORCEWAKE_VLV);
+
+	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
+		DRM_ERROR("Force wake wait timed out\n");
+
+	__gen6_gt_wait_for_thread_c0(dev_priv);
+}
+
+static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
+{
+	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
+	/* FIXME: confirm VLV behavior with Punit folks */
+	POSTING_READ(FORCEWAKE_VLV);
+}
+
+void intel_gt_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	spin_lock_init(&dev_priv->gt_lock);
+
+	if (IS_VALLEYVIEW(dev)) {
+		dev_priv->gt.force_wake_get = vlv_force_wake_get;
+		dev_priv->gt.force_wake_put = vlv_force_wake_put;
+	} else if (INTEL_INFO(dev)->gen >= 6) {
+		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
+		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
+
+		/* IVB configs may use multi-threaded forcewake */
+		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
+			u32 ecobus;
+
+			/* 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 = I915_READ_NOTRACE(ECOBUS);
+			__gen6_gt_force_wake_mt_put(dev_priv);
+			mutex_unlock(&dev->struct_mutex);
+
+			if (ecobus & FORCEWAKE_MT_ENABLE) {
+				DRM_DEBUG_KMS("Using MT version of forcewake\n");
+				dev_priv->gt.force_wake_get =
+					__gen6_gt_force_wake_mt_get;
+				dev_priv->gt.force_wake_put =
+					__gen6_gt_force_wake_mt_put;
+			}
+		}
+	}
+}
+
-- 
1.7.11.1

Re: [PATCH 10/10] drm/i915: move force wake support into intel_pm

[Ben Widawsky]

On Mon,  2 Jul 2012 11:51:11 -0300
Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> This commit moves force wake support routines into intel_pm modules, and
> exports the gen6_gt_check_fifodbg routine (used in I915_READ).
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

I seem to recall Chris not liking this idea. Did I make that up?

Acked-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c  | 191 ---------------------------------------
>  drivers/gpu/drm/i915/intel_drv.h |   1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 191 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 192 insertions(+), 191 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3ac414f..c7e76e0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -433,197 +433,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  	return 1;
>  }
>  
> -static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
> -{
> -	u32 gt_thread_status_mask;
> -
> -	if (IS_HASWELL(dev_priv->dev))
> -		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
> -	else
> -		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
> -
> -	/* 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))
> -		DRM_ERROR("GT thread status wait timed out\n");
> -}
> -
> -static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
> -{
> -	u32 forcewake_ack;
> -
> -	if (IS_HASWELL(dev_priv->dev))
> -		forcewake_ack = FORCEWAKE_ACK_HSW;
> -	else
> -		forcewake_ack = FORCEWAKE_ACK;
> -
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
> -		DRM_ERROR("Force wake wait timed out\n");
> -
> -	I915_WRITE_NOTRACE(FORCEWAKE, 1);
> -
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
> -		DRM_ERROR("Force wake wait timed out\n");
> -
> -	__gen6_gt_wait_for_thread_c0(dev_priv);
> -}
> -
> -static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> -{
> -	u32 forcewake_ack;
> -
> -	if (IS_HASWELL(dev_priv->dev))
> -		forcewake_ack = FORCEWAKE_ACK_HSW;
> -	else
> -		forcewake_ack = FORCEWAKE_MT_ACK;
> -
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
> -		DRM_ERROR("Force wake wait timed out\n");
> -
> -	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
> -
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
> -		DRM_ERROR("Force wake wait timed out\n");
> -
> -	__gen6_gt_wait_for_thread_c0(dev_priv);
> -}
> -
> -/*
> - * Generally this is called implicitly by the register read function. However,
> - * if some sequence requires the GT to not power down then this function should
> - * be called at the beginning of the sequence followed by a call to
> - * gen6_gt_force_wake_put() at the end of the sequence.
> - */
> -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
> -{
> -	unsigned long irqflags;
> -
> -	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
> -	if (dev_priv->forcewake_count++ == 0)
> -		dev_priv->gt.force_wake_get(dev_priv);
> -	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
> -}
> -
> -static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
> -{
> -	u32 gtfifodbg;
> -	gtfifodbg = I915_READ_NOTRACE(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);
> -}
> -
> -static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> -{
> -	I915_WRITE_NOTRACE(FORCEWAKE, 0);
> -	/* The below doubles as a POSTING_READ */
> -	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(1));
> -	/* The below doubles as a POSTING_READ */
> -	gen6_gt_check_fifodbg(dev_priv);
> -}
> -
> -/*
> - * see gen6_gt_force_wake_get()
> - */
> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> -{
> -	unsigned long irqflags;
> -
> -	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
> -	if (--dev_priv->forcewake_count == 0)
> -		dev_priv->gt.force_wake_put(dev_priv);
> -	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
> -}
> -
> -int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> -{
> -	int ret = 0;
> -
> -	if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
> -		int loop = 500;
> -		u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> -		while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
> -			udelay(10);
> -			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> -		}
> -		if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
> -			++ret;
> -		dev_priv->gt_fifo_count = fifo;
> -	}
> -	dev_priv->gt_fifo_count--;
> -
> -	return ret;
> -}
> -
> -static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> -{
> -	/* Already awake? */
> -	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
> -		return;
> -
> -	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
> -	POSTING_READ(FORCEWAKE_VLV);
> -
> -	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
> -		DRM_ERROR("Force wake wait timed out\n");
> -
> -	__gen6_gt_wait_for_thread_c0(dev_priv);
> -}
> -
> -static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> -{
> -	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
> -	/* FIXME: confirm VLV behavior with Punit folks */
> -	POSTING_READ(FORCEWAKE_VLV);
> -}
> -
> -void intel_gt_init(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	spin_lock_init(&dev_priv->gt_lock);
> -
> -	if (IS_VALLEYVIEW(dev)) {
> -		dev_priv->gt.force_wake_get = vlv_force_wake_get;
> -		dev_priv->gt.force_wake_put = vlv_force_wake_put;
> -	} else if (INTEL_INFO(dev)->gen >= 6) {
> -		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
> -		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
> -
> -		/* IVB configs may use multi-threaded forcewake */
> -		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> -			u32 ecobus;
> -
> -			/* 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 = I915_READ_NOTRACE(ECOBUS);
> -			__gen6_gt_force_wake_mt_put(dev_priv);
> -			mutex_unlock(&dev->struct_mutex);
> -
> -			if (ecobus & FORCEWAKE_MT_ENABLE) {
> -				DRM_DEBUG_KMS("Using MT version of forcewake\n");
> -				dev_priv->gt.force_wake_get =
> -					__gen6_gt_force_wake_mt_get;
> -				dev_priv->gt.force_wake_put =
> -					__gen6_gt_force_wake_mt_put;
> -			}
> -		}
> -	}
> -}
> -
>  static int i915_drm_freeze(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a741a02..6dd25e9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -533,6 +533,7 @@ extern void intel_gpu_ips_teardown(void);
>  
>  extern void intel_enable_gt_powersave(struct drm_device *dev);
>  extern void intel_disable_gt_powersave(struct drm_device *dev);
> +extern void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv);
>  
>  extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode);
>  extern void intel_ddi_mode_set(struct drm_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 604f9bf..425e7d4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3946,3 +3946,194 @@ void intel_init_pm(struct drm_device *dev)
>  	intel_init_power_wells(dev);
>  }
>  
> +static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
> +{
> +	u32 gt_thread_status_mask;
> +
> +	if (IS_HASWELL(dev_priv->dev))
> +		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
> +	else
> +		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
> +
> +	/* 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))
> +		DRM_ERROR("GT thread status wait timed out\n");
> +}
> +
> +static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
> +{
> +	u32 forcewake_ack;
> +
> +	if (IS_HASWELL(dev_priv->dev))
> +		forcewake_ack = FORCEWAKE_ACK_HSW;
> +	else
> +		forcewake_ack = FORCEWAKE_ACK;
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
> +		DRM_ERROR("Force wake wait timed out\n");
> +
> +	I915_WRITE_NOTRACE(FORCEWAKE, 1);
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
> +		DRM_ERROR("Force wake wait timed out\n");
> +
> +	__gen6_gt_wait_for_thread_c0(dev_priv);
> +}
> +
> +static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> +{
> +	u32 forcewake_ack;
> +
> +	if (IS_HASWELL(dev_priv->dev))
> +		forcewake_ack = FORCEWAKE_ACK_HSW;
> +	else
> +		forcewake_ack = FORCEWAKE_MT_ACK;
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
> +		DRM_ERROR("Force wake wait timed out\n");
> +
> +	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
> +		DRM_ERROR("Force wake wait timed out\n");
> +
> +	__gen6_gt_wait_for_thread_c0(dev_priv);
> +}
> +
> +/*
> + * Generally this is called implicitly by the register read function. However,
> + * if some sequence requires the GT to not power down then this function should
> + * be called at the beginning of the sequence followed by a call to
> + * gen6_gt_force_wake_put() at the end of the sequence.
> + */
> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
> +{
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
> +	if (dev_priv->forcewake_count++ == 0)
> +		dev_priv->gt.force_wake_get(dev_priv);
> +	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
> +}
> +
> +void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
> +{
> +	u32 gtfifodbg;
> +	gtfifodbg = I915_READ_NOTRACE(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);
> +}
> +
> +static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> +{
> +	I915_WRITE_NOTRACE(FORCEWAKE, 0);
> +	/* The below doubles as a POSTING_READ */
> +	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(1));
> +	/* The below doubles as a POSTING_READ */
> +	gen6_gt_check_fifodbg(dev_priv);
> +}
> +
> +/*
> + * see gen6_gt_force_wake_get()
> + */
> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> +{
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
> +	if (--dev_priv->forcewake_count == 0)
> +		dev_priv->gt.force_wake_put(dev_priv);
> +	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
> +}
> +
> +int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> +{
> +	int ret = 0;
> +
> +	if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
> +		int loop = 500;
> +		u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> +		while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
> +			udelay(10);
> +			fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
> +		}
> +		if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
> +			++ret;
> +		dev_priv->gt_fifo_count = fifo;
> +	}
> +	dev_priv->gt_fifo_count--;
> +
> +	return ret;
> +}
> +
> +static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> +{
> +	/* Already awake? */
> +	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
> +		return;
> +
> +	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
> +	POSTING_READ(FORCEWAKE_VLV);
> +
> +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
> +		DRM_ERROR("Force wake wait timed out\n");
> +
> +	__gen6_gt_wait_for_thread_c0(dev_priv);
> +}
> +
> +static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> +{
> +	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
> +	/* FIXME: confirm VLV behavior with Punit folks */
> +	POSTING_READ(FORCEWAKE_VLV);
> +}
> +
> +void intel_gt_init(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	spin_lock_init(&dev_priv->gt_lock);
> +
> +	if (IS_VALLEYVIEW(dev)) {
> +		dev_priv->gt.force_wake_get = vlv_force_wake_get;
> +		dev_priv->gt.force_wake_put = vlv_force_wake_put;
> +	} else if (INTEL_INFO(dev)->gen >= 6) {
> +		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
> +		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
> +
> +		/* IVB configs may use multi-threaded forcewake */
> +		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> +			u32 ecobus;
> +
> +			/* 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 = I915_READ_NOTRACE(ECOBUS);
> +			__gen6_gt_force_wake_mt_put(dev_priv);
> +			mutex_unlock(&dev->struct_mutex);
> +
> +			if (ecobus & FORCEWAKE_MT_ENABLE) {
> +				DRM_DEBUG_KMS("Using MT version of forcewake\n");
> +				dev_priv->gt.force_wake_get =
> +					__gen6_gt_force_wake_mt_get;
> +				dev_priv->gt.force_wake_put =
> +					__gen6_gt_force_wake_mt_put;
> +			}
> +		}
> +	}
> +}
> +



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 10/10] drm/i915: move force wake support into intel_pm

[Daniel Vetter]

On Mon, Jul 02, 2012 at 11:41:43AM -0700, Ben Widawsky wrote:
> On Mon,  2 Jul 2012 11:51:11 -0300
> Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:
> 
> > This commit moves force wake support routines into intel_pm modules, and
> > exports the gen6_gt_check_fifodbg routine (used in I915_READ).
> > 
> > Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> 
> I seem to recall Chris not liking this idea. Did I make that up?
> 
> Acked-by: Ben Widawsky <ben@bwidawsk.net>

Ok, I've slurped in the entire series, thanks for patches and review.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48