[Intel-gfx] [PATCH 09/32] drm/i915: Prevent leaking of -EIO from i915_wait_request()

[Chris Wilson]

On Wed, Dec 16, 2015 at 10:52:06AM +0100, Daniel Vetter wrote:
> On Fri, Dec 11, 2015 at 11:33:05AM +0000, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d7bbd015de35..875bdf814d73 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13405,10 +13405,6 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
> >  
> >  			ret = __i915_wait_request(intel_plane_state->wait_req,
> >  						  true, NULL, NULL);
> > -
> > -			/* Swallow -EIO errors to allow updates during hw lockup. */
> > -			if (ret == -EIO)
> 
> I'd like to keep a WARN_ON here since it's ABI relevant, and consistent
> with the other places we've had a WARN_ON(-EIO).
> 
> > -				ret = 0;
> >  			if (ret) {
> >  				mutex_lock(&dev->struct_mutex);
> >  				drm_atomic_helper_cleanup_planes(dev, state);
> > @@ -13744,9 +13740,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> >  		 */
> >  		if (needs_modeset(crtc_state))
> >  			ret = i915_gem_object_wait_rendering(old_obj, true);
> > -
> > -		/* Swallow -EIO errors to allow updates during hw lockup. */
> > -		if (ret && ret != -EIO)
> > +		if (ret)
> 
> Same here really.

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 875bdf814d73..1586d9107cba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13406,6 +13406,8 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
                        ret = __i915_wait_request(intel_plane_state->wait_req,
                                                  true, NULL, NULL);
                        if (ret) {
+                               /* Any hang should be swallowed by the wait */
+                               WARN_ON(ret == -EIO);
                                mutex_lock(&dev->struct_mutex);
                                drm_atomic_helper_cleanup_planes(dev, state);
                                mutex_unlock(&dev->struct_mutex);
@@ -13740,8 +13742,11 @@ intel_prepare_plane_fb(struct drm_plane *plane,
                 */
                if (needs_modeset(crtc_state))
                        ret = i915_gem_object_wait_rendering(old_obj, true);
-               if (ret)
+               if (ret) {
+                       /* GPU hangs should have been swallowed by the wait */
+                       WARN_ON(ret == -EIO);
                        return ret;
+               }
        }
 
        /* For framebuffer backed by dmabuf, wait for fence */



> We already have a WARN_ON in the mmio_flip, so are covered there already.
> 
> Otherwise looks good, and with the above addressed and under the
> assumption that the last caller of check_wedge is in request_alloc (which
> I was too lazy to check perfectly):

Yes, later it is moved to the new i915_gem_request.c and made static
alongside it's only caller request_alloc().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre