From patchwork Mon Jan 11 09:16:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 8000531 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BB889BEEE5 for ; Mon, 11 Jan 2016 09:22:41 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 970BC20251 for ; Mon, 11 Jan 2016 09:22:40 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id A5D6A20256 for ; Mon, 11 Jan 2016 09:22:38 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1BDCA6E273; Mon, 11 Jan 2016 01:22:35 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by gabe.freedesktop.org (Postfix) with ESMTPS id 64DE16E23F for ; Mon, 11 Jan 2016 01:19:44 -0800 (PST) Received: by mail-wm0-f66.google.com with SMTP id u188so25291864wmu.0 for ; Mon, 11 Jan 2016 01:19:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=QMLPh7jYcIC8nglrh2/CX9ICRvJVS2Jumrn6O26NUJc=; b=xckq0hPBEojtJ+y0FStswDVJexAcmxm8GxD68aBcvUo78X4bLmb8pEBcgtnW5P1LgF C9mujOl66o/YMB2B8jsWRKTTcfemG2ym97rY7zYfnFxXtrYalshPWZNCK8ByQqUaTie2 H80cMp8Kz5sEDOGQN0IvUplShIu+EJ6rloYm8Pn7sXKfMWZ09GL/Uyvn2mPc/Zc7vDmM JOTcZ/IgDNk7rZyz8sfxDCd3q8aM43quST6bHeACSWNgkivDOl3E24nUx0LGQzA3qwTD whuP3KAHtjepcaWBx7vBQCkQaRouoU3y/UbGv0uef1YveTHkrR1JQJr0DL5AEvHMbw8s lmvw== X-Received: by 10.28.97.135 with SMTP id v129mr12853911wmb.78.1452503983073; Mon, 11 Jan 2016 01:19:43 -0800 (PST) Received: from haswell.alporthouse.com ([78.156.65.138]) by smtp.gmail.com with ESMTPSA id v2sm11834679wmv.12.2016.01.11.01.19.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 11 Jan 2016 01:19:41 -0800 (PST) From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Mon, 11 Jan 2016 09:16:20 +0000 Message-Id: <1452503961-14837-9-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.7.0.rc3 In-Reply-To: <1452503961-14837-1-git-send-email-chris@chris-wilson.co.uk> References: <1452503961-14837-1-git-send-email-chris@chris-wilson.co.uk> Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 009/190] drm/i915: Tighten reset_counter for reset status 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-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In the reset_counter, we use two bits to track a GPU hang and reset. The low bit is a "reset-in-progress" flag that we set to signal when we need to break waiters in order for the recovery task to grab the mutex. As soon as the recovery task has the mutex, we can clear that flag (which we do by incrementing the reset_counter thereby incrementing the gobal reset epoch). By clearing that flag when the recovery task holds the struct_mutex, we can forgo a second flag that simply tells GEM to ignore the "reset-in-progress" flag. The second flag we store in the reset_counter is whether the reset failed and we consider the GPU terminally wedged. Whilst this flag is set, all access to the GPU (at least through GEM rather than direct mmio access) is verboten. PS: Fun is in store, as in the future we want to move from a global reset epoch to a per-engine reset engine with request recovery. Signed-off-by: Chris Wilson Cc: Daniel Vetter Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 4 ++-- drivers/gpu/drm/i915/i915_drv.c | 39 ++++++++++++++++++++++--------------- drivers/gpu/drm/i915/i915_drv.h | 3 --- drivers/gpu/drm/i915/i915_gem.c | 27 +++++++++---------------- drivers/gpu/drm/i915/i915_irq.c | 21 ++------------------ 5 files changed, 36 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 932af05b8eec..6ff2d23faaa7 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4696,7 +4696,7 @@ i915_wedged_get(void *data, u64 *val) struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; - *val = i915_reset_counter(&dev_priv->gpu_error); + *val = i915_terminally_wedged(&dev_priv->gpu_error); return 0; } @@ -4715,7 +4715,7 @@ i915_wedged_set(void *data, u64 val) * while it is writing to 'i915_wedged' */ - if (i915_reset_in_progress_or_wedged(&dev_priv->gpu_error)) + if (i915_reset_in_progress(&dev_priv->gpu_error)) return -EAGAIN; intel_runtime_pm_get(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 288fec7691dc..2f03379cdb4b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -873,23 +873,32 @@ int i915_resume_switcheroo(struct drm_device *dev) int i915_reset(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - bool simulated; + struct i915_gpu_error *error = &dev_priv->gpu_error; + unsigned reset_counter; int ret; intel_reset_gt_powersave(dev); mutex_lock(&dev->struct_mutex); - i915_gem_reset(dev); + /* Clear any previous failed attempts at recovery. Time to try again. */ + atomic_andnot(I915_WEDGED, &error->reset_counter); - simulated = dev_priv->gpu_error.stop_rings != 0; + /* Clear the reset-in-progress flag and increment the reset epoch. */ + reset_counter = atomic_inc_return(&error->reset_counter); + if (WARN_ON(__i915_reset_in_progress(reset_counter))) { + ret = -EIO; + goto error; + } + + i915_gem_reset(dev); ret = intel_gpu_reset(dev); /* Also reset the gpu hangman. */ - if (simulated) { + if (error->stop_rings != 0) { DRM_INFO("Simulated gpu hang, resetting stop_rings\n"); - dev_priv->gpu_error.stop_rings = 0; + error->stop_rings = 0; if (ret == -ENODEV) { DRM_INFO("Reset not implemented, but ignoring " "error for simulated gpu hangs\n"); @@ -902,8 +911,7 @@ int i915_reset(struct drm_device *dev) if (ret) { DRM_ERROR("Failed to reset chip: %i\n", ret); - mutex_unlock(&dev->struct_mutex); - return ret; + goto error; } intel_overlay_reset(dev_priv); @@ -922,20 +930,14 @@ int i915_reset(struct drm_device *dev) * was running at the time of the reset (i.e. we weren't VT * switched away). */ - - /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ - dev_priv->gpu_error.reload_in_reset = true; - ret = i915_gem_init_hw(dev); - - dev_priv->gpu_error.reload_in_reset = false; - - mutex_unlock(&dev->struct_mutex); if (ret) { DRM_ERROR("Failed hw init on reset %d\n", ret); - return ret; + goto error; } + mutex_unlock(&dev->struct_mutex); + /* * rps/rc6 re-init is necessary to restore state lost after the * reset and the re-install of gt irqs. Skip for ironlake per @@ -946,6 +948,11 @@ int i915_reset(struct drm_device *dev) intel_enable_gt_powersave(dev); return 0; + +error: + atomic_or(I915_WEDGED, &error->reset_counter); + mutex_unlock(&dev->struct_mutex); + return ret; } static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b274237726de..60531df3844c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1381,9 +1381,6 @@ struct i915_gpu_error { /* For missed irq/seqno simulation. */ unsigned int test_irq_rings; - - /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ - bool reload_in_reset; }; enum modeset_restore { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 78bf980a69bf..2cdd20b3aeaf 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -83,9 +83,7 @@ i915_gem_wait_for_error(struct i915_gpu_error *error) { int ret; -#define EXIT_COND (!i915_reset_in_progress_or_wedged(error) || \ - i915_terminally_wedged(error)) - if (EXIT_COND) + if (!i915_reset_in_progress(error)) return 0; /* @@ -94,17 +92,16 @@ i915_gem_wait_for_error(struct i915_gpu_error *error) * we should simply try to bail out and fail as gracefully as possible. */ ret = wait_event_interruptible_timeout(error->reset_queue, - EXIT_COND, + !i915_reset_in_progress(error), 10*HZ); if (ret == 0) { DRM_ERROR("Timed out waiting for the gpu reset to complete\n"); return -EIO; } else if (ret < 0) { return ret; + } else { + return 0; } -#undef EXIT_COND - - return 0; } int i915_mutex_lock_interruptible(struct drm_device *dev) @@ -1112,22 +1109,16 @@ i915_gem_check_wedge(struct i915_gpu_error *error, bool interruptible) { if (i915_reset_in_progress_or_wedged(error)) { + /* Recovery complete, but the reset failed ... */ + if (i915_terminally_wedged(error)) + return -EIO; + /* Non-interruptible callers can't handle -EAGAIN, hence return * -EIO unconditionally for these. */ if (!interruptible) return -EIO; - /* Recovery complete, but the reset failed ... */ - if (i915_terminally_wedged(error)) - return -EIO; - - /* - * Check if GPU Reset is in progress - we need intel_ring_begin - * to work properly to reinit the hw state while the gpu is - * still marked as reset-in-progress. Handle this with a flag. - */ - if (!error->reload_in_reset) - return -EAGAIN; + return -EAGAIN; } return 0; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9a6b0ac54d01..15973e917566 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2466,7 +2466,6 @@ static void i915_error_wake_up(struct drm_i915_private *dev_priv, static void i915_reset_and_wakeup(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); - struct i915_gpu_error *error = &dev_priv->gpu_error; char *error_event[] = { I915_ERROR_UEVENT "=1", NULL }; char *reset_event[] = { I915_RESET_UEVENT "=1", NULL }; char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL }; @@ -2484,7 +2483,7 @@ static void i915_reset_and_wakeup(struct drm_device *dev) * the reset in-progress bit is only ever set by code outside of this * work we don't need to worry about any other races. */ - if (i915_reset_in_progress_or_wedged(error) && !i915_terminally_wedged(error)) { + if (i915_reset_in_progress(&dev_priv->gpu_error)) { DRM_DEBUG_DRIVER("resetting chip\n"); kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, reset_event); @@ -2512,25 +2511,9 @@ static void i915_reset_and_wakeup(struct drm_device *dev) intel_runtime_pm_put(dev_priv); - if (ret == 0) { - /* - * After all the gem state is reset, increment the reset - * counter and wake up everyone waiting for the reset to - * complete. - * - * Since unlock operations are a one-sided barrier only, - * we need to insert a barrier here to order any seqno - * updates before - * the counter increment. - */ - smp_mb__before_atomic(); - atomic_inc(&dev_priv->gpu_error.reset_counter); - + if (ret == 0) kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, reset_done_event); - } else { - atomic_or(I915_WEDGED, &error->reset_counter); - } /* * Note: The wake_up also serves as a memory barrier so that