On Wed, 11 Oct 2017 10:54:05 +0200, Sagar Arun Kamble
<sagar.a.kamble@xxxxxxxxx> wrote:
Suspending GuC involves bunch of tasks controlled by GuC OS and some
controlled by Host OS.
Host needs to disable submission to GuC and any other GuC functions.
Then,
GuC's task is initiated by Host sending action to GuC to enter sleep
state. On this action, GuC preempts engines to idle context and then
saves
internal state to a buffer. It also disables internal interrupts/timers
to
avoid any wake-ups.
After this, Host should disable GuC interrupts, communication with GuC
(intel_guc_send/notify). GGTT invalidate update will have to be done in
conjunction with GTT related suspend/resume tasks.
v2: Rebase w.r.t removal of GuC code restructuring.
v3: Removed GuC specific helpers as tasks other than send H2G for
sleep/resume are to be done from uc generic functions. (Michal Wajdeczko)
v4: Simplified/Unified the error messaging in uc_runtime_suspend/resume.
(Michal Wajdeczko). Rebase w.r.t i915_modparams change.
Added documentation to intel_uc_runtime_suspend/resume.
v5: Removed enable_guc_loading based check from intel_uc_runtime_suspend
and intel_uc_runtime_resume and pulled FW load_status based checks from
intel_guc_suspend/resume into these functions. (Michal Wajdeczko)
v6: Adjusted intel_uc_runtime_resume with prototype change to not return
value.
v7: Rebase.
v8: Updated commit description and added submission enable/disable in
GuC suspend/resume paths. Removed GGTT invalidate update functions.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> #6
---
drivers/gpu/drm/i915/intel_guc.c | 11 -------
drivers/gpu/drm/i915/intel_uc.c | 65
+++++++++++++++++++++++++++++++++++++---
2 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c
b/drivers/gpu/drm/i915/intel_guc.c
index 9a2df69..55a0158 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -177,11 +177,6 @@ int intel_guc_suspend(struct intel_guc *guc)
struct i915_gem_context *ctx;
u32 data[3];
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
- return 0;
-
- gen9_disable_guc_interrupts(i915);
-
ctx = i915->kernel_context;
data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
@@ -204,12 +199,6 @@ int intel_guc_resume(struct intel_guc *guc)
struct i915_gem_context *ctx;
u32 data[3];
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
- return 0;
-
- if (i915_modparams.guc_log_level >= 0)
- gen9_enable_guc_interrupts(i915);
-
ctx = i915->kernel_context;
data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
diff --git a/drivers/gpu/drm/i915/intel_uc.c
b/drivers/gpu/drm/i915/intel_uc.c
index b5c132c..297a321 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -284,18 +284,75 @@ void intel_uc_fini_hw(struct drm_i915_private
*dev_priv)
i915_ggtt_disable_guc(dev_priv);
}
+/**
+ * intel_uc_suspend() - Suspend uC operation.
+ * @dev_priv: i915 device private
+ *
Ha! found missing kerneldoc ... maybe it can be partially moved to
previous patch ?
+ * This function disables GuC submission, invokes GuC OS suspension,
+ * disables GuC interrupts and disable communication with GuC.
+ *
+ * Return: non-zero code on error
+ */
int intel_uc_suspend(struct drm_i915_private *dev_priv)
{
- int ret;
+ struct intel_guc *guc = &dev_priv->guc;
+ int ret = 0;
+
+ if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ goto out;
Hmm, is it ok to report DRM_ERROR if Guc was not started/loaded ?
Return 0 seems to be still the best option here.
+
+ i915_guc_submission_disable(dev_priv);
- ret = intel_guc_suspend(&dev_priv->guc);
+ ret = intel_guc_suspend(guc);
if (ret)
- DRM_ERROR("Failed to suspend GuC\n");
+ goto out_suspend;
+
+ gen9_disable_guc_interrupts(dev_priv);
+ guc_disable_communication(guc);
+
+ goto out;
+
+out_suspend:
+ i915_guc_submission_enable(dev_priv);
+out:
+ if (ret)
+ DRM_ERROR("uC Suspend failed (%d)\n", ret);
Unless I read wrong, we are re-enabling guc submission on failure,
so maybe error should say something like:
DRM_ERROR("Failed to suspend uC, aborting suspend\n");
return ret;
}
+/**
+ * intel_uc_resume() - Resume uC operation.
+ * @dev_priv: i915 device private
+ *
+ * This function enables communication with GuC, enables GuC interrupts,
+ * invokes GuC OS resumption and enables GuC submission.
+ */
void intel_uc_resume(struct drm_i915_private *dev_priv)
{
- intel_guc_resume(&dev_priv->guc);
+ struct intel_guc *guc = &dev_priv->guc;
+ int ret;
+
+ if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ return;
+
+ ret = guc_enable_communication(guc);
Hmm, not too early ? CT will try to talk with Guc.
+ if (ret) {
+ DRM_DEBUG_DRIVER("GuC communication enable failed (%d)\n", ret);
+ return;
+ }
+
+ if (i915_modparams.guc_log_level >= 0)
+ gen9_enable_guc_interrupts(dev_priv);
+
+ ret = intel_guc_resume(guc);
+ if (ret)
+ DRM_ERROR("GuC resume failed (%d)."
+ "GuC functions may not work\n", ret);
+
+ i915_guc_submission_enable(dev_priv);
+
+ DRM_DEBUG_DRIVER("GuC submission %s\n",
+ i915_guc_submission_enabled(guc) ?
+ "enabled" : "disabled");
Hmm, this message can be part of i915_guc_submission_enable
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
