* [PATCH] drm/i915: Don't die in wait_for_pending_flips @ 2014-05-19 14:09 Daniel Vetter 2014-05-19 15:06 ` Jesse Barnes 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2014-05-19 14:09 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter 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@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); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Don't die in wait_for_pending_flips 2014-05-19 14:09 [PATCH] drm/i915: Don't die in wait_for_pending_flips Daniel Vetter @ 2014-05-19 15:06 ` Jesse Barnes 2014-05-19 15:18 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Jesse Barnes @ 2014-05-19 15:06 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel Graphics Development On Mon, 19 May 2014 16:09:35 +0200 Daniel Vetter <daniel.vetter@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@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. :) -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Don't die in wait_for_pending_flips 2014-05-19 15:06 ` Jesse Barnes @ 2014-05-19 15:18 ` Daniel Vetter 2014-05-19 15:35 ` Jesse Barnes 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2014-05-19 15:18 UTC (permalink / raw) To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development 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@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@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. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Don't die in wait_for_pending_flips 2014-05-19 15:18 ` Daniel Vetter @ 2014-05-19 15:35 ` Jesse Barnes 2014-05-19 15:41 ` Chris Wilson 0 siblings, 1 reply; 6+ messages in thread From: Jesse Barnes @ 2014-05-19 15:35 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development On Mon, 19 May 2014 17:18:40 +0200 Daniel Vetter <daniel@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@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@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. Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Don't die in wait_for_pending_flips 2014-05-19 15:35 ` Jesse Barnes @ 2014-05-19 15:41 ` Chris Wilson 2014-05-19 15:52 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Chris Wilson @ 2014-05-19 15:41 UTC (permalink / raw) To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development 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@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@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@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@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@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Don't die in wait_for_pending_flips 2014-05-19 15:41 ` Chris Wilson @ 2014-05-19 15:52 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2014-05-19 15:52 UTC (permalink / raw) To: Chris Wilson, Jesse Barnes, Daniel Vetter, Daniel Vetter, Intel Graphics Development 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 <daniel@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@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@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@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. 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.co.uk> > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-19 15:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-19 14:09 [PATCH] drm/i915: Don't die in wait_for_pending_flips Daniel Vetter 2014-05-19 15:06 ` Jesse Barnes 2014-05-19 15:18 ` Daniel Vetter 2014-05-19 15:35 ` Jesse Barnes 2014-05-19 15:41 ` Chris Wilson 2014-05-19 15:52 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox