All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Add ppgtt init/release trace points
@ 2014-06-18 16:16 oscar.mateo
  2014-06-18 16:16 ` [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point oscar.mateo
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: oscar.mateo @ 2014-06-18 16:16 UTC (permalink / raw)
  To: intel-gfx

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

These tracepoints are useful for observing the creation and
destruction of Full PPGTTs.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 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)
+);
+
+TRACE_EVENT(ppgtt_release,
+
+	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 = 1;
+	),
+
+	TP_printk("ppgtt op: %u", __entry->ppgtt_op_code)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point
  2014-06-18 16:16 [PATCH 1/3] drm/i915: Add ppgtt init/release trace points oscar.mateo
@ 2014-06-18 16:16 ` oscar.mateo
  2014-06-18 16:16 ` [PATCH 3/3] drm/i915: Trace point callbacks for validation oscar.mateo
  2014-06-19  7:56 ` [PATCH 1/3] drm/i915: Add ppgtt init/release trace points Chris Wilson
  2 siblings, 0 replies; 8+ messages in thread
From: oscar.mateo @ 2014-06-18 16:16 UTC (permalink / raw)
  To: intel-gfx

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

The context used top execute a batchbuffer is becoming increasingly
important.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
 drivers/gpu/drm/i915/i915_trace.h          | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 93d7f72..b273257 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1374,7 +1374,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 	}
 
-	trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
+	trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags, ctx);
 
 	i915_gem_execbuffer_move_to_active(&eb->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 2d206d8..756be7e 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -352,8 +352,8 @@ TRACE_EVENT(i915_gem_ring_sync_to,
 );
 
 TRACE_EVENT(i915_gem_ring_dispatch,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno, u32 flags),
-	    TP_ARGS(ring, seqno, flags),
+	    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)
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] drm/i915: Trace point callbacks for validation
  2014-06-18 16:16 [PATCH 1/3] drm/i915: Add ppgtt init/release trace points oscar.mateo
  2014-06-18 16:16 ` [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point oscar.mateo
@ 2014-06-18 16:16 ` oscar.mateo
  2014-06-19  7:56 ` [PATCH 1/3] drm/i915: Add ppgtt init/release trace points Chris Wilson
  2 siblings, 0 replies; 8+ messages in thread
From: oscar.mateo @ 2014-06-18 16:16 UTC (permalink / raw)
  To: intel-gfx

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

These callbacks can be assigned to specific functions inside an external
validation kernel module. This module can then extract run-time information
to make sure everything is working as expected.

Specifically, these two callbacks extract information about ifull PPGGT and
batch dispatch.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c    |  3 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +++
 drivers/gpu/drm/i915/i915_trace.h          | 21 +++++++++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index bdfe3f5..42b583d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -89,6 +89,9 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
+i915_ppgtt_validation_callback_t ppgtt_validation_callback = NULL;
+EXPORT_SYMBOL_GPL(ppgtt_validation_callback);
+
 /* This is a HW constraint. The value below is the largest known requirement
  * I've seen in a spec to date, and that was a workaround for a non-shipping
  * part. It should be safe to decrease this, but it's more future proof as is.
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index b273257..14984585 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -33,6 +33,9 @@
 #include "intel_drv.h"
 #include <linux/dma_remapping.h>
 
+i915_gem_ring_dispatch_callback_t i915_gem_ring_dispatch_validation_callback = NULL;
+EXPORT_SYMBOL_GPL(i915_gem_ring_dispatch_validation_callback);
+
 #define  __EXEC_OBJECT_HAS_PIN (1<<31)
 #define  __EXEC_OBJECT_HAS_FENCE (1<<30)
 #define  __EXEC_OBJECT_NEEDS_BIAS (1<<28)
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 756be7e..2511618 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -15,6 +15,17 @@
 #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
 #define TRACE_INCLUDE_FILE i915_trace
 
+/* validation callbacks */
+
+typedef void (*i915_ppgtt_validation_callback_t)(unsigned int action_code,
+						 struct i915_hw_ppgtt *ppgtt);
+extern i915_ppgtt_validation_callback_t ppgtt_validation_callback;
+
+typedef void (*i915_gem_ring_dispatch_callback_t)(struct intel_engine_cs *ring,
+						  u32 seqno, u32 flags,
+						  struct intel_context *ctx);
+extern i915_gem_ring_dispatch_callback_t i915_gem_ring_dispatch_validation_callback;
+
 /* pipe updates */
 
 TRACE_EVENT(i915_pipe_update_start,
@@ -368,6 +379,10 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 			   __entry->seqno = seqno;
 			   __entry->flags = flags;
 			   i915_trace_irq_get(ring, seqno);
+
+			   if (i915_gem_ring_dispatch_validation_callback)
+				   i915_gem_ring_dispatch_validation_callback(ring,
+						   seqno, flags, ctx);
 			   ),
 
 	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
@@ -601,6 +616,9 @@ TRACE_EVENT(ppgtt_init,
 	TP_fast_assign(
 		__entry->trace_ppgtt = ppgtt;
 		__entry->ppgtt_op_code = 0;
+
+		if (ppgtt_validation_callback)
+			ppgtt_validation_callback(0, ppgtt);
 	),
 
 	TP_printk("ppgtt op: %u", __entry->ppgtt_op_code)
@@ -620,6 +638,9 @@ TRACE_EVENT(ppgtt_release,
 	TP_fast_assign(
 		__entry->trace_ppgtt = ppgtt;
 		__entry->ppgtt_op_code = 1;
+
+		if (ppgtt_validation_callback)
+			ppgtt_validation_callback(1, ppgtt);
 	),
 
 	TP_printk("ppgtt op: %u", __entry->ppgtt_op_code)
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] drm/i915: Add ppgtt init/release trace points
  2014-06-18 16:16 [PATCH 1/3] drm/i915: Add ppgtt init/release trace points oscar.mateo
  2014-06-18 16:16 ` [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point oscar.mateo
  2014-06-18 16:16 ` [PATCH 3/3] drm/i915: Trace point callbacks for validation oscar.mateo
@ 2014-06-19  7:56 ` Chris Wilson
  2014-06-19 14:22   ` Mateo Lozano, Oscar
  2 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2014-06-19  7:56 UTC (permalink / raw)
  To: oscar.mateo; +Cc: intel-gfx

On Wed, Jun 18, 2014 at 05:16:39PM +0100, oscar.mateo@intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> These tracepoints are useful for observing the creation and
> destruction of Full PPGTTs.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  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

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] drm/i915: Add ppgtt init/release trace points
  2014-06-19  7:56 ` [PATCH 1/3] drm/i915: Add ppgtt init/release trace points Chris Wilson
@ 2014-06-19 14:22   ` Mateo Lozano, Oscar
  2014-06-19 15:41     ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 8+ messages in thread
From: Mateo Lozano, Oscar @ 2014-06-19 14:22 UTC (permalink / raw)
  To: Chris Wilson, dan.cer.spu; +Cc: intel-gfx

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Thursday, June 19, 2014 8:57 AM
> To: Mateo Lozano, Oscar
> Cc: intel-gfx@lists.freedesktop.org
> 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, oscar.mateo@intel.com wrote:
> > From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >
> > These tracepoints are useful for observing the creation and
> > destruction of Full PPGTTs.
> >
> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] drm/i915: Add ppgtt init/release trace points
  2014-06-19 14:22   ` Mateo Lozano, Oscar
@ 2014-06-19 15:41     ` Daniele Ceraolo Spurio
  2014-06-19 16:05       ` Chris Wilson
  0 siblings, 1 reply; 8+ messages in thread
From: Daniele Ceraolo Spurio @ 2014-06-19 15:41 UTC (permalink / raw)
  To: Mateo Lozano, Oscar; +Cc: intel-gfx

On 19 June 2014 15:22, Mateo Lozano, Oscar <oscar.mateo@intel.com> wrote:
>
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Thursday, June 19, 2014 8:57 AM
> > To: Mateo Lozano, Oscar
> > Cc: intel-gfx@lists.freedesktop.org
> > 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, oscar.mateo@intel.com wrote:
> > > From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > >
> > > These tracepoints are useful for observing the creation and
> > > destruction of Full PPGTTs.
> > >
> > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > > ---
> > >  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,

Would something like "i915_create_ppgtt" be ok?

> > 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.

There are several things that we could use to identify the ppgtt. My
choice would be the VM pointer, which is unique for each ppgtt and is
also printed by the i915_vma_bind and i915_vma_unbind traces, but we
could also use the context id. The problem with using the context id
is that we won't be able to print it in the ppgtt_release trace.
Regarding the client, my idea is to print the task pid and/or the
process name; would that look good for you?

> > -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

Thanks,
-- Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] drm/i915: Add ppgtt init/release trace points
  2014-06-19 15:41     ` Daniele Ceraolo Spurio
@ 2014-06-19 16:05       ` Chris Wilson
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2014-06-19 16:05 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

On Thu, Jun 19, 2014 at 04:41:50PM +0100, Daniele Ceraolo Spurio wrote:
> Regarding the client, my idea is to print the task pid and/or the
> process name; would that look good for you?

Going off an tangent. We have a problem with file->pid. In the dri3,
or rendernodes, future all pids belong to the master. :(

Suggestions? Complete solutions?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point
  2014-07-01 16:24 daniele.ceraolospurio
@ 2014-07-01 16:24 ` daniele.ceraolospurio
  0 siblings, 0 replies; 8+ messages in thread
From: daniele.ceraolospurio @ 2014-07-01 16:24 UTC (permalink / raw)
  To: intel-gfx

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

The context used to execute a batchbuffer is becoming increasingly
important.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
 drivers/gpu/drm/i915/i915_trace.h          | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d815ef5..0b2b76e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1372,7 +1372,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 	}
 
-	trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
+	trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags, ctx);
 
 	i915_gem_execbuffer_move_to_active(&eb->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..4e73e3a 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -352,14 +352,16 @@ TRACE_EVENT(i915_gem_ring_sync_to,
 );
 
 TRACE_EVENT(i915_gem_ring_dispatch,
-	    TP_PROTO(struct intel_engine_cs *ring, u32 seqno, u32 flags),
-	    TP_ARGS(ring, seqno, flags),
+	    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(
@@ -367,11 +369,13 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 			   __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",
-		      __entry->dev, __entry->ring, __entry->seqno, __entry->flags)
+	    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,
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-07-01 16:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-18 16:16 [PATCH 1/3] drm/i915: Add ppgtt init/release trace points oscar.mateo
2014-06-18 16:16 ` [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point oscar.mateo
2014-06-18 16:16 ` [PATCH 3/3] drm/i915: Trace point callbacks for validation oscar.mateo
2014-06-19  7:56 ` [PATCH 1/3] drm/i915: Add ppgtt init/release trace points Chris Wilson
2014-06-19 14:22   ` Mateo Lozano, Oscar
2014-06-19 15:41     ` Daniele Ceraolo Spurio
2014-06-19 16:05       ` Chris Wilson
2014-07-01 16:24 daniele.ceraolospurio
2014-07-01 16:24 ` [PATCH 2/3] drm/i915: Add ctx param to i915_gem_ring_dispatch trace point daniele.ceraolospurio

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.