On 11/6/2017 5:43 PM, Chris Wilson wrote:
Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
Before GT device suspend, i915 should release GuC client doorbells by
stopping doorbell controller snooping and doorbell deallocation through
GuC. They need to be acquired again on resume. This will also resolve
the driver unload issue with GuC, where doorbell deallocation was being
done post GEM suspend.
Release function is called from guc_suspend prior to sending sleep action
during runtime and drm suspend. Acquiral is different in runtime and drm
resume paths as on drm resume we are currently doing full reinit. Release
should be done in synchronization with acquire hence GuC suspend/resume
along with doorbell release/acquire should be under struct_mutex. Upcoming
suspend and resume restructuring for GuC will update these changes.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 3 +++
drivers/gpu/drm/i915/i915_gem.c | 5 +++--
drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
drivers/gpu/drm/i915/intel_guc.c | 2 ++
5 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7e9e06..3df8a7d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -47,6 +47,7 @@
#include <drm/i915_drm.h>
#include "i915_drv.h"
+#include "i915_guc_submission.h"
#include "i915_trace.h"
#include "i915_vgpu.h"
#include "intel_drv.h"
@@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
intel_guc_resume(dev_priv);
+ i915_guc_clients_acquire_doorbells(&dev_priv->guc);
intel_guc_acquire_doorbells();
Prefixed "i915_guc_clients" since this modifies submission state
(clients/doorbells). Should have kept dev_priv as parameter.
what should be the correct option here: intel_guc*(guc) or
i915_guc*(dev_priv)
Though, if we need to specialise between resume and runtime_resume, I
suggest adding intel_guc_resume() and intel_guc_runtime_resume() entry
points. (We probably should find a better way to distinguish those two
paths, system_resume vs runtime_resume?)
Yes. these will be added in the upcoming GuC suspend/resume
restructuring series.
Functionality along runtime/system suspend path will be same for GuC.
Will defer in
the resume path since we are doing full gem reinit on resume from system
suspend.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
