From patchwork Fri Sep 9 13:11:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 9323385 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 565FF60231 for ; Fri, 9 Sep 2016 13:12:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4A1D229E9B for ; Fri, 9 Sep 2016 13:12:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3CD1229DD3; Fri, 9 Sep 2016 13:12:23 +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.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID 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 B593B29DD3 for ; Fri, 9 Sep 2016 13:12:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1FF386E872; Fri, 9 Sep 2016 13:12:22 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id B20FE6E0B7 for ; Fri, 9 Sep 2016 13:12:19 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id a6so2720431wmc.2 for ; Fri, 09 Sep 2016 06:12:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:subject:date:message-id:in-reply-to:references; bh=NaUUePrZTMFrnxrhULE/BvznM7BAtts90OeDHbVdXcM=; b=Uu3ydsdSvg8Gq7EY/Jz54LXAO758h/Xaldgzpu6M3L2lkUs/H8Ep7/ElVFC2En7Lzg YZmbnHlgTddvRVWek3pxAL0/ZMRAhzHfX15CAmjhw9Bt9Fb9ig0bkmrYuP6uCIHWHeon vkP/NYFW0+VZ7aA923LCGL913bDYr7fVQhsSaImHEIgq0hGDuUDkv4WjvHCJGNbRg5dq x49zb/nq4Nln7Z5S3n+aDO6TJCsCVsdwydAe5R9EBelhKtx5NvfuwRzN1A00POtaBDF5 Pw/q3ivZ7iN8ZKn0BKpQkr7R4lU1EabJ6EkGd3AD23TwrcV8rid9GqZC4mFCbOEigs7u D70w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:subject:date:message-id :in-reply-to:references; bh=NaUUePrZTMFrnxrhULE/BvznM7BAtts90OeDHbVdXcM=; b=cEqoibhbYRM1VCpECIIr0qcJ+9cJmJo1K44C+sN0yjiZKtyQLlxvjE4xpP8CKlJayE S3ULW/egb0qA/fR1+yjaoAXh7fWNUIcn1yVGLSPz5JwfJAidk1pXvFeGRB8DnAYaRsni lNzEyVATDlwGxcfqMM70M4DmzY+xF2bJ/ST1bK4Wwz519hx9GgV5SKe8+vTOjvfdB1uS 818yDjKNJ2ag4mcWcuIV/Xa5uwiNlHu0eoj26SveYcfgfMyc4BGaHprc/hcqkO57Zwes IsuyzCUsqWmgOq5eitlswfvvZ92Zl0KSzh2OX9EIofQsjveBiADeJdyIkPOiij9cCjZF D0XQ== X-Gm-Message-State: AE9vXwMxfEMPY5mb9vrG99JJ2uwzvr+/ZWX68o4PpGGkxMr2cs3w5OoqJEN72KHTJd3aQg== X-Received: by 10.28.58.7 with SMTP id h7mr3022119wma.35.1473426737951; Fri, 09 Sep 2016 06:12:17 -0700 (PDT) Received: from haswell.alporthouse.com ([78.156.65.138]) by smtp.gmail.com with ESMTPSA id u124sm3330468wmu.10.2016.09.09.06.12.17 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 Sep 2016 06:12:17 -0700 (PDT) From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2016 14:11:51 +0100 Message-Id: <20160909131201.16673-11-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20160909131201.16673-1-chris@chris-wilson.co.uk> References: <20160909131201.16673-1-chris@chris-wilson.co.uk> Subject: [Intel-gfx] [CI 11/21] drm/i915: Perform a direct reset of the GPU from the waiter 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 If a waiter is holding the struct_mutex, then the reset worker cannot reset the GPU until the waiter returns. We do not want to return -EAGAIN form i915_wait_request as that breaks delicate operations like i915_vma_unbind() which often cannot be restarted easily, and returning -EIO is just as useless (and has in the past proven dangerous). The remaining WARN_ON(i915_wait_request) serve as a valuable reminder that handling errors from an indefinite wait are tricky. We can keep the current semantic that knowing after a reset is complete, so is the request, by performing the reset ourselves if we hold the mutex. uevent emission is still handled by the reset worker, so it may appear slightly out of order with respect to the actual reset (and concurrent use of the device). Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.c | 11 ++++++----- drivers/gpu/drm/i915/i915_drv.h | 15 +++------------ drivers/gpu/drm/i915/i915_gem_request.c | 29 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_irq.c | 2 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 3 --- 5 files changed, 40 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 47a676d859db..ff4173e6e298 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1729,6 +1729,8 @@ int i915_resume_switcheroo(struct drm_device *dev) * Reset the chip. Useful if a hang is detected. Returns zero on successful * reset or otherwise an error code. * + * Caller must hold the struct_mutex. + * * Procedure is fairly simple: * - reset the chip using the reset reg * - re-init context state @@ -1743,7 +1745,10 @@ int i915_reset(struct drm_i915_private *dev_priv) struct i915_gpu_error *error = &dev_priv->gpu_error; int ret; - mutex_lock(&dev->struct_mutex); + lockdep_assert_held(&dev->struct_mutex); + + if (!test_and_clear_bit(I915_RESET_IN_PROGRESS, &error->flags)) + return test_bit(I915_WEDGED, &error->flags) ? -EIO : 0; /* Clear any previous failed attempts at recovery. Time to try again. */ __clear_bit(I915_WEDGED, &error->flags); @@ -1784,9 +1789,6 @@ int i915_reset(struct drm_i915_private *dev_priv) goto error; } - clear_bit(I915_RESET_IN_PROGRESS, &error->flags); - 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 @@ -1800,7 +1802,6 @@ int i915_reset(struct drm_i915_private *dev_priv) error: set_bit(I915_WEDGED, &error->flags); - mutex_unlock(&dev->struct_mutex); return ret; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 20b7743f8ec5..15f1977e356a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3863,7 +3863,9 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms) schedule_timeout_uninterruptible(remaining_jiffies); } } -static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req) + +static inline bool +__i915_request_irq_complete(struct drm_i915_gem_request *req) { struct intel_engine_cs *engine = req->engine; @@ -3925,17 +3927,6 @@ static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req) return true; } - /* We need to check whether any gpu reset happened in between - * the request being submitted and now. If a reset has occurred, - * the seqno will have been advance past ours and our request - * is complete. If we are in the process of handling a reset, - * the request is effectively complete as the rendering will - * be discarded, but we need to return in order to drop the - * struct_mutex. - */ - if (i915_reset_in_progress(&req->i915->gpu_error)) - return true; - return false; } diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 5f89801e6a16..64c370681a81 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -533,6 +533,16 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches) engine->submit_request(request); } +static void reset_wait_queue(wait_queue_head_t *q, wait_queue_t *wait) +{ + unsigned long flags; + + spin_lock_irqsave(&q->lock, flags); + if (list_empty(&wait->task_list)) + __add_wait_queue(q, wait); + spin_unlock_irqrestore(&q->lock, flags); +} + static unsigned long local_clock_us(unsigned int *cpu) { unsigned long t; @@ -710,6 +720,25 @@ wakeup: if (__i915_request_irq_complete(req)) break; + /* If the GPU is hung, and we hold the lock, reset the GPU + * and then check for completion. On a full reset, the engine's + * HW seqno will be advanced passed us and we are complete. + * If we do a partial reset, we have to wait for the GPU to + * resume and update the breadcrumb. + * + * If we don't hold the mutex, we can just wait for the worker + * to come along and update the breadcrumb (either directly + * itself, or indirectly by recovering the GPU). + */ + if (flags & I915_WAIT_LOCKED && + i915_reset_in_progress(&req->i915->gpu_error)) { + __set_current_state(TASK_RUNNING); + i915_reset(req->i915); + reset_wait_queue(&req->i915->gpu_error.wait_queue, + &reset); + continue; + } + /* Only spin if we know the GPU is processing this request */ if (i915_spin_request(req, state, 2)) break; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ed172d7beecb..2c7cb5041511 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2521,7 +2521,9 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv) * pending state and not properly drop locks, resulting in * deadlocks with the reset work. */ + mutex_lock(&dev_priv->drm.struct_mutex); ret = i915_reset(dev_priv); + mutex_unlock(&dev_priv->drm.struct_mutex); intel_finish_reset(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8687a84a7ff3..d2d85fc869e1 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2229,9 +2229,6 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes) if (ret) return ret; - if (i915_reset_in_progress(&target->i915->gpu_error)) - return -EAGAIN; - i915_gem_request_retire_upto(target); intel_ring_update_space(ring);