From patchwork Sat Jun 16 20:25:34 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10468093 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A9C91600F4 for ; Sat, 16 Jun 2018 20:25:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 86A2928AB8 for ; Sat, 16 Jun 2018 20:25:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7884228B12; Sat, 16 Jun 2018 20:25:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A36AE28AB8 for ; Sat, 16 Jun 2018 20:25:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3F4FA6E2BA; Sat, 16 Jun 2018 20:25:54 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id A12B16E2BA for ; Sat, 16 Jun 2018 20:25:52 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 12069235-1500050 for multiple; Sat, 16 Jun 2018 21:25:34 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Sat, 16 Jun 2018 21:25:35 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Sat, 16 Jun 2018 21:25:34 +0100 Message-Id: <20180616202534.18767-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180616171239.18434-1-chris@chris-wilson.co.uk> References: <20180616171239.18434-1-chris@chris-wilson.co.uk> X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH v2] drm/i915: Fix fallout of fake reset along resume X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP commit b2209e62a450 ("drm/i915/execlists: Reset the CSB head tracking on reset/sanitization") and commit 1288786b18f7 ("drm/i915: Move GEM sanitize from resume_early to resume") show the conflicting requirements on the code. We must reset the GPU before trashing live state on a fast resume (hibernation debug, or error paths), but we must only reset our state tracking iff the GPU is reset (or power cycled). This is tricky if we are disabling GPU reset to simulate broken hardware; we reset our state tracking but the GPU is left intact and recovers from its stale state. v2: Again without the assertion for forcewake, no longer required since commit b3ee09a4de33 ("drm/i915/ringbuffer: Fix context restore upon reset") as the contexts are reset from the CS ensuring everything is powered up. Fixes: b2209e62a450 ("drm/i915/execlists: Reset the CSB head tracking on reset/sanitization") Fixes: 1288786b18f7 ("drm/i915: Move GEM sanitize from resume_early to resume") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Mika Kuoppala Cc: Joonas Lahtinen Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 14 +++++--------- drivers/gpu/drm/i915/intel_engine_cs.c | 22 ++++++++++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.c | 8 -------- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ 5 files changed, 31 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7423d78f38f4..beb0951001ce 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1841,6 +1841,8 @@ static int i915_drm_resume_early(struct drm_device *dev) else intel_display_set_init_power(dev_priv, true); + intel_engines_sanitize(dev_priv); + enable_rpm_wakeref_asserts(dev_priv); out: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5155e458b11e..822abf444378 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4990,8 +4990,7 @@ void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj) void i915_gem_sanitize(struct drm_i915_private *i915) { - struct intel_engine_cs *engine; - enum intel_engine_id id; + int err; GEM_TRACE("\n"); @@ -5017,14 +5016,11 @@ void i915_gem_sanitize(struct drm_i915_private *i915) * it may impact the display and we are uncertain about the stability * of the reset, so this could be applied to even earlier gen. */ + err = -ENODEV; if (INTEL_GEN(i915) >= 5 && intel_has_gpu_reset(i915)) - WARN_ON(intel_gpu_reset(i915, ALL_ENGINES)); - - /* Reset the submission backend after resume as well as the GPU reset */ - for_each_engine(engine, i915, id) { - if (engine->reset.reset) - engine->reset.reset(engine, NULL); - } + err = WARN_ON(intel_gpu_reset(i915, ALL_ENGINES)); + if (!err) + intel_engines_sanitize(i915); intel_uncore_forcewake_put(i915, FORCEWAKE_ALL); intel_runtime_pm_put(i915); diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 13bb8c7d2621..32bf3a408d46 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1077,6 +1077,28 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) engine->set_default_submission(engine); } +/** + * intel_engines_sanitize: called after the GPU has lost power + * @i915: the i915 device + * + * Anytime we reset the GPU, either with an explicit GPU reset or through a + * PCI power cycle, the GPU loses state and we must reset our state tracking + * to match. Note that calling intel_engines_sanitize() if the GPU has not + * been reset results in much confusion! + */ +void intel_engines_sanitize(struct drm_i915_private *i915) +{ + struct intel_engine_cs *engine; + enum intel_engine_id id; + + GEM_TRACE("\n"); + + for_each_engine(engine, i915, id) { + if (engine->reset.reset) + engine->reset.reset(engine, NULL); + } +} + /** * intel_engines_park: called when the GT is transitioning from busy->idle * @i915: the i915 device diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ef3c76425843..e0448eff12bd 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -563,14 +563,6 @@ static void reset_ring(struct intel_engine_cs *engine, struct i915_request *rq) { GEM_TRACE("%s seqno=%x\n", engine->name, rq ? rq->global_seqno : 0); - /* - * RC6 must be prevented until the reset is complete and the engine - * reinitialised. If it occurs in the middle of this sequence, the - * state written to/loaded from the power context is ill-defined (e.g. - * the PP_BASE_DIR may be lost). - */ - assert_forcewakes_active(engine->i915, FORCEWAKE_ALL); - /* * Try to restore the logical GPU state to match the continuation * of the request queue. If we skip the context/PD restore, then diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 4003f3ebe3d1..a0bc7a8222b4 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -1052,6 +1052,8 @@ gen8_emit_ggtt_write(u32 *cs, u32 value, u32 gtt_offset) return cs; } +void intel_engines_sanitize(struct drm_i915_private *i915); + bool intel_engine_is_idle(struct intel_engine_cs *engine); bool intel_engines_are_idle(struct drm_i915_private *dev_priv);