[Intel-gfx] [PATCH 2/2] drm/i915: Tag GEM_TRACE with device name

Lucas De Marchi lucas.demarchi at intel.com
Thu Dec 12 21:14:23 UTC 2019


On Thu, Dec 12, 2019 at 08:30:20AM +0000, Chris Wilson wrote:
>Quoting Venkata Sandeep Dhanalakota (2019-12-12 07:35:22)
>> Adding device name to trace makes debugging easier,
>> when dealing with multiple gpus.
>
>I'm not going to type that by hand, so let's try a little bit of
>ingenuity.
>
>---
> drivers/gpu/drm/i915/gt/intel_engine.h        |   8 ++
> drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   6 +-
> drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   6 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c           | 106 ++++++++----------
> drivers/gpu/drm/i915/gt/intel_reset.c         |   2 +-
> .../gpu/drm/i915/gt/intel_ring_submission.c   |  11 +-
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c |   8 +-
> 7 files changed, 72 insertions(+), 75 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
>index c294ea80605e..d9c7121fa09e 100644
>--- a/drivers/gpu/drm/i915/gt/intel_engine.h
>+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
>@@ -29,6 +29,14 @@ struct intel_gt;
> #define CACHELINE_BYTES 64
> #define CACHELINE_DWORDS (CACHELINE_BYTES / sizeof(u32))
>
>+#define ENGINE_TRACE(e__, fmt, ...) do {				\
>+	typecheck(struct intel_engine_cs, *(e__));			\
>+	GEM_TRACE("%s %s: " fmt, 					\
>+		  dev_name((e__)->i915->drm.dev),			\
>+		  (e__)->name,						\
>+		  ##__VA_ARGS__);					\
>+} while (0)

yeah, looks better. For this version:

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

