[PATCH 1/2] drm/i915: Do forcewake reset on gen8

[PATCH 1/2] drm/i915: Do forcewake reset on gen8

[mika.kuoppala]

From: Mika Kuoppala <mika.kuoppala@intel.com>

When we get control from BIOS there might be mt forcewake
bits already set. Apparently double write into mt forcewake
without proper clear/ack sequence in between will cause
system hang.

Fix this by clearing mt forcewake register on init,
like we do with older gens.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index c628414..25ceac4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -305,13 +305,13 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev))
 		vlv_force_wake_reset(dev_priv);
-	} else if (INTEL_INFO(dev)->gen >= 6) {
+	else if (IS_GEN6(dev) || IS_GEN7(dev))
 		__gen6_gt_force_wake_reset(dev_priv);
-		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
-			__gen6_gt_force_wake_mt_reset(dev_priv);
-	}
+
+	if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_GEN8(dev))
+		__gen6_gt_force_wake_mt_reset(dev_priv);
 }
 
 void intel_uncore_early_sanitize(struct drm_device *dev)
-- 
1.7.9.5

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 2/2] drm/i915: Don't access fifodbg registers on gen8

[mika.kuoppala]

From: Mika Kuoppala <mika.kuoppala@intel.com>

as they don't exists.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 25ceac4..cfb8011 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -143,7 +143,9 @@ static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv,
 			   _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
 	/* something from same cacheline, but !FORCEWAKE_MT */
 	__raw_posting_read(dev_priv, ECOBUS);
-	gen6_gt_check_fifodbg(dev_priv);
+
+	if (IS_GEN6(dev_priv->dev) || IS_GEN7(dev_priv->dev))
+		gen6_gt_check_fifodbg(dev_priv);
 }
 
 static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
@@ -973,7 +975,10 @@ static int gen6_do_reset(struct drm_device *dev)
 		dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
 
 	/* Restore fifo count */
-	dev_priv->uncore.fifo_count = __raw_i915_read32(dev_priv, GTFIFOCTL) & GT_FIFO_FREE_ENTRIES_MASK;
+	if (IS_GEN6(dev) || IS_GEN7(dev))
+		dev_priv->uncore.fifo_count =
+			__raw_i915_read32(dev_priv, GTFIFOCTL) &
+			GT_FIFO_FREE_ENTRIES_MASK;
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 	return ret;
-- 
1.7.9.5

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 2/2] drm/i915: Don't access fifodbg registers on gen8

[Ben Widawsky]

On Wed, Feb 19, 2014 at 06:59:26PM +0200, mika.kuoppala@intel.com wrote:
> From: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> as they don't exists.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 25ceac4..cfb8011 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -143,7 +143,9 @@ static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv,
>  			   _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
>  	/* something from same cacheline, but !FORCEWAKE_MT */
>  	__raw_posting_read(dev_priv, ECOBUS);
> -	gen6_gt_check_fifodbg(dev_priv);
> +
> +	if (IS_GEN6(dev_priv->dev) || IS_GEN7(dev_priv->dev))
> +		gen6_gt_check_fifodbg(dev_priv);
>  }

I think at this point, gen8 specific forcewake functions would be just
as good. Whatever you prefer though.

>  
>  static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> @@ -973,7 +975,10 @@ static int gen6_do_reset(struct drm_device *dev)
>  		dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
>  
>  	/* Restore fifo count */
> -	dev_priv->uncore.fifo_count = __raw_i915_read32(dev_priv, GTFIFOCTL) & GT_FIFO_FREE_ENTRIES_MASK;
> +	if (IS_GEN6(dev) || IS_GEN7(dev))
> +		dev_priv->uncore.fifo_count =
> +			__raw_i915_read32(dev_priv, GTFIFOCTL) &
> +			GT_FIFO_FREE_ENTRIES_MASK;
>  
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  	return ret;


This code looks somewhat suspicious in that I cannot fathom a case where
fifo_count should be anything other than 0 here. It seems this behavior has
existed since its induction:

commit 286fed412a134e76be55899bc628c6fa59cb70da
Author: Keith Packard <keithp@keithp.com>
Date:   Fri Jan 6 11:44:11 2012 -0800

    drm/i915: Hold gt_lock during reset

I for one would mind another patch with a WARN if it's non-zero... just
saying, and in such a case we could make the WARN gen specific, and the
dev_priv->uncore.fifo_count = 0 for all platforms.


Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 1/2] drm/i915: Do forcewake reset on gen8

[Ben Widawsky]

On Wed, Feb 19, 2014 at 06:59:25PM +0200, mika.kuoppala@intel.com wrote:
> From: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> When we get control from BIOS there might be mt forcewake
> bits already set. Apparently double write into mt forcewake
> without proper clear/ack sequence in between will cause
> system hang.
> 

I'd remove the bit about the hang, since we can't really explain it, and
while I am at it, I'd remove it from the previous patch I reviewed to,
but whatever.

> Fix this by clearing mt forcewake register on init,
> like we do with older gens.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Either way:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/intel_uncore.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index c628414..25ceac4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -305,13 +305,13 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (IS_VALLEYVIEW(dev))
>  		vlv_force_wake_reset(dev_priv);
> -	} else if (INTEL_INFO(dev)->gen >= 6) {
> +	else if (IS_GEN6(dev) || IS_GEN7(dev))
>  		__gen6_gt_force_wake_reset(dev_priv);
> -		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
> -			__gen6_gt_force_wake_mt_reset(dev_priv);
> -	}
> +
> +	if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_GEN8(dev))
> +		__gen6_gt_force_wake_mt_reset(dev_priv);
>  }
>  
>  void intel_uncore_early_sanitize(struct drm_device *dev)
> -- 
> 1.7.9.5
> 
> ---------------------------------------------------------------------
> Intel Finland Oy
> Registered Address: PL 281, 00181 Helsinki 
> Business Identity Code: 0357606 - 4 
> Domiciled in Helsinki 
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center