On 20/07/2021 02:59, Matthew Brost wrote:
On Tue, Jul 13, 2021 at 10:06:17AM +0100, Tvrtko Ursulin wrote:
On 24/06/2021 08:04, Matthew Brost wrote:
Add trace points for request dependencies and GuC submit. Extended
existing request trace points to include submit fence value,, guc_id,
and ring tail value.
Cc: John Harrison <john.c.harrison@xxxxxxxxx>
Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 ++
drivers/gpu/drm/i915/i915_request.c | 3 ++
drivers/gpu/drm/i915/i915_trace.h | 39 ++++++++++++++++++-
3 files changed, 43 insertions(+), 2 deletions(-)
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 89b3c7e5d15b..c2327eebc09c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -422,6 +422,7 @@ static int guc_dequeue_one_context(struct intel_guc *guc)
guc->stalled_request = last;
return false;
}
+ trace_i915_request_guc_submit(last);
}
guc->stalled_request = NULL;
@@ -642,6 +643,8 @@ static int guc_bypass_tasklet_submit(struct intel_guc *guc,
ret = guc_add_request(guc, rq);
if (ret == -EBUSY)
guc->stalled_request = rq;
+ else
+ trace_i915_request_guc_submit(rq);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index d92c9f25c9f4..7f7aa096e873 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1344,6 +1344,9 @@ __i915_request_await_execution(struct i915_request *to,
return err;
}
+ trace_i915_request_dep_to(to);
+ trace_i915_request_dep_from(from);
Are those two guaranteed to be atomic ie. no other dep_to/dep_from can get
injected in the middle of them and if so what guarantees that?
These are not atomic but in practice I've never seen an out of order
tracepoints.
Actually we had an internal discussion going in November 2019 on these very
tracepoints which I think was left hanging in the air.
There I was suggesting you create a single tracepoint in the format of "from
-> to", so it's clear without any doubt what is going on.
Not sure if it worth adding a custom trace point fo rthis.
Custom as in not inherit from i915_request class you mean? It's not that
hard really.
I also suggested this should out outside the GuC patch since it is backend
agnostic.
I guess, but it really matter?
IMO following best practices and established conventions matters a lot.
Regards,
Tvrtko
