From patchwork Thu Apr 27 23:12:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michel Thierry X-Patchwork-Id: 9703741 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 76CE3602CC for ; Thu, 27 Apr 2017 23:13:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 70FC3285EA for ; Thu, 27 Apr 2017 23:13:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 65EB72863B; Thu, 27 Apr 2017 23:13:21 +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=-4.2 required=2.0 tests=BAYES_00, 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 8AE3E285EA for ; Thu, 27 Apr 2017 23:13:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0664D6E71D; Thu, 27 Apr 2017 23:13:08 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5BCBD6E195 for ; Thu, 27 Apr 2017 23:13:02 +0000 (UTC) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Apr 2017 16:13:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,386,1488873600"; d="scan'208";a="81571422" Received: from relo-linux-11.sc.intel.com ([10.3.160.214]) by orsmga004.jf.intel.com with ESMTP; 27 Apr 2017 16:13:01 -0700 From: Michel Thierry To: intel-gfx@lists.freedesktop.org Date: Thu, 27 Apr 2017 16:12:43 -0700 Message-Id: <20170427231300.32841-4-michel.thierry@intel.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170427231300.32841-1-michel.thierry@intel.com> References: <20170427231300.32841-1-michel.thierry@intel.com> Subject: [Intel-gfx] [PATCH v7 03/20] drm/i915: Add support for per engine reset recovery X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 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 From: Arun Siluvery This change implements support for per-engine reset as an initial, less intrusive hang recovery option to be attempted before falling back to the legacy full GPU reset recovery mode if necessary. This is only supported from Gen8 onwards. Hangchecker determines which engines are hung and invokes error handler to recover from it. Error handler schedules recovery for each of those engines that are hung. The recovery procedure is as follows, - identifies the request that caused the hang and it is dropped - force engine to idle: this is done by issuing a reset request - reset and re-init engine - restart submissions to the engine If engine reset fails then we fall back to heavy weight full gpu reset which resets all engines and reinitiazes complete state of HW and SW. v2: Rebase. v3: s/*engine_reset*/*reset_engine*/; freeze engine and irqs before calling i915_gem_reset_engine (Chris). v4: Rebase, modify i915_gem_reset_prepare to use a ring mask and reuse the function for reset_engine. v5: intel_reset_engine_start/cancel instead of request/unrequest_reset. v6: Clean up reset_engine function to not require mutex, i.e. no need to call revoke/restore_fences and _retire_requests (Chris). v7: Remove leftovers from v5, i.e. no need to disable irq, hold forcewake or wakeup the handoff bit (Chris). Cc: Chris Wilson Cc: Mika Kuoppala Signed-off-by: Tomas Elf Signed-off-by: Arun Siluvery Signed-off-by: Michel Thierry --- drivers/gpu/drm/i915/i915_drv.c | 60 ++++++++++++++++++-- drivers/gpu/drm/i915/i915_drv.h | 12 +++- drivers/gpu/drm/i915/i915_gem.c | 97 +++++++++++++++++++-------------- drivers/gpu/drm/i915/i915_gem_request.c | 2 +- drivers/gpu/drm/i915/intel_uncore.c | 20 +++++++ 5 files changed, 142 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 48c8b69d9bde..ae891529dedd 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1810,7 +1810,7 @@ void i915_reset(struct drm_i915_private *dev_priv) pr_notice("drm/i915: Resetting chip after gpu hang\n"); disable_irq(dev_priv->drm.irq); - ret = i915_gem_reset_prepare(dev_priv); + ret = i915_gem_reset_prepare(dev_priv, ALL_ENGINES); if (ret) { DRM_ERROR("GPU recovery failed\n"); intel_gpu_reset(dev_priv, ALL_ENGINES); @@ -1852,7 +1852,7 @@ void i915_reset(struct drm_i915_private *dev_priv) i915_queue_hangcheck(dev_priv); finish: - i915_gem_reset_finish(dev_priv); + i915_gem_reset_finish(dev_priv, ALL_ENGINES); enable_irq(dev_priv->drm.irq); wakeup: @@ -1871,11 +1871,63 @@ void i915_reset(struct drm_i915_private *dev_priv) * * Reset a specific GPU engine. Useful if a hang is detected. * Returns zero on successful reset or otherwise an error code. + * + * Procedure is: + * - identifies the request that caused the hang and it is dropped + * - force engine to idle: this is done by issuing a reset request + * - reset engine + * - restart submissions to the engine */ int i915_reset_engine(struct intel_engine_cs *engine) { - /* FIXME: replace me with engine reset sequence */ - return -ENODEV; + int ret; + struct drm_i915_private *dev_priv = engine->i915; + struct i915_gpu_error *error = &dev_priv->gpu_error; + + GEM_BUG_ON(!test_bit(I915_RESET_BACKOFF, &error->flags)); + + DRM_DEBUG_DRIVER("resetting %s\n", engine->name); + + ret = i915_gem_reset_prepare_engine(engine); + if (ret) { + DRM_ERROR("Previous reset failed - promote to full reset\n"); + goto out; + } + + /* + * the request that caused the hang is stuck on elsp, identify the + * active request and drop it, adjust head to skip the offending + * request to resume executing remaining requests in the queue. + */ + i915_gem_reset_engine(engine); + + /* forcing engine to idle */ + ret = intel_reset_engine_start(engine); + if (ret) { + DRM_ERROR("Failed to disable %s\n", engine->name); + goto out; + } + + /* finally, reset engine */ + ret = intel_gpu_reset(dev_priv, intel_engine_flag(engine)); + if (ret) { + DRM_ERROR("Failed to reset %s, ret=%d\n", engine->name, ret); + intel_reset_engine_cancel(engine); + goto out; + } + + /* be sure the request reset bit gets cleared */ + intel_reset_engine_cancel(engine); + + i915_gem_reset_finish_engine(engine); + + /* replay remaining requests in the queue */ + ret = engine->init_hw(engine); + if (ret) + goto out; //XXX: ignore this line for now + +out: + return ret; } static int i915_pm_suspend(struct device *kdev) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ab7e68626c49..efbf34318893 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3022,6 +3022,8 @@ extern bool intel_has_gpu_reset(struct drm_i915_private *dev_priv); extern void i915_reset(struct drm_i915_private *dev_priv); extern int i915_reset_engine(struct intel_engine_cs *engine); extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv); +extern int intel_reset_engine_start(struct intel_engine_cs *engine); +extern void intel_reset_engine_cancel(struct intel_engine_cs *engine); extern int intel_guc_reset(struct drm_i915_private *dev_priv); extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine); extern void intel_hangcheck_init(struct drm_i915_private *dev_priv); @@ -3410,7 +3412,6 @@ int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno); struct drm_i915_gem_request * i915_gem_find_active_request(struct intel_engine_cs *engine); - void i915_gem_retire_requests(struct drm_i915_private *dev_priv); static inline bool i915_reset_backoff(struct i915_gpu_error *error) @@ -3438,11 +3439,16 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error) return READ_ONCE(error->reset_count); } -int i915_gem_reset_prepare(struct drm_i915_private *dev_priv); +int i915_gem_reset_prepare_engine(struct intel_engine_cs *engine); +int i915_gem_reset_prepare(struct drm_i915_private *dev_priv, + unsigned int engine_mask); void i915_gem_reset(struct drm_i915_private *dev_priv); -void i915_gem_reset_finish(struct drm_i915_private *dev_priv); +void i915_gem_reset_finish_engine(struct intel_engine_cs *engine); +void i915_gem_reset_finish(struct drm_i915_private *dev_priv, + unsigned int engine_mask); void i915_gem_set_wedged(struct drm_i915_private *dev_priv); bool i915_gem_unset_wedged(struct drm_i915_private *dev_priv); +void i915_gem_reset_engine(struct intel_engine_cs *engine); void i915_gem_init_mmio(struct drm_i915_private *i915); int __must_check i915_gem_init(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 33fb11cc5acc..bce38062f94e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2793,48 +2793,57 @@ static bool engine_stalled(struct intel_engine_cs *engine) return true; } -int i915_gem_reset_prepare(struct drm_i915_private *dev_priv) +/* Ensure irq handler finishes, and not run again. */ +int i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) { - struct intel_engine_cs *engine; - enum intel_engine_id id; + struct drm_i915_gem_request *request; int err = 0; - /* Ensure irq handler finishes, and not run again. */ - for_each_engine(engine, dev_priv, id) { - struct drm_i915_gem_request *request; - - /* Prevent the signaler thread from updating the request - * state (by calling dma_fence_signal) as we are processing - * the reset. The write from the GPU of the seqno is - * asynchronous and the signaler thread may see a different - * value to us and declare the request complete, even though - * the reset routine have picked that request as the active - * (incomplete) request. This conflict is not handled - * gracefully! - */ - kthread_park(engine->breadcrumbs.signaler); - - /* Prevent request submission to the hardware until we have - * completed the reset in i915_gem_reset_finish(). If a request - * is completed by one engine, it may then queue a request - * to a second via its engine->irq_tasklet *just* as we are - * calling engine->init_hw() and also writing the ELSP. - * Turning off the engine->irq_tasklet until the reset is over - * prevents the race. - */ - tasklet_kill(&engine->irq_tasklet); - tasklet_disable(&engine->irq_tasklet); - if (engine->irq_seqno_barrier) - engine->irq_seqno_barrier(engine); + /* Prevent the signaler thread from updating the request + * state (by calling dma_fence_signal) as we are processing + * the reset. The write from the GPU of the seqno is + * asynchronous and the signaler thread may see a different + * value to us and declare the request complete, even though + * the reset routine have picked that request as the active + * (incomplete) request. This conflict is not handled + * gracefully! + */ + kthread_park(engine->breadcrumbs.signaler); + + /* Prevent request submission to the hardware until we have + * completed the reset in i915_gem_reset_finish(). If a request + * is completed by one engine, it may then queue a request + * to a second via its engine->irq_tasklet *just* as we are + * calling engine->init_hw() and also writing the ELSP. + * Turning off the engine->irq_tasklet until the reset is over + * prevents the race. + */ + tasklet_kill(&engine->irq_tasklet); + tasklet_disable(&engine->irq_tasklet); - if (engine_stalled(engine)) { - request = i915_gem_find_active_request(engine); - if (request && request->fence.error == -EIO) - err = -EIO; /* Previous reset failed! */ - } + if (engine->irq_seqno_barrier) + engine->irq_seqno_barrier(engine); + + if (engine_stalled(engine)) { + request = i915_gem_find_active_request(engine); + if (request && request->fence.error == -EIO) + err = -EIO; /* Previous reset failed! */ } + return err; +} + +int i915_gem_reset_prepare(struct drm_i915_private *dev_priv, + unsigned int engine_mask) +{ + struct intel_engine_cs *engine; + unsigned int tmp; + int err = 0; + + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) + err = i915_gem_reset_prepare_engine(engine); + i915_gem_revoke_fences(dev_priv); return err; @@ -2920,7 +2929,7 @@ static bool i915_gem_reset_request(struct drm_i915_gem_request *request) return guilty; } -static void i915_gem_reset_engine(struct intel_engine_cs *engine) +void i915_gem_reset_engine(struct intel_engine_cs *engine) { struct drm_i915_gem_request *request; @@ -2966,16 +2975,22 @@ void i915_gem_reset(struct drm_i915_private *dev_priv) } } -void i915_gem_reset_finish(struct drm_i915_private *dev_priv) +void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) +{ + tasklet_enable(&engine->irq_tasklet); + kthread_unpark(engine->breadcrumbs.signaler); +} + +void i915_gem_reset_finish(struct drm_i915_private *dev_priv, + unsigned int engine_mask) { struct intel_engine_cs *engine; - enum intel_engine_id id; + unsigned int tmp; lockdep_assert_held(&dev_priv->drm.struct_mutex); - for_each_engine(engine, dev_priv, id) { - tasklet_enable(&engine->irq_tasklet); - kthread_unpark(engine->breadcrumbs.signaler); + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) { + i915_gem_reset_finish_engine(engine); } } diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 6198f6997d05..f69a8c535d5f 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -1216,7 +1216,7 @@ long i915_wait_request(struct drm_i915_gem_request *req, return timeout; } -static void engine_retire_requests(struct intel_engine_cs *engine) +void engine_retire_requests(struct intel_engine_cs *engine) { struct drm_i915_gem_request *request, *next; u32 seqno = intel_engine_get_seqno(engine); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index ab5bdd110ac3..3ebba6b2dd74 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1801,6 +1801,26 @@ int intel_guc_reset(struct drm_i915_private *dev_priv) return ret; } +/* + * On gen8+ a reset request has to be issued via the reset control register + * before a GPU engine can be reset in order to stop the command streamer + * and idle the engine. This replaces the legacy way of stopping an engine + * by writing to the stop ring bit in the MI_MODE register. + */ +int intel_reset_engine_start(struct intel_engine_cs *engine) +{ + return gen8_reset_engine_start(engine); +} + +/* + * It is possible to back off from a previously issued reset request by simply + * clearing the reset request bit in the reset control register. + */ +void intel_reset_engine_cancel(struct intel_engine_cs *engine) +{ + gen8_reset_engine_cancel(engine); +} + bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv) { return check_for_unclaimed_mmio(dev_priv);