Re: [PATCH v13 14/21] drm/i915/uc: Update GEM runtime resume with need for reload of GuC/HuC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/11/2017 10:49 PM, Michal Wajdeczko wrote:
On Wed, 11 Oct 2017 10:54:09 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> wrote:

On resume from drm sleep/suspend, we have gem_init_hw path to reload
the GuC/HuC firmware. However, on resume from runtime suspend we needed
to add support to reload the GuC/HuC firmware and resume.
We can leverage intel_uc_init_hw for this based on skip_load_on_resume.

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_gem.c |  2 +-
 drivers/gpu/drm/i915/intel_uc.c | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7d1b7e1..9e257e2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2113,7 +2113,7 @@ void i915_gem_runtime_resume(struct drm_i915_private *dev_priv)
     i915_gem_init_swizzling(dev_priv);
     i915_gem_restore_fences(dev_priv);
-    intel_uc_resume(dev_priv);
+    intel_uc_runtime_resume(dev_priv);
    mutex_unlock(&dev_priv->drm.struct_mutex);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f641872..25acf8f 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -410,3 +410,31 @@ void intel_uc_resume(struct drm_i915_private *dev_priv)
         guc->skip_load_on_resume = false;
     }
 }
+
+/**
+ * intel_uc_runtime_resume() - Resume uC operation.
+ * @dev_priv: i915 device private
+ *
+ * This function invokes intel_uc_suspend that will if GuC is loaded
                            ^^^^^^^^^^^^^^^^
Please focus on tasks rather than function names.
Sure.

+ * enable communication with GuC, enable GuC interrupts, invoke GuC OS
+ * resumption and enable GuC submission.
+ * If GuC is not loaded, GuC needs to be loaded and do the entire setup
+ * by leveraging intel_uc_init_hw.
+ *
+ */
+void intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
+{
+    struct intel_guc *guc = &dev_priv->guc;
+
+    if (!guc->suspended)
+        return;
+
+    intel_uc_resume(dev_priv);
+
+    if (guc->skip_load_on_resume)

Hmm, I may be lost, but I feel that some changes from 13/21 done
in intel_uc_resume() looks like good candidate for this function.

What I'm missing is clear distinction what each function will do,
due to lot of conditions and cross calls.
Have introduced separate runtime and drm uc_suspend/resume functions in v14 and will help understand this better.

+        return;
+
+    WARN_ON(guc_wopcm_locked(guc));

Why here?
will remove.

+
+    intel_uc_init_hw(dev_priv);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 7d9dd9c..f741ccc 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -36,5 +36,6 @@
 void intel_uc_cleanup(struct drm_i915_private *dev_priv);
 int intel_uc_suspend(struct drm_i915_private *dev_priv);
 void intel_uc_resume(struct drm_i915_private *dev_priv);
+void intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
#endif

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux