On Fri, Apr 20, 2012 at 06:23:29PM -0700, Ben Widawsky wrote:
> Insert a wait parameter in the code so we can possibly timeout on a
> seqno wait if need be. The code should be functionally the same as
> before because all the callers will continue to retry if an arbitrary
> timeout elapses.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky at intel.com>
Interface bikeshed: I think handling the wait_forever case in here would
be better, the do {} while (-ETIME) loops don't look to cute. That way we
can also ditch the rather arbitrary SEQNO_WAIT_DEFAULT.
> ---
> drivers/gpu/drm/i915/i915_gem.c | 41 ++++++++++++++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 920dbc1..7bfad04 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1847,25 +1847,36 @@ i915_gem_retire_work_handler(struct work_struct *work)
> mutex_unlock(&dev->struct_mutex);
> }
>
> +#define SEQNO_WAIT_DEFAULT (3000000UL)
> static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> - bool interruptible)
> + bool interruptible, long *usecs)
> {
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> - int ret = 0;
> + const long timeout = usecs_to_jiffies(*usecs);
> + long end;
>
> #define EXIT_COND \
> (i915_seqno_passed(ring->get_seqno(ring), seqno) || \
> atomic_read(&dev_priv->mm.wedged))
>
> if (interruptible)
> - ret = wait_event_interruptible(ring->irq_queue,
> - EXIT_COND);
> + end = wait_event_interruptible_timeout(ring->irq_queue,
> + EXIT_COND,
> + timeout);
> else
> - wait_event(ring->irq_queue, EXIT_COND);
> -
> + end = wait_event_timeout(ring->irq_queue, EXIT_COND, timeout);
> #undef EXIT_COND
>
> - return ret;
> + switch (end) {
> + case 0: /* Tiemout */
> + return -ETIME;
> + case -ERESTARTSYS: /* Signal */
> + return -ERESTARTSYS;
> + default: /* Completed */
> + BUG_ON(end < 0); /* We're not aware of other errors */
WARN_ON, we won't die right away when this ever happens.
> + *usecs = jiffies_to_usecs(timeout - end);
Safe when the ioctl restarting works different, you also need this for the
-ERESTARTSYS case. And it doesn't hurt for the timeout case.
> + return 0;
> + }
> }
>
> /**
> @@ -1916,7 +1927,12 @@ i915_wait_request(struct intel_ring_buffer *ring,
> if (WARN_ON(!ring->irq_get(ring)))
> return -EBUSY;
>
> - ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible);
> + do {
> + long time = SEQNO_WAIT_DEFAULT;
> + ret = __wait_seqno(ring, seqno,
> + dev_priv->mm.interruptible,
> + &time);
> + } while (ret == -ETIME);
>
> ring->irq_put(ring);
> trace_i915_gem_request_wait_end(ring, seqno);
> @@ -3012,13 +3028,16 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
> * lockless.
> */
> if (ring->irq_get(ring)) {
> - ret = __wait_seqno(ring, seqno, true);
> + do {
> + long time = SEQNO_WAIT_DEFAULT;
> + ret = __wait_seqno(ring, seqno, true, &time);
> + } while (ret == -ETIME);
> ring->irq_put(ring);
> -
> if (ret == 0 && atomic_read(&dev_priv->mm.wedged))
> ret = -EIO;
> - } else
> + } else {
> ret = -EBUSY;
> + }
> }
>
> if (ret == 0)
> --
> 1.7.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
