[PATCH] drm/i915: Distinguish last emitted request from last submitted request

[PATCH] drm/i915: Distinguish last emitted request from last submitted request

[Chris Wilson]

In order not to trigger hangcheck on a idle-but-waiting engine, we need
to distinguish between the pending request queue and the actual
execution queue. This is done later in "drm/i915: Enable multiple
timelines" but for now we need a temporary fix to prevent blaming the
wrong engine for a GPU hang.

Fixes: 0a046a0e93d2 ("drm/i915: Nonblocking request submission")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 5 +++--
 drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 40978bc12ceb..8832f8ec1583 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -328,6 +328,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 
 	switch (state) {
 	case FENCE_COMPLETE:
+		request->engine->last_submitted_seqno = request->fence.seqno;
 		request->engine->submit_request(request);
 		break;
 
@@ -641,8 +642,8 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
 					     &request->submitq);
 
 	request->emitted_jiffies = jiffies;
-	request->previous_seqno = engine->last_submitted_seqno;
-	engine->last_submitted_seqno = request->fence.seqno;
+	request->previous_seqno = engine->last_pending_seqno;
+	engine->last_pending_seqno = request->fence.seqno;
 	i915_gem_active_set(&engine->last_request, request);
 	list_add_tail(&request->link, &engine->request_list);
 	list_add_tail(&request->ring_link, &ring->request_list);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 498931f0b1f1..34954ca03a4a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -355,6 +355,7 @@ struct intel_engine_cs {
 	 * inspecting request list.
 	 */
 	u32 last_submitted_seqno;
+	u32 last_pending_seqno;
 
 	/* An RCU guarded pointer to the last request. No reference is
 	 * held to the request, users must carefully acquire a reference to
-- 
2.9.3

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

✓ Fi.CI.BAT: success for drm/i915: Distinguish last emitted request from last submitted request

[Patchwork]

== Series Details ==

Series: drm/i915: Distinguish last emitted request from last submitted request
URL   : https://patchwork.freedesktop.org/series/13347/
State : success

== Summary ==

Series 13347v1 drm/i915: Distinguish last emitted request from last submitted request
https://patchwork.freedesktop.org/api/1.0/series/13347/revisions/1/mbox/


fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:244  pass:214  dwarn:0   dfail:0   fail:0   skip:30 
fi-hsw-4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-kbl-7200u     total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2632/

2dff18acaa95a26b882a5f9910d7ded514f18415 drm-intel-nightly: 2016y-10m-05d-13h-58m-08s UTC integration manifest
987735b drm/i915: Distinguish last emitted request from last submitted request

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

Re: [PATCH] drm/i915: Distinguish last emitted request from last submitted request

[Joonas Lahtinen]

On ke, 2016-10-05 at 21:05 +0100, Chris Wilson wrote:
> In order not to trigger hangcheck on a idle-but-waiting engine, we need
> to distinguish between the pending request queue and the actual
> execution queue. This is done later in "drm/i915: Enable multiple
> timelines" but for now we need a temporary fix to prevent blaming the
> wrong engine for a GPU hang.
> 
> Fixes: 0a046a0e93d2 ("drm/i915: Nonblocking request submission")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

Kerneldoc to tell the difference would be useful in the struct.

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: Distinguish last emitted request from last submitted request

[Chris Wilson]

On Thu, Oct 06, 2016 at 09:57:29AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-10-05 at 21:05 +0100, Chris Wilson wrote:
> > In order not to trigger hangcheck on a idle-but-waiting engine, we need
> > to distinguish between the pending request queue and the actual
> > execution queue. This is done later in "drm/i915: Enable multiple
> > timelines" but for now we need a temporary fix to prevent blaming the
> > wrong engine for a GPU hang.
> > 
> > Fixes: 0a046a0e93d2 ("drm/i915: Nonblocking request submission")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Kerneldoc to tell the difference would be useful in the struct.

It purely a temporary hack. (And hack it is.)
-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: Distinguish last emitted request from last submitted request

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In order not to trigger hangcheck on a idle-but-waiting engine, we need
> to distinguish between the pending request queue and the actual
> execution queue. This is done later in "drm/i915: Enable multiple
> timelines" but for now we need a temporary fix to prevent blaming the
> wrong engine for a GPU hang.
>

You described this as a hack, but on top of current nightly,
this doesn't seem so hackish at all.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> Fixes: 0a046a0e93d2 ("drm/i915: Nonblocking request submission")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 5 +++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 40978bc12ceb..8832f8ec1583 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -328,6 +328,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  
>  	switch (state) {
>  	case FENCE_COMPLETE:
> +		request->engine->last_submitted_seqno = request->fence.seqno;
>  		request->engine->submit_request(request);
>  		break;
>  
> @@ -641,8 +642,8 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>  					     &request->submitq);
>  
>  	request->emitted_jiffies = jiffies;
> -	request->previous_seqno = engine->last_submitted_seqno;
> -	engine->last_submitted_seqno = request->fence.seqno;
> +	request->previous_seqno = engine->last_pending_seqno;
> +	engine->last_pending_seqno = request->fence.seqno;
>  	i915_gem_active_set(&engine->last_request, request);
>  	list_add_tail(&request->link, &engine->request_list);
>  	list_add_tail(&request->ring_link, &ring->request_list);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 498931f0b1f1..34954ca03a4a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -355,6 +355,7 @@ struct intel_engine_cs {
>  	 * inspecting request list.
>  	 */
>  	u32 last_submitted_seqno;
> +	u32 last_pending_seqno;
>  
>  	/* An RCU guarded pointer to the last request. No reference is
>  	 * held to the request, users must carefully acquire a reference to
> -- 
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx