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

2016-10-06 Thread Mika Kuoppala
Chris Wilson  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 

> Fixes: 0a046a0e93d2 ("drm/i915: Nonblocking request submission")
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Mika Kuoppala 
> ---
>  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)
>>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(>last_request, request);
>   list_add_tail(>link, >request_list);
>   list_add_tail(>ring_link, >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


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

2016-10-06 Thread 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 
> > Cc: Joonas Lahtinen 
> > Cc: Mika Kuoppala 
> 
> 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: [Intel-gfx] [PATCH] drm/i915: Distinguish last emitted request from last submitted request

2016-10-06 Thread 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 
> Cc: Joonas Lahtinen 
> Cc: Mika Kuoppala 

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

Reviewed-by: Joonas Lahtinen 

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