> -----Original Message-----
> From: Chris Wilson [mailto:[email protected]]
> Sent: Thursday, June 19, 2014 8:57 AM
> To: Mateo Lozano, Oscar
> Cc: [email protected]
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Add ppgtt init/release trace
> points
> 
> On Wed, Jun 18, 2014 at 05:16:39PM +0100, [email protected] wrote:
> > From: Daniele Ceraolo Spurio <[email protected]>
> >
> > These tracepoints are useful for observing the creation and
> > destruction of Full PPGTTs.
> >
> > Signed-off-by: Oscar Mateo <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.c |  5 +++++
> >  drivers/gpu/drm/i915/i915_trace.h       | 38
> +++++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> > b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 5a62a19..bdfe3f5 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -136,6 +136,8 @@ static void ppgtt_release(struct kref *kref)
> >     struct i915_hw_ppgtt *ppgtt =
> >             container_of(kref, struct i915_hw_ppgtt, ref);
> >
> > +   trace_ppgtt_release(ppgtt);
> > +
> >     do_ppgtt_cleanup(ppgtt);
> >     kfree(ppgtt);
> >  }
> > @@ -215,6 +217,9 @@ create_vm_for_ctx(struct drm_device *dev, struct
> intel_context *ctx)
> >     }
> >
> >     ppgtt->ctx = ctx;
> > +
> > +   trace_ppgtt_init(ppgtt);
> > +
> >     return ppgtt;
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/i915_trace.h
> > b/drivers/gpu/drm/i915/i915_trace.h
> > index f5aa006..2d206d8 100644
> > --- a/drivers/gpu/drm/i915/i915_trace.h
> > +++ b/drivers/gpu/drm/i915/i915_trace.h
> > @@ -587,6 +587,44 @@ TRACE_EVENT(intel_gpu_freq_change,
> >         TP_printk("new_freq=%u", __entry->freq)  );
> >
> > +TRACE_EVENT(ppgtt_init,
> > +
> > +   TP_PROTO(struct i915_hw_ppgtt *ppgtt),
> > +
> > +   TP_ARGS(ppgtt),
> > +
> > +   TP_STRUCT__entry(
> > +           __field(struct i915_hw_ppgtt*, trace_ppgtt)
> > +           __field(unsigned int, ppgtt_op_code)
> > +   ),
> > +
> > +   TP_fast_assign(
> > +           __entry->trace_ppgtt = ppgtt;
> > +           __entry->ppgtt_op_code = 0;
> > +   ),
> > +
> > +   TP_printk("ppgtt op: %u", __entry->ppgtt_op_code)
> 
> op is redundant and unuseful since it is already encoded into the tracepoint
> itself. I'm not happy with the tracepoint name either, but it is close. Note,
> that we do like to pretend that our driver can coexist with itself. That means
> we have to pass along the dev->minor here so that listeners can distinguish
> events between imaginary devices. You need to say which client created the
> ppgtt and include some method for identifying the ppgtt.
> -Chris

Hmmm... I was submitting this on behalf of our validation team but in 
retrospect that was a bad idea: it´s much better if they participate in the 
discussion. Daniele, care to join?

-- Oscar
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to