[Intel-gfx] [PATCH] drm/i915/execlists: Log fence context & seqno throughout GEM_TRACE

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-04-06 10:54:57)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Include fence context and seqno in low level tracing so it is easier to
> follow flows of individual requests when things go bad.
> 
> Also added tracing on the reset side of things.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f6631ff11caf..a5cd698a2e3c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -468,10 +468,11 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
>                         desc = execlists_update_context(rq);
>                         GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc));
>  
> -                       GEM_TRACE("%s in[%d]:  ctx=%d.%d, seqno=%d (current %d), prio=%d\n",
> +                       GEM_TRACE("%s in[%d]:  ctx=%d.%d, seqno=%d (fence %llx:%d) (current %d), prio=%d\n",

We mix seqno=%d and global_seqno=%d. Please fix my inconsistency. :)

Using seqno is fine as we've standardised on calling the other (fence
x:y).

>                                   engine->name, n,
>                                   port[n].context_id, count,
>                                   rq->global_seqno,
> +                                 rq->fence.context, rq->fence.seqno,
>                                   intel_engine_get_seqno(engine),
>                                   rq_prio(rq));
>                 } else {
> @@ -742,6 +743,10 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>         while (num_ports-- && port_isset(port)) {
>                 struct i915_request *rq = port_request(port);
>  
> +               GEM_TRACE("%s:port%lu fence %llx:%d (current %d)\n",
> +                         rq->engine->name, port - execlists->port,
> +                         rq->fence.context, rq->fence.seqno, rq->global_seqno);

Please always keep current == intel_engine_get_seqno(). So _also_ add
seqno=%d.

Hmm, do want to use x=y, or "x: y", or "x y". Again, we might as well
stick to our internal rules for notation. Actually I'd love to adopt a
yaml approach { x: value; y: value }. (But until we have yaml output
throughout, the inconsistency might be annoying ;) Basically I'm
thinking from a scripting aspect, the very least being grep or ^F.

Overall, a very big "yes please" to the improvements.
-Chris