diff mbox

[v3,2/2] drm/i915: sanitize RPS resetting during GPU reset

Message ID 1418662768-29085-2-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak Dec. 15, 2014, 4:59 p.m. UTC
Atm, we don't disable RPS interrupts and related work items before
resetting the GPU. This may interfere with the following GPU
initialization and cause RPS interrupts to show up in PM_IIR too early
before calling gen6_enable_rps_interrupts() (triggering a WARN there).

Solve this by disabling RPS interrupts and flushing any related work
items before resetting the GPU.

v2:
- split out the common parts of the gt suspend and the new gt reset
  functions (Paulo)
v3:
- remove the check for UMS, it's a NOP nowadays (Daniel)

Reported-by: He, Shuang <shuang.he@intel.com>
Testcase: igt/gem_reset_stats/ban-render
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86644
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  4 +++-
 drivers/gpu/drm/i915/intel_pm.c | 28 +++++++++++++++++++---------
 2 files changed, 22 insertions(+), 10 deletions(-)

Comments

Jani Nikula Dec. 15, 2014, 5:15 p.m. UTC | #1
On Mon, 15 Dec 2014, Imre Deak <imre.deak@intel.com> wrote:
> Atm, we don't disable RPS interrupts and related work items before
> resetting the GPU. This may interfere with the following GPU
> initialization and cause RPS interrupts to show up in PM_IIR too early
> before calling gen6_enable_rps_interrupts() (triggering a WARN there).
>
> Solve this by disabling RPS interrupts and flushing any related work
> items before resetting the GPU.
>
> v2:
> - split out the common parts of the gt suspend and the new gt reset
>   functions (Paulo)
> v3:
> - remove the check for UMS, it's a NOP nowadays (Daniel)
>
> Reported-by: He, Shuang <shuang.he@intel.com>
> Testcase: igt/gem_reset_stats/ban-render
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86644
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Pushed both to drm-intel-next-fixes, thanks for the patches and review.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/i915_drv.c |  4 +++-
>  drivers/gpu/drm/i915/intel_pm.c | 28 +++++++++++++++++++---------
>  2 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f990ab4..fc8cfdd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -811,6 +811,8 @@ int i915_reset(struct drm_device *dev)
>  	if (!i915.reset)
>  		return 0;
>  
> +	intel_reset_gt_powersave(dev);
> +
>  	mutex_lock(&dev->struct_mutex);
>  
>  	i915_gem_reset(dev);
> @@ -880,7 +882,7 @@ int i915_reset(struct drm_device *dev)
>  		 * of re-init after reset.
>  		 */
>  		if (INTEL_INFO(dev)->gen > 5)
> -			intel_reset_gt_powersave(dev);
> +			intel_enable_gt_powersave(dev);
>  	} else {
>  		mutex_unlock(&dev->struct_mutex);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1f4b56e..964b28e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6191,6 +6191,20 @@ void intel_cleanup_gt_powersave(struct drm_device *dev)
>  		valleyview_cleanup_gt_powersave(dev);
>  }
>  
> +static void gen6_suspend_rps(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> +
> +	/*
> +	 * TODO: disable RPS interrupts on GEN9+ too once RPS support
> +	 * is added for it.
> +	 */
> +	if (INTEL_INFO(dev)->gen < 9)
> +		gen6_disable_rps_interrupts(dev);
> +}
> +
>  /**
>   * intel_suspend_gt_powersave - suspend PM work and helper threads
>   * @dev: drm device
> @@ -6206,14 +6220,7 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
>  	if (INTEL_INFO(dev)->gen < 6)
>  		return;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> -	/*
> -	 * TODO: disable RPS interrupts on GEN9+ too once RPS support
> -	 * is added for it.
> -	 */
> -	if (INTEL_INFO(dev)->gen < 9)
> -		gen6_disable_rps_interrupts(dev);
> +	gen6_suspend_rps(dev);
>  
>  	/* Force GPU to min freq during suspend */
>  	gen6_rps_idle(dev_priv);
> @@ -6316,8 +6323,11 @@ void intel_reset_gt_powersave(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	if (INTEL_INFO(dev)->gen < 6)
> +		return;
> +
> +	gen6_suspend_rps(dev);
>  	dev_priv->rps.enabled = false;
> -	intel_enable_gt_powersave(dev);
>  }
>  
>  static void ibx_init_clock_gating(struct drm_device *dev)
> -- 
> 1.8.4
>
Shuang He Dec. 15, 2014, 11:58 p.m. UTC | #2
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +5-2              360/366              363/366
SNB                                  448/450              448/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_kms_flip_rcs-flip-vs-panning-interruptible      DMESG_WARN(1, M26)PASS(1, M26)      DMESG_WARN(1, M26)
 ILK  igt_drv_suspend_fence-restore-untiled      DMESG_WARN(1, M26)PASS(5, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(5, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_busy-flip-interruptible      DMESG_WARN(1, M26)PASS(5, M37M26)      PASS(1, M26)
*ILK  igt_kms_flip_flip-vs-panning      PASS(2, M26)      NSPT(1, M26)
 ILK  igt_kms_flip_flip-vs-rmfb-interruptible      DMESG_WARN(1, M26)PASS(5, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_rcs-flip-vs-dpms      DMESG_WARN(1, M26)PASS(4, M37M26)      PASS(1, M26)
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f990ab4..fc8cfdd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -811,6 +811,8 @@  int i915_reset(struct drm_device *dev)
 	if (!i915.reset)
 		return 0;
 
+	intel_reset_gt_powersave(dev);
+
 	mutex_lock(&dev->struct_mutex);
 
 	i915_gem_reset(dev);
@@ -880,7 +882,7 @@  int i915_reset(struct drm_device *dev)
 		 * of re-init after reset.
 		 */
 		if (INTEL_INFO(dev)->gen > 5)
-			intel_reset_gt_powersave(dev);
+			intel_enable_gt_powersave(dev);
 	} else {
 		mutex_unlock(&dev->struct_mutex);
 	}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1f4b56e..964b28e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6191,6 +6191,20 @@  void intel_cleanup_gt_powersave(struct drm_device *dev)
 		valleyview_cleanup_gt_powersave(dev);
 }
 
+static void gen6_suspend_rps(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
+
+	/*
+	 * TODO: disable RPS interrupts on GEN9+ too once RPS support
+	 * is added for it.
+	 */
+	if (INTEL_INFO(dev)->gen < 9)
+		gen6_disable_rps_interrupts(dev);
+}
+
 /**
  * intel_suspend_gt_powersave - suspend PM work and helper threads
  * @dev: drm device
@@ -6206,14 +6220,7 @@  void intel_suspend_gt_powersave(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen < 6)
 		return;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
-	/*
-	 * TODO: disable RPS interrupts on GEN9+ too once RPS support
-	 * is added for it.
-	 */
-	if (INTEL_INFO(dev)->gen < 9)
-		gen6_disable_rps_interrupts(dev);
+	gen6_suspend_rps(dev);
 
 	/* Force GPU to min freq during suspend */
 	gen6_rps_idle(dev_priv);
@@ -6316,8 +6323,11 @@  void intel_reset_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	if (INTEL_INFO(dev)->gen < 6)
+		return;
+
+	gen6_suspend_rps(dev);
 	dev_priv->rps.enabled = false;
-	intel_enable_gt_powersave(dev);
 }
 
 static void ibx_init_clock_gating(struct drm_device *dev)