Re: [PATCH 3/5] drm/i915: Make wait_for_flips interruptible.

From: Daniel Vetter <daniel@ffwll.ch>
To: Ander Conselvan De Oliveira <conselvan2@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/5] drm/i915: Make wait_for_flips interruptible.
Date: Mon, 19 Oct 2015 15:30:37 +0200	[thread overview]
Message-ID: <20151019133037.GC13786@phenom.ffwll.local> (raw)
In-Reply-To: <1445260613.3042.62.camel@gmail.com>

On Mon, Oct 19, 2015 at 04:16:53PM +0300, Ander Conselvan De Oliveira wrote:
> On Wed, 2015-09-23 at 13:27 +0200, Maarten Lankhorst wrote:
> > @@ -13306,6 +13299,29 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (old_obj) {
> > +		struct drm_crtc_state *crtc_state =
> > +			drm_atomic_get_existing_crtc_state(new_state->state,
> > plane->state->crtc);
> > +
> > +		/* Big Hammer, we also need to ensure that any pending
> > +		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
> > +		 * current scanout is retired before unpinning the old
> > +		 * framebuffer. Note that we rely on userspace rendering
> > +		 * into the buffer attached to the pipe they are waiting
> > +		 * on. If not, userspace generates a GPU hang with IPEHR
> > +		 * point to the MI_WAIT_FOR_EVENT.
> > +		 *
> > +		 * This should only fail upon a hung GPU, in which case we
> > +		 * can safely continue.
> > +		 */
> > +		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)
> > +			goto out;
> 
> Doesn't this change the behavior of a modeset after a GPU hang? Since
> mm.interruptible is true, i915_gem_check_wedge() might return -EAGAIN instead of
> -EIO. Previously the modeset would continue in that scenario, but now, somewhat
> contrary to the comment above, we don't continue and instead pass the -EAGAIN to
> user space.

It's "while the gpu hang is pending" not "after", but this change is the
hole point of making pinning interruptible. With current modeset code the
only thing we could hope for is that the reset would go through, and
otherwise we'd have to fail the modeset. Now we can correctly retry the
operation if it has run into a concurrent gpu hang/reset.

Note that we still should eat any -EIO, since modesets must continue even
if the render side is completely dead.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx