drm/i915: Defer rc6 shutdown to suspend_late
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 1 Nov 2019 14:10:09 +0000 (14:10 +0000)
committerChris Wilson <chris@chris-wilson.co.uk>
Fri, 1 Nov 2019 14:47:36 +0000 (14:47 +0000)
Currently we shutdown rc6 during i915_gem_resume() but this is called
during the preparation phase (i915_drm_prepare) for all suspend paths,
but we only want to shutdown rc6 for S3+. Move the actual shutdown to
i915_gem_suspend_late().

We then need to differentiate between suspend targets, to distinguish S0
(s2idle) where the device is kept awake but needs to be in a low power
mode (the same as runtime suspend) from the device suspend levels where
we lose control of HW and so must disable any HW access to dangling
memory.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111909
Fixes: c113236718e8 ("drm/i915: Extract GT render sleep (rc6) management")
Testcase: igt/gem_exec_suspend/power-S0
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andi Shyti <andi.shyti@intel.com>
Acked-by: Andi Shyti <andi.shyti@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-4-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/gem/i915_gem_pm.c
drivers/gpu/drm/i915/gt/intel_gt_pm.c
drivers/gpu/drm/i915/gt/intel_gt_pm.h
drivers/gpu/drm/i915/gt/intel_rc6.c
drivers/gpu/drm/i915/gt/selftest_gt_pm.c

index 6779ab34101b996756ab4df6166ba60eccd05300..f88ee1317bb45f98916038c85a68680ebf707f51 100644 (file)
@@ -27,7 +27,7 @@ void i915_gem_suspend(struct drm_i915_private *i915)
         * state. Fortunately, the kernel_context is disposable and we do
         * not rely on its state.
         */
-       intel_gt_suspend(&i915->gt);
+       intel_gt_suspend_prepare(&i915->gt);
 
        i915_gem_drain_freed_objects(i915);
 }
@@ -69,6 +69,8 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
         * machine in an unusable condition.
         */
 
+       intel_gt_suspend_late(&i915->gt);
+
        spin_lock_irqsave(&i915->mm.obj_lock, flags);
        for (phase = phases; *phase; phase++) {
                LIST_HEAD(keep);
index 11661de8c40b1cc515288ae2cf5f36a804f306d8..6374744bb65ebb0c3d8d42974e82a212841442ee 100644 (file)
@@ -4,6 +4,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include <linux/suspend.h>
+
 #include "i915_drv.h"
 #include "i915_globals.h"
 #include "i915_params.h"
@@ -236,8 +238,11 @@ int intel_gt_resume(struct intel_gt *gt)
        return err;
 }
 
-static void wait_for_idle(struct intel_gt *gt)
+static void wait_for_suspend(struct intel_gt *gt)
 {
+       if (!intel_gt_pm_is_awake(gt))
+               return;
+
        if (intel_gt_wait_for_idle(gt, I915_GEM_IDLE_TIMEOUT) == -ETIME) {
                /*
                 * Forcibly cancel outstanding work and leave
@@ -246,19 +251,46 @@ static void wait_for_idle(struct intel_gt *gt)
                intel_gt_set_wedged(gt);
        }
 
+       GEM_BUG_ON(atomic_read(&gt->user_wakeref));
        intel_gt_pm_wait_for_idle(gt);
 }
 
-void intel_gt_suspend(struct intel_gt *gt)
+void intel_gt_suspend_prepare(struct intel_gt *gt)
 {
-       intel_wakeref_t wakeref;
-
        user_forcewake(gt, true);
+       wait_for_suspend(gt);
+
+       intel_uc_suspend(&gt->uc);
+}
+
+static suspend_state_t pm_suspend_target(void)
+{
+#if IS_ENABLED(CONFIG_PM_SLEEP)
+       return pm_suspend_target_state;
+#else
+       return PM_SUSPEND_TO_IDLE;
+#endif
+}
+
+void intel_gt_suspend_late(struct intel_gt *gt)
+{
+       intel_wakeref_t wakeref;
 
        /* We expect to be idle already; but also want to be independent */
-       wait_for_idle(gt);
+       wait_for_suspend(gt);
 
-       intel_uc_suspend(&gt->uc);
+       /*
+        * On disabling the device, we want to turn off HW access to memory
+        * that we no longer own.
+        *
+        * However, not all suspend-states disable the device. S0 (s2idle)
+        * is effectively runtime-suspend, the device is left powered on
+        * but needs to be put into a low power state. We need to keep
+        * powermanagement enabled, but we also retain system state and so
+        * it remains safe to keep on using our allocated memory.
+        */
+       if (pm_suspend_target() == PM_SUSPEND_TO_IDLE)
+               return;
 
        with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
                intel_rps_disable(&gt->rps);
index d924c984c74d908f5380c73348068d7e2ae6ad33..b3e17399be9baf9e1ff2ec088e3974dc00c3a63b 100644 (file)
@@ -43,8 +43,9 @@ void intel_gt_pm_fini(struct intel_gt *gt);
 
 void intel_gt_sanitize(struct intel_gt *gt, bool force);
 
+void intel_gt_suspend_prepare(struct intel_gt *gt);
+void intel_gt_suspend_late(struct intel_gt *gt);
 int intel_gt_resume(struct intel_gt *gt);
-void intel_gt_suspend(struct intel_gt *gt);
 
 void intel_gt_runtime_suspend(struct intel_gt *gt);
 int intel_gt_runtime_resume(struct intel_gt *gt);
index 70f0e01a38b97599dd71bc5822177e7f29dffd2c..5ad4a92a9582da016134c3e05560cdc8256eacce 100644 (file)
@@ -525,6 +525,11 @@ void intel_rc6_init(struct intel_rc6 *rc6)
 
 void intel_rc6_sanitize(struct intel_rc6 *rc6)
 {
+       if (rc6->enabled) { /* unbalanced suspend/resume */
+               rpm_get(rc6);
+               rc6->enabled = false;
+       }
+
        if (rc6->supported)
                __intel_rc6_disable(rc6);
 }
index 5d429037cdad04176631860810795d8689568bca..3d4e6a008af8bb61f3a5acaffa91a2ec85e78dc3 100644 (file)
@@ -15,7 +15,7 @@ static int live_gt_resume(void *arg)
 
        /* Do several suspend/resume cycles to check we don't explode! */
        do {
-               intel_gt_suspend(gt);
+               intel_gt_suspend_late(gt);
 
                if (gt->rc6.enabled) {
                        pr_err("rc6 still enabled after suspend!\n");