From patchwork Wed Jul 4 20:52:50 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1157321 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 17BF23FC36 for ; Wed, 4 Jul 2012 20:53:15 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 04E1E9E7C2 for ; Wed, 4 Jul 2012 13:53:14 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wg0-f43.google.com (mail-wg0-f43.google.com [74.125.82.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 0D8969E765 for ; Wed, 4 Jul 2012 13:52:59 -0700 (PDT) Received: by wgbdr1 with SMTP id dr1so5939959wgb.12 for ; Wed, 04 Jul 2012 13:52:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=iQqwm6N0iiAWebeAol+hYQnzUuNVeiuKNFr/YC324mA=; b=a83qx7xwiV+nv4TLv9+jdFSfcpL7CxQmryxtpDQE2gbk8zc1dZ7DiZJ3375iF/lbyx miC01Q8vNb+SNlPirZ/P7ZjB7ObtFS2rUOhh3DamwWQq4Ba3VO0iOL9DYOS1GN8pnMZR /mu6oQX3K3f+SOgI2GmZCT+R0Y2XR9j78PL7Q= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=iQqwm6N0iiAWebeAol+hYQnzUuNVeiuKNFr/YC324mA=; b=UBkBcdF/1hwnQTdFer+Bv91NIpAXkUjN4b65inAD4yy0jXC5TKX8i44VkBfJ7o006j 0QHsTpFnVxCBS+Jtoq1p3DfdvDoHYJ0KFxuGy9llcTKbiBMtzvNVt+3gv61RECrFzp1w FuAoYhrF4D9T4KWtD0pDkiinackjRzDBcfVXm35KH8isgIG0VX9/+FYDh4R/Ggp7i4UO 5PtgLVjN9/MTGwC5T07QvbSae3senQ+Pdw1EhplBHvcGQc/iC8myRdOkCMQihKzUbOb2 9UNUE3lW8HOIvxBP225uBZsA62gWrN2ogkHrmehVsp0ZglXc1uY3qBckMbqT0Dw0t7sX +ecA== Received: by 10.216.207.151 with SMTP id n23mr7295440weo.100.1341435179160; Wed, 04 Jul 2012 13:52:59 -0700 (PDT) Received: from aaron.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id f7sm69872534wiv.2.2012.07.04.13.52.57 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 04 Jul 2012 13:52:58 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Wed, 4 Jul 2012 22:52:50 +0200 Message-Id: <1341435170-30391-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1341433123-23055-6-git-send-email-daniel.vetter@ffwll.ch> References: <1341433123-23055-6-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQlxlw0feQK4J/8FeIfm2SgfFOqBu57mU+aP148s4k+hgs/WN0ZzLXnG6e738bZ2XBU9uy+B Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH] drm/i915: don't return a spurious -EIO from intel_ring_begin X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org The issue with this check is that it results in userspace receiving an -EIO while the gpu reset hasn't completed, resulting in fallback to sw rendering or worse. Now there's also a stern comment in intel_ring_wait_seqno saying that intel_ring_begin should not return -EAGAIN, ever, because some callers can't handle that. But after an audit of the callsites I don't see any issues. I guess the last problematic spot disappeared with the removal of the pipelined fencing code. So do the right thing and call check_wedge, which should properly decide whether an -EAGAIN or -EIO is appropriate if wedged is set. Note that the early check for a wedged gpu before touching the ring is rather important (and it took me quite some time of acting like the densest doofus to figure that out): If we don't do that and the gpu died for good, not having been resurrect by the reset code, userspace can merrily fill up the entire ring until it notices that something is amiss. Allowing userspace to emit more render, despite that we know that it will fail can't lead to anything good (and by experience can lead to all sorts of havoc, including angering the OOM gods and hard-hanging the hw for good). v2: Fix EAGAIN mispell, noticed by Chris Wilson. Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index cd35ad4..d42d821 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1117,20 +1117,9 @@ static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring) static int intel_ring_wait_seqno(struct intel_ring_buffer *ring, u32 seqno) { - struct drm_i915_private *dev_priv = ring->dev->dev_private; - bool was_interruptible; int ret; - /* XXX As we have not yet audited all the paths to check that - * they are ready for ERESTARTSYS from intel_ring_begin, do not - * allow us to be interruptible by a signal. - */ - was_interruptible = dev_priv->mm.interruptible; - dev_priv->mm.interruptible = false; - ret = i915_wait_seqno(ring, seqno); - - dev_priv->mm.interruptible = was_interruptible; if (!ret) i915_gem_retire_requests_ring(ring); @@ -1240,12 +1229,13 @@ int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n) int intel_ring_begin(struct intel_ring_buffer *ring, int num_dwords) { - struct drm_i915_private *dev_priv = ring->dev->dev_private; + drm_i915_private_t *dev_priv = ring->dev->dev_private; int n = 4*num_dwords; int ret; - if (unlikely(atomic_read(&dev_priv->mm.wedged))) - return -EIO; + ret = i915_gem_check_wedge(dev_priv, dev_priv->mm.interruptible); + if (ret) + return ret; if (unlikely(ring->tail + n > ring->effective_size)) { ret = intel_wrap_ring_buffer(ring);