From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 1/4] drm/i915: don't return a spurious -EIO from intel_ring_begin Date: Sat, 30 Jun 2012 20:09:36 -0700 Message-ID: <20120630200936.5a802fe1@bwidawsk.net> References: <1340744933-11835-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 437C59E754 for ; Sat, 30 Jun 2012 20:11:30 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by cloud01.chad-versace.us (Postfix) with ESMTP id A418B180E61 for ; Sun, 1 Jul 2012 03:13:47 +0000 (UTC) Received: from cloud01.chad-versace.us ([127.0.0.1]) by localhost (cloud01.static.cloud-ips.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xQQ-0zF6uMc7 for ; Sun, 1 Jul 2012 03:13:42 +0000 (UTC) Received: from localhost.localdomain (static-50-53-63-81.bvtn.or.frontiernet.net [50.53.63.81]) by cloud01.chad-versace.us (Postfix) with ESMTPSA id 55ABA18023E for ; Sun, 1 Jul 2012 03:13:42 +0000 (UTC) In-Reply-To: <1340744933-11835-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 26 Jun 2012 23:08:50 +0200 Daniel Vetter wrote: > Having this early check in intel_ring_begin doesn't buy us anything, > since we'll be calling into wait_request in the usual case already > anyway. In the corner case of not waiting for free space using the > last_retired_head we simply need to do the same check, too. > > With these changes we'll only ever get an -EIO from intel_ring_begin > if the gpu has truely been declared dead. > > v2: Don't conflate the change to prevent intel_ring_begin from returning > a spurious -EIO with the refactor to use i915_gem_check_wedge, which is > just prep work to avoid returning -EAGAIN in callsites that can't handle > syscall restarting. I'm really scared by this change. It's diffuclt to review because there are SO many callers of intel_ring_begin, and figuring out if they all work in the wedged case is simply too difficult. I've yet to review the rest of the series, but it may make more sense to put this change last perhaps? > > Signed-Off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index f30a53a..501546e 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1230,13 +1230,9 @@ 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; > int n = 4*num_dwords; > int ret; > > - if (unlikely(atomic_read(&dev_priv->mm.wedged))) > - return -EIO; > - > if (unlikely(ring->tail + n > ring->effective_size)) { > ret = intel_wrap_ring_buffer(ring); > if (unlikely(ret)) -- Ben Widawsky, Intel Open Source Technology Center