From patchwork Fri Dec 16 20:20:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michel Thierry X-Patchwork-Id: 9478489 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 41018601C2 for ; Fri, 16 Dec 2016 20:20:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 30E5127D13 for ; Fri, 16 Dec 2016 20:20:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 25C7928723; Fri, 16 Dec 2016 20:20:34 +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 42B3A27D13 for ; Fri, 16 Dec 2016 20:20:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B3FEB6ED2C; Fri, 16 Dec 2016 20:20:31 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id BEBA16ED2F for ; Fri, 16 Dec 2016 20:20:11 +0000 (UTC) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP; 16 Dec 2016 12:20:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.33,359,1477983600"; d="scan'208"; a="1082813291" Received: from relo-linux-11.sc.intel.com ([10.3.160.214]) by fmsmga001.fm.intel.com with ESMTP; 16 Dec 2016 12:20:10 -0800 From: Michel Thierry To: intel-gfx@lists.freedesktop.org Date: Fri, 16 Dec 2016 12:20:04 -0800 Message-Id: <20161216202010.7983-4-michel.thierry@intel.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20161216202010.7983-1-michel.thierry@intel.com> References: <20161216202010.7983-1-michel.thierry@intel.com> Subject: [Intel-gfx] [PATCH 3/9] drm/i915/tdr: Modify error handler for per engine hang 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 is a preparatory patch which modifies error handler to do per engine hang recovery. The actual patch which implements this sequence follows later in the series. The aim is to prepare existing recovery function to adapt to this new function where applicable (which fails at this point because core implementation is lacking) and continue recovery using legacy full gpu reset. A helper function is also added to query the availability of engine reset. The error events behaviour that are used to notify user of reset are adapted to engine reset such that it doesn't break users listening to these events. In legacy we report an error event, a reset event before resetting the gpu and a reset done event marking the completion of reset. The same behaviour is adapted but reset event is only dispatched once even when multiple engines are hung. Finally once reset is complete we send reset done event as usual. Note that this implementation of engine reset is for i915 directly submitting to the ELSP, where the driver manages the hang detection, recovery and resubmission. With GuC submission these tasks are shared between driver and firmware; i915 will still responsible for detecting a hang, and when it does it will have to request GuC to reset that Engine and remind the firmware about the outstanding submissions. v2: rebase, advertise engine reset availability in platform definition, add note about GuC submission. Cc: Chris Wilson Cc: Mika Kuoppala Signed-off-by: Ian Lister Signed-off-by: Tomas Elf Signed-off-by: Arun Siluvery Signed-off-by: Michel Thierry --- drivers/gpu/drm/i915/i915_drv.c | 21 +++++++++ drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/i915_irq.c | 88 +++++++++++++++++++++++++++---------- drivers/gpu/drm/i915/i915_pci.c | 5 ++- drivers/gpu/drm/i915/intel_uncore.c | 11 +++++ 5 files changed, 103 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6428588518aa..e5688edd62cd 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1824,6 +1824,27 @@ void i915_reset(struct drm_i915_private *dev_priv) goto wakeup; } +/** + * i915_reset_engine - reset GPU engine to recover from a hang + * @engine: engine to reset + * + * Reset a specific GPU engine. Useful if a hang is detected. + * Returns zero on successful reset or otherwise an error code. + */ +int i915_reset_engine(struct intel_engine_cs *engine) +{ + int ret; + struct drm_i915_private *dev_priv = engine->i915; + + /* FIXME: replace me with engine reset sequence */ + ret = -ENODEV; + + /* use full gpu reset to recover on error */ + set_bit(I915_RESET_IN_PROGRESS, &dev_priv->gpu_error.flags); + + return ret; +} + static int i915_pm_suspend(struct device *kdev) { struct pci_dev *pdev = to_pci_dev(kdev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index fb3c88144aa8..6b4e8ee19905 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -781,6 +781,7 @@ struct intel_csr { func(has_ddi); \ func(has_decoupled_mmio); \ func(has_dp_mst); \ + func(has_engine_reset); \ func(has_fbc); \ func(has_fpga_dbg); \ func(has_full_ppgtt); \ @@ -3007,6 +3008,8 @@ extern void i915_driver_unload(struct drm_device *dev); extern int intel_gpu_reset(struct drm_i915_private *dev_priv, u32 engine_mask); extern bool intel_has_gpu_reset(struct drm_i915_private *dev_priv); extern void i915_reset(struct drm_i915_private *dev_priv); +extern bool intel_has_engine_reset(struct drm_i915_private *dev_priv); +extern int i915_reset_engine(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); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 77caeeb5ee55..da619810f59e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2594,7 +2594,8 @@ static void i915_error_wake_up(struct drm_i915_private *dev_priv) * Fire an error uevent so userspace can see that a hang or error * was detected. */ -static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv) +static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv, + u32 engine_mask) { struct kobject *kobj = &dev_priv->drm.primary->kdev->kobj; char *error_event[] = { I915_ERROR_UEVENT "=1", NULL }; @@ -2603,7 +2604,15 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv) kobject_uevent_env(kobj, KOBJ_CHANGE, error_event); - DRM_DEBUG_DRIVER("resetting chip\n"); + /* + * This event needs to be sent before performing gpu reset. When + * engine resets are supported we iterate through all engines and + * reset hung engines individually. To keep the event dispatch + * mechanism consistent with full gpu reset, this is only sent once + * even when multiple engines are hung. It is also safe to move this + * here because when we are in this function, we will definitely + * perform gpu reset. + */ kobject_uevent_env(kobj, KOBJ_CHANGE, reset_event); /* @@ -2614,30 +2623,55 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv) * simulated reset via debugs, so get an RPM reference. */ intel_runtime_pm_get(dev_priv); - intel_prepare_reset(dev_priv); - do { - /* - * All state reset _must_ be completed before we update the - * reset counter, for otherwise waiters might miss the reset - * pending state and not properly drop locks, resulting in - * deadlocks with the reset work. - */ - if (mutex_trylock(&dev_priv->drm.struct_mutex)) { - i915_reset(dev_priv); - mutex_unlock(&dev_priv->drm.struct_mutex); + /* If hardware supports it (GEN8+), try engine reset first */ + if (intel_has_engine_reset(dev_priv)) { + struct intel_engine_cs *engine; + unsigned int tmp, ret; + + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) { + ret = i915_reset_engine(engine); + /* on failure fallback to full gpu reset for recovery */ + if (ret) + break; } + } - /* We need to wait for anyone holding the lock to wakeup */ - } while (wait_on_bit_timeout(&dev_priv->gpu_error.flags, - I915_RESET_IN_PROGRESS, - TASK_UNINTERRUPTIBLE, - HZ)); + /* + * If the waiter already held the struct_mutex lock, it may have already + * triggered the GPU reset and the reset_in_progress can be false. + */ + if (i915_reset_in_progress(&dev_priv->gpu_error)) { + DRM_DEBUG_DRIVER("resetting chip\n"); + intel_prepare_reset(dev_priv); + + do { + /* + * All state reset _must_ be completed before we update + * the reset counter, for otherwise waiters might miss + * the reset pending state and not properly drop locks, + * resulting in deadlocks with the reset work. + */ + if (mutex_trylock(&dev_priv->drm.struct_mutex)) { + i915_reset(dev_priv); + mutex_unlock(&dev_priv->drm.struct_mutex); + } + + /* + * We need to wait for anyone holding the lock to + * wakeup. + */ + } while (wait_on_bit_timeout(&dev_priv->gpu_error.flags, + I915_RESET_IN_PROGRESS, + TASK_UNINTERRUPTIBLE, + HZ)); + + intel_finish_reset(dev_priv); + } - intel_finish_reset(dev_priv); intel_runtime_pm_put(dev_priv); - if (!test_bit(I915_WEDGED, &dev_priv->gpu_error.flags)) + if (!i915_terminally_wedged(&dev_priv->gpu_error)) kobject_uevent_env(kobj, KOBJ_CHANGE, reset_done_event); @@ -2728,9 +2762,15 @@ void i915_handle_error(struct drm_i915_private *dev_priv, if (!engine_mask) return; - if (test_and_set_bit(I915_RESET_IN_PROGRESS, - &dev_priv->gpu_error.flags)) - return; + /* + * Engine reset support is only available from Gen8 onwards so if + * it is not available or explicity disabled, use full gpu reset. + */ + if (!intel_has_engine_reset(dev_priv)) { + if (test_and_set_bit(I915_RESET_IN_PROGRESS, + &dev_priv->gpu_error.flags)) + return; + } /* * Wakeup waiting processes so that the reset function @@ -2746,7 +2786,7 @@ void i915_handle_error(struct drm_i915_private *dev_priv, */ i915_error_wake_up(dev_priv); - i915_reset_and_wakeup(dev_priv); + i915_reset_and_wakeup(dev_priv, engine_mask); } /* Called from drm generic code, passed 'crtc' which diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 37244fad9b03..6e9716ae4c21 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -308,7 +308,8 @@ static const struct intel_device_info intel_haswell_info = { BDW_COLORS, \ .has_logical_ring_contexts = 1, \ .has_full_48bit_ppgtt = 1, \ - .has_64bit_reloc = 1 + .has_64bit_reloc = 1, \ + .has_engine_reset = 1 static const struct intel_device_info intel_broadwell_info = { BDW_FEATURES, @@ -339,6 +340,7 @@ static const struct intel_device_info intel_cherryview_info = { .has_gmch_display = 1, .has_aliasing_ppgtt = 1, .has_full_ppgtt = 1, + .has_engine_reset = 1, .display_mmio_offset = VLV_DISPLAY_BASE, GEN_CHV_PIPEOFFSETS, CURSOR_OFFSETS, @@ -390,6 +392,7 @@ static const struct intel_device_info intel_skylake_gt3_info = { .has_aliasing_ppgtt = 1, \ .has_full_ppgtt = 1, \ .has_full_48bit_ppgtt = 1, \ + .has_engine_reset = 1, \ GEN_DEFAULT_PIPEOFFSETS, \ IVB_CURSOR_OFFSETS, \ BDW_COLORS diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 8fc5f29e79a8..97ce324570ad 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1851,6 +1851,17 @@ bool intel_has_gpu_reset(struct drm_i915_private *dev_priv) return intel_get_gpu_reset(dev_priv) != NULL; } +/* + * When GuC submission is enabled, GuC manages ELSP and can initiate the + * engine reset too. For now, fall back to full GPU reset if it is enabled. + */ +bool intel_has_engine_reset(struct drm_i915_private *dev_priv) +{ + return (dev_priv->info.has_engine_reset && + !dev_priv->guc.execbuf_client && + i915.reset == 2); +} + int intel_guc_reset(struct drm_i915_private *dev_priv) { int ret;