diff mbox

Revert "drm/i915: unconditionally use mt forcewake on hsw/ivb"

Message ID 1373400849-30351-1-git-send-email-linux@roeck-us.net (mailing list archive)
State New, archived
Headers show

Commit Message

Guenter Roeck July 9, 2013, 8:14 p.m. UTC
This reverts commit 36ec8f877481449bdfa072e6adf2060869e2b970.

The commit results in repeated 'Timed out waiting for forcewake old ack
to clear' messages on a Supermicro C7H61 board (BIOS version 2.00 and 2.00b)
with i7-3770K CPU. It ultimately results in a hangup if the system is highly
loaded. Reverting the commit fixes the issue.

While I understand that this should be fixed in the BIOS, a more recent BIOS
is not available fron the vendor. If/since no better solution is not available,
reverting the patch seems to be the way to go (otherwise me and others with
the same problem won't be able to run the upstream kernel on affected boards).

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/gpu/drm/i915/intel_pm.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

Comments

Daniel Vetter July 9, 2013, 8:33 p.m. UTC | #1
On Tue, Jul 09, 2013 at 01:14:09PM -0700, Guenter Roeck wrote:
> This reverts commit 36ec8f877481449bdfa072e6adf2060869e2b970.
> 
> The commit results in repeated 'Timed out waiting for forcewake old ack
> to clear' messages on a Supermicro C7H61 board (BIOS version 2.00 and 2.00b)
> with i7-3770K CPU. It ultimately results in a hangup if the system is highly
> loaded. Reverting the commit fixes the issue.
> 
> While I understand that this should be fixed in the BIOS, a more recent BIOS
> is not available fron the vendor. If/since no better solution is not available,
> reverting the patch seems to be the way to go (otherwise me and others with
> the same problem won't be able to run the upstream kernel on affected boards).
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Can you please add the bugzilla link here?

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index aa01128..5bae9ba 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4510,12 +4510,35 @@ void intel_gt_init(struct drm_device *dev)
>  	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 (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> -		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_mt_get;
> -		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_mt_put;

Please keep the unconditional use of mt forcewake for haswell, afaik we
don't have any reports that there are still broken BIOS out there.


> -	} else if (IS_GEN6(dev)) {
> +	} 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;
> +			}

This needs a DRM_INFO warning if MT forcewake is not available telling the
user telling the user that this might result in hangs with recent DDX
version, e.g.: "No MT forcewake available on Ivybridge, this can result in
issues when using vblank-synced partial screen updates."

Thanks, Daniel

> +		}
>  	}
>  	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
>  			  intel_gen6_powersave_work);
> -- 
> 1.7.9.7
>
Guenter Roeck July 9, 2013, 10:41 p.m. UTC | #2
On Tue, Jul 09, 2013 at 10:33:22PM +0200, Daniel Vetter wrote:
> On Tue, Jul 09, 2013 at 01:14:09PM -0700, Guenter Roeck wrote:
> > This reverts commit 36ec8f877481449bdfa072e6adf2060869e2b970.
> > 
> > The commit results in repeated 'Timed out waiting for forcewake old ack
> > to clear' messages on a Supermicro C7H61 board (BIOS version 2.00 and 2.00b)
> > with i7-3770K CPU. It ultimately results in a hangup if the system is highly
> > loaded. Reverting the commit fixes the issue.
> > 
> > While I understand that this should be fixed in the BIOS, a more recent BIOS
> > is not available fron the vendor. If/since no better solution is not available,
> > reverting the patch seems to be the way to go (otherwise me and others with
> > the same problem won't be able to run the upstream kernel on affected boards).
> > 
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> Can you please add the bugzilla link here?
> 
I'll create one and add a reference.

Thanks,
Guenter
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aa01128..5bae9ba 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4510,12 +4510,35 @@  void intel_gt_init(struct drm_device *dev)
 	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 (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_mt_get;
-		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_mt_put;
-	} else if (IS_GEN6(dev)) {
+	} 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;
+			}
+		}
 	}
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);