[5/6] drm/i915/uc: move uc_resume under gt_resume
diff mbox series

Message ID 20190729234727.28286-6-daniele.ceraolospurio@intel.com
State New
Headers show
Series
  • Call uC functions from GT ones
Related show

Commit Message

Daniele Ceraolo Spurio July 29, 2019, 11:47 p.m. UTC
intel_uc is part of intel_gt so it makes logical sense for it to be
resumed as part of it. Note that, since gt_resume is also called during
the init flow, a state variable has been added to intel_uc to avoid
asking an already running GuC to resume.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_pm.c |  2 --
 drivers/gpu/drm/i915/gt/intel_gt_pm.c  |  3 +++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 10 ++++++++--
 drivers/gpu/drm/i915/gt/uc/intel_uc.h  |  2 ++
 4 files changed, 13 insertions(+), 4 deletions(-)

Comments

Chris Wilson July 30, 2019, 8:09 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2019-07-30 00:47:26)
> intel_uc is part of intel_gt so it makes logical sense for it to be
> resumed as part of it. Note that, since gt_resume is also called during
> the init flow, a state variable has been added to intel_uc to avoid
> asking an already running GuC to resume.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_pm.c |  2 --
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c  |  3 +++
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 10 ++++++++--
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h  |  2 ++
>  4 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> index b5561cbdc5ea..25610de3961b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> @@ -265,8 +265,6 @@ void i915_gem_resume(struct drm_i915_private *i915)
>         if (intel_gt_resume(&i915->gt))
>                 goto err_wedged;
>  
> -       intel_uc_resume(&i915->gt.uc);
> -
>         /* Always reload a context for powersaving. */
>         if (!i915_gem_load_power_context(i915))
>                 goto err_wedged;

This sequence itself does not belong to i915_gem_resume() and needs to
be lifted to intel_gt_resume (that is establishing the baseline GT power
context). So I would rather postpone this to try and avoid having to
introduce bool suspended if at all possible.

Another rule of thumb to consider is that we should be able to throw gt
initialisation into an async task (and I'm considering an async task
per engine, although for a large part we can achieve asynchronicity via
HW queues).
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index b5561cbdc5ea..25610de3961b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -265,8 +265,6 @@  void i915_gem_resume(struct drm_i915_private *i915)
 	if (intel_gt_resume(&i915->gt))
 		goto err_wedged;
 
-	intel_uc_resume(&i915->gt.uc);
-
 	/* Always reload a context for powersaving. */
 	if (!i915_gem_load_power_context(i915))
 		goto err_wedged;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 1a32e3e523c0..2250ffbd2f32 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -162,6 +162,9 @@  int intel_gt_resume(struct intel_gt *gt)
 	}
 	intel_gt_pm_put(gt);
 
+	if (!err)
+		err = intel_uc_resume(&gt->uc);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 2f71f3704c46..a9a893c5fbe0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -380,6 +380,8 @@  static void __uc_sanitize(struct intel_uc *uc)
 	intel_guc_sanitize(guc);
 
 	__intel_uc_reset_hw(uc);
+
+	uc->suspended = false;
 }
 
 void intel_uc_sanitize(struct intel_uc *uc)
@@ -598,6 +600,8 @@  void intel_uc_runtime_suspend(struct intel_uc *uc)
 		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
 
 	guc_disable_communication(guc);
+
+	uc->suspended = true;
 }
 
 void intel_uc_suspend(struct intel_uc *uc)
@@ -605,7 +609,7 @@  void intel_uc_suspend(struct intel_uc *uc)
 	struct intel_guc *guc = &uc->guc;
 	intel_wakeref_t wakeref;
 
-	if (!intel_guc_is_running(guc))
+	if (!intel_guc_is_running(guc) || uc->suspended)
 		return;
 
 	with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref)
@@ -617,7 +621,7 @@  int intel_uc_resume(struct intel_uc *uc)
 	struct intel_guc *guc = &uc->guc;
 	int err;
 
-	if (!intel_guc_is_running(guc))
+	if (!intel_guc_is_running(guc) || !uc->suspended)
 		return 0;
 
 	guc_enable_communication(guc);
@@ -628,5 +632,7 @@  int intel_uc_resume(struct intel_uc *uc)
 		return err;
 	}
 
+	uc->suspended = false;
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index fe3362fd7706..ea58fc164d1e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -31,6 +31,8 @@ 
 struct intel_uc {
 	struct intel_guc guc;
 	struct intel_huc huc;
+
+	bool suspended;
 };
 
 void intel_uc_init_early(struct intel_uc *uc);