* [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs @ 2018-05-24 15:04 Tvrtko Ursulin 2018-05-24 15:04 ` [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich Tvrtko Ursulin ` (5 more replies) 0 siblings, 6 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-24 15:04 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> 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 <tvrtko.ursulin@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: svetlana.kukanova@intel.com --- 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 = rq->fence.seqno; __entry->global = rq->global_seqno; ), - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u", - __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx, - __entry->seqno, __entry->global) + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u", + __entry->dev, __entry->hw_id, __entry->class, + __entry->instance, __entry->ctx, __entry->seqno, + __entry->global) ); DEFINE_EVENT(i915_request, i915_request_add, @@ -686,7 +697,8 @@ TRACE_EVENT(i915_request_in, 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_seqno) @@ -697,7 +709,8 @@ TRACE_EVENT(i915_request_in, 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->global_seqno = rq->global_seqno; @@ -705,10 +718,10 @@ TRACE_EVENT(i915_request_in, __entry->port = port; ), - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, prio=%u, global=%u, port=%u", - __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx, - __entry->seqno, __entry->prio, __entry->global_seqno, - __entry->port) + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, prio=%u, global=%u, port=%u", + __entry->dev, __entry->hw_id, __entry->class, + __entry->instance, __entry->ctx, __entry->seqno, + __entry->prio, __entry->global_seqno, __entry->port) ); TRACE_EVENT(i915_request_out, @@ -718,7 +731,8 @@ TRACE_EVENT(i915_request_out, 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_seqno) @@ -728,16 +742,17 @@ TRACE_EVENT(i915_request_out, 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->global_seqno = rq->global_seqno; __entry->completed = i915_request_completed(rq); ), - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, completed?=%u", - __entry->dev, __entry->hw_id, __entry->ring, - __entry->ctx, __entry->seqno, + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, completed?=%u", + __entry->dev, __entry->hw_id, __entry->class, + __entry->instance, __entry->ctx, __entry->seqno, __entry->global_seqno, __entry->completed) ); @@ -771,21 +786,23 @@ TRACE_EVENT(intel_engine_notify, TP_STRUCT__entry( __field(u32, dev) - __field(u32, ring) + __field(u32, class) + __field(u32, instance) __field(u32, seqno) __field(bool, waiters) ), TP_fast_assign( __entry->dev = engine->i915->drm.primary->index; - __entry->ring = engine->id; + __entry->class = engine->uabi_class; + __entry->instance = engine->instance; __entry->seqno = intel_engine_get_seqno(engine); __entry->waiters = waiters; ), - TP_printk("dev=%u, ring=%u, seqno=%u, waiters=%u", - __entry->dev, __entry->ring, __entry->seqno, - __entry->waiters) + TP_printk("dev=%u, engine=%u:%u, seqno=%u, waiters=%u", + __entry->dev, __entry->class, __entry->instance, + __entry->seqno, __entry->waiters) ); DEFINE_EVENT(i915_request, i915_request_retire, @@ -800,7 +817,8 @@ TRACE_EVENT(i915_request_wait_begin, 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) @@ -816,17 +834,19 @@ TRACE_EVENT(i915_request_wait_begin, 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->global = rq->global_seqno; __entry->flags = flags; ), - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", - __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx, - __entry->seqno, __entry->global, - !!(__entry->flags & I915_WAIT_LOCKED), __entry->flags) + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", + __entry->dev, __entry->hw_id, __entry->class, + __entry->instance, __entry->ctx, __entry->seqno, + __entry->global, !!(__entry->flags & I915_WAIT_LOCKED), + __entry->flags) ); DEFINE_EVENT(i915_request, i915_request_wait_end, @@ -966,21 +986,24 @@ TRACE_EVENT(switch_mm, TP_ARGS(engine, to), TP_STRUCT__entry( - __field(u32, ring) + __field(u32, class) + __field(u32, instance) __field(struct i915_gem_context *, to) __field(struct i915_address_space *, vm) __field(u32, dev) ), TP_fast_assign( - __entry->ring = engine->id; + __entry->class = engine->uabi_class; + __entry->instance = engine->instance; __entry->to = to; __entry->vm = to->ppgtt? &to->ppgtt->base : NULL; __entry->dev = engine->i915->drm.primary->index; ), - TP_printk("dev=%u, ring=%u, ctx=%p, ctx_vm=%p", - __entry->dev, __entry->ring, __entry->to, __entry->vm) + TP_printk("dev=%u, engine=%u:%u, ctx=%p, ctx_vm=%p", + __entry->dev, __entry->class, __entry->instance, __entry->to, + __entry->vm) ); #endif /* _I915_TRACE_H_ */ -- 2.17.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin @ 2018-05-24 15:04 ` Tvrtko Ursulin 2018-05-24 15:33 ` Chris Wilson 2018-05-24 15:48 ` Lionel Landwerlin 2018-05-24 15:29 ` [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Chris Wilson ` (4 subsequent siblings) 5 siblings, 2 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-24 15:04 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> In the string tracepoint representation we ended up with the engine sandwiched between context hardware id and context fence id. Move the two pieces of context data together and consolidate for redability using the format of ctx=hw_id:fence_context_id. Binary records are left as is, that is both fields remaing under the existing name and ordering. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_trace.h | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 3465cc1f9345..ab67b1661de4 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -638,9 +638,9 @@ TRACE_EVENT(i915_request_queue, __entry->flags = 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, + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, flags=0x%x", + __entry->dev, __entry->class, __entry->instance, + __entry->hw_id, __entry->ctx, __entry->seqno, __entry->flags) ); @@ -668,9 +668,9 @@ DECLARE_EVENT_CLASS(i915_request, __entry->global = rq->global_seqno; ), - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u", - __entry->dev, __entry->hw_id, __entry->class, - __entry->instance, __entry->ctx, __entry->seqno, + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, global=%u", + __entry->dev, __entry->class, __entry->instance, + __entry->hw_id, __entry->ctx, __entry->seqno, __entry->global) ); @@ -718,9 +718,9 @@ TRACE_EVENT(i915_request_in, __entry->port = port; ), - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, prio=%u, global=%u, port=%u", - __entry->dev, __entry->hw_id, __entry->class, - __entry->instance, __entry->ctx, __entry->seqno, + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, prio=%u, global=%u, port=%u", + __entry->dev, __entry->class, __entry->instance, + __entry->hw_id, __entry->ctx, __entry->seqno, __entry->prio, __entry->global_seqno, __entry->port) ); @@ -750,9 +750,9 @@ TRACE_EVENT(i915_request_out, __entry->completed = i915_request_completed(rq); ), - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, completed?=%u", - __entry->dev, __entry->hw_id, __entry->class, - __entry->instance, __entry->ctx, __entry->seqno, + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, global=%u, completed?=%u", + __entry->dev, __entry->class, __entry->instance, + __entry->hw_id, __entry->ctx, __entry->seqno, __entry->global_seqno, __entry->completed) ); @@ -842,9 +842,9 @@ TRACE_EVENT(i915_request_wait_begin, __entry->flags = flags; ), - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", - __entry->dev, __entry->hw_id, __entry->class, - __entry->instance, __entry->ctx, __entry->seqno, + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", + __entry->dev, __entry->class, __entry->instance, + __entry->hw_id, __entry->ctx, __entry->seqno, __entry->global, !!(__entry->flags & I915_WAIT_LOCKED), __entry->flags) ); -- 2.17.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 15:04 ` [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich Tvrtko Ursulin @ 2018-05-24 15:33 ` Chris Wilson 2018-05-24 15:48 ` Tvrtko Ursulin 2018-05-24 15:48 ` Lionel Landwerlin 1 sibling, 1 reply; 19+ messages in thread From: Chris Wilson @ 2018-05-24 15:33 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx Quoting Tvrtko Ursulin (2018-05-24 16:04:47) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > In the string tracepoint representation we ended up with the engine > sandwiched between context hardware id and context fence id. > > Move the two pieces of context data together and consolidate for > redability using the format of ctx=hw_id:fence_context_id. Grr, bring backs my memory of ctx being the most important! In order of segregation, I think of it as device+context as being the outer container (a user isn't meant to escape their context). The timelines and engines they use are all contained within their context. But what we call ctx here isn't really context, but timeline; how about if we switch to the fence=%llx:%d representation we've mostly settled on for the debug messages? -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 15:33 ` Chris Wilson @ 2018-05-24 15:48 ` Tvrtko Ursulin 2018-05-24 16:00 ` Chris Wilson 0 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-24 15:48 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx On 24/05/2018 16:33, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-05-24 16:04:47) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> In the string tracepoint representation we ended up with the engine >> sandwiched between context hardware id and context fence id. >> >> Move the two pieces of context data together and consolidate for >> redability using the format of ctx=hw_id:fence_context_id. > > Grr, bring backs my memory of ctx being the most important! I knew it! :)) > In order of segregation, I think of it as device+context as being the > outer container (a user isn't meant to escape their context). The > timelines and engines they use are all contained within their context. If I move ctx before engine, then seqno is left to hang after the engine. And honestly I forgot all my arguments in this topic. By the virtue of exhaustion I am prepared to give in. :)) > But what we call ctx here isn't really context, but timeline; how about > if we switch to the fence=%llx:%d representation we've mostly settled on > for the debug messages? For the ctx and seqno pair? But here we have the additional issue of hw_id. I think context is better than timeline at this level. Or you mean keep explicit hw_id and join ctx and seqno into fence=%llx:%d? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 15:48 ` Tvrtko Ursulin @ 2018-05-24 16:00 ` Chris Wilson 2018-05-25 8:30 ` Tvrtko Ursulin 0 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2018-05-24 16:00 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx Quoting Tvrtko Ursulin (2018-05-24 16:48:45) > > On 24/05/2018 16:33, Chris Wilson wrote: > > But what we call ctx here isn't really context, but timeline; how about > > if we switch to the fence=%llx:%d representation we've mostly settled on > > for the debug messages? > > For the ctx and seqno pair? But here we have the additional issue of > hw_id. I think context is better than timeline at this level. > > Or you mean keep explicit hw_id and join ctx and seqno into fence=%llx:%d? Right. I think what we call ctx here is very confusing, as it's just the fence.context (i.e timeline id) and not any of the ids we assign to the context (neither hw_id or uabi_id), so I don't think ctx refers to i915_gem_context/intel_context at all and so would rather stop using 'ctx'. Pardon the rambling, -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 16:00 ` Chris Wilson @ 2018-05-25 8:30 ` Tvrtko Ursulin 2018-05-25 8:38 ` Chris Wilson 0 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-25 8:30 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx On 24/05/2018 17:00, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-05-24 16:48:45) >> >> On 24/05/2018 16:33, Chris Wilson wrote: >>> But what we call ctx here isn't really context, but timeline; how about >>> if we switch to the fence=%llx:%d representation we've mostly settled on >>> for the debug messages? >> >> For the ctx and seqno pair? But here we have the additional issue of >> hw_id. I think context is better than timeline at this level. >> >> Or you mean keep explicit hw_id and join ctx and seqno into fence=%llx:%d? > > Right. I think what we call ctx here is very confusing, as it's just the > fence.context (i.e timeline id) and not any of the ids we assign to the > context (neither hw_id or uabi_id), so I don't think ctx refers to > i915_gem_context/intel_context at all and so would rather stop using > 'ctx'. I couldn't make myself re-order ctx and engine, since I did not find the solution for the resulting ctx and seqno split. And I did not like the fence=%llu:%u for tracepoints. Just can't think of requests as fences. Wrt hw_id and dev moving to u16, both fields theoretically can be wider so I did not do that either. What's left is class:instance, group two context id's together, and expand ctx to u64. Hopefully not controversial at this time, or as a first step, or something. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-25 8:30 ` Tvrtko Ursulin @ 2018-05-25 8:38 ` Chris Wilson 0 siblings, 0 replies; 19+ messages in thread From: Chris Wilson @ 2018-05-25 8:38 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx Quoting Tvrtko Ursulin (2018-05-25 09:30:29) > > On 24/05/2018 17:00, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-05-24 16:48:45) > >> > >> On 24/05/2018 16:33, Chris Wilson wrote: > >>> But what we call ctx here isn't really context, but timeline; how about > >>> if we switch to the fence=%llx:%d representation we've mostly settled on > >>> for the debug messages? > >> > >> For the ctx and seqno pair? But here we have the additional issue of > >> hw_id. I think context is better than timeline at this level. > >> > >> Or you mean keep explicit hw_id and join ctx and seqno into fence=%llx:%d? > > > > Right. I think what we call ctx here is very confusing, as it's just the > > fence.context (i.e timeline id) and not any of the ids we assign to the > > context (neither hw_id or uabi_id), so I don't think ctx refers to > > i915_gem_context/intel_context at all and so would rather stop using > > 'ctx'. > > I couldn't make myself re-order ctx and engine, since I did not find the > solution for the resulting ctx and seqno split. And I did not like the > fence=%llu:%u for tracepoints. Just can't think of requests as fences. But we are referring to the fence id of the request (rq->fence.context, rq->fence.seqno), not its global seqno. > Wrt hw_id and dev moving to u16, both fields theoretically can be wider > so I did not do that either. dev cannot. We are limited to 16 DRM devices on the system, so minor is always 0-15. Immaterial if we need padding between members in the struct, we might as well use up the padding. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 15:04 ` [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich Tvrtko Ursulin 2018-05-24 15:33 ` Chris Wilson @ 2018-05-24 15:48 ` Lionel Landwerlin 2018-05-24 15:51 ` Tvrtko Ursulin 1 sibling, 1 reply; 19+ messages in thread From: Lionel Landwerlin @ 2018-05-24 15:48 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx On 24/05/18 16:04, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > In the string tracepoint representation we ended up with the engine > sandwiched between context hardware id and context fence id. > > Move the two pieces of context data together and consolidate for > redability using the format of ctx=hw_id:fence_context_id. Arg! Will need to update the tracepoint parser in igt :( > > Binary records are left as is, that is both fields remaing under the > existing name and ordering. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/i915_trace.h | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h > index 3465cc1f9345..ab67b1661de4 100644 > --- a/drivers/gpu/drm/i915/i915_trace.h > +++ b/drivers/gpu/drm/i915/i915_trace.h > @@ -638,9 +638,9 @@ TRACE_EVENT(i915_request_queue, > __entry->flags = 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, > + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, flags=0x%x", > + __entry->dev, __entry->class, __entry->instance, > + __entry->hw_id, __entry->ctx, __entry->seqno, > __entry->flags) > ); > > @@ -668,9 +668,9 @@ DECLARE_EVENT_CLASS(i915_request, > __entry->global = rq->global_seqno; > ), > > - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u", > - __entry->dev, __entry->hw_id, __entry->class, > - __entry->instance, __entry->ctx, __entry->seqno, > + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, global=%u", > + __entry->dev, __entry->class, __entry->instance, > + __entry->hw_id, __entry->ctx, __entry->seqno, > __entry->global) > ); > > @@ -718,9 +718,9 @@ TRACE_EVENT(i915_request_in, > __entry->port = port; > ), > > - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, prio=%u, global=%u, port=%u", > - __entry->dev, __entry->hw_id, __entry->class, > - __entry->instance, __entry->ctx, __entry->seqno, > + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, prio=%u, global=%u, port=%u", > + __entry->dev, __entry->class, __entry->instance, > + __entry->hw_id, __entry->ctx, __entry->seqno, > __entry->prio, __entry->global_seqno, __entry->port) > ); > > @@ -750,9 +750,9 @@ TRACE_EVENT(i915_request_out, > __entry->completed = i915_request_completed(rq); > ), > > - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, completed?=%u", > - __entry->dev, __entry->hw_id, __entry->class, > - __entry->instance, __entry->ctx, __entry->seqno, > + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, global=%u, completed?=%u", > + __entry->dev, __entry->class, __entry->instance, > + __entry->hw_id, __entry->ctx, __entry->seqno, > __entry->global_seqno, __entry->completed) > ); > > @@ -842,9 +842,9 @@ TRACE_EVENT(i915_request_wait_begin, > __entry->flags = flags; > ), > > - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", > - __entry->dev, __entry->hw_id, __entry->class, > - __entry->instance, __entry->ctx, __entry->seqno, > + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", > + __entry->dev, __entry->class, __entry->instance, > + __entry->hw_id, __entry->ctx, __entry->seqno, > __entry->global, !!(__entry->flags & I915_WAIT_LOCKED), > __entry->flags) > ); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich 2018-05-24 15:48 ` Lionel Landwerlin @ 2018-05-24 15:51 ` Tvrtko Ursulin 0 siblings, 0 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-24 15:51 UTC (permalink / raw) To: Lionel Landwerlin, Tvrtko Ursulin, Intel-gfx On 24/05/2018 16:48, Lionel Landwerlin wrote: > On 24/05/18 16:04, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> In the string tracepoint representation we ended up with the engine >> sandwiched between context hardware id and context fence id. >> >> Move the two pieces of context data together and consolidate for >> redability using the format of ctx=hw_id:fence_context_id. > > Arg! Will need to update the tracepoint parser in igt :( I'll leave them separate then, was mostly aiming to remove engine out of the sandwich. Regards, Tvrtko >> >> Binary records are left as is, that is both fields remaing under the >> existing name and ordering. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> --- >> drivers/gpu/drm/i915/i915_trace.h | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_trace.h >> b/drivers/gpu/drm/i915/i915_trace.h >> index 3465cc1f9345..ab67b1661de4 100644 >> --- a/drivers/gpu/drm/i915/i915_trace.h >> +++ b/drivers/gpu/drm/i915/i915_trace.h >> @@ -638,9 +638,9 @@ TRACE_EVENT(i915_request_queue, >> __entry->flags = 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, >> + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, >> flags=0x%x", >> + __entry->dev, __entry->class, __entry->instance, >> + __entry->hw_id, __entry->ctx, __entry->seqno, >> __entry->flags) >> ); >> @@ -668,9 +668,9 @@ DECLARE_EVENT_CLASS(i915_request, >> __entry->global = rq->global_seqno; >> ), >> - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, >> global=%u", >> - __entry->dev, __entry->hw_id, __entry->class, >> - __entry->instance, __entry->ctx, __entry->seqno, >> + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, >> global=%u", >> + __entry->dev, __entry->class, __entry->instance, >> + __entry->hw_id, __entry->ctx, __entry->seqno, >> __entry->global) >> ); >> @@ -718,9 +718,9 @@ TRACE_EVENT(i915_request_in, >> __entry->port = port; >> ), >> - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, >> prio=%u, global=%u, port=%u", >> - __entry->dev, __entry->hw_id, __entry->class, >> - __entry->instance, __entry->ctx, __entry->seqno, >> + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, >> prio=%u, global=%u, port=%u", >> + __entry->dev, __entry->class, __entry->instance, >> + __entry->hw_id, __entry->ctx, __entry->seqno, >> __entry->prio, __entry->global_seqno, __entry->port) >> ); >> @@ -750,9 +750,9 @@ TRACE_EVENT(i915_request_out, >> __entry->completed = i915_request_completed(rq); >> ), >> - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, >> seqno=%u, global=%u, completed?=%u", >> - __entry->dev, __entry->hw_id, __entry->class, >> - __entry->instance, __entry->ctx, __entry->seqno, >> + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, >> global=%u, completed?=%u", >> + __entry->dev, __entry->class, __entry->instance, >> + __entry->hw_id, __entry->ctx, __entry->seqno, >> __entry->global_seqno, __entry->completed) >> ); >> @@ -842,9 +842,9 @@ TRACE_EVENT(i915_request_wait_begin, >> __entry->flags = flags; >> ), >> - TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, >> global=%u, blocking=%u, flags=0x%x", >> - __entry->dev, __entry->hw_id, __entry->class, >> - __entry->instance, __entry->ctx, __entry->seqno, >> + TP_printk("dev=%u, engine=%u:%u, ctx=%u:%u, seqno=%u, >> global=%u, blocking=%u, flags=0x%x", >> + __entry->dev, __entry->class, __entry->instance, >> + __entry->hw_id, __entry->ctx, __entry->seqno, >> __entry->global, !!(__entry->flags & I915_WAIT_LOCKED), >> __entry->flags) >> ); > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin 2018-05-24 15:04 ` [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich Tvrtko Ursulin @ 2018-05-24 15:29 ` Chris Wilson 2018-05-24 15:42 ` Tvrtko Ursulin 2018-05-24 15:53 ` Lionel Landwerlin ` (3 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2018-05-24 15:29 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx Quoting Tvrtko Ursulin (2018-05-24 16:04:46) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > 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 <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: svetlana.kukanova@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@chris-wilson.co.uk> -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:29 ` [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Chris Wilson @ 2018-05-24 15:42 ` Tvrtko Ursulin 0 siblings, 0 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-24 15:42 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx On 24/05/2018 16:29, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-05-24 16:04:46) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> 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 <tvrtko.ursulin@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: svetlana.kukanova@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@chris-wilson.co.uk> 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin 2018-05-24 15:04 ` [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich Tvrtko Ursulin 2018-05-24 15:29 ` [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Chris Wilson @ 2018-05-24 15:53 ` Lionel Landwerlin 2018-05-24 16:07 ` Tvrtko Ursulin 2018-05-24 16:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork ` (2 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Lionel Landwerlin @ 2018-05-24 15:53 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx On 24/05/18 16:04, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > 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 <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: svetlana.kukanova@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 = rq->fence.seqno; > __entry->global = rq->global_seqno; > ), > > - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u", > - __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx, > - __entry->seqno, __entry->global) > + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u", > + __entry->dev, __entry->hw_id, __entry->class, > + __entry->instance, __entry->ctx, __entry->seqno, > + __entry->global) > ); > > DEFINE_EVENT(i915_request, i915_request_add, > @@ -686,7 +697,8 @@ TRACE_EVENT(i915_request_in, > 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_seqno) > @@ -697,7 +709,8 @@ TRACE_EVENT(i915_request_in, > 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->global_seqno = rq->global_seqno; > @@ -705,10 +718,10 @@ TRACE_EVENT(i915_request_in, > __entry->port = port; > ), > > - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, prio=%u, global=%u, port=%u", > - __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx, > - __entry->seqno, __entry->prio, __entry->global_seqno, > - __entry->port) > + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, prio=%u, global=%u, port=%u", > + __entry->dev, __entry->hw_id, __entry->class, > + __entry->instance, __entry->ctx, __entry->seqno, > + __entry->prio, __entry->global_seqno, __entry->port) > ); > > TRACE_EVENT(i915_request_out, > @@ -718,7 +731,8 @@ TRACE_EVENT(i915_request_out, > 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_seqno) > @@ -728,16 +742,17 @@ TRACE_EVENT(i915_request_out, > 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->global_seqno = rq->global_seqno; > __entry->completed = i915_request_completed(rq); > ), > > - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, completed?=%u", > - __entry->dev, __entry->hw_id, __entry->ring, > - __entry->ctx, __entry->seqno, > + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, completed?=%u", > + __entry->dev, __entry->hw_id, __entry->class, > + __entry->instance, __entry->ctx, __entry->seqno, > __entry->global_seqno, __entry->completed) > ); > > @@ -771,21 +786,23 @@ TRACE_EVENT(intel_engine_notify, > > TP_STRUCT__entry( > __field(u32, dev) > - __field(u32, ring) > + __field(u32, class) > + __field(u32, instance) > __field(u32, seqno) > __field(bool, waiters) > ), > > TP_fast_assign( > __entry->dev = engine->i915->drm.primary->index; > - __entry->ring = engine->id; > + __entry->class = engine->uabi_class; > + __entry->instance = engine->instance; > __entry->seqno = intel_engine_get_seqno(engine); > __entry->waiters = waiters; > ), > > - TP_printk("dev=%u, ring=%u, seqno=%u, waiters=%u", > - __entry->dev, __entry->ring, __entry->seqno, > - __entry->waiters) > + TP_printk("dev=%u, engine=%u:%u, seqno=%u, waiters=%u", > + __entry->dev, __entry->class, __entry->instance, > + __entry->seqno, __entry->waiters) > ); > > DEFINE_EVENT(i915_request, i915_request_retire, > @@ -800,7 +817,8 @@ TRACE_EVENT(i915_request_wait_begin, > 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) > @@ -816,17 +834,19 @@ TRACE_EVENT(i915_request_wait_begin, > 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->global = rq->global_seqno; > __entry->flags = flags; > ), > > - TP_printk("dev=%u, hw_id=%u, ring=%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", > - __entry->dev, __entry->hw_id, __entry->ring, __entry->ctx, > - __entry->seqno, __entry->global, > - !!(__entry->flags & I915_WAIT_LOCKED), __entry->flags) > + TP_printk("dev=%u, hw_id=%u, engine=%u:%u, ctx=%u, seqno=%u, global=%u, blocking=%u, flags=0x%x", > + __entry->dev, __entry->hw_id, __entry->class, > + __entry->instance, __entry->ctx, __entry->seqno, > + __entry->global, !!(__entry->flags & I915_WAIT_LOCKED), > + __entry->flags) > ); > > DEFINE_EVENT(i915_request, i915_request_wait_end, > @@ -966,21 +986,24 @@ TRACE_EVENT(switch_mm, > TP_ARGS(engine, to), > > TP_STRUCT__entry( > - __field(u32, ring) > + __field(u32, class) > + __field(u32, instance) > __field(struct i915_gem_context *, to) > __field(struct i915_address_space *, vm) > __field(u32, dev) > ), > > TP_fast_assign( > - __entry->ring = engine->id; > + __entry->class = engine->uabi_class; > + __entry->instance = engine->instance; > __entry->to = to; > __entry->vm = to->ppgtt? &to->ppgtt->base : NULL; > __entry->dev = engine->i915->drm.primary->index; > ), > > - TP_printk("dev=%u, ring=%u, ctx=%p, ctx_vm=%p", > - __entry->dev, __entry->ring, __entry->to, __entry->vm) > + TP_printk("dev=%u, engine=%u:%u, ctx=%p, ctx_vm=%p", > + __entry->dev, __entry->class, __entry->instance, __entry->to, > + __entry->vm) > ); > > #endif /* _I915_TRACE_H_ */ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:53 ` Lionel Landwerlin @ 2018-05-24 16:07 ` Tvrtko Ursulin 2018-05-24 16:13 ` Lionel Landwerlin 0 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-24 16:07 UTC (permalink / raw) To: Lionel Landwerlin, Tvrtko Ursulin, Intel-gfx On 24/05/2018 16:53, Lionel Landwerlin wrote: > On 24/05/18 16:04, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> 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 <tvrtko.ursulin@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: svetlana.kukanova@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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 16:07 ` Tvrtko Ursulin @ 2018-05-24 16:13 ` Lionel Landwerlin 2018-05-25 7:11 ` Tvrtko Ursulin 0 siblings, 1 reply; 19+ messages in thread From: Lionel Landwerlin @ 2018-05-24 16:13 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx 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 <tvrtko.ursulin@intel.com> >>> >>> 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 <tvrtko.ursulin@intel.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: svetlana.kukanova@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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 16:13 ` Lionel Landwerlin @ 2018-05-25 7:11 ` Tvrtko Ursulin 2018-05-25 7:21 ` Chris Wilson 0 siblings, 1 reply; 19+ messages in thread From: Tvrtko Ursulin @ 2018-05-25 7:11 UTC (permalink / raw) To: Lionel Landwerlin, Tvrtko Ursulin, Intel-gfx 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 <tvrtko.ursulin@intel.com> >>>> >>>> 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 <tvrtko.ursulin@intel.com> >>>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>>> Cc: svetlana.kukanova@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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-25 7:11 ` Tvrtko Ursulin @ 2018-05-25 7:21 ` Chris Wilson 0 siblings, 0 replies; 19+ messages in thread From: Chris Wilson @ 2018-05-25 7:21 UTC (permalink / raw) To: Tvrtko Ursulin, Lionel Landwerlin, Tvrtko Ursulin, Intel-gfx 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 <tvrtko.ursulin@intel.com> > >>>> > >>>> 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 <tvrtko.ursulin@intel.com> > >>>> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >>>> Cc: svetlana.kukanova@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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin ` (2 preceding siblings ...) 2018-05-24 15:53 ` Lionel Landwerlin @ 2018-05-24 16:00 ` Patchwork 2018-05-24 16:18 ` ✓ Fi.CI.BAT: success " Patchwork 2018-05-24 20:50 ` ✓ Fi.CI.IGT: " Patchwork 5 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2018-05-24 16:00 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/trace: Describe engines as class:instance pairs URL : https://patchwork.freedesktop.org/series/43709/ State : warning == Summary == $ dim checkpatch origin/drm-tip 29e13710369f drm/i915/trace: Describe engines as class:instance pairs 4f5293dac612 drm/i915/trace: Remove engine out of the context sandwich -:31: ERROR:CODE_INDENT: code indent should use tabs where possible #31: FILE: drivers/gpu/drm/i915/i915_trace.h:643: +^I __entry->hw_id, __entry->ctx, __entry->seqno,$ total: 1 errors, 0 warnings, 0 checks, 60 lines checked _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin ` (3 preceding siblings ...) 2018-05-24 16:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork @ 2018-05-24 16:18 ` Patchwork 2018-05-24 20:50 ` ✓ Fi.CI.IGT: " Patchwork 5 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2018-05-24 16:18 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/trace: Describe engines as class:instance pairs URL : https://patchwork.freedesktop.org/series/43709/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4233 -> Patchwork_9106 = == Summary - WARNING == Minor unknown changes coming with Patchwork_9106 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9106, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43709/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9106: === IGT changes === ==== Warnings ==== igt@gem_exec_gttfill@basic: fi-pnv-d510: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_9106 that come from known issues: === IGT changes === ==== Issues hit ==== igt@debugfs_test@read_all_entries: fi-bsw-n3050: PASS -> DMESG-WARN (fdo#106207) igt@gem_mmap_gtt@basic-small-bo-tiledx: fi-gdg-551: PASS -> FAIL (fdo#102575) ==== Possible fixes ==== igt@kms_flip@basic-flip-vs-wf_vblank: fi-hsw-4200u: FAIL (fdo#100368) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fdo#106207 https://bugs.freedesktop.org/show_bug.cgi?id=106207 == Participating hosts (44 -> 39) == Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6700hq == Build changes == * Linux: CI_DRM_4233 -> Patchwork_9106 CI_DRM_4233: 0b7643db57abff1223f77fb8cbb8dd8a00ca9938 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4498: f9ecb79ad8b02278cfdb5b82495df47061c04f8f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9106: 4f5293dac6122cdcb6456a96f29f8c69c73e0346 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 4f5293dac612 drm/i915/trace: Remove engine out of the context sandwich 29e13710369f drm/i915/trace: Describe engines as class:instance pairs == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9106/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/trace: Describe engines as class:instance pairs 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin ` (4 preceding siblings ...) 2018-05-24 16:18 ` ✓ Fi.CI.BAT: success " Patchwork @ 2018-05-24 20:50 ` Patchwork 5 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2018-05-24 20:50 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/trace: Describe engines as class:instance pairs URL : https://patchwork.freedesktop.org/series/43709/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4233_full -> Patchwork_9106_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9106_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9106_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43709/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9106_full: === IGT changes === ==== Warnings ==== igt@gem_mocs_settings@mocs-rc6-vebox: shard-kbl: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_9106_full that come from known issues: === IGT changes === ==== Issues hit ==== igt@drv_selftest@live_gtt: shard-glk: PASS -> INCOMPLETE (k.org#198133, fdo#103359) igt@kms_atomic_transition@1x-modeset-transitions-nonblocking: shard-glk: PASS -> FAIL (fdo#105703) igt@kms_flip_tiling@flip-y-tiled: shard-glk: PASS -> FAIL (fdo#104724) ==== Possible fixes ==== igt@drv_selftest@live_gtt: shard-kbl: INCOMPLETE (fdo#103665) -> PASS shard-apl: INCOMPLETE (fdo#103927) -> PASS igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing: shard-glk: FAIL (fdo#105703) -> PASS igt@kms_flip@plain-flip-fb-recreate: shard-glk: FAIL (fdo#100368) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703 k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4233 -> Patchwork_9106 CI_DRM_4233: 0b7643db57abff1223f77fb8cbb8dd8a00ca9938 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4498: f9ecb79ad8b02278cfdb5b82495df47061c04f8f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9106: 4f5293dac6122cdcb6456a96f29f8c69c73e0346 @ git://anongit.freedesktop.org/gfx-ci/linux == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9106/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-05-25 8:38 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-24 15:04 [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Tvrtko Ursulin 2018-05-24 15:04 ` [PATCH 2/2] drm/i915/trace: Remove engine out of the context sandwich Tvrtko Ursulin 2018-05-24 15:33 ` Chris Wilson 2018-05-24 15:48 ` Tvrtko Ursulin 2018-05-24 16:00 ` Chris Wilson 2018-05-25 8:30 ` Tvrtko Ursulin 2018-05-25 8:38 ` Chris Wilson 2018-05-24 15:48 ` Lionel Landwerlin 2018-05-24 15:51 ` Tvrtko Ursulin 2018-05-24 15:29 ` [PATCH 1/2] drm/i915/trace: Describe engines as class:instance pairs Chris Wilson 2018-05-24 15:42 ` Tvrtko Ursulin 2018-05-24 15:53 ` Lionel Landwerlin 2018-05-24 16:07 ` Tvrtko Ursulin 2018-05-24 16:13 ` Lionel Landwerlin 2018-05-25 7:11 ` Tvrtko Ursulin 2018-05-25 7:21 ` Chris Wilson 2018-05-24 16:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] " Patchwork 2018-05-24 16:18 ` ✓ Fi.CI.BAT: success " Patchwork 2018-05-24 20:50 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.