* [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