[i-g-t] intel-gpu-overlay: Update for renamed tracepoints

Commit Message

Tvrtko Ursulin March 5, 2018, 12:41 p.m. UTC

From: Tvrtko Ursulin <[e-mail]>

Request tracepoints have been renames so update the tool to be able to
find them.

Also support falling back to the old name for compatibility.

Signed-off-by: Tvrtko Ursulin <[e-mail]>
---
Compile tested only so also looking for someone to smoke test it if
possible. Otherwise I'll do it in due time once back to a test machine
with display.

I was also 50-50 whether it is worth supporting the old names, but it was
easy enough so I did it.
---
 overlay/gpu-perf.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Chris Wilson March 6, 2018, 10:58 a.m. UTC | #1

Quoting Tvrtko Ursulin (2018-03-05 12:41:58)
> From: Tvrtko Ursulin <[e-mail]>
> 
> Request tracepoints have been renames so update the tool to be able to
> find them.
> 
> Also support falling back to the old name for compatibility.
> 
> Signed-off-by: Tvrtko Ursulin <[e-mail]>
> ---
> Compile tested only so also looking for someone to smoke test it if
> possible. Otherwise I'll do it in due time once back to a test machine
> with display.

I use it on remote boxes with ssh -X.
 
> I was also 50-50 whether it is worth supporting the old names, but it was
> easy enough so I did it.

Yeah, but a slippery slope? If we just rule this is a dev tool and using
it on anything other than drm-tip requires you to fix it up, do we
create more or less hassle for ourselves?
-Chris

Tvrtko Ursulin March 6, 2018, 11:17 a.m. UTC | #2

On 06/03/2018 10:58, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-03-05 12:41:58)
>> From: Tvrtko Ursulin <[e-mail]>
>>
>> Request tracepoints have been renames so update the tool to be able to
>> find them.
>>
>> Also support falling back to the old name for compatibility.
>>
>> Signed-off-by: Tvrtko Ursulin <[e-mail]>
>> ---
>> Compile tested only so also looking for someone to smoke test it if
>> possible. Otherwise I'll do it in due time once back to a test machine
>> with display.
> 
> I use it on remote boxes with ssh -X.

Ha.. never crossed my mind.

>> I was also 50-50 whether it is worth supporting the old names, but it was
>> easy enough so I did it.
> 
> Yeah, but a slippery slope? If we just rule this is a dev tool and using
> it on anything other than drm-tip requires you to fix it up, do we
> create more or less hassle for ourselves?

You're probably right. Will drop the backwards compatibility.

Regards,

Tvrtko

Patch

diff --git a/overlay/gpu-perf.c b/overlay/gpu-perf.c
index 0abd937e3618..bc49175da386 100644
--- a/overlay/gpu-perf.c
+++ b/overlay/gpu-perf.c
@@ -72,7 +72,7 @@  enum {
 };
 
 struct tracepoint {
-	const char *name;
+	const char *name, *altname;
 	int event_id;
 
 	struct {
@@ -90,9 +90,12 @@  struct tracepoint {
 	int global_seqno_field;
 	int plane_field;
 } tracepoints[TP_NB] = {
-	[TP_GEM_REQUEST_ADD]         = { .name = "i915/i915_gem_request_add", },
-	[TP_GEM_REQUEST_WAIT_BEGIN]  = { .name = "i915/i915_gem_request_wait_begin", },
-	[TP_GEM_REQUEST_WAIT_END]    = { .name = "i915/i915_gem_request_wait_end", },
+	[TP_GEM_REQUEST_ADD]         = { .name = "i915/i915_request_add",
+					 .altname = "i915/i915_gem_request_add" },
+	[TP_GEM_REQUEST_WAIT_BEGIN]  = { .name = "i915/i915_request_wait_begin",
+					 .altname = "i915/i915_gem_request_wait_begin" },
+	[TP_GEM_REQUEST_WAIT_END]    = { .name = "i915/i915_request_wait_end",
+					 .altname = "i915/i915_gem_request_wait_end" },
 	[TP_FLIP_COMPLETE]           = { .name = "i915/flip_complete", },
 	[TP_GEM_RING_SYNC_TO]        = { .name = "i915/gem_ring_sync_to", },
 	[TP_GEM_RING_SWITCH_CONTEXT] = { .name = "i915/gem_ring_switch_context", },
@@ -142,6 +145,12 @@  tracepoint_id(int tp_id)
 	ctx.ctx.tp = tp;
 	ctx.ctx.fp = fopen(buf, "r");
 
+	if (ctx.ctx.fp == NULL) {
+		snprintf(buf, sizeof(buf), "%s/tracing/events/%s/format",
+			 debugfs_path, tp->altname);
+		ctx.ctx.fp = fopen(buf, "r");
+	}
+
 	if (ctx.ctx.fp == NULL)
 		return 0;