On Wed, 11 Oct 2017 10:54:08 +0200, Sagar Arun Kamble
<sagar.a.kamble@xxxxxxxxx> wrote:
GuC/HuC resume operation depends on whether firmwares are available
in the WOPCM region. This is known through register WOPCM_SIZE BIT(0).
If it indicates WOPCM is locked (bit is set) we just need to send action
to GuC to resume and enable other related GuC functionality such as
communication, interrupts and submission.
If it indicates WOPCM is not locked then we need to first reload the
GuC/HuC and then do all resume tasks. Currently on resume from sleep,
GuC/HuC are not loaded as GPU is reset at the end of suspend/early
resume.
So we will have to reload the firmware and send action to resume.
Resume will be done through uc_init_hw from gem_init_hw based on newly
introduced state "guc->suspended". During gem_init_hw firmware load will
be skipped based on resume status during intel_uc_resume.
Also updated the accesses to dev_priv->guc and dev_priv->huc structure by
reusing initial declared pointer.
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>
---
drivers/gpu/drm/i915/i915_guc_reg.h | 1 +
drivers/gpu/drm/i915/intel_guc.c | 2 +
drivers/gpu/drm/i915/intel_guc.h | 3 ++
drivers/gpu/drm/i915/intel_uc.c | 99
+++++++++++++++++++++++++++++--------
4 files changed, 85 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h
b/drivers/gpu/drm/i915/i915_guc_reg.h
index 35cf991..532296b 100644
--- a/drivers/gpu/drm/i915/i915_guc_reg.h
+++ b/drivers/gpu/drm/i915/i915_guc_reg.h
@@ -75,6 +75,7 @@
/* Defines WOPCM space available to GuC firmware */
#define GUC_WOPCM_SIZE _MMIO(0xc050)
+#define GUC_WOPCM_LOCKED BIT(0)
/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
diff --git a/drivers/gpu/drm/i915/intel_guc.c
b/drivers/gpu/drm/i915/intel_guc.c
index 55a0158..73be382 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -65,6 +65,8 @@ void intel_guc_init_early(struct intel_guc *guc)
mutex_init(&guc->send_mutex);
guc->send = intel_guc_send_nop;
guc->notify = gen8_guc_raise_irq;
+ guc->suspended = false;
+ guc->skip_load_on_resume = false;
}
int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
diff --git a/drivers/gpu/drm/i915/intel_guc.h
b/drivers/gpu/drm/i915/intel_guc.h
index a587210..9f84033 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -38,6 +38,9 @@ struct intel_guc {
struct intel_guc_log log;
struct intel_guc_ct ct;
+ bool suspended;
+ bool skip_load_on_resume;
maybe bool xxx:1 to save space.
+
/* Log snapshot if GuC errors during load */
struct drm_i915_gem_object *load_err_log;
diff --git a/drivers/gpu/drm/i915/intel_uc.c
b/drivers/gpu/drm/i915/intel_uc.c
index 1365724..f641872 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -152,14 +152,35 @@ static void guc_disable_communication(struct
intel_guc *guc)
guc->send = intel_guc_send_nop;
}
+static inline bool guc_wopcm_locked(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+ return I915_READ(GUC_WOPCM_SIZE) & GUC_WOPCM_LOCKED;
what about adding !! trick?
+}
+
int intel_uc_init_hw(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
+ struct intel_huc *huc = &dev_priv->huc;
int ret, attempts;
if (!i915_modparams.enable_guc_loading)
return 0;
+ /*
+ * If on resume from sleep GuC was available we resumed GuC during
+ * i915_gem_resume. We need to skip load here. Reset
skip_load_on_resume
+ * to allow load during module reload/reset/next resume behavior.
+ */
+ if (guc->skip_load_on_resume) {
+ guc->skip_load_on_resume = false;
+ return 0;
+ }
+
+ WARN_ON_ONCE(guc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS);
+ WARN_ON_ONCE(huc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS);
+
guc_disable_communication(guc);
gen9_reset_guc_interrupts(dev_priv);
@@ -197,8 +218,8 @@ int intel_uc_init_hw(struct drm_i915_private
*dev_priv)
if (ret)
goto err_submission;
- intel_huc_init_hw(&dev_priv->huc);
- ret = intel_guc_init_hw(&dev_priv->guc);
+ intel_huc_init_hw(huc);
+ ret = intel_guc_init_hw(guc);
if (ret == 0 || ret != -EAGAIN)
break;
@@ -214,7 +235,21 @@ int intel_uc_init_hw(struct drm_i915_private
*dev_priv)
if (ret)
goto err_log_capture;
- intel_huc_auth(&dev_priv->huc);
+ /*
+ * If WOPCM was not locked during resume from sleep, GuC/HuC need to
+ * be reloaded. For this we are using intel_uc_init_hw path as we want
+ * to handle HuC/GuC reload, GuC resume, HuC authentication and
+ * submission enabling etc. all here.
+ */
+ if (guc->suspended) {
+ ret = intel_guc_resume(guc);
+ if (ret)
+ DRM_ERROR("GuC resume failed (%d)."
+ " GuC functions may not work\n", ret);
+ guc->suspended = false;
+ }
+
+ intel_huc_auth(huc);
if (i915_guc_submission_initialized(guc)) {
if (i915_modparams.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
@@ -305,6 +340,8 @@ int intel_uc_suspend(struct drm_i915_private
*dev_priv)
gen9_disable_guc_interrupts(dev_priv);
guc_disable_communication(guc);
+ guc->suspended = true;
+
goto out;
out_suspend:
@@ -326,28 +363,50 @@ int intel_uc_suspend(struct drm_i915_private
*dev_priv)
void intel_uc_resume(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
+ struct intel_huc *huc = &dev_priv->huc;
int ret;
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
- return;
-
- ret = guc_enable_communication(guc);
- if (ret) {
- DRM_DEBUG_DRIVER("GuC communication enable failed (%d)\n", ret);
+ if (!guc->suspended)
return;
- }
- if (i915_modparams.guc_log_level >= 0)
- gen9_enable_guc_interrupts(dev_priv);
+ /*
+ * If WOPCM is locked then GuC and HuC are still loaded. We just
+ * need to enable communication with GuC, enable interrupts,
+ * invoke GuC action to resume from sleep and enable submission.
+ * If WOPCM is not locked it is similar to fresh boot and we need
+ * reload the GuC/HuC firmwares and enable other GuC related
+ * mechanisms. Post reloading GuC we need to send action to resume
+ * from sleep for GuC to restore its state prior to suspend.
+ */
+ if (guc_wopcm_locked(guc)) {
+ huc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
+ guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
Hmm, as we didn't clear load_status (or at least I can't find it) then
load_status should still be INTEL_UC_FIRMWARE_SUCCESS here.
Maybe instead of introducing new skip_load_on_resume flag we can
just rely on load_status that is correctly maintained across suspend
resume.
- ret = intel_guc_resume(guc);
- if (ret)
- DRM_ERROR("GuC resume failed (%d)."
- "GuC functions may not work\n", ret);
+ ret = guc_enable_communication(guc);
+ if (ret) {
+ DRM_DEBUG_DRIVER("GuC communication enable failed"
+ " (%d)\n", ret);
Btw, the only path in guc_enable_communication that can fail reports
error so maybe this extra debug is not necessary ?
Maybe better option will be to reorg this code to include note about
forced GuC reload during init_hw ?
+ return;
+ }
- i915_guc_submission_enable(dev_priv);
+ if (i915_modparams.guc_log_level >= 0)
+ gen9_enable_guc_interrupts(dev_priv);
- DRM_DEBUG_DRIVER("GuC submission %s\n",
- i915_guc_submission_enabled(guc) ?
- "enabled" : "disabled");
+ ret = intel_guc_resume(guc);
+ if (ret)
+ DRM_ERROR("GuC resume failed (%d)."
+ " GuC functions may not work\n", ret);
+
Hmm, is it safe to continue with submission after failing resume ?
+ i915_guc_submission_enable(dev_priv);
+
+ DRM_DEBUG_DRIVER("GuC submission %s\n",
+ i915_guc_submission_enabled(guc) ?
+ "enabled" : "disabled");
+ guc->suspended = false;
+ guc->skip_load_on_resume = true;
+ } else {
+ DRM_DEBUG_DRIVER("GuC not available. Resume will be done"
+ " during i915_gem_init_hw\n");
+ guc->skip_load_on_resume = false;
+ }
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
