diff mbox

[2/3] drm/i915: Don't frob with RPS around GPU reset

Message ID 1464012583-22480-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä May 23, 2016, 2:09 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Based on my observations GPU reset doesn't clobber the RPS state, so
frobbing with RPS around GPU reset seems rather pointless. Just get
rid of it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 11 -----------
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 drivers/gpu/drm/i915/intel_pm.c  |  9 ---------
 3 files changed, 21 deletions(-)

Comments

Chris Wilson May 23, 2016, 2:32 p.m. UTC | #1
On Mon, May 23, 2016 at 05:09:42PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Based on my observations GPU reset doesn't clobber the RPS state, so
> frobbing with RPS around GPU reset seems rather pointless. Just get
> rid of it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Testcase: igt/pm_rpm/reset ?

Looks like that is the desired test.

Maybe make it basic and see what CI says :)

Actually it probably should be a basic test considering how often we
hang.
-Chris
Daniel Vetter May 24, 2016, 8:24 a.m. UTC | #2
On Mon, May 23, 2016 at 03:32:40PM +0100, Chris Wilson wrote:
> On Mon, May 23, 2016 at 05:09:42PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Based on my observations GPU reset doesn't clobber the RPS state, so
> > frobbing with RPS around GPU reset seems rather pointless. Just get
> > rid of it.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Testcase: igt/pm_rpm/reset ?

pm_rps&rc6_residency seem more appropriate. pm_rpm just outright yanks the
power cable for the GT. Unfortunately those 2 tests aren't particularly
nasty ...
-Daniel

> 
> Looks like that is the desired test.
> 
> Maybe make it basic and see what CI says :)
> 
> Actually it probably should be a basic test considering how often we
> hang.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson May 24, 2016, 8:36 a.m. UTC | #3
On Tue, May 24, 2016 at 10:24:58AM +0200, Daniel Vetter wrote:
> On Mon, May 23, 2016 at 03:32:40PM +0100, Chris Wilson wrote:
> > On Mon, May 23, 2016 at 05:09:42PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Based on my observations GPU reset doesn't clobber the RPS state, so
> > > frobbing with RPS around GPU reset seems rather pointless. Just get
> > > rid of it.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Testcase: igt/pm_rpm/reset ?
> 
> pm_rps&rc6_residency seem more appropriate. pm_rpm just outright yanks the
> power cable for the GT. Unfortunately those 2 tests aren't particularly
> nasty ...

Typo, I was looking at pm_rps since it has the reset test that should be
checking whether RPS works after hang/reset.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dba03c026151..3794dc67caa4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -930,8 +930,6 @@  int i915_reset(struct drm_i915_private *dev_priv)
 	unsigned reset_counter;
 	int ret;
 
-	intel_reset_gt_powersave(dev_priv);
-
 	mutex_lock(&dev->struct_mutex);
 
 	/* Clear any previous failed attempts at recovery. Time to try again. */
@@ -994,15 +992,6 @@  int i915_reset(struct drm_i915_private *dev_priv)
 
 	mutex_unlock(&dev->struct_mutex);
 
-	/*
-	 * rps/rc6 re-init is necessary to restore state lost after the
-	 * reset and the re-install of gt irqs. Skip for ironlake per
-	 * previous concerns that it doesn't respond well to some forms
-	 * of re-init after reset.
-	 */
-	if (INTEL_INFO(dev)->gen > 5)
-		intel_enable_gt_powersave(dev_priv);
-
 	return 0;
 
 error:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0741b2d3aa65..ca3b69b61f06 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1652,7 +1652,6 @@  void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_enable_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_disable_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_reset_gt_powersave(struct drm_i915_private *dev_priv);
 void gen6_update_ring_freq(struct drm_i915_private *dev_priv);
 void gen6_rps_busy(struct drm_i915_private *dev_priv);
 void gen6_rps_reset_ei(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 576e98744a2d..817c84c22782 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6508,15 +6508,6 @@  void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
 	}
 }
 
-void intel_reset_gt_powersave(struct drm_i915_private *dev_priv)
-{
-	if (INTEL_INFO(dev_priv)->gen < 6)
-		return;
-
-	gen6_suspend_rps(dev_priv);
-	dev_priv->rps.enabled = false;
-}
-
 static void ibx_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;