From 8eb4704b124cbd44f189709959137d77063ecfa1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 3 Nov 2019 16:23:05 +0000 Subject: [PATCH 1/7] drm/i915: Protect request peeking with RCU Since the execlists_active() is no longer protected by the engine->active.lock, we need to protect the request pointer with RCU to prevent it being freed as we evaluate whether or not we need to preempt. Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock") Fixes: 13ed13a4dcbf ("drm/i915: Don't set queue_priority_hint if we don't kick the submission") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Tvrtko Ursulin Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20191104090158.2959-2-chris@chris-wilson.co.uk (cherry picked from commit 7d148635253328dda7cfe55d57e3c828e9564427) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index d2edb527dcb8..010d67f48ad9 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -202,21 +202,26 @@ static void kick_submission(struct intel_engine_cs *engine, if (prio <= engine->execlists.queue_priority_hint) return; + rcu_read_lock(); + /* Nothing currently active? We're overdue for a submission! */ inflight = execlists_active(&engine->execlists); if (!inflight) - return; + goto unlock; /* * If we are already the currently executing context, don't * bother evaluating if we should preempt ourselves. */ if (inflight->hw_context == rq->hw_context) - return; + goto unlock; engine->execlists.queue_priority_hint = prio; if (need_preempt(prio, rq_prio(inflight))) tasklet_hi_schedule(&engine->execlists.tasklet); + +unlock: + rcu_read_unlock(); } static void __i915_schedule(struct i915_sched_node *node, From fd6fe087ca470439d554185a2f6bd4d8749bb385 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 1 Nov 2019 14:10:06 +0000 Subject: [PATCH 2/7] drm/i915/gt: Call intel_gt_sanitize() directly Assume all responsibility for operating on the HW to sanitize the GT state upon load/resume in intel_gt_sanitize() itself. Signed-off-by: Chris Wilson Reviewed-by: Andi Shyti Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-1-chris@chris-wilson.co.uk (cherry picked from commit 797a615357ac0feb79c9ce41f5eaac3eb738a51f) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 5 --- drivers/gpu/drm/i915/gt/intel_gt.c | 6 ++- drivers/gpu/drm/i915/gt/intel_gt.h | 2 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 36 +++++++++++++++++- drivers/gpu/drm/i915/i915_drv.c | 8 +--- drivers/gpu/drm/i915/i915_drv.h | 2 - drivers/gpu/drm/i915/i915_gem.c | 37 ------------------- drivers/gpu/drm/i915/selftests/i915_gem.c | 1 - .../gpu/drm/i915/selftests/mock_gem_device.c | 1 - drivers/gpu/drm/i915/selftests/mock_gtt.c | 3 +- 10 files changed, 43 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index c99bb94fe41e..e2ee9c04ece8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -74,7 +74,6 @@ void i915_gem_suspend(struct drm_i915_private *i915) * not rely on its state. */ intel_gt_suspend(&i915->gt); - intel_uc_suspend(&i915->gt.uc); i915_gem_drain_freed_objects(i915); } @@ -140,8 +139,6 @@ void i915_gem_suspend_late(struct drm_i915_private *i915) list_splice_tail(&keep, *phase); } spin_unlock_irqrestore(&i915->mm.obj_lock, flags); - - i915_gem_sanitize(i915); } void i915_gem_resume(struct drm_i915_private *i915) @@ -161,8 +158,6 @@ void i915_gem_resume(struct drm_i915_private *i915) if (intel_gt_resume(&i915->gt)) goto err_wedged; - intel_uc_resume(&i915->gt.uc); - /* Always reload a context for powersaving. */ if (!switch_to_kernel_context_sync(&i915->gt)) goto err_wedged; diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 898662c158ad..4c26daf7ee46 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -31,9 +31,11 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) intel_uc_init_early(>->uc); } -void intel_gt_init_hw_early(struct drm_i915_private *i915) +void intel_gt_init_hw_early(struct intel_gt *gt, struct i915_ggtt *ggtt) { - i915->gt.ggtt = &i915->ggtt; + gt->ggtt = ggtt; + + intel_gt_sanitize(gt, false); } static void init_unused_ring(struct intel_gt *gt, u32 base) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 5b6effed3713..5436f8c30708 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -28,7 +28,7 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc *huc) } void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915); -void intel_gt_init_hw_early(struct drm_i915_private *i915); +void intel_gt_init_hw_early(struct intel_gt *gt, struct i915_ggtt *ggtt); int __must_check intel_gt_init_hw(struct intel_gt *gt); int intel_gt_init(struct intel_gt *gt); void intel_gt_driver_register(struct intel_gt *gt); diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 32becf15d4e0..1679f4f78d65 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -118,8 +118,22 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force) { struct intel_engine_cs *engine; enum intel_engine_id id; + intel_wakeref_t wakeref; - GEM_TRACE("\n"); + GEM_TRACE("force:%s\n", yesno(force)); + + /* Use a raw wakeref to avoid calling intel_display_power_get early */ + wakeref = intel_runtime_pm_get(gt->uncore->rpm); + intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL); + + /* + * As we have just resumed the machine and woken the device up from + * deep PCI sleep (presumably D3_cold), assume the HW has been reset + * back to defaults, recovering from whatever wedged state we left it + * in and so worth trying to use the device once more. + */ + if (intel_gt_is_wedged(gt)) + intel_gt_unset_wedged(gt); intel_uc_sanitize(>->uc); @@ -127,6 +141,8 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force) if (engine->reset.prepare) engine->reset.prepare(engine); + intel_uc_reset_prepare(>->uc); + if (reset_engines(gt) || force) { for_each_engine(engine, gt, id) __intel_engine_reset(engine, false); @@ -135,6 +151,9 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force) for_each_engine(engine, gt, id) if (engine->reset.finish) engine->reset.finish(engine); + + intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL); + intel_runtime_pm_put(gt->uncore->rpm, wakeref); } void intel_gt_pm_fini(struct intel_gt *gt) @@ -148,6 +167,8 @@ int intel_gt_resume(struct intel_gt *gt) enum intel_engine_id id; int err = 0; + GEM_TRACE("\n"); + /* * After resume, we may need to poke into the pinned kernel * contexts to paper over any damage caused by the sudden suspend. @@ -186,6 +207,9 @@ int intel_gt_resume(struct intel_gt *gt) } intel_rc6_enable(>->rc6); + + intel_uc_resume(>->uc); + intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL); intel_gt_pm_put(gt); @@ -212,20 +236,30 @@ void intel_gt_suspend(struct intel_gt *gt) /* We expect to be idle already; but also want to be independent */ wait_for_idle(gt); + intel_uc_suspend(>->uc); + with_intel_runtime_pm(gt->uncore->rpm, wakeref) { intel_rps_disable(>->rps); intel_rc6_disable(>->rc6); intel_llc_disable(>->llc); } + + intel_gt_sanitize(gt, false); + + GEM_TRACE("\n"); } void intel_gt_runtime_suspend(struct intel_gt *gt) { intel_uc_runtime_suspend(>->uc); + + GEM_TRACE("\n"); } int intel_gt_runtime_resume(struct intel_gt *gt) { + GEM_TRACE("\n"); + intel_gt_init_swizzling(gt); return intel_uc_runtime_resume(>->uc); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3340485c12e3..ccb5b566795f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -603,8 +603,6 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) if (ret) goto err_uncore; - i915_gem_init_mmio(dev_priv); - return 0; err_uncore: @@ -1177,7 +1175,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv) if (ret) goto err_ggtt; - intel_gt_init_hw_early(dev_priv); + intel_gt_init_hw_early(&dev_priv->gt, &dev_priv->ggtt); ret = i915_ggtt_enable_hw(dev_priv); if (ret) { @@ -1821,7 +1819,7 @@ static int i915_drm_resume(struct drm_device *dev) disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); - i915_gem_sanitize(dev_priv); + intel_gt_sanitize(&dev_priv->gt, true); ret = i915_ggtt_enable_hw(dev_priv); if (ret) @@ -1952,8 +1950,6 @@ static int i915_drm_resume_early(struct drm_device *dev) intel_power_domains_resume(dev_priv); - intel_gt_sanitize(&dev_priv->gt, true); - enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); return ret; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1e6118f62b29..0567fe67e8c5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1779,7 +1779,6 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data, /* i915_gem.c */ int i915_gem_init_userptr(struct drm_i915_private *dev_priv); void i915_gem_cleanup_userptr(struct drm_i915_private *dev_priv); -void i915_gem_sanitize(struct drm_i915_private *i915); void i915_gem_init_early(struct drm_i915_private *dev_priv); void i915_gem_cleanup_early(struct drm_i915_private *dev_priv); int i915_gem_freeze(struct drm_i915_private *dev_priv); @@ -1863,7 +1862,6 @@ static inline u32 i915_reset_engine_count(struct i915_gpu_error *error, return atomic_read(&error->reset_engine_count[engine->uabi_class]); } -void i915_gem_init_mmio(struct drm_i915_private *i915); int __must_check i915_gem_init(struct drm_i915_private *dev_priv); void i915_gem_driver_register(struct drm_i915_private *i915); void i915_gem_driver_unregister(struct drm_i915_private *i915); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b1574ab104d7..ac23322fb382 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1039,38 +1039,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, return err; } -void i915_gem_sanitize(struct drm_i915_private *i915) -{ - intel_wakeref_t wakeref; - - GEM_TRACE("\n"); - - wakeref = intel_runtime_pm_get(&i915->runtime_pm); - intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL); - - /* - * As we have just resumed the machine and woken the device up from - * deep PCI sleep (presumably D3_cold), assume the HW has been reset - * back to defaults, recovering from whatever wedged state we left it - * in and so worth trying to use the device once more. - */ - if (intel_gt_is_wedged(&i915->gt)) - intel_gt_unset_wedged(&i915->gt); - - /* - * If we inherit context state from the BIOS or earlier occupants - * of the GPU, the GPU may be in an inconsistent state when we - * try to take over. The only way to remove the earlier state - * is by resetting. However, resetting on earlier gen is tricky as - * it may impact the display and we are uncertain about the stability - * of the reset, so this could be applied to even earlier gen. - */ - intel_gt_sanitize(&i915->gt, false); - - intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); - intel_runtime_pm_put(&i915->runtime_pm, wakeref); -} - static int __intel_engines_record_defaults(struct intel_gt *gt) { struct i915_request *requests[I915_NUM_ENGINES] = {}; @@ -1413,11 +1381,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv) WARN_ON(!list_empty(&dev_priv->gem.contexts.list)); } -void i915_gem_init_mmio(struct drm_i915_private *i915) -{ - i915_gem_sanitize(i915); -} - static void i915_gem_init__mm(struct drm_i915_private *i915) { spin_lock_init(&i915->mm.obj_lock); diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c index e378543ed453..d83f6bf6d9d4 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c @@ -124,7 +124,6 @@ static void pm_resume(struct drm_i915_private *i915) */ with_intel_runtime_pm(&i915->runtime_pm, wakeref) { intel_gt_sanitize(&i915->gt, false); - i915_gem_sanitize(i915); i915_gem_restore_gtt_mappings(i915); i915_gem_restore_fences(&i915->ggtt); diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index a0da5944dd33..27ed3cee6a9b 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -183,7 +183,6 @@ struct drm_i915_private *mock_gem_device(void) intel_timelines_init(i915); mock_init_ggtt(i915, &i915->ggtt); - i915->gt.ggtt = &i915->ggtt; mkwrite_device_info(i915)->engine_mask = BIT(0); diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c index 9ec93dc27fb5..20ac3844edec 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gtt.c +++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c @@ -118,8 +118,7 @@ void mock_init_ggtt(struct drm_i915_private *i915, struct i915_ggtt *ggtt) ggtt->vm.vma_ops.clear_pages = clear_pages; i915_address_space_init(&ggtt->vm, VM_CLASS_GGTT); - - intel_gt_init_hw_early(i915); + i915->gt.ggtt = ggtt; } void mock_fini_ggtt(struct i915_ggtt *ggtt) From 489d1953c12ea4e7b36f20219fb2a6b1896c1f80 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 1 Nov 2019 14:10:07 +0000 Subject: [PATCH 3/7] drm/i915/gem: Leave reloading kernel context on resume to GT As we already do reload the kernel context in intel_gt_resume, repeating that action inside i915_gem_resume() as well is redundant. Signed-off-by: Chris Wilson Reviewed-by: Andi Shyti Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-2-chris@chris-wilson.co.uk (cherry picked from commit c8f6cfc56fc86999725e71a19d91269482bd2c01) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 30 -------------------------- 1 file changed, 30 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index e2ee9c04ece8..64dd04ae3d57 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -11,32 +11,6 @@ #include "i915_drv.h" -static bool switch_to_kernel_context_sync(struct intel_gt *gt) -{ - bool result = !intel_gt_is_wedged(gt); - - if (intel_gt_wait_for_idle(gt, I915_GEM_IDLE_TIMEOUT) == -ETIME) { - /* XXX hide warning from gem_eio */ - if (i915_modparams.reset) { - dev_err(gt->i915->drm.dev, - "Failed to idle engines, declaring wedged!\n"); - GEM_TRACE_DUMP(); - } - - /* - * Forcibly cancel outstanding work and leave - * the gpu quiet. - */ - intel_gt_set_wedged(gt); - result = false; - } - - if (intel_gt_pm_wait_for_idle(gt)) - result = false; - - return result; -} - static void user_forcewake(struct intel_gt *gt, bool suspend) { int count = atomic_read(>->user_wakeref); @@ -158,10 +132,6 @@ void i915_gem_resume(struct drm_i915_private *i915) if (intel_gt_resume(&i915->gt)) goto err_wedged; - /* Always reload a context for powersaving. */ - if (!switch_to_kernel_context_sync(&i915->gt)) - goto err_wedged; - user_forcewake(&i915->gt, false); out_unlock: From d4033a9b03d8df375bca65290e9419c14392ddd0 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 1 Nov 2019 14:10:08 +0000 Subject: [PATCH 4/7] drm/i915/gt: Move user_forcewake application to GT We already track the debugfs user_forcewake on the GT, so it is natural to pull the suspend/resume handling under gt/ as well. Signed-off-by: Chris Wilson Reviewed-by: Andi Shyti Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-3-chris@chris-wilson.co.uk (cherry picked from commit 9ab3fe2d7dc39b088591b0121f041cbfd6bb1ef8) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 22 ---------------------- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index 64dd04ae3d57..6779ab34101b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -11,24 +11,6 @@ #include "i915_drv.h" -static void user_forcewake(struct intel_gt *gt, bool suspend) -{ - int count = atomic_read(>->user_wakeref); - - /* Inside suspend/resume so single threaded, no races to worry about. */ - if (likely(!count)) - return; - - intel_gt_pm_get(gt); - if (suspend) { - GEM_BUG_ON(count > atomic_read(>->wakeref.count)); - atomic_sub(count, >->wakeref.count); - } else { - atomic_add(count, >->wakeref.count); - } - intel_gt_pm_put(gt); -} - void i915_gem_suspend(struct drm_i915_private *i915) { GEM_TRACE("\n"); @@ -36,8 +18,6 @@ void i915_gem_suspend(struct drm_i915_private *i915) intel_wakeref_auto(&i915->ggtt.userfault_wakeref, 0); flush_workqueue(i915->wq); - user_forcewake(&i915->gt, true); - /* * We have to flush all the executing contexts to main memory so * that they can saved in the hibernation image. To ensure the last @@ -132,8 +112,6 @@ void i915_gem_resume(struct drm_i915_private *i915) if (intel_gt_resume(&i915->gt)) goto err_wedged; - user_forcewake(&i915->gt, false); - out_unlock: intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL); return; diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 1679f4f78d65..11661de8c40b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -18,6 +18,24 @@ #include "intel_rps.h" #include "intel_wakeref.h" +static void user_forcewake(struct intel_gt *gt, bool suspend) +{ + int count = atomic_read(>->user_wakeref); + + /* Inside suspend/resume so single threaded, no races to worry about. */ + if (likely(!count)) + return; + + intel_gt_pm_get(gt); + if (suspend) { + GEM_BUG_ON(count > atomic_read(>->wakeref.count)); + atomic_sub(count, >->wakeref.count); + } else { + atomic_add(count, >->wakeref.count); + } + intel_gt_pm_put(gt); +} + static int __gt_unpark(struct intel_wakeref *wf) { struct intel_gt *gt = container_of(wf, typeof(*gt), wakeref); @@ -210,6 +228,8 @@ int intel_gt_resume(struct intel_gt *gt) intel_uc_resume(>->uc); + user_forcewake(gt, false); + intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL); intel_gt_pm_put(gt); @@ -233,6 +253,8 @@ void intel_gt_suspend(struct intel_gt *gt) { intel_wakeref_t wakeref; + user_forcewake(gt, true); + /* We expect to be idle already; but also want to be independent */ wait_for_idle(gt); From a70a9e998e8e64417c1213572c1dda617e355653 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 1 Nov 2019 14:10:09 +0000 Subject: [PATCH 5/7] drm/i915: Defer rc6 shutdown to suspend_late 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 Cc: Andi Shyti Acked-by: Andi Shyti Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-4-chris@chris-wilson.co.uk (cherry picked from commit c601cb2135fda0b5fb9d08153b0125fcb153c7e0) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 4 ++- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 44 ++++++++++++++++++++---- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 3 +- drivers/gpu/drm/i915/gt/intel_rc6.c | 5 +++ drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 2 +- 5 files changed, 49 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index 6779ab34101b..f88ee1317bb4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -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); diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 11661de8c40b..6374744bb65e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -4,6 +4,8 @@ * Copyright © 2019 Intel Corporation */ +#include + #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(>->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) +{ + user_forcewake(gt, true); + wait_for_suspend(gt); + + intel_uc_suspend(>->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; - user_forcewake(gt, true); - /* We expect to be idle already; but also want to be independent */ - wait_for_idle(gt); + wait_for_suspend(gt); - intel_uc_suspend(>->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(>->rps); diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index d924c984c74d..b3e17399be9b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -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); diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c index 70f0e01a38b9..5ad4a92a9582 100644 --- a/drivers/gpu/drm/i915/gt/intel_rc6.c +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c @@ -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); } diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c index 5d429037cdad..3d4e6a008af8 100644 --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c @@ -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"); From 3fd850dda83d0e5a4e49e7d093b0b72f75a9391f Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 3 Nov 2019 09:50:38 +0000 Subject: [PATCH 6/7] drm/i915/gt: Drop false assertion on user_forcewake The counter is removed from the pm wakeref count, but it remains intact so that we can restore it upon resume. Ergo inside suspend, it may have a value. Signed-off-by: Chris Wilson Acked-by: Andi Shyti Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20191104090158.2959-1-chris@chris-wilson.co.uk (cherry picked from commit 83c55ee82f3ac5a1c36dab9f7150554c4da773a8) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 6374744bb65e..060a27d9af34 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -251,7 +251,6 @@ static void wait_for_suspend(struct intel_gt *gt) intel_gt_set_wedged(gt); } - GEM_BUG_ON(atomic_read(>->user_wakeref)); intel_gt_pm_wait_for_idle(gt); } From d9dace9438945e7c13d91e62927c5c6c88a37ee5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 1 Nov 2019 17:44:05 +0000 Subject: [PATCH 7/7] drm/i915/selftests: Add intel_gt_suspend_prepare Call suspend_prepare first so that we don't leave GuC so confused. Signed-off-by: Chris Wilson Acked-by: Andi Shyti Link: https://patchwork.freedesktop.org/patch/msgid/20191101174405.7389-1-chris@chris-wilson.co.uk (cherry picked from commit 833e979db36c0202f21e1e0bdd7339a27e50b8e3) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c index 3d4e6a008af8..d1752f15702a 100644 --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c @@ -15,6 +15,7 @@ static int live_gt_resume(void *arg) /* Do several suspend/resume cycles to check we don't explode! */ do { + intel_gt_suspend_prepare(gt); intel_gt_suspend_late(gt); if (gt->rc6.enabled) {