Re: [PATCH 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Nov 24, 2014 at 10:34:42AM +0100, Daniel Vetter wrote:
> On Fri, Nov 21, 2014 at 11:10:31PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 21, 2014 at 09:49:21PM +0100, Daniel Vetter wrote:
> > > On Fri, Nov 21, 2014 at 09:54:29PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > +
> > > > +	/*
> > > > +	 * Flips in the rings will be nuked by the reset,
> > > > +	 * so complete all pending flips so that user space
> > > > +	 * will get its events and not get stuck.
> > > > +	 *
> > > > +	 * Old platforms will also reset the display, so we
> > > > +	 * need to grab the modeset locks around the reset.
> > > > +	 * But in order to do that we must let any pending
> > > > +	 * page flip wait complete since the waiters may be
> > > > +	 * holding some modeset locks.
> > > > +	 */
> > > > +	intel_complete_page_flips(dev);
> > > 
> > > Is this really required? We complete them afterwards, and all the pageflip
> > > waiters I've found do check for gpu hangs and abort the pageflip wait.
> > > That's already required since the mmio flip might go missing, and thus far
> > > we've only completed the flip _after_ having reset the gpu and gem state
> > > (and grabbed dev->struct_mutex).
> > 
> > Hmm. Yeah, just waking them up ought to be sufficient to dislodge
> > things. And we already do that before scheduling the error work, but
> > after setting the reset_in_progress flag, which is very much critical
> > here. So I guess I could just move the complete pending flips bit to
> > intel_finish_reset().
> > 
> > But then I do wonder a bit why I originally needed to add the unlocked
> > page flip complete before the locked .update_plane() call. Did we miss
> > a wakeup somewhere or did we not abort pending flip waits on reset?
> 
> gpu hang vs. pending flip deadlocks should have been fixed with
> 
> commit 17e1df07df0fbc77696a1e1b6ccf9f2e5af70e40
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Sun Sep 8 21:57:13 2013 +0200
> 
>     drm/i915: fix wait_for_pending_flips vs gpu hang deadlock
> 
> Maybe it's been broken meanwhile but it should have worked since quite a
> while. Or are these gen3/4 reset patches really this old?

Nah. But I was actually wondering why the unlocked flip complete prior to
.update_plane() was supposedly necessary at all. But yeah the missing
wake_up before that commit explains it.

I'll see about respinning the patch with the flip complete moved back
after the reset. I guess I could actually move it occur even after
.update_plane() now.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx