[PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Chris Wilson]

We first wait for a request to be submitted to hw and assigned a seqno,
before we can wait for the hw to signal completion (otherwise we don't
know the hw id we need to wait upon). Whilst waiting for the request to
be submitted, we may exceed the user's timeout and need to propagate the
error back.

Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
---
 drivers/gpu/drm/i915/i915_gem_request.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..69aff559cf8e 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
 		if (timeout < 0)
 			goto complete;
 
+		if (!timeout)
+			return -ETIME;
+
 		GEM_BUG_ON(!i915_sw_fence_done(&req->execute));
 	}
 	GEM_BUG_ON(!i915_sw_fence_done(&req->submit));
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Tvrtko Ursulin]

On 08/02/2017 16:54, Chris Wilson wrote:
> We first wait for a request to be submitted to hw and assigned a seqno,
> before we can wait for the hw to signal completion (otherwise we don't
> know the hw id we need to wait upon). Whilst waiting for the request to
> be submitted, we may exceed the user's timeout and need to propagate the
> error back.
>
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 72b7f7d9461d..69aff559cf8e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  		if (timeout < 0)
>  			goto complete;
>
> +		if (!timeout)
> +			return -ETIME;
> +
>  		GEM_BUG_ON(!i915_sw_fence_done(&req->execute));
>  	}
>  	GEM_BUG_ON(!i915_sw_fence_done(&req->submit));
>

Perhaps "else if" would be more typical, but still OK for a fix.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Chris Wilson]

On Wed, Feb 08, 2017 at 05:28:39PM +0000, Tvrtko Ursulin wrote:
> 
> On 08/02/2017 16:54, Chris Wilson wrote:
> >We first wait for a request to be submitted to hw and assigned a seqno,
> >before we can wait for the hw to signal completion (otherwise we don't
> >know the hw id we need to wait upon). Whilst waiting for the request to
> >be submitted, we may exceed the user's timeout and need to propagate the
> >error back.
> >
> >Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
> >---
> > drivers/gpu/drm/i915/i915_gem_request.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> >index 72b7f7d9461d..69aff559cf8e 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_request.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_request.c
> >@@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
> > 		if (timeout < 0)
> > 			goto complete;
> >
> >+		if (!timeout)
> >+			return -ETIME;
> >+
> > 		GEM_BUG_ON(!i915_sw_fence_done(&req->execute));
> > 	}
> > 	GEM_BUG_ON(!i915_sw_fence_done(&req->submit));
> >
> 
> Perhaps "else if" would be more typical, but still OK for a fix.

What I did later on in the series, was

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..c33f537f02b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1026,7 +1026,11 @@ __i915_request_wait_for_execute(struct drm_i915_gem_request *request,
                }
 
                timeout = io_schedule_timeout(timeout);
-       } while (timeout);
+               if (!timeout) {
+                       timeout = -ETIME;
+                       break;
+               }
+       } while (1);
        finish_wait(&request->execute.wait, &wait);
 
        if (flags & I915_WAIT_LOCKED)

That seemed like a more consistent pattern to use. Care to consider that as a v2?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Tvrtko Ursulin]

On 08/02/2017 17:54, Chris Wilson wrote:
> On Wed, Feb 08, 2017 at 05:28:39PM +0000, Tvrtko Ursulin wrote:
>>
>> On 08/02/2017 16:54, Chris Wilson wrote:
>>> We first wait for a request to be submitted to hw and assigned a seqno,
>>> before we can wait for the hw to signal completion (otherwise we don't
>>> know the hw id we need to wait upon). Whilst waiting for the request to
>>> be submitted, we may exceed the user's timeout and need to propagate the
>>> error back.
>>>
>>> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
>>> ---
>>> drivers/gpu/drm/i915/i915_gem_request.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
>>> index 72b7f7d9461d..69aff559cf8e 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_request.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
>>> @@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>>> 		if (timeout < 0)
>>> 			goto complete;
>>>
>>> +		if (!timeout)
>>> +			return -ETIME;
>>> +
>>> 		GEM_BUG_ON(!i915_sw_fence_done(&req->execute));
>>> 	}
>>> 	GEM_BUG_ON(!i915_sw_fence_done(&req->submit));
>>>
>>
>> Perhaps "else if" would be more typical, but still OK for a fix.
>
> What I did later on in the series, was
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 72b7f7d9461d..c33f537f02b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1026,7 +1026,11 @@ __i915_request_wait_for_execute(struct drm_i915_gem_request *request,
>                 }
>
>                 timeout = io_schedule_timeout(timeout);
> -       } while (timeout);
> +               if (!timeout) {
> +                       timeout = -ETIME;
> +                       break;
> +               }
> +       } while (1);
>         finish_wait(&request->execute.wait, &wait);
>
>         if (flags & I915_WAIT_LOCKED)
>
> That seemed like a more consistent pattern to use. Care to consider that as a v2?

Don't like to see "while (1)", how about:

	return timeout == 0 ? -ETIME : timeout;

For the exit of __i915_request_wait_for_execute?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Chris Wilson]

We first wait for a request to be submitted to hw and assigned a seqno,
before we can wait for the hw to signal completion (otherwise we don't
know the hw id we need to wait upon). Whilst waiting for the request to
be submitted, we may exceed the user's timeout and need to propagate the
error back.

v2: Make ETIME into an error from wait_for_execute for consistent exit
handling.

Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
Testcase: igt/gem_wait/basic-await
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
---
 drivers/gpu/drm/i915/i915_gem_request.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..f31deeb72703 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1025,8 +1025,13 @@ __i915_request_wait_for_execute(struct drm_i915_gem_request *request,
 			break;
 		}
 
+		if (!timeout) {
+			timeout = -ETIME;
+			break;
+		}
+
 		timeout = io_schedule_timeout(timeout);
-	} while (timeout);
+	} while (1);
 	finish_wait(&request->execute.wait, &wait);
 
 	if (flags & I915_WAIT_LOCKED)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Chris Wilson]

On Wed, Feb 08, 2017 at 06:08:43PM +0000, Tvrtko Ursulin wrote:
> 
> On 08/02/2017 17:54, Chris Wilson wrote:
> >On Wed, Feb 08, 2017 at 05:28:39PM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 08/02/2017 16:54, Chris Wilson wrote:
> >>>We first wait for a request to be submitted to hw and assigned a seqno,
> >>>before we can wait for the hw to signal completion (otherwise we don't
> >>>know the hw id we need to wait upon). Whilst waiting for the request to
> >>>be submitted, we may exceed the user's timeout and need to propagate the
> >>>error back.
> >>>
> >>>Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> >>>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >>>Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
> >>>---
> >>>drivers/gpu/drm/i915/i915_gem_request.c | 3 +++
> >>>1 file changed, 3 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> >>>index 72b7f7d9461d..69aff559cf8e 100644
> >>>--- a/drivers/gpu/drm/i915/i915_gem_request.c
> >>>+++ b/drivers/gpu/drm/i915/i915_gem_request.c
> >>>@@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
> >>>		if (timeout < 0)
> >>>			goto complete;
> >>>
> >>>+		if (!timeout)
> >>>+			return -ETIME;
> >>>+
> >>>		GEM_BUG_ON(!i915_sw_fence_done(&req->execute));
> >>>	}
> >>>	GEM_BUG_ON(!i915_sw_fence_done(&req->submit));
> >>>
> >>
> >>Perhaps "else if" would be more typical, but still OK for a fix.
> >
> >What I did later on in the series, was
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> >index 72b7f7d9461d..c33f537f02b3 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_request.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_request.c
> >@@ -1026,7 +1026,11 @@ __i915_request_wait_for_execute(struct drm_i915_gem_request *request,
> >                }
> >
> >                timeout = io_schedule_timeout(timeout);
> >-       } while (timeout);
> >+               if (!timeout) {
> >+                       timeout = -ETIME;
> >+                       break;
> >+               }
> >+       } while (1);
> >        finish_wait(&request->execute.wait, &wait);
> >
> >        if (flags & I915_WAIT_LOCKED)
> >
> >That seemed like a more consistent pattern to use. Care to consider that as a v2?
> 
> Don't like to see "while (1)", how about:

for (;;) ? I prefer do while (1) personally.

Actually, the patch should be doing the if (!timeout) before the schedule
so that we check the fence condition before declaring a TIMEOUT.

Have a look at the v2 patch and see what you think.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Check for timeout completion when waiting for the rq to submitted (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Check for timeout completion when waiting for the rq to submitted (rev3)
URL   : https://patchwork.freedesktop.org/series/19323/
State : success

== Summary ==

Series 19323v3 drm/i915: Check for timeout completion when waiting for the rq to submitted
https://patchwork.freedesktop.org/api/1.0/series/19323/revisions/3/mbox/


fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:230  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:252  pass:199  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:252  pass:229  dwarn:0   dfail:0   fail:2   skip:21 
fi-skl-6260u     total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:252  pass:232  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:252  pass:227  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:252  pass:220  dwarn:0   dfail:0   fail:0   skip:32 
fi-bdw-5557u failed to collect. IGT log at Patchwork_3745/fi-bdw-5557u/igt.log

29b6014fdfab0a48deb05fc5cc5b33ba00042118 drm-tip: 2017y-02m-08d-21h-01m-50s UTC integration manifest
5a8e4e7 drm/i915: Check for timeout completion when waiting for the rq to submitted

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3745/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Tvrtko Ursulin]

On 08/02/2017 18:12, Chris Wilson wrote:
> We first wait for a request to be submitted to hw and assigned a seqno,
> before we can wait for the hw to signal completion (otherwise we don't
> know the hw id we need to wait upon). Whilst waiting for the request to
> be submitted, we may exceed the user's timeout and need to propagate the
> error back.
>
> v2: Make ETIME into an error from wait_for_execute for consistent exit
> handling.
>
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> Testcase: igt/gem_wait/basic-await
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 72b7f7d9461d..f31deeb72703 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1025,8 +1025,13 @@ __i915_request_wait_for_execute(struct drm_i915_gem_request *request,
>  			break;
>  		}
>
> +		if (!timeout) {
> +			timeout = -ETIME;
> +			break;
> +		}
> +
>  		timeout = io_schedule_timeout(timeout);
> -	} while (timeout);
> +	} while (1);
>  	finish_wait(&request->execute.wait, &wait);
>
>  	if (flags & I915_WAIT_LOCKED)
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Chris Wilson]

On Thu, Feb 09, 2017 at 09:10:12AM +0000, Tvrtko Ursulin wrote:
> 
> On 08/02/2017 18:12, Chris Wilson wrote:
> >We first wait for a request to be submitted to hw and assigned a seqno,
> >before we can wait for the hw to signal completion (otherwise we don't
> >know the hw id we need to wait upon). Whilst waiting for the request to
> >be submitted, we may exceed the user's timeout and need to propagate the
> >error back.
> >
> >v2: Make ETIME into an error from wait_for_execute for consistent exit
> >handling.
> >
> >Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> >Testcase: igt/gem_wait/basic-await
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
> >---
> > drivers/gpu/drm/i915/i915_gem_request.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> >index 72b7f7d9461d..f31deeb72703 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_request.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_request.c
> >@@ -1025,8 +1025,13 @@ __i915_request_wait_for_execute(struct drm_i915_gem_request *request,
> > 			break;
> > 		}
> >
> >+		if (!timeout) {
> >+			timeout = -ETIME;
> >+			break;
> >+		}
> >+
> > 		timeout = io_schedule_timeout(timeout);
> >-	} while (timeout);
> >+	} while (1);
> > 	finish_wait(&request->execute.wait, &wait);
> >
> > 	if (flags & I915_WAIT_LOCKED)
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for catching it. Hopefully the window it exists in 4.10 is short
enough that we aren't deluged with bug reports...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Joonas Lahtinen]

On ke, 2017-02-08 at 16:54 +0000, Chris Wilson wrote:
> We first wait for a request to be submitted to hw and assigned a seqno,
> before we can wait for the hw to signal completion (otherwise we don't
> know the hw id we need to wait upon). Whilst waiting for the request to
> be submitted, we may exceed the user's timeout and need to propagate the
> error back.
> 
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  		if (timeout < 0)
>  			goto complete;
>  
> +		if (!timeout)
> +			return -ETIME;
> +

Misses the tracepoint, if that's deliberate;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check for timeout completion when waiting for the rq to submitted

[Chris Wilson]

On Wed, Feb 15, 2017 at 02:54:40PM +0200, Joonas Lahtinen wrote:
> On ke, 2017-02-08 at 16:54 +0000, Chris Wilson wrote:
> > We first wait for a request to be submitted to hw and assigned a seqno,
> > before we can wait for the hw to signal completion (otherwise we don't
> > know the hw id we need to wait upon). Whilst waiting for the request to
> > be submitted, we may exceed the user's timeout and need to propagate the
> > error back.
> > 
> > Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+
> 
> <SNIP>
> 
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> > @@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
> >  		if (timeout < 0)
> >  			goto complete;
> >  
> > +		if (!timeout)
> > +			return -ETIME;
> > +
> 
> Misses the tracepoint, if that's deliberate;

Hmm, this should be ret = -ETIME; goto complete; It is at the end of the
series, or at least at the end in my current tree.

Ah, yes, v2 of this patch is already in the tree. Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx