diff mbox

[v8,3/8] drm/i915: Create uC runtime and system suspend/resume helpers

Message ID 1506432285-14979-3-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Sept. 26, 2017, 1:24 p.m. UTC 
Prepared generic helpers intel_uc_suspend, intel_uc_resume,
intel_uc_runtime_suspend, intel_uc_runtime_resume. These are
called from respective GEM functions.

v2: Rebase w.r.t removal of GuC code restructuring.

v3: Calling intel_uc_resume from i915_gem_resume post resuming
i915 gem setup. This is symmetrical with i915_gem_suspend.
Removed error messages from i915 suspend/resume routines as
uC suspend/resume routines will have those. (Michal Wajdeczko)
Declare wedged on uc_suspend failure and uc_resume failure.
(Michał Winiarski)
Added DRM_DEBUG message about skipping changes in intel_uc_resume.
Keeping the uC suspend/resume function definitions close to other
uC functions.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  1 -
 drivers/gpu/drm/i915/i915_gem.c | 32 +++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_uc.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h |  4 ++++
 4 files changed, 49 insertions(+), 10 deletions(-)

Comments

Chris Wilson Sept. 26, 2017, 1:43 p.m. UTC | #1 
Quoting Sagar Arun Kamble (2017-09-26 14:24:40)
> @@ -2077,7 +2079,7 @@ int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
>                 reg->dirty = true;
>         }
>  
> -       return 0;
> +       return ret;
OI! These are written as return 0 because you are very much not meant to
reach this point with an error.
-Chris
sagar.a.kamble@intel.com Sept. 26, 2017, 2:22 p.m. UTC | #2 
On 9/26/2017 7:13 PM, Chris Wilson wrote:
> Quoting Sagar Arun Kamble (2017-09-26 14:24:40)
>> @@ -2077,7 +2079,7 @@ int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
>>                  reg->dirty = true;
>>          }
>>   
>> -       return 0;
>> +       return ret;
> OI! These are written as return 0 because you are very much not meant to
> reach this point with an error.
> -Chris
What if GuC suspend fail? Do we still want to return 0. It might cause 
issues as GuC will be active when PCI Device is set to D3.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d67b8c..8bfebe3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2518,7 +2518,6 @@  static int intel_runtime_suspend(struct device *kdev)
 		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
 		intel_runtime_pm_enable_interrupts(dev_priv);
 
-		intel_guc_resume(dev_priv);
 		i915_gem_runtime_resume(dev_priv);
 		enable_rpm_wakeref_asserts(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5dcd8c0..8e6e2bd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2034,9 +2034,11 @@  int i915_gem_fault(struct vm_fault *vmf)
 int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
 {
 	struct drm_i915_gem_object *obj, *on;
-	int i;
+	int i, ret;
 
-	intel_guc_suspend(dev_priv);
+	ret = intel_uc_runtime_suspend(dev_priv);
+	if (ret)
+		return ret;
 
 	/*
 	 * Only called during RPM suspend. All users of the userfault_list
@@ -2077,7 +2079,7 @@  int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
 		reg->dirty = true;
 	}
 
-	return 0;
+	return ret;
 }
 
 /**
@@ -2097,9 +2099,7 @@  int i915_gem_runtime_resume(struct drm_i915_private *dev_priv)
 	i915_gem_init_swizzling(dev_priv);
 	i915_gem_restore_fences(dev_priv);
 
-	intel_guc_resume(dev_priv);
-
-	return 0;
+	return intel_uc_runtime_resume(dev_priv);
 }
 
 static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
@@ -4600,7 +4600,9 @@  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_guc_suspend(dev_priv);
+	ret = intel_uc_suspend(dev_priv);
+	if (ret)
+		i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
 
 	/*
 	 * Neither the BIOS, ourselves or any other kernel
@@ -4644,6 +4646,7 @@  int i915_gem_suspend(struct drm_i915_private *dev_priv)
 int i915_gem_resume(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
+	int ret;
 
 	WARN_ON(dev_priv->gt.awake);
 
@@ -4656,10 +4659,21 @@  int i915_gem_resume(struct drm_i915_private *dev_priv)
 	 * it and start again.
 	 */
 	dev_priv->gt.resume(dev_priv);
-	intel_guc_resume(dev_priv);
+
+	/*
+	 * NB: Full gem reinitialization is being done in i915_drm_resume
+	 * after gem_resume, so currently intel_uc_resume will be of no use.
+	 * Hence, intel_uc_resume is nop currently. If full reinitialization is
+	 * removed, will need to put functionality to resume from sleep in
+	 * intel_uc_resume.
+	 */
+	ret = intel_uc_resume(dev_priv);
+	if (ret)
+		i915_gem_set_wedged(dev_priv);
+
 	mutex_unlock(&dev->struct_mutex);
 
-	return 0;
+	return ret;
 }
 
 void i915_gem_init_swizzling(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2774778..b9376e4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -481,6 +481,28 @@  void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	i915_ggtt_disable_guc(dev_priv);
 }
 
+int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
+{
+	return intel_guc_suspend(dev_priv);
+}
+
+int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
+{
+	return intel_guc_resume(dev_priv);
+}
+
+int intel_uc_suspend(struct drm_i915_private *dev_priv)
+{
+	return intel_guc_suspend(dev_priv);
+}
+
+int intel_uc_resume(struct drm_i915_private *dev_priv)
+{
+	DRM_DEBUG_DRIVER("uC resume does nothing as full reinit is done\n");
+
+	return 0;
+}
+
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
 {
 	WARN(1, "Unexpected send: action=%#x\n", *action);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 6966349..0a79e17 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,10 @@  struct intel_huc {
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
+int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
+int intel_uc_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_resume(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);