>+
> /*
>  * The register defines to be used with the following macros need to accept a
>  * base param, e.g:
>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>index 49473c25916c..3d1d48bf90cf 100644
>--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>@@ -912,7 +912,7 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine)
> 	if (INTEL_GEN(engine->i915) < 3)
> 		return -ENODEV;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	intel_uncore_write_fw(uncore, mode, _MASKED_BIT_ENABLE(STOP_RING));
>
>@@ -921,7 +921,7 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine)
> 					 mode, MODE_IDLE, MODE_IDLE,
> 					 1000, stop_timeout(engine),
> 					 NULL)) {
>-		GEM_TRACE("%s: timed out on STOP_RING -> IDLE\n", engine->name);
>+		ENGINE_TRACE(engine, "timed out on STOP_RING -> IDLE\n");
> 		err = -ETIMEDOUT;
> 	}
>
>@@ -933,7 +933,7 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine)
>
> void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
> {
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
> }
>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>index 889eb37e386a..bcbda8e52d41 100644
>--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>@@ -21,7 +21,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
> 		container_of(wf, typeof(*engine), wakeref);
> 	void *map;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	intel_gt_pm_get(engine->gt);
>
>@@ -80,7 +80,7 @@ __queue_and_release_pm(struct i915_request *rq,
> {
> 	struct intel_gt_timelines *timelines = &engine->gt->timelines;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	/*
> 	 * We have to serialise all potential retirement paths with our
>@@ -204,7 +204,7 @@ static int __engine_park(struct intel_wakeref *wf)
> 	if (!switch_to_kernel_context(engine))
> 		return -EBUSY;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	call_idle_barriers(engine); /* cleanup after wedging */
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>index 929f6bae4eba..e402b3b28150 100644
>--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>@@ -1069,8 +1069,8 @@ static void reset_active(struct i915_request *rq,
> 	 * remain correctly ordered. And we defer to __i915_request_submit()
> 	 * so that all asynchronous waits are correctly handled.
> 	 */
>-	GEM_TRACE("%s(%s): { rq=%llx:%lld }\n",
>-		  __func__, engine->name, rq->fence.context, rq->fence.seqno);
>+	ENGINE_TRACE(engine, "{ rq=%llx:%lld }\n",
>+		     rq->fence.context, rq->fence.seqno);
>
> 	/* On resubmission of the active request, payload will be scrubbed */
> 	if (i915_request_completed(rq))
>@@ -1274,15 +1274,14 @@ trace_ports(const struct intel_engine_execlists *execlists,
> 	if (!ports[0])
> 		return;
>
>-	GEM_TRACE("%s: %s { %llx:%lld%s, %llx:%lld }\n",
>-		  engine->name, msg,
>-		  ports[0]->fence.context,
>-		  ports[0]->fence.seqno,
>-		  i915_request_completed(ports[0]) ? "!" :
>-		  i915_request_started(ports[0]) ? "*" :
>-		  "",
>-		  ports[1] ? ports[1]->fence.context : 0,
>-		  ports[1] ? ports[1]->fence.seqno : 0);
>+	ENGINE_TRACE(engine, "%s { %llx:%lld%s, %llx:%lld }\n", msg,
>+		     ports[0]->fence.context,
>+		     ports[0]->fence.seqno,
>+		     i915_request_completed(ports[0]) ? "!" :
>+		     i915_request_started(ports[0]) ? "*" :
>+		     "",
>+		     ports[1] ? ports[1]->fence.context : 0,
>+		     ports[1] ? ports[1]->fence.seqno : 0);
> }
>
> static __maybe_unused bool
>@@ -1700,12 +1699,11 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> 	last = last_active(execlists);
> 	if (last) {
> 		if (need_preempt(engine, last, rb)) {
>-			GEM_TRACE("%s: preempting last=%llx:%lld, prio=%d, hint=%d\n",
>-				  engine->name,
>-				  last->fence.context,
>-				  last->fence.seqno,
>-				  last->sched.attr.priority,
>-				  execlists->queue_priority_hint);
>+			ENGINE_TRACE(engine, "preempting last=%llx:%lld, prio=%d, hint=%d\n",
>+				     last->fence.context,
>+				     last->fence.seqno,
>+				     last->sched.attr.priority,
>+				     execlists->queue_priority_hint);
> 			record_preemption(execlists);
>
> 			/*
>@@ -1735,12 +1733,12 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> 			last = NULL;
> 		} else if (need_timeslice(engine, last) &&
> 			   timer_expired(&engine->execlists.timer)) {
>-			GEM_TRACE("%s: expired last=%llx:%lld, prio=%d, hint=%d\n",
>-				  engine->name,
>-				  last->fence.context,
>-				  last->fence.seqno,
>-				  last->sched.attr.priority,
>-				  execlists->queue_priority_hint);
>+			ENGINE_TRACE(engine,
>+				     "expired last=%llx:%lld, prio=%d, hint=%d\n",
>+				     last->fence.context,
>+				     last->fence.seqno,
>+				     last->sched.attr.priority,
>+				     execlists->queue_priority_hint);
>
> 			ring_set_paused(engine, 1);
> 			defer_active(engine);
>@@ -1817,14 +1815,13 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> 				return; /* leave this for another */
> 			}
>
>-			GEM_TRACE("%s: virtual rq=%llx:%lld%s, new engine? %s\n",
>-				  engine->name,
>-				  rq->fence.context,
>-				  rq->fence.seqno,
>-				  i915_request_completed(rq) ? "!" :
>-				  i915_request_started(rq) ? "*" :
>-				  "",
>-				  yesno(engine != ve->siblings[0]));
>+			ENGINE_TRACE(engine, "virtual rq=%llx:%lld%s, new engine? %s\n",
>+				     rq->fence.context,
>+				     rq->fence.seqno,
>+				     i915_request_completed(rq) ? "!" :
>+				     i915_request_started(rq) ? "*" :
>+				     "",
>+				     yesno(engine != ve->siblings[0]));
>
> 			ve->request = NULL;
> 			ve->base.execlists.queue_priority_hint = INT_MIN;
>@@ -1980,9 +1977,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> 	 * interrupt for secondary ports).
> 	 */
> 	execlists->queue_priority_hint = queue_prio(execlists);
>-	GEM_TRACE("%s: queue_priority_hint:%d, submit:%s\n",
>-		  engine->name, execlists->queue_priority_hint,
>-		  yesno(submit));
>
> 	if (submit) {
> 		*port = execlists_schedule_in(last, port - execlists->pending);
>@@ -2131,7 +2125,7 @@ static void process_csb(struct intel_engine_cs *engine)
> 	 */
> 	head = execlists->csb_head;
> 	tail = READ_ONCE(*execlists->csb_write);
>-	GEM_TRACE("%s cs-irq head=%d, tail=%d\n", engine->name, head, tail);
>+	ENGINE_TRACE(engine, "cs-irq head=%d, tail=%d\n", head, tail);
> 	if (unlikely(head == tail))
> 		return;
>
>@@ -2169,9 +2163,8 @@ static void process_csb(struct intel_engine_cs *engine)
> 		 * status notifier.
> 		 */
>
>-		GEM_TRACE("%s csb[%d]: status=0x%08x:0x%08x\n",
>-			  engine->name, head,
>-			  buf[2 * head + 0], buf[2 * head + 1]);
>+		ENGINE_TRACE(engine, "csb[%d]: status=0x%08x:0x%08x\n",
>+			     head, buf[2 * head + 0], buf[2 * head + 1]);
>
> 		if (INTEL_GEN(engine->i915) >= 12)
> 			promote = gen12_csb_parse(execlists, buf + 2 * head);
>@@ -2262,10 +2255,9 @@ static noinline void preempt_reset(struct intel_engine_cs *engine)
> 	/* Mark this tasklet as disabled to avoid waiting for it to complete */
> 	tasklet_disable_nosync(&engine->execlists.tasklet);
>
>-	GEM_TRACE("%s: preempt timeout %lu+%ums\n",
>-		  engine->name,
>-		  READ_ONCE(engine->props.preempt_timeout_ms),
>-		  jiffies_to_msecs(jiffies - engine->execlists.preempt.expires));
>+	ENGINE_TRACE(engine, "preempt timeout %lu+%ums\n",
>+		     READ_ONCE(engine->props.preempt_timeout_ms),
>+		     jiffies_to_msecs(jiffies - engine->execlists.preempt.expires));
> 	intel_engine_reset(engine, "preemption time out");
>
> 	tasklet_enable(&engine->execlists.tasklet);
>@@ -2971,8 +2963,8 @@ static void execlists_reset_prepare(struct intel_engine_cs *engine)
> 	struct intel_engine_execlists * const execlists = &engine->execlists;
> 	unsigned long flags;
>
>-	GEM_TRACE("%s: depth<-%d\n", engine->name,
>-		  atomic_read(&execlists->tasklet.count));
>+	ENGINE_TRACE(engine, "depth<-%d\n",
>+		     atomic_read(&execlists->tasklet.count));
>
> 	/*
> 	 * Prevent request submission to the hardware until we have
>@@ -3134,8 +3126,8 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
> 	restore_default_state(ce, engine);
>
> out_replay:
>-	GEM_TRACE("%s replay {head:%04x, tail:%04x}\n",
>-		  engine->name, ce->ring->head, ce->ring->tail);
>+	ENGINE_TRACE(engine, "replay {head:%04x, tail:%04x}\n",
>+		     ce->ring->head, ce->ring->tail);
> 	intel_ring_update_space(ce->ring);
> 	__execlists_reset_reg_state(ce, engine);
> 	__execlists_update_reg_state(ce, engine);
>@@ -3151,7 +3143,7 @@ static void execlists_reset(struct intel_engine_cs *engine, bool stalled)
> {
> 	unsigned long flags;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	spin_lock_irqsave(&engine->active.lock, flags);
>
>@@ -3172,7 +3164,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
> 	struct rb_node *rb;
> 	unsigned long flags;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	/*
> 	 * Before we call engine->cancel_requests(), we should have exclusive
>@@ -3259,8 +3251,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
> 	if (__tasklet_enable(&execlists->tasklet))
> 		/* And kick in case we missed a new request submission. */
> 		tasklet_hi_schedule(&execlists->tasklet);
>-	GEM_TRACE("%s: depth->%d\n", engine->name,
>-		  atomic_read(&execlists->tasklet.count));
>+	ENGINE_TRACE(engine, "depth->%d\n",
>+		     atomic_read(&execlists->tasklet.count));
> }
>
> static int gen8_emit_bb_start(struct i915_request *rq,
>@@ -4309,10 +4301,9 @@ static intel_engine_mask_t virtual_submission_mask(struct virtual_engine *ve)
> 		mask = ve->siblings[0]->mask;
> 	}
>
>-	GEM_TRACE("%s: rq=%llx:%lld, mask=%x, prio=%d\n",
>-		  ve->base.name,
>-		  rq->fence.context, rq->fence.seqno,
>-		  mask, ve->base.execlists.queue_priority_hint);
>+	ENGINE_TRACE(&ve->base, "rq=%llx:%lld, mask=%x, prio=%d\n",
>+		     rq->fence.context, rq->fence.seqno,
>+		     mask, ve->base.execlists.queue_priority_hint);
>
> 	return mask;
> }
>@@ -4403,10 +4394,9 @@ static void virtual_submit_request(struct i915_request *rq)
> 	struct i915_request *old;
> 	unsigned long flags;
>
>-	GEM_TRACE("%s: rq=%llx:%lld\n",
>-		  ve->base.name,
>-		  rq->fence.context,
>-		  rq->fence.seqno);
>+	ENGINE_TRACE(&ve->base, "rq=%llx:%lld\n",
>+		     rq->fence.context,
>+		     rq->fence.seqno);
>
> 	GEM_BUG_ON(ve->base.submit_request != virtual_submit_request);
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
>index 8408cb84e52c..f5b2e7c7e6c8 100644
>--- a/drivers/gpu/drm/i915/gt/intel_reset.c
>+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>@@ -1089,7 +1089,7 @@ int intel_engine_reset(struct intel_engine_cs *engine, const char *msg)
> 	bool uses_guc = intel_engine_in_guc_submission_mode(engine);
> 	int ret;
>
>-	GEM_TRACE("%s flags=%lx\n", engine->name, gt->reset.flags);
>+	ENGINE_TRACE(engine, "flags=%lx\n", gt->reset.flags);
> 	GEM_BUG_ON(!test_bit(I915_RESET_ENGINE + engine->id, &gt->reset.flags));
>
> 	if (!intel_engine_pm_get_if_awake(engine))
>diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
>index 5c22ca6f998a..32334476cd77 100644
>--- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
>+++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
>@@ -632,8 +632,8 @@ static int xcs_resume(struct intel_engine_cs *engine)
> 	struct intel_ring *ring = engine->legacy.ring;
> 	int ret = 0;
>
>-	GEM_TRACE("%s: ring:{HEAD:%04x, TAIL:%04x}\n",
>-		  engine->name, ring->head, ring->tail);
>+	ENGINE_TRACE(engine, "ring:{HEAD:%04x, TAIL:%04x}\n",
>+		     ring->head, ring->tail);
>
> 	intel_uncore_forcewake_get(engine->uncore, FORCEWAKE_ALL);
>
>@@ -746,10 +746,10 @@ static void reset_prepare(struct intel_engine_cs *engine)
> 	 *
> 	 * FIXME: Wa for more modern gens needs to be validated
> 	 */
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	if (intel_engine_stop_cs(engine))
>-		GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
>+		ENGINE_TRACE(engine, "timed out on STOP_RING\n");
>
> 	intel_uncore_write_fw(uncore,
> 			      RING_HEAD(base),
>@@ -765,8 +765,7 @@ static void reset_prepare(struct intel_engine_cs *engine)
>
> 	/* Check acts as a post */
> 	if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
>-		GEM_TRACE("%s: ring head [%x] not parked\n",
>-			  engine->name,
>+		ENGINE_TRACE(engine, "ring head [%x] not parked\n",
> 			  intel_uncore_read_fw(uncore, RING_HEAD(base)));
> }
>
>diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>index 172220e83079..af04ed6e48d9 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>@@ -375,7 +375,7 @@ static void guc_reset_prepare(struct intel_engine_cs *engine)
> {
> 	struct intel_engine_execlists * const execlists = &engine->execlists;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	/*
> 	 * Prevent request submission to the hardware until we have
>@@ -434,7 +434,7 @@ static void guc_cancel_requests(struct intel_engine_cs *engine)
> 	struct rb_node *rb;
> 	unsigned long flags;
>
>-	GEM_TRACE("%s\n", engine->name);
>+	ENGINE_TRACE(engine, "\n");
>
> 	/*
> 	 * Before we call engine->cancel_requests(), we should have exclusive
>@@ -495,8 +495,8 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
> 		/* And kick in case we missed a new request submission. */
> 		tasklet_hi_schedule(&execlists->tasklet);
>
>-	GEM_TRACE("%s: depth->%d\n", engine->name,
>-		  atomic_read(&execlists->tasklet.count));
>+	ENGINE_TRACE(engine, "depth->%d\n",
>+		     atomic_read(&execlists->tasklet.count));
> }
>
> /*
>-- 
>2.24.0
>
>---------------------------------------------------------------------
>Intel Corporation (UK) Limited
>Registered No. 1134945 (England)
>Registered Office: Pipers Way, Swindon SN3 1RJ
>VAT No: 860 2173 47
>
>This e-mail and any attachments may contain confidential material for
>the sole use of the intended recipient(s). Any review or distribution
>by others is strictly prohibited. If you are not the intended
>recipient, please contact the sender and delete all copies.
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx


More information about the Intel-gfx mailing list