From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Don't die in wait_for_pending_flips Date: Mon, 19 May 2014 17:52:31 +0200 Message-ID: <20140519155230.GV8790@phenom.ffwll.local> References: <1400508575-8041-1-git-send-email-daniel.vetter@ffwll.ch> <20140519080606.19afa359@jbarnes-desktop> <20140519151840.GS8790@phenom.ffwll.local> <20140519083527.1110ff95@jbarnes-desktop> <20140519154131.GF11754@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id D2CA76E503 for ; Mon, 19 May 2014 08:52:35 -0700 (PDT) Received: by mail-ee0-f49.google.com with SMTP id e53so3716954eek.22 for ; Mon, 19 May 2014 08:52:35 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140519154131.GF11754@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Jesse Barnes , Daniel Vetter , Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Mon, May 19, 2014 at 04:41:31PM +0100, Chris Wilson wrote: > On Mon, May 19, 2014 at 08:35:27AM -0700, Jesse Barnes wrote: > > On Mon, 19 May 2014 17:18:40 +0200 > > Daniel Vetter 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 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 > > > > > --- > > > > > 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 > > Anyhow, I'd been meaning to do this myself, just kept wondering if > perhaps hangcheck was a better place to drive it from. We still lack the add_request for flips ... And imo there's too many fragile parts in our pageflip sequence to go wrong, so a simple timeout at the very end seems best. Patch pulled in. -Daniel > > Reviewed-by: Chris Wilson > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch