Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-25 Thread Chris Wilson 
Quoting Tvrtko Ursulin (2018-05-25 08:11:41)
> 
> On 24/05/2018 17:13, Lionel Landwerlin wrote:
> > On 24/05/18 17:07, Tvrtko Ursulin wrote:
> >>
> >> On 24/05/2018 16:53, Lionel Landwerlin wrote:
> >>> On 24/05/18 16:04, Tvrtko Ursulin wrote:
>  From: Tvrtko Ursulin 
> 
>  Instead of using the engine->id, use uabi_class:instance pairs in 
>  trace-
>  points including engine info.
> 
>  This will be more readable, more future proof and more stable for
>  userspace consumption.
> 
>  Signed-off-by: Tvrtko Ursulin 
>  Cc: Chris Wilson 
>  Cc: svetlana.kukan...@intel.com
> >>> Don't you want engine->uabi_id instead of engine->instance ?
> >>
> >> No, class:instance is the new engine identifier - why do you think we 
> >> would need legacy engine->uabi_id?
> > 
> > Maybe I forgot about your engine listing series...
> > I would expect the tracepoint to match the engines listed through that 
> > uapi.
> 
> Yeah I don't have engine->uabi_id exported in engine discovery. I could 
> add it, but given how we don't plan to extend it (the legacy engine 
> selection), I think it is not needed. If there will be popular demand 
> though can do it.

Also that we plan to conflate the I915_EXEC_RING selector so that
engine->uabi_id would no longer be a unique mapping, will add to the
confusion.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-25 Thread Tvrtko Ursulin 


On 24/05/2018 17:13, Lionel Landwerlin wrote:

On 24/05/18 17:07, Tvrtko Ursulin wrote:


On 24/05/2018 16:53, Lionel Landwerlin wrote:

On 24/05/18 16:04, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Instead of using the engine->id, use uabi_class:instance pairs in 
trace-

points including engine info.

This will be more readable, more future proof and more stable for
userspace consumption.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: svetlana.kukan...@intel.com

Don't you want engine->uabi_id instead of engine->instance ?


No, class:instance is the new engine identifier - why do you think we 
would need legacy engine->uabi_id?


Maybe I forgot about your engine listing series...
I would expect the tracepoint to match the engines listed through that 
uapi.


Yeah I don't have engine->uabi_id exported in engine discovery. I could 
add it, but given how we don't plan to extend it (the legacy engine 
selection), I think it is not needed. If there will be popular demand 
though can do it.


Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-24 Thread Lionel Landwerlin 

On 24/05/18 17:07, Tvrtko Ursulin wrote:


On 24/05/2018 16:53, Lionel Landwerlin wrote:

On 24/05/18 16:04, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Instead of using the engine->id, use uabi_class:instance pairs in 
trace-

points including engine info.

This will be more readable, more future proof and more stable for
userspace consumption.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: svetlana.kukan...@intel.com

Don't you want engine->uabi_id instead of engine->instance ?


No, class:instance is the new engine identifier - why do you think we 
would need legacy engine->uabi_id?


Maybe I forgot about your engine listing series...
I would expect the tracepoint to match the engines listed through that uapi.

-
Lionel



Regards,

Tvrtko



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-24 Thread Tvrtko Ursulin 


On 24/05/2018 16:53, Lionel Landwerlin wrote:

On 24/05/18 16:04, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Instead of using the engine->id, use uabi_class:instance pairs in trace-
points including engine info.

This will be more readable, more future proof and more stable for
userspace consumption.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: svetlana.kukan...@intel.com

Don't you want engine->uabi_id instead of engine->instance ?


No, class:instance is the new engine identifier - why do you think we 
would need legacy engine->uabi_id?


Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-24 Thread Lionel Landwerlin 

On 24/05/18 16:04, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Instead of using the engine->id, use uabi_class:instance pairs in trace-
points including engine info.

This will be more readable, more future proof and more stable for
userspace consumption.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: svetlana.kukan...@intel.com

Don't you want engine->uabi_id instead of engine->instance ?

---
  drivers/gpu/drm/i915/i915_trace.h | 107 ++
  1 file changed, 65 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index 5d4f78765083..3465cc1f9345 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -591,21 +591,26 @@ TRACE_EVENT(i915_gem_ring_sync_to,
  
  	TP_STRUCT__entry(

 __field(u32, dev)
-__field(u32, sync_from)
-__field(u32, sync_to)
+__field(u32, from_class)
+__field(u32, from_instance)
+__field(u32, to_class)
+__field(u32, to_instance)
 __field(u32, seqno)
 ),
  
  	TP_fast_assign(

   __entry->dev = from->i915->drm.primary->index;
-  __entry->sync_from = from->engine->id;
-  __entry->sync_to = to->engine->id;
+  __entry->from_class = from->engine->uabi_class;
+  __entry->from_instance = from->engine->instance;
+  __entry->to_class = to->engine->uabi_class;
+  __entry->to_instance = to->engine->instance;
   __entry->seqno = from->global_seqno;
   ),
  
-	TP_printk("dev=%u, sync-from=%u, sync-to=%u, seqno=%u",

+   TP_printk("dev=%u, sync-from=%u:%u, sync-to=%u:%u, seqno=%u",
  __entry->dev,
- __entry->sync_from, __entry->sync_to,
+ __entry->from_class, __entry->from_instance,
+ __entry->to_class, __entry->to_instance,
  __entry->seqno)
  );
  
@@ -616,7 +621,8 @@ TRACE_EVENT(i915_request_queue,

TP_STRUCT__entry(
 __field(u32, dev)
 __field(u32, hw_id)
-__field(u32, ring)
+__field(u32, class)
+__field(u32, instance)
 __field(u32, ctx)
 __field(u32, seqno)
 __field(u32, flags)
@@ -625,15 +631,17 @@ TRACE_EVENT(i915_request_queue,
TP_fast_assign(
   __entry->dev = rq->i915->drm.primary->index;
   __entry->hw_id = rq->gem_context->hw_id;
-  __entry->ring = rq->engine->id;
+  __entry->class = rq->engine->uabi_class;
+  __entry->instance = rq->engine->instance;
   __entry->ctx = rq->fence.context;
   __entry->seqno = rq->fence.seqno;
   __entry->flags = flags;
   ),
  
-	TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, flags=0x%x",

- __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx,
- __entry->seqno, __entry->flags)
+   TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, 
flags=0x%x",
+ __entry->dev, __entry->hw_id, __entry->class,
+ __entry->instance, __entry->ctx, __entry->seqno,
+ __entry->flags)
  );
  
  DECLARE_EVENT_CLASS(i915_request,

@@ -643,7 +651,8 @@ DECLARE_EVENT_CLASS(i915_request,
TP_STRUCT__entry(
 __field(u32, dev)
 __field(u32, hw_id)
-__field(u32, ring)
+__field(u32, class)
+__field(u32, instance)
 __field(u32, ctx)
 __field(u32, seqno)
 __field(u32, global)
@@ -652,15 +661,17 @@ DECLARE_EVENT_CLASS(i915_request,
TP_fast_assign(
   __entry->dev = rq->i915->drm.primary->index;
   __entry->hw_id = rq->gem_context->hw_id;
-  __entry->ring = rq->engine->id;
+  __entry->class = rq->engine->uabi_class;
+  __entry->instance = rq->engine->instance;
   __entry->ctx = rq->fence.context;
   __entry->seqno =

Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-24 Thread Tvrtko Ursulin 


On 24/05/2018 16:29, Chris Wilson wrote:

Quoting Tvrtko Ursulin (2018-05-24 16:04:46)

From: Tvrtko Ursulin 

Instead of using the engine->id, use uabi_class:instance pairs in trace-
points including engine info.


Should we not pack dev,hw_id,class,instance into u16s?


Can do.


This will be more readable, more future proof and more stable for
userspace consumption.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: svetlana.kukan...@intel.com
---
@@ -616,7 +621,8 @@ TRACE_EVENT(i915_request_queue,
 TP_STRUCT__entry(
  __field(u32, dev)
  __field(u32, hw_id)
-__field(u32, ring)
+__field(u32, class)
+__field(u32, instance)
  __field(u32, ctx)


ctx needs u64 :(
I've no objection to switching to our hopefully futureproof uabi
nomenclature.

Reviewed-by: Chris Wilson 


Thanks, I'll grow the series with all of the above.

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs

2018-05-24 Thread Chris Wilson 
Quoting Tvrtko Ursulin (2018-05-24 16:04:46)
> From: Tvrtko Ursulin 
> 
> Instead of using the engine->id, use uabi_class:instance pairs in trace-
> points including engine info.

Should we not pack dev,hw_id,class,instance into u16s?

> This will be more readable, more future proof and more stable for
> userspace consumption.
> 
> Signed-off-by: Tvrtko Ursulin 
> Cc: Chris Wilson 
> Cc: svetlana.kukan...@intel.com
> ---
> @@ -616,7 +621,8 @@ TRACE_EVENT(i915_request_queue,
> TP_STRUCT__entry(
>  __field(u32, dev)
>  __field(u32, hw_id)
> -__field(u32, ring)
> +__field(u32, class)
> +__field(u32, instance)
>  __field(u32, ctx)

ctx needs u64 :(

I've no objection to switching to our hopefully futureproof uabi
nomenclature.

Reviewed-by: Chris Wilson 
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx