[Intel-gfx] [PATCH] drm/i915: Don't die in wait_for_pending_flips

[Chris Wilson]

On Mon, May 19, 2014 at 08:35:27AM -0700, Jesse Barnes wrote:
> On Mon, 19 May 2014 17:18:40 +0200
> Daniel Vetter <daniel at ffwll.ch> wrote:
> 
> > On Mon, May 19, 2014 at 08:06:06AM -0700, Jesse Barnes wrote:
> > > On Mon, 19 May 2014 16:09:35 +0200
> > > Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > > 
> > > > We can apperently miss them, but breaking the entire driver hampers
> > > > testing. So bail out after one minute, our customerary "this is a lost
> > > > cause" timeout.
> > > > 
> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=78383
> > > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 0f8f9bcb3012..6eca24d8b282 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -3284,8 +3284,9 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
> > > >  
> > > >  	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
> > > >  
> > > > -	wait_event(dev_priv->pending_flip_queue,
> > > > -		   !intel_crtc_has_pending_flip(crtc));
> > > > +	WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
> > > > +				   !intel_crtc_has_pending_flip(crtc),
> > > > +				   60*HZ) == 0);
> > > >  
> > > >  	mutex_lock(&dev->struct_mutex);
> > > >  	intel_finish_fb(crtc->primary->fb);
> > > 
> > > Updating our page flip ioctl man page (hah!) with the timeout info
> > > would be good, in case people like Mario queue flips for after lunch. :)
> > 
> > We don't do that in the kernel though, we only ever queue flips for the
> > next vblank after rendering completed. Completed rendering we can detect
> > (and have 60s timeouts in other places where the hangcheck isn't
> > guaranteed to be around already), the additional vblank is negligible imo.
> > 
> > Of course if we add support for flip queues in the kernel we might need to
> > cancel outstanding flips properly when we kill the crtc, like we already
> > do for vblank events.
> > 
> > So imo no need to document anything.
> 
> Ah right so we'd only be affected here by ridiculous refresh rates...
> so yeah should be fine.

If we are that concerned we could factor those into the timeout,
say hangcheck + 10 * frame_interval.
 
> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

Anyhow, I'd been meaning to do this myself, just kept wondering if
perhaps hangcheck was a better place to drive it from.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre