On Tue, Dec 10, 2013 at 10:35:13AM +0200, Mika Kuoppala wrote:
> Commit 094f9a54e355 ("drm/i915: Fix __wait_seqno to use true infinite
> timeouts") added support for __wait_seqno to detect missing interrupts and
> go around them by polling. As there is also timeout detection in
> __wait_seqno, the polling and timeout detection were done with the same
> timer.
>
> When there has been missed interrupts and polling is needed, the timer is
> set to trigger in (now + 1) jiffies in future, instead of the caller
> specified timeout.
>
> Now when io_schedule() returns, we calculate the jiffies left to timeout
> using the timer expiration value. As the current jiffies is now bound to be
> always equal or greater than the expiration value, the timeout_jiffies will
> become zero or negative and we return -ETIME to caller even tho the
> timeout was never reached.
>
> Fix this by decoupling timeout calculation from timer expiration.
>
> v2: Commit message with some sense in it (Chris Wilson)
>
> v3: add parenthesis on timeout_expire calculation
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 92149bc..71df9be 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1017,7 +1017,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> struct timespec before, now;
> DEFINE_WAIT(wait);
> - long timeout_jiffies;
> + unsigned long timeout_expire;
> int ret;
>
> WARN(dev_priv->pc8.irqs_disabled, "IRQs disabled\n");
> @@ -1025,7 +1025,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
> return 0;
>
> - timeout_jiffies = timeout ? timespec_to_jiffies_timeout(timeout) : 1;
> + timeout_expire = jiffies + (timeout ? timespec_to_jiffies_timeout(timeout) : 1);
I would jiggle this slightly to not read jiffies if timeout == NULL, i.e.
timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
(Hmm, we should be keeping these helpers separate from i915_drv.h.). As we
only set timeout_expire here if !timeout to keep the compiler quiet.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
