drm/i915: don't return a spurious -EIO from intel_ring_begin

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.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

1 files changed, 4 insertions, 14 deletions

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cd35ad4922de..d42d821c64d6 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);