diff mbox

drm/i915: fix FORCEWAKE posting reads

Message ID 1357890833-8523-1-git-send-email-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula Jan. 11, 2013, 7:53 a.m. UTC
We stopped reading FORCEWAKE for posting reads in

commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Sat Sep 1 22:59:50 2012 -0700

    drm/i915: Never read FORCEWAKE

and started using something from the same cacheline instead. This was true
for ECOBUS, but not for GTFIFODBG. Add posting reads on ECOBUS where
GTFIFODBG was relied on.

In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as
well.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411
Reported-and-tested-by: Alexander Bersenev <bay@hackerdom.ru>
CC: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Ben Widawsky Jan. 11, 2013, 6:15 p.m. UTC | #1
On Fri, Jan 11, 2013 at 09:53:53AM +0200, Jani Nikula wrote:
> We stopped reading FORCEWAKE for posting reads in
> 
> commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Sat Sep 1 22:59:50 2012 -0700
> 
>     drm/i915: Never read FORCEWAKE
> 
> and started using something from the same cacheline instead. This was true
> for ECOBUS, but not for GTFIFODBG. Add posting reads on ECOBUS where
> GTFIFODBG was relied on.
> 
> In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as
> well.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411
> Reported-and-tested-by: Alexander Bersenev <bay@hackerdom.ru>
> CC: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Am I the only skeptic about what is actually fixing the bug here?
Without WC MMIO, the cacheline we readback from makes no difference.

ie. I put ECOBUS there for a warm and fuzzy feeling, nothing else.

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index abfff29..06b75a1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4122,7 +4122,8 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff));
> -	POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
> +	/* something from same cacheline, but !FORCEWAKE_MT */
> +	POSTING_READ(ECOBUS);
>  }
>  
>  static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> @@ -4139,7 +4140,8 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
>  		DRM_ERROR("Timed out waiting for forcewake old ack to clear.\n");
>  
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
> -	POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
> +	/* something from same cacheline, but !FORCEWAKE_MT */
> +	POSTING_READ(ECOBUS);
>  
>  	if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
>  			    FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -4176,14 +4178,16 @@ void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
>  static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE, 0);
> -	/* gen6_gt_check_fifodbg doubles as the POSTING_READ */
> +	/* something from same cacheline, but !FORCEWAKE */
> +	POSTING_READ(ECOBUS);
>  	gen6_gt_check_fifodbg(dev_priv);
>  }
>  
>  static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> -	/* gen6_gt_check_fifodbg doubles as the POSTING_READ */
> +	/* something from same cacheline, but !FORCEWAKE_MT */
> +	POSTING_READ(ECOBUS);
>  	gen6_gt_check_fifodbg(dev_priv);
>  }
>  
> @@ -4223,6 +4227,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>  static void vlv_force_wake_reset(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(0xffff));
> +	/* something from same cacheline, but !FORCEWAKE_VLV */
> +	POSTING_READ(FORCEWAKE_ACK_VLV);
>  }
>  
>  static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> @@ -4243,7 +4249,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
>  static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> -	/* The below doubles as a POSTING_READ */
> +	/* something from same cacheline, but !FORCEWAKE_VLV */
> +	POSTING_READ(FORCEWAKE_ACK_VLV);
>  	gen6_gt_check_fifodbg(dev_priv);
>  }
>  
> -- 
> 1.7.9.5
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index abfff29..06b75a1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4122,7 +4122,8 @@  static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff));
-	POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
+	/* something from same cacheline, but !FORCEWAKE_MT */
+	POSTING_READ(ECOBUS);
 }
 
 static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
@@ -4139,7 +4140,8 @@  static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
 		DRM_ERROR("Timed out waiting for forcewake old ack to clear.\n");
 
 	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
-	POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
+	/* something from same cacheline, but !FORCEWAKE_MT */
+	POSTING_READ(ECOBUS);
 
 	if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
 			    FORCEWAKE_ACK_TIMEOUT_MS))
@@ -4176,14 +4178,16 @@  void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
 static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE, 0);
-	/* gen6_gt_check_fifodbg doubles as the POSTING_READ */
+	/* something from same cacheline, but !FORCEWAKE */
+	POSTING_READ(ECOBUS);
 	gen6_gt_check_fifodbg(dev_priv);
 }
 
 static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
-	/* gen6_gt_check_fifodbg doubles as the POSTING_READ */
+	/* something from same cacheline, but !FORCEWAKE_MT */
+	POSTING_READ(ECOBUS);
 	gen6_gt_check_fifodbg(dev_priv);
 }
 
@@ -4223,6 +4227,8 @@  int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
 static void vlv_force_wake_reset(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(0xffff));
+	/* something from same cacheline, but !FORCEWAKE_VLV */
+	POSTING_READ(FORCEWAKE_ACK_VLV);
 }
 
 static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
@@ -4243,7 +4249,8 @@  static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
 static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
-	/* The below doubles as a POSTING_READ */
+	/* something from same cacheline, but !FORCEWAKE_VLV */
+	POSTING_READ(FORCEWAKE_ACK_VLV);
 	gen6_gt_check_fifodbg(dev_priv);
 }