diff mbox

[4/4] drm/i915: Only sanitize GEM from late suspend

Message ID 20180531082246.9763-4-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 31, 2018, 8:22 a.m. UTC
During testing we encounter a conflict between SUSPEND_TEST_DEVICES and
disabling reset (gem_eio/suspend). This results in the device continuing
on without being reset, but since it has gone through HW sanitization to
account for the suspend/resume cycle, we have to assume the device has
been reset to its defaults. A simple way around this is to skip the
sanitize phase for SUSPEND_TEST_DEVICES by moving it to suspend-late.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c |  6 +++++-
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 22 +++++++++++++---------
 3 files changed, 19 insertions(+), 10 deletions(-)

Comments

Tvrtko Ursulin May 31, 2018, 3:09 p.m. UTC | #1
On 31/05/2018 09:22, Chris Wilson wrote:
> During testing we encounter a conflict between SUSPEND_TEST_DEVICES and
> disabling reset (gem_eio/suspend). This results in the device continuing
> on without being reset, but since it has gone through HW sanitization to
> account for the suspend/resume cycle, we have to assume the device has
> been reset to its defaults. A simple way around this is to skip the
> sanitize phase for SUSPEND_TEST_DEVICES by moving it to suspend-late.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.c |  6 +++++-
>   drivers/gpu/drm/i915/i915_drv.h |  1 +
>   drivers/gpu/drm/i915/i915_gem.c | 22 +++++++++++++---------
>   3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 8f002ae22e62..0d9b8cc0436d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -636,6 +636,8 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>   
>   static void i915_gem_fini(struct drm_i915_private *dev_priv)
>   {
> +	i915_gem_suspend_late(dev_priv);
> +
>   	/* Flush any outstanding unpin_work. */
>   	i915_gem_drain_workqueue(dev_priv);
>   
> @@ -1611,7 +1613,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>   	opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
>   	intel_opregion_notify_adapter(dev_priv, opregion_target_state);
>   
> -	intel_uncore_suspend(dev_priv);
>   	intel_opregion_unregister(dev_priv);
>   
>   	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
> @@ -1633,7 +1634,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>   
>   	disable_rpm_wakeref_asserts(dev_priv);
>   
> +	i915_gem_suspend_late(dev_priv);
> +
>   	intel_display_set_init_power(dev_priv, false);
> +	intel_uncore_suspend(dev_priv);
>   
>   	/*
>   	 * In case of firmware assisted context save/restore don't manually
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 32a65de7222d..38157df6ff5c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3169,6 +3169,7 @@ void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
>   int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
>   			   unsigned int flags);
>   int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
> +void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
>   void i915_gem_resume(struct drm_i915_private *dev_priv);
>   int i915_gem_fault(struct vm_fault *vmf);
>   int i915_gem_object_wait(struct drm_i915_gem_object *obj,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9b8fa1866cc9..f5c4ef052001 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5055,6 +5055,17 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>   	if (WARN_ON(!intel_engines_are_idle(dev_priv)))
>   		i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
>   
> +	intel_runtime_pm_put(dev_priv);
> +	return 0;
> +
> +err_unlock:
> +	mutex_unlock(&dev->struct_mutex);
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
> +}
> +
> +void i915_gem_suspend_late(struct drm_i915_private *i915)
> +{
>   	/*
>   	 * Neither the BIOS, ourselves or any other kernel
>   	 * expects the system to be in execlists mode on startup,
> @@ -5074,16 +5085,9 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>   	 * machines is a good idea, we don't - just in case it leaves the
>   	 * machine in an unusable condition.
>   	 */
> -	intel_uc_sanitize(dev_priv);
> -	i915_gem_sanitize(dev_priv);
>   
> -	intel_runtime_pm_put(dev_priv);
> -	return 0;
> -
> -err_unlock:
> -	mutex_unlock(&dev->struct_mutex);
> -	intel_runtime_pm_put(dev_priv);
> -	return ret;
> +	intel_uc_sanitize(i915);
> +	i915_gem_sanitize(i915);
>   }
>   
>   void i915_gem_resume(struct drm_i915_private *i915)
> 

It's hackery for PM core not suspending devices fully in device test 
mode, but I have no better ideas, nor can I figure out why PM core does 
it like that:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8f002ae22e62..0d9b8cc0436d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -636,6 +636,8 @@  static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 
 static void i915_gem_fini(struct drm_i915_private *dev_priv)
 {
+	i915_gem_suspend_late(dev_priv);
+
 	/* Flush any outstanding unpin_work. */
 	i915_gem_drain_workqueue(dev_priv);
 
@@ -1611,7 +1613,6 @@  static int i915_drm_suspend(struct drm_device *dev)
 	opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
 	intel_opregion_notify_adapter(dev_priv, opregion_target_state);
 
-	intel_uncore_suspend(dev_priv);
 	intel_opregion_unregister(dev_priv);
 
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
@@ -1633,7 +1634,10 @@  static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 
 	disable_rpm_wakeref_asserts(dev_priv);
 
+	i915_gem_suspend_late(dev_priv);
+
 	intel_display_set_init_power(dev_priv, false);
+	intel_uncore_suspend(dev_priv);
 
 	/*
 	 * In case of firmware assisted context save/restore don't manually
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 32a65de7222d..38157df6ff5c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3169,6 +3169,7 @@  void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
 int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
+void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
 int i915_gem_fault(struct vm_fault *vmf);
 int i915_gem_object_wait(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9b8fa1866cc9..f5c4ef052001 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5055,6 +5055,17 @@  int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	if (WARN_ON(!intel_engines_are_idle(dev_priv)))
 		i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
 
+	intel_runtime_pm_put(dev_priv);
+	return 0;
+
+err_unlock:
+	mutex_unlock(&dev->struct_mutex);
+	intel_runtime_pm_put(dev_priv);
+	return ret;
+}
+
+void i915_gem_suspend_late(struct drm_i915_private *i915)
+{
 	/*
 	 * Neither the BIOS, ourselves or any other kernel
 	 * expects the system to be in execlists mode on startup,
@@ -5074,16 +5085,9 @@  int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 * machines is a good idea, we don't - just in case it leaves the
 	 * machine in an unusable condition.
 	 */
-	intel_uc_sanitize(dev_priv);
-	i915_gem_sanitize(dev_priv);
 
-	intel_runtime_pm_put(dev_priv);
-	return 0;
-
-err_unlock:
-	mutex_unlock(&dev->struct_mutex);
-	intel_runtime_pm_put(dev_priv);
-	return ret;
+	intel_uc_sanitize(i915);
+	i915_gem_sanitize(i915);
 }
 
 void i915_gem_resume(struct drm_i915_private *i915)