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@intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com
.../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);@@ -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);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