| Message ID | 1405527759-957-3-git-send-email-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 16, 2014 at 05:22:38PM +0100, daniele.ceraolospurio@intel.com wrote: > From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > The context used to execute a batchbuffer is becoming increasingly > important. Duplicating to avoid modifications to the original trace. I am sure we don't want both. The structure encoding is exposed to userspace so we are free to update the tracepoints within reason. I would also like a better ctx identifier than its pointer. Using the pointer for tracking objects makes it more difficult to read traces (although it is easy for scripts). -Chris
On 7/17/2014 5:25 PM, Chris Wilson wrote: > On Wed, Jul 16, 2014 at 05:22:38PM +0100, daniele.ceraolospurio@intel.com wrote: >> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> >> The context used to execute a batchbuffer is becoming increasingly >> important. Duplicating to avoid modifications to the original trace. > > I am sure we don't want both. The structure encoding is exposed to > userspace so we are free to update the tracepoints within reason. As you can see by the next patch in the series, I plan to add a callback inside the trace. My original patch modified the existing trace, but (if I've understood correctly) Daniel asked for a duplicated trace to avoid adding the callback into the existing one. > I would also like a better ctx identifier than its pointer. Using the > pointer for tracking objects makes it more difficult to read traces > (although it is easy for scripts). I use the VM pointer to track the ppgtt; that pointer is also printed by several other traces, including the ppgtt init/release ones that I've submitted for comments in this series. However, I don't mind changing the way we identify the ctx as long as I still have access to the VM pointer. I'll have a look at the possible ways of identifying the ctx and I'll try to find a better solution than the current one. thanks, Daniele
On Fri, Jul 18, 2014 at 10:43:36AM +0100, Ceraolo Spurio, Daniele wrote: > On 7/17/2014 5:25 PM, Chris Wilson wrote: > >On Wed, Jul 16, 2014 at 05:22:38PM +0100, daniele.ceraolospurio@intel.com wrote: > >>From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >> > >>The context used to execute a batchbuffer is becoming increasingly > >>important. Duplicating to avoid modifications to the original trace. > > > >I am sure we don't want both. The structure encoding is exposed to > >userspace so we are free to update the tracepoints within reason. > > As you can see by the next patch in the series, I plan to add a callback > inside the trace. My original patch modified the existing trace, but (if > I've understood correctly) Daniel asked for a duplicated trace to avoid > adding the callback into the existing one. I guess there was a misunderstanding. I was worried about changing the tracepoint, but apparently Chris disagrees. The callback in the tracepoint is an unrelated issue and a no-go either way. -Daniel
>> I would also like a better ctx identifier than its pointer. Using the >> pointer for tracking objects makes it more difficult to read traces >> (although it is easy for scripts). > > I use the VM pointer to track the ppgtt; that pointer is also printed by > several other traces, including the ppgtt init/release ones that I've > submitted for comments in this series. However, I don't mind changing > the way we identify the ctx as long as I still have access to the VM > pointer. I'll have a look at the possible ways of identifying the ctx > and I'll try to find a better solution than the current one. > I've looked for other ways to identify the ctx, but I could't find anything more readable than a pointer. The best alternative in my opinion is to use the user_handle, but that is dependent on the file_priv so it is not enough on its own. Coupling the user_handle with the file_priv pointer doesn't seem like a good idea, so I still think that using the ctx pointer or the vm pointer is the best way to identify the ctx at the moment. Did you have something in mind that I've missed? Thanks, Daniele
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 60998fc..6b0dd9f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1174,6 +1174,8 @@ legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file, } trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags); + trace_i915_gem_ring_dispatch_validation(ring, + intel_ring_get_seqno(ring), flags, ctx); i915_gem_execbuffer_move_to_active(vmas, ring); i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj); diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 9be1421..d639d6c 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -374,6 +374,33 @@ TRACE_EVENT(i915_gem_ring_dispatch, __entry->dev, __entry->ring, __entry->seqno, __entry->flags) ); +TRACE_EVENT(i915_gem_ring_dispatch_validation, + TP_PROTO(struct intel_engine_cs *ring, u32 seqno, u32 flags, + struct intel_context *ctx), + TP_ARGS(ring, seqno, flags, ctx), + + TP_STRUCT__entry( + __field(u32, dev) + __field(u32, ring) + __field(u32, seqno) + __field(u32, flags) + __field(struct i915_address_space *, vm) + ), + + TP_fast_assign( + __entry->dev = ring->dev->primary->index; + __entry->ring = ring->id; + __entry->seqno = seqno; + __entry->flags = flags; + __entry->vm = ctx->vm; + i915_trace_irq_get(ring, seqno); + ), + + TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x, vm=%p", + __entry->dev, __entry->ring, __entry->seqno, + __entry->flags, __entry->vm) +); + TRACE_EVENT(i915_gem_ring_flush, TP_PROTO(struct intel_engine_cs *ring, u32 invalidate, u32 flush), TP_ARGS(ring, invalidate, flush),