| Message ID | 20180406095457.14802-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Tvrtko Ursulin (2018-04-06 10:54:57) > From: Tvrtko Ursulin <tvrtko.ursulin@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@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
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", 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); + GEM_BUG_ON(!execlists->active); intel_engine_context_out(rq->engine); @@ -817,7 +822,8 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) struct rb_node *rb; unsigned long flags; - GEM_TRACE("%s\n", engine->name); + GEM_TRACE("%s current %d\n", + engine->name, intel_engine_get_seqno(engine)); /* * Before we call engine->cancel_requests(), we should have exclusive @@ -1014,10 +1020,12 @@ static void execlists_submission_tasklet(unsigned long data) EXECLISTS_ACTIVE_USER)); rq = port_unpack(port, &count); - GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%d (current %d), prio=%d\n", + GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%d (fence %llx:%d) (current %d), prio=%d\n", engine->name, port->context_id, count, rq ? rq->global_seqno : 0, + rq ? rq->fence.context : 0, + rq ? rq->fence.seqno : 0, intel_engine_get_seqno(engine), rq ? rq_prio(rq) : 0); @@ -1745,8 +1753,9 @@ static void reset_common_ring(struct intel_engine_cs *engine, struct intel_context *ce; unsigned long flags; - GEM_TRACE("%s seqno=%x\n", - engine->name, request ? request->global_seqno : 0); + GEM_TRACE("%s seqno=%x current=%d\n", + engine->name, request ? request->global_seqno : 0, + intel_engine_get_seqno(engine)); /* See execlists_cancel_requests() for the irq/spinlock split. */ local_irq_save(flags);