public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v3] drm/i915: Add ppgtt create/release trace points
@ 2014-09-25 16:10 daniele.ceraolospurio
  2014-09-29 13:06 ` Daniel Vetter
  2014-10-22 13:28 ` [PATCH v4] " daniele.ceraolospurio
  0 siblings, 2 replies; 16+ messages in thread
From: daniele.ceraolospurio @ 2014-09-25 16:10 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: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++++
 drivers/gpu/drm/i915/i915_trace.h   | 40 +++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index be0aa29..5577e86 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
 
 	ppgtt->file_priv = fpriv;
 
+	trace_i915_ppgtt_create(ppgtt);
+
 	return ppgtt;
 }
 
@@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
 	struct i915_hw_ppgtt *ppgtt =
 		container_of(kref, struct i915_hw_ppgtt, ref);
 
+	trace_i915_ppgtt_release(ppgtt);
+
 	/* vmas should already be unbound */
 	WARN_ON(!list_empty(&ppgtt->base.active_list));
 	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..ca84c49 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -587,6 +587,46 @@ TRACE_EVENT(intel_gpu_freq_change,
 	    TP_printk("new_freq=%u", __entry->freq)
 );
 
+TRACE_EVENT(i915_ppgtt_create,
+	TP_PROTO(struct i915_hw_ppgtt *ppgtt),
+
+	TP_ARGS(ppgtt),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+			__field(u32, pid)
+	),
+
+	TP_fast_assign(
+			__entry->vm = &ppgtt->base;
+			__entry->dev = ppgtt->base.dev->primary->index;
+			__entry->pid = (unsigned int)task_pid_nr(current);
+	),
+
+	TP_printk("dev=%u, task_pid=%u, vm=%p",
+		  __entry->dev, __entry->pid, __entry->vm)
+);
+
+TRACE_EVENT(i915_ppgtt_release,
+
+	TP_PROTO(struct i915_hw_ppgtt *ppgtt),
+
+	TP_ARGS(ppgtt),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+		__entry->vm = &ppgtt->base;
+		__entry->dev = ppgtt->base.dev->primary->index;
+	),
+
+	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
-- 
1.8.5.2

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

* Re: [PATCH v3] drm/i915: Add ppgtt create/release trace points
  2014-09-25 16:10 [PATCH v3] drm/i915: Add ppgtt create/release trace points daniele.ceraolospurio
@ 2014-09-29 13:06 ` Daniel Vetter
  2014-10-22 13:28 ` [PATCH v4] " daniele.ceraolospurio
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-09-29 13:06 UTC (permalink / raw)
  To: daniele.ceraolospurio; +Cc: intel-gfx

On Thu, Sep 25, 2014 at 05:10:57PM +0100, daniele.ceraolospurio@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: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Ok, I think this is an excellent opportunity to properly document the
tracepoints we already have. Since kerneldocs sucks I guess we need to do
it all with DOC: comments that get manually pulled in, with an overview
comment at the top of i915_trace.h.

That then also leaves you with a nice place to explain a bit what this
should be used for.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++++
>  drivers/gpu/drm/i915/i915_trace.h   | 40 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index be0aa29..5577e86 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
>  
>  	ppgtt->file_priv = fpriv;
>  
> +	trace_i915_ppgtt_create(ppgtt);
> +
>  	return ppgtt;
>  }
>  
> @@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
>  	struct i915_hw_ppgtt *ppgtt =
>  		container_of(kref, struct i915_hw_ppgtt, ref);
>  
> +	trace_i915_ppgtt_release(ppgtt);
> +
>  	/* vmas should already be unbound */
>  	WARN_ON(!list_empty(&ppgtt->base.active_list));
>  	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index f5aa006..ca84c49 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -587,6 +587,46 @@ TRACE_EVENT(intel_gpu_freq_change,
>  	    TP_printk("new_freq=%u", __entry->freq)
>  );
>  
> +TRACE_EVENT(i915_ppgtt_create,
> +	TP_PROTO(struct i915_hw_ppgtt *ppgtt),
> +
> +	TP_ARGS(ppgtt),
> +
> +	TP_STRUCT__entry(
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +			__field(u32, pid)
> +	),
> +
> +	TP_fast_assign(
> +			__entry->vm = &ppgtt->base;
> +			__entry->dev = ppgtt->base.dev->primary->index;
> +			__entry->pid = (unsigned int)task_pid_nr(current);
> +	),
> +
> +	TP_printk("dev=%u, task_pid=%u, vm=%p",
> +		  __entry->dev, __entry->pid, __entry->vm)
> +);
> +
> +TRACE_EVENT(i915_ppgtt_release,
> +
> +	TP_PROTO(struct i915_hw_ppgtt *ppgtt),
> +
> +	TP_ARGS(ppgtt),
> +
> +	TP_STRUCT__entry(
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vm = &ppgtt->base;
> +		__entry->dev = ppgtt->base.dev->primary->index;
> +	),
> +
> +	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
> +);
> +
>  #endif /* _I915_TRACE_H_ */
>  
>  /* This part must be outside protection */
> -- 
> 1.8.5.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* [PATCH v4] drm/i915: Add ppgtt create/release trace points
  2014-09-25 16:10 [PATCH v3] drm/i915: Add ppgtt create/release trace points daniele.ceraolospurio
  2014-09-29 13:06 ` Daniel Vetter
@ 2014-10-22 13:28 ` daniele.ceraolospurio
  2014-10-22 15:29   ` Daniel Vetter
  2014-10-24 15:30   ` [PATCH v5] " daniele.ceraolospurio
  1 sibling, 2 replies; 16+ messages in thread
From: daniele.ceraolospurio @ 2014-10-22 13:28 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.

v4: add DOC information

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |  4 +++
 drivers/gpu/drm/i915/i915_trace.h   | 58 +++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e0bcba0..ed9ec67 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
 
 	ppgtt->file_priv = fpriv;
 
+	trace_i915_ppgtt_create(&ppgtt->base);
+
 	return ppgtt;
 }
 
@@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
 	struct i915_hw_ppgtt *ppgtt =
 		container_of(kref, struct i915_hw_ppgtt, ref);
 
+	trace_i915_ppgtt_release(&ppgtt->base);
+
 	/* vmas should already be unbound */
 	WARN_ON(!list_empty(&ppgtt->base.active_list));
 	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..6cf5ca2 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -15,6 +15,14 @@
 #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
 #define TRACE_INCLUDE_FILE i915_trace
 
+/**
+ * DOC: i915 tracepoints
+ *
+ * This file has the fancy tracepoints available in the drm/i915 driver.
+ * I encourage people to update theirs!
+ *
+ */
+
 /* pipe updates */
 
 TRACE_EVENT(i915_pipe_update_start,
@@ -587,6 +595,56 @@ TRACE_EVENT(intel_gpu_freq_change,
 	    TP_printk("new_freq=%u", __entry->freq)
 );
 
+/**
+ * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints
+ *
+ * With full ppgtt enabled each process using drm will allocate at least one
+ * translation table. With these traces it is possible to keep track of the
+ * allocation and of the lifetime of the tables; this can be used during
+ * testing/debug to verify that we are not leaking ppgtts.
+ * These traces identify the ppgtt through the vm pointer, which is also printed
+ * by the i915_vma_bind and i915_vma_unbind tracepoints.
+ */
+TRACE_EVENT(i915_ppgtt_create,
+	TP_PROTO(struct i915_address_space *vm),
+
+	TP_ARGS(vm),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+			__field(int, pid)
+	),
+
+	TP_fast_assign(
+			__entry->vm = vm;
+			__entry->dev = vm->dev->primary->index;
+			__entry->pid = (int)task_pid_nr(current);
+	),
+
+	TP_printk("dev=%u, task_pid=%d, vm=%p",
+		  __entry->dev, __entry->pid, __entry->vm)
+);
+
+TRACE_EVENT(i915_ppgtt_release,
+
+	TP_PROTO(struct i915_address_space *vm),
+
+	TP_ARGS(vm),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+		__entry->vm = vm;
+		__entry->dev = vm->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
-- 
1.8.5.2

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

* Re: [PATCH v4] drm/i915: Add ppgtt create/release trace points
  2014-10-22 13:28 ` [PATCH v4] " daniele.ceraolospurio
@ 2014-10-22 15:29   ` Daniel Vetter
  2014-10-24 15:30   ` [PATCH v5] " daniele.ceraolospurio
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-10-22 15:29 UTC (permalink / raw)
  To: daniele.ceraolospurio; +Cc: intel-gfx

On Wed, Oct 22, 2014 at 02:28:45PM +0100, daniele.ceraolospurio@intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> These tracepoints are useful for observing the creation and
> destruction of Full PPGTTs.
> 
> v4: add DOC information
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  4 +++
>  drivers/gpu/drm/i915/i915_trace.h   | 58 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index e0bcba0..ed9ec67 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
>  
>  	ppgtt->file_priv = fpriv;
>  
> +	trace_i915_ppgtt_create(&ppgtt->base);
> +
>  	return ppgtt;
>  }
>  
> @@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
>  	struct i915_hw_ppgtt *ppgtt =
>  		container_of(kref, struct i915_hw_ppgtt, ref);
>  
> +	trace_i915_ppgtt_release(&ppgtt->base);
> +
>  	/* vmas should already be unbound */
>  	WARN_ON(!list_empty(&ppgtt->base.active_list));
>  	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index f5aa006..6cf5ca2 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -15,6 +15,14 @@
>  #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
>  #define TRACE_INCLUDE_FILE i915_trace
>  
> +/**
> + * DOC: i915 tracepoints
> + *
> + * This file has the fancy tracepoints available in the drm/i915 driver.
> + * I encourage people to update theirs!

I think we cand drop this part, but to make these DOC sections really
useful you also need to pull them into a new "Tracing" section in the i915
part of the drm DocBook. See Documentation/DocBook/drm.tmpl and lock at
the recent commits from i915 people for examples.
-Daniel

> + *
> + */
> +
>  /* pipe updates */
>  
>  TRACE_EVENT(i915_pipe_update_start,
> @@ -587,6 +595,56 @@ TRACE_EVENT(intel_gpu_freq_change,
>  	    TP_printk("new_freq=%u", __entry->freq)
>  );
>  
> +/**
> + * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints
> + *
> + * With full ppgtt enabled each process using drm will allocate at least one
> + * translation table. With these traces it is possible to keep track of the
> + * allocation and of the lifetime of the tables; this can be used during
> + * testing/debug to verify that we are not leaking ppgtts.
> + * These traces identify the ppgtt through the vm pointer, which is also printed
> + * by the i915_vma_bind and i915_vma_unbind tracepoints.
> + */
> +TRACE_EVENT(i915_ppgtt_create,
> +	TP_PROTO(struct i915_address_space *vm),
> +
> +	TP_ARGS(vm),
> +
> +	TP_STRUCT__entry(
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +			__field(int, pid)
> +	),
> +
> +	TP_fast_assign(
> +			__entry->vm = vm;
> +			__entry->dev = vm->dev->primary->index;
> +			__entry->pid = (int)task_pid_nr(current);
> +	),
> +
> +	TP_printk("dev=%u, task_pid=%d, vm=%p",
> +		  __entry->dev, __entry->pid, __entry->vm)
> +);
> +
> +TRACE_EVENT(i915_ppgtt_release,
> +
> +	TP_PROTO(struct i915_address_space *vm),
> +
> +	TP_ARGS(vm),
> +
> +	TP_STRUCT__entry(
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vm = vm;
> +		__entry->dev = vm->dev->primary->index;
> +	),
> +
> +	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
> +);
> +
>  #endif /* _I915_TRACE_H_ */
>  
>  /* This part must be outside protection */
> -- 
> 1.8.5.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* [PATCH v5] drm/i915: Add ppgtt create/release trace points
  2014-10-22 13:28 ` [PATCH v4] " daniele.ceraolospurio
  2014-10-22 15:29   ` Daniel Vetter
@ 2014-10-24 15:30   ` daniele.ceraolospurio
  2014-10-27  8:49     ` Chris Wilson
  2014-11-07 17:45     ` [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime daniele.ceraolospurio
  1 sibling, 2 replies; 16+ messages in thread
From: daniele.ceraolospurio @ 2014-10-24 15:30 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.

v4: add DOC information
v5: pull the DOC in drm.tmpl

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 Documentation/DocBook/drm.tmpl      | 13 ++++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.c |  4 +++
 drivers/gpu/drm/i915/i915_trace.h   | 50 +++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index f6a9d7b..a372f52 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3963,6 +3963,19 @@ int num_ioctls;</synopsis>
 !Idrivers/gpu/drm/i915/intel_lrc.c
       </sect2>
     </sect1>
+
+    <sect1>
+      <title> Tracing </title>
+      <para>
+    This sections covers all things related to the tracepoints implemented in
+    the i915 driver.
+      </para>
+      <sect2>
+        <title> i915_ppgtt_create and i915_ppgtt_release </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_ppgtt_create and i915_ppgtt_release tracepoints
+      </sect2>
+    </sect1>
+
   </chapter>
 !Cdrivers/gpu/drm/i915/i915_irq.c
 </part>
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e0bcba0..ed9ec67 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
 
 	ppgtt->file_priv = fpriv;
 
+	trace_i915_ppgtt_create(&ppgtt->base);
+
 	return ppgtt;
 }
 
@@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
 	struct i915_hw_ppgtt *ppgtt =
 		container_of(kref, struct i915_hw_ppgtt, ref);
 
+	trace_i915_ppgtt_release(&ppgtt->base);
+
 	/* vmas should already be unbound */
 	WARN_ON(!list_empty(&ppgtt->base.active_list));
 	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..525be1b 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -587,6 +587,56 @@ TRACE_EVENT(intel_gpu_freq_change,
 	    TP_printk("new_freq=%u", __entry->freq)
 );
 
+/**
+ * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints
+ *
+ * With full ppgtt enabled each process using drm will allocate at least one
+ * translation table. With these traces it is possible to keep track of the
+ * allocation and of the lifetime of the tables; this can be used during
+ * testing/debug to verify that we are not leaking ppgtts.
+ * These traces identify the ppgtt through the vm pointer, which is also printed
+ * by the i915_vma_bind and i915_vma_unbind tracepoints.
+ */
+TRACE_EVENT(i915_ppgtt_create,
+	TP_PROTO(struct i915_address_space *vm),
+
+	TP_ARGS(vm),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+			__field(int, pid)
+	),
+
+	TP_fast_assign(
+			__entry->vm = vm;
+			__entry->dev = vm->dev->primary->index;
+			__entry->pid = (int)task_pid_nr(current);
+	),
+
+	TP_printk("dev=%u, task_pid=%d, vm=%p",
+		  __entry->dev, __entry->pid, __entry->vm)
+);
+
+TRACE_EVENT(i915_ppgtt_release,
+
+	TP_PROTO(struct i915_address_space *vm),
+
+	TP_ARGS(vm),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+		__entry->vm = vm;
+		__entry->dev = vm->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
-- 
1.8.5.2

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

* Re: [PATCH v5] drm/i915: Add ppgtt create/release trace points
  2014-10-24 15:30   ` [PATCH v5] " daniele.ceraolospurio
@ 2014-10-27  8:49     ` Chris Wilson
  2014-10-27 11:03       ` Ceraolo Spurio, Daniele
  2014-11-07 17:45     ` [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime daniele.ceraolospurio
  1 sibling, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2014-10-27  8:49 UTC (permalink / raw)
  To: daniele.ceraolospurio; +Cc: intel-gfx

On Fri, Oct 24, 2014 at 04:30:52PM +0100, daniele.ceraolospurio@intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> These tracepoints are useful for observing the creation and
> destruction of Full PPGTTs.
> 
> v4: add DOC information
> v5: pull the DOC in drm.tmpl
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> +TRACE_EVENT(i915_ppgtt_create,
> +	TP_PROTO(struct i915_address_space *vm),
> +
> +	TP_ARGS(vm),
> +
> +	TP_STRUCT__entry(
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +			__field(int, pid)
> +	),
> +
> +	TP_fast_assign(
> +			__entry->vm = vm;
> +			__entry->dev = vm->dev->primary->index;
> +			__entry->pid = (int)task_pid_nr(current);

This is redundant. Current pid is part of the perf header (iirc at
least). Besides which storing the creator pid is useful elsewhere when
debugging vm (especially as now vm->pid != file->pid).

> +	),
> +
> +	TP_printk("dev=%u, task_pid=%d, vm=%p",
> +		  __entry->dev, __entry->pid, __entry->vm)
> +);
> +
> +TRACE_EVENT(i915_ppgtt_release,
> +
> +	TP_PROTO(struct i915_address_space *vm),
> +
> +	TP_ARGS(vm),
> +
> +	TP_STRUCT__entry(
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vm = vm;
> +		__entry->dev = vm->dev->primary->index;
> +	),
> +
> +	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
> +);

So what about switch_mm (accounting for both execlists/non-execlists)?
ppgtt_close is also another important point in the lifetime, and so you
also want ppgtt_open for symmetry.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v5] drm/i915: Add ppgtt create/release trace points
  2014-10-27  8:49     ` Chris Wilson
@ 2014-10-27 11:03       ` Ceraolo Spurio, Daniele
  2014-10-28  8:47         ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Ceraolo Spurio, Daniele @ 2014-10-27 11:03 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 10/27/2014 8:49 AM, Chris Wilson wrote:
> On Fri, Oct 24, 2014 at 04:30:52PM +0100, daniele.ceraolospurio@intel.com wrote:
>> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>> These tracepoints are useful for observing the creation and
>> destruction of Full PPGTTs.
>>
>> v4: add DOC information
>> v5: pull the DOC in drm.tmpl
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> +TRACE_EVENT(i915_ppgtt_create,
>> +	TP_PROTO(struct i915_address_space *vm),
>> +
>> +	TP_ARGS(vm),
>> +
>> +	TP_STRUCT__entry(
>> +			__field(struct i915_address_space *, vm)
>> +			__field(u32, dev)
>> +			__field(int, pid)
>> +	),
>> +
>> +	TP_fast_assign(
>> +			__entry->vm = vm;
>> +			__entry->dev = vm->dev->primary->index;
>> +			__entry->pid = (int)task_pid_nr(current);
>
> This is redundant. Current pid is part of the perf header (iirc at
> least). Besides which storing the creator pid is useful elsewhere when
> debugging vm (especially as now vm->pid != file->pid).
>

You're right, I'll just remove it.

>> +	),
>> +
>> +	TP_printk("dev=%u, task_pid=%d, vm=%p",
>> +		  __entry->dev, __entry->pid, __entry->vm)
>> +);
>> +
>> +TRACE_EVENT(i915_ppgtt_release,
>> +
>> +	TP_PROTO(struct i915_address_space *vm),
>> +
>> +	TP_ARGS(vm),
>> +
>> +	TP_STRUCT__entry(
>> +			__field(struct i915_address_space *, vm)
>> +			__field(u32, dev)
>> +	),
>> +
>> +	TP_fast_assign(
>> +		__entry->vm = vm;
>> +		__entry->dev = vm->dev->primary->index;
>> +	),
>> +
>> +	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
>> +);
>
> So what about switch_mm (accounting for both execlists/non-execlists)?

I'll add a couple of tracepoints to cover them.

> ppgtt_close is also another important point in the lifetime, and so you
> also want ppgtt_open for symmetry.
> -Chris
>

What do you mean with ppgtt_close and ppgtt_open? I don't see anything 
like that in my local d-i-n tree (pulled this morning).

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

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

* Re: [PATCH v5] drm/i915: Add ppgtt create/release trace points
  2014-10-27 11:03       ` Ceraolo Spurio, Daniele
@ 2014-10-28  8:47         ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2014-10-28  8:47 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele; +Cc: intel-gfx

On Mon, Oct 27, 2014 at 11:03:26AM +0000, Ceraolo Spurio, Daniele wrote:
> On 10/27/2014 8:49 AM, Chris Wilson wrote:
> >On Fri, Oct 24, 2014 at 04:30:52PM +0100, daniele.ceraolospurio@intel.com wrote:
> >>From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >>
> >>These tracepoints are useful for observing the creation and
> >>destruction of Full PPGTTs.
> >>
> >>v4: add DOC information
> >>v5: pull the DOC in drm.tmpl
> >>
> >>Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >>+TRACE_EVENT(i915_ppgtt_create,
> >>+	TP_PROTO(struct i915_address_space *vm),
> >>+
> >>+	TP_ARGS(vm),
> >>+
> >>+	TP_STRUCT__entry(
> >>+			__field(struct i915_address_space *, vm)
> >>+			__field(u32, dev)
> >>+			__field(int, pid)
> >>+	),
> >>+
> >>+	TP_fast_assign(
> >>+			__entry->vm = vm;
> >>+			__entry->dev = vm->dev->primary->index;
> >>+			__entry->pid = (int)task_pid_nr(current);
> >
> >This is redundant. Current pid is part of the perf header (iirc at
> >least). Besides which storing the creator pid is useful elsewhere when
> >debugging vm (especially as now vm->pid != file->pid).
> >
> 
> You're right, I'll just remove it.
> 
> >>+	),
> >>+
> >>+	TP_printk("dev=%u, task_pid=%d, vm=%p",
> >>+		  __entry->dev, __entry->pid, __entry->vm)
> >>+);
> >>+
> >>+TRACE_EVENT(i915_ppgtt_release,
> >>+
> >>+	TP_PROTO(struct i915_address_space *vm),
> >>+
> >>+	TP_ARGS(vm),
> >>+
> >>+	TP_STRUCT__entry(
> >>+			__field(struct i915_address_space *, vm)
> >>+			__field(u32, dev)
> >>+	),
> >>+
> >>+	TP_fast_assign(
> >>+		__entry->vm = vm;
> >>+		__entry->dev = vm->dev->primary->index;
> >>+	),
> >>+
> >>+	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
> >>+);
> >
> >So what about switch_mm (accounting for both execlists/non-execlists)?
> 
> I'll add a couple of tracepoints to cover them.
> 
> >ppgtt_close is also another important point in the lifetime, and so you
> >also want ppgtt_open for symmetry.
> >-Chris
> >
> 
> What do you mean with ppgtt_close and ppgtt_open? I don't see
> anything like that in my local d-i-n tree (pulled this morning).

I was thinking of context open/close. They need tracepoints too. The
issue is that we may hold onto the vm long after the process is closed
so tracing that would be useful.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime
  2014-10-24 15:30   ` [PATCH v5] " daniele.ceraolospurio
  2014-10-27  8:49     ` Chris Wilson
@ 2014-11-07 17:45     ` daniele.ceraolospurio
  2014-11-08  8:44       ` Chris Wilson
  2014-11-10 13:44       ` [PATCH v7] " daniele.ceraolospurio
  1 sibling, 2 replies; 16+ messages in thread
From: daniele.ceraolospurio @ 2014-11-07 17:45 UTC (permalink / raw)
  To: intel-gfx

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

- ppgtt init/release: these tracepoints are useful for observing the
  creation and destruction of Full PPGTTs.

- ctx create/free: we can use the ctx_free trace in combination with the
  ppgtt_release one to be sure that the ppgtt doesn't stay alive for too
  long after the ctx is destroyed. ctx_create is there for simmetry

- switch_mm: important point in the lifetime of the vm

- execlist_submit_context: a switch_mm is automatically performed during
  the ctx switch (this is the "execlist version" of the switch_mm trace)

v4: add DOC information
v5: pull the DOC in drm.tmpl
v6: clean ppgtt init/release traces + add ctx create/free and switch_mm
    tracepoints (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 Documentation/DocBook/drm.tmpl          |  25 ++++++
 drivers/gpu/drm/i915/i915_gem_context.c |   6 ++
 drivers/gpu/drm/i915/i915_gem_gtt.c     |   4 +
 drivers/gpu/drm/i915/i915_trace.h       | 139 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        |   3 +
 5 files changed, 177 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7277a7f..16e67eb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3994,6 +3994,31 @@ int num_ioctls;</synopsis>
 !Idrivers/gpu/drm/i915/intel_lrc.c
       </sect2>
     </sect1>
+
+    <sect1>
+      <title> Tracing </title>
+      <para>
+    This sections covers all things related to the tracepoints implemented in
+    the i915 driver.
+      </para>
+      <sect2>
+        <title> i915_ppgtt_create and i915_ppgtt_release </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_ppgtt_create and i915_ppgtt_release tracepoints
+      </sect2>
+      <sect2>
+        <title> i915_context_create and i915_context_free </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_context_create and i915_context_free tracepoints
+      </sect2>
+      <sect2>
+        <title> switch_mm </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h switch_mm tracepoint
+      </sect2>
+      <sect2>
+        <title> execlist_submit_context </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h execlist_submit_context tracepoint
+      </sect2>
+    </sect1>
+
   </chapter>
 !Cdrivers/gpu/drm/i915/i915_irq.c
 </part>
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 7d32571..1fb0000 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -88,6 +88,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "i915_trace.h"
 
 /* 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
@@ -137,6 +138,8 @@ void i915_gem_context_free(struct kref *ctx_ref)
 	struct intel_context *ctx = container_of(ctx_ref,
 						 typeof(*ctx), ref);
 
+	trace_i915_context_free(ctx);
+
 	if (i915.enable_execlists)
 		intel_lr_context_free(ctx);
 
@@ -274,6 +277,8 @@ i915_gem_create_context(struct drm_device *dev,
 		ctx->ppgtt = ppgtt;
 	}
 
+	trace_i915_context_create(ctx);
+
 	return ctx;
 
 err_unpin:
@@ -549,6 +554,7 @@ static int do_switch(struct intel_engine_cs *ring,
 	from = ring->last_context;
 
 	if (to->ppgtt) {
+		trace_switch_mm(ring, to);
 		ret = to->ppgtt->switch_mm(to->ppgtt, ring);
 		if (ret)
 			goto unpin_out;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c1cf332..7737e55 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
 
 	ppgtt->file_priv = fpriv;
 
+	trace_i915_ppgtt_create(&ppgtt->base);
+
 	return ppgtt;
 }
 
@@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
 	struct i915_hw_ppgtt *ppgtt =
 		container_of(kref, struct i915_hw_ppgtt, ref);
 
+	trace_i915_ppgtt_release(&ppgtt->base);
+
 	/* vmas should already be unbound */
 	WARN_ON(!list_empty(&ppgtt->base.active_list));
 	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..1d25018 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -587,6 +587,145 @@ TRACE_EVENT(intel_gpu_freq_change,
 	    TP_printk("new_freq=%u", __entry->freq)
 );
 
+/**
+ * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints
+ *
+ * With full ppgtt enabled each process using drm will allocate at least one
+ * translation table. With these traces it is possible to keep track of the
+ * allocation and of the lifetime of the tables; this can be used during
+ * testing/debug to verify that we are not leaking ppgtts.
+ * These traces identify the ppgtt through the vm pointer, which is also printed
+ * by the i915_vma_bind and i915_vma_unbind tracepoints.
+ */
+DECLARE_EVENT_CLASS(i915_ppgtt,
+	TP_PROTO(struct i915_address_space *vm),
+	TP_ARGS(vm),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+			__entry->vm = vm;
+			__entry->dev = vm->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
+)
+
+DEFINE_EVENT(i915_ppgtt, i915_ppgtt_create,
+	TP_PROTO(struct i915_address_space *vm),
+	TP_ARGS(vm)
+);
+
+DEFINE_EVENT(i915_ppgtt, i915_ppgtt_release,
+	TP_PROTO(struct i915_address_space *vm),
+	TP_ARGS(vm)
+);
+
+/**
+ * DOC: i915_context_create and i915_context_free tracepoints
+ *
+ * These tracepoints are used to track creation and deletion of contexts.
+ * If full ppgtt is enabled, they also print the address of the vm assigned to
+ * the context.
+ */
+DECLARE_EVENT_CLASS(i915_context,
+	TP_PROTO(struct intel_context *ctx),
+	TP_ARGS(ctx),
+
+	TP_STRUCT__entry(
+			__field(u32, dev)
+			__field(struct intel_context *, ctx)
+			__field(struct i915_address_space *, vm)
+	),
+
+	TP_fast_assign(
+			__entry->ctx = ctx;
+			__entry->vm = ctx->ppgtt ? &ctx->ppgtt->base : NULL;
+			__entry->dev = ctx->file_priv->dev_priv->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, ctx=%p, ctx_vm=%p",
+		  __entry->dev, __entry->ctx, __entry->vm)
+)
+
+DEFINE_EVENT(i915_context, i915_context_create,
+	TP_PROTO(struct intel_context *ctx),
+	TP_ARGS(ctx)
+);
+
+DEFINE_EVENT(i915_context, i915_context_free,
+	TP_PROTO(struct intel_context *ctx),
+	TP_ARGS(ctx)
+);
+
+/**
+ * DOC: switch_mm tracepoint
+ *
+ * This tracepoint allows tracking of the mm switch, which is an important point
+ * in the lifetime of the vm in the legacy submission path. This tracepoint is
+ * called only if full ppgtt is enabled.
+ */
+TRACE_EVENT(switch_mm,
+	TP_PROTO(struct intel_engine_cs *ring, struct intel_context *to),
+
+	TP_ARGS(ring, to),
+
+	TP_STRUCT__entry(
+			__field(u32, ring)
+			__field(struct intel_context *, to)
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+			__entry->ring = ring->id;
+			__entry->to = to;
+			__entry->vm = to->ppgtt? &to->ppgtt->base : NULL;
+			__entry->dev = ring->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, ring=%u, ctx=%p, ctx_vm=%p",
+		  __entry->dev, __entry->ring, __entry->to, __entry->vm)
+);
+
+/**
+ * DOC: execlist_submit_context tracepoint
+ *
+ * These tracepoint are used to track the contexts that are submitted to the
+ * ring. An mm switch is automatically performed by the GPU during the context
+ * switch. Given the fact that the mm switch is an important point in the
+ * lifetime of a vm, the vm assigned to the context is also printed by the
+ * tracepoint when full ppgtt is enabled.
+ */
+TRACE_EVENT(execlists_submit_context,
+	TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct intel_context *to),
+
+	TP_ARGS(ring, to_num, to),
+
+	TP_STRUCT__entry(
+			__field(u32, ring)
+			__field(u32, to_num)
+			__field(struct intel_context *, to)
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+			__entry->ring = ring->id;
+			__entry->to_num = to_num;
+			__entry->to = to;
+			__entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
+			__entry->dev = ring->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
+		  __entry->dev, __entry->ring,
+		  __entry->to_num, __entry->to, __entry->vm)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6025ac7..e72759d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -135,6 +135,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "i915_trace.h"
 
 #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
 #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
@@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
 	BUG_ON(!ctx_obj0);
 	WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
 
+	trace_execlists_submit_context(ring, 0, to0);
 	execlists_ctx_write_tail(ctx_obj0, tail0);
 
 	if (to1) {
@@ -374,6 +376,7 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
 		BUG_ON(!ctx_obj1);
 		WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
 
+		trace_execlists_submit_context(ring, 1, to1);
 		execlists_ctx_write_tail(ctx_obj1, tail1);
 	}
 
-- 
1.8.5.2

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

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

* Re: [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime
  2014-11-07 17:45     ` [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime daniele.ceraolospurio
@ 2014-11-08  8:44       ` Chris Wilson
       [not found]         ` <5460A438.5080804@intel.com>
  2014-11-10 13:44       ` [PATCH v7] " daniele.ceraolospurio
  1 sibling, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2014-11-08  8:44 UTC (permalink / raw)
  To: daniele.ceraolospurio; +Cc: intel-gfx

On Fri, Nov 07, 2014 at 05:45:01PM +0000, daniele.ceraolospurio@intel.com wrote:
> +/**
> + * DOC: execlist_submit_context tracepoint
> + *
> + * These tracepoint are used to track the contexts that are submitted to the
> + * ring. An mm switch is automatically performed by the GPU during the context
> + * switch. Given the fact that the mm switch is an important point in the
> + * lifetime of a vm, the vm assigned to the context is also printed by the
> + * tracepoint when full ppgtt is enabled.
> + */
> +TRACE_EVENT(execlists_submit_context,
> +	TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct intel_context *to),
> +
> +	TP_ARGS(ring, to_num, to),
> +
> +	TP_STRUCT__entry(
> +			__field(u32, ring)
> +			__field(u32, to_num)
> +			__field(struct intel_context *, to)
> +			__field(struct i915_address_space *, vm)
> +			__field(u32, dev)
> +	),
> +
> +	TP_fast_assign(
> +			__entry->ring = ring->id;
> +			__entry->to_num = to_num;
> +			__entry->to = to;
> +			__entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
> +			__entry->dev = ring->dev->primary->index;
> +	),
> +
> +	TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
> +		  __entry->dev, __entry->ring,
> +		  __entry->to_num, __entry->to, __entry->vm)
> +);
> +
>  #endif /* _I915_TRACE_H_ */
>  
>  /* This part must be outside protection */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6025ac7..e72759d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -135,6 +135,7 @@
>  #include <drm/drmP.h>
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "i915_trace.h"
>  
>  #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
>  #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
> @@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
>  	BUG_ON(!ctx_obj0);
>  	WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
>  
> +	trace_execlists_submit_context(ring, 0, to0);
>  	execlists_ctx_write_tail(ctx_obj0, tail0);

This is very tenuous. This is not part of the context lifetime but of
the request.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime
       [not found]         ` <5460A438.5080804@intel.com>
@ 2014-11-10 11:43           ` Ceraolo Spurio, Daniele
       [not found]           ` <20141110115431.GC22109@nuc-i3427.alporthouse.com>
  1 sibling, 0 replies; 16+ messages in thread
From: Ceraolo Spurio, Daniele @ 2014-11-10 11:43 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 11/10/2014 11:40 AM, Ceraolo Spurio, Daniele wrote:
> On 11/8/2014 8:44 AM, Chris Wilson wrote:
>> On Fri, Nov 07, 2014 at 05:45:01PM +0000,
>> daniele.ceraolospurio@intel.com wrote:
>>> +/**
>>> + * DOC: execlist_submit_context tracepoint
>>> + *
>>> + * These tracepoint are used to track the contexts that are
>>> submitted to the
>>> + * ring. An mm switch is automatically performed by the GPU during
>>> the context
>>> + * switch. Given the fact that the mm switch is an important point
>>> in the
>>> + * lifetime of a vm, the vm assigned to the context is also printed
>>> by the
>>> + * tracepoint when full ppgtt is enabled.
>>> + */
>>> +TRACE_EVENT(execlists_submit_context,
>>> +    TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct
>>> intel_context *to),
>>> +
>>> +    TP_ARGS(ring, to_num, to),
>>> +
>>> +    TP_STRUCT__entry(
>>> +            __field(u32, ring)
>>> +            __field(u32, to_num)
>>> +            __field(struct intel_context *, to)
>>> +            __field(struct i915_address_space *, vm)
>>> +            __field(u32, dev)
>>> +    ),
>>> +
>>> +    TP_fast_assign(
>>> +            __entry->ring = ring->id;
>>> +            __entry->to_num = to_num;
>>> +            __entry->to = to;
>>> +            __entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
>>> +            __entry->dev = ring->dev->primary->index;
>>> +    ),
>>> +
>>> +    TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
>>> +          __entry->dev, __entry->ring,
>>> +          __entry->to_num, __entry->to, __entry->vm)
>>> +);
>>> +
>>>   #endif /* _I915_TRACE_H_ */
>>>
>>>   /* This part must be outside protection */
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 6025ac7..e72759d 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -135,6 +135,7 @@
>>>   #include <drm/drmP.h>
>>>   #include <drm/i915_drm.h>
>>>   #include "i915_drv.h"
>>> +#include "i915_trace.h"
>>>
>>>   #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
>>>   #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
>>> @@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct
>>> intel_engine_cs *ring,
>>>       BUG_ON(!ctx_obj0);
>>>       WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
>>>
>>> +    trace_execlists_submit_context(ring, 0, to0);
>>>       execlists_ctx_write_tail(ctx_obj0, tail0);
>>
>> This is very tenuous. This is not part of the context lifetime but of
>> the request.
>> -Chris
>>
>
> The aim wasn't to track the context or the request but to track the
> switch_mm. considering that in execlist mode that is automatically done
> by the GPU when the ctx is moved on the ring, this looked like a good
> place to put the trace. What were you exactly concerned about?
>
> Thanks,
> Daniele
>

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

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

* Re: [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime
       [not found]           ` <20141110115431.GC22109@nuc-i3427.alporthouse.com>
@ 2014-11-10 12:28             ` Ceraolo Spurio, Daniele
  2014-11-10 12:34               ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Ceraolo Spurio, Daniele @ 2014-11-10 12:28 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 11/10/2014 11:54 AM, Chris Wilson wrote:
> On Mon, Nov 10, 2014 at 11:40:40AM +0000, Ceraolo Spurio, Daniele wrote:
>> On 11/8/2014 8:44 AM, Chris Wilson wrote:
>>> On Fri, Nov 07, 2014 at 05:45:01PM +0000, daniele.ceraolospurio@intel.com wrote:
>>>> +/**
>>>> + * DOC: execlist_submit_context tracepoint
>>>> + *
>>>> + * These tracepoint are used to track the contexts that are submitted to the
>>>> + * ring. An mm switch is automatically performed by the GPU during the context
>>>> + * switch. Given the fact that the mm switch is an important point in the
>>>> + * lifetime of a vm, the vm assigned to the context is also printed by the
>>>> + * tracepoint when full ppgtt is enabled.
>>>> + */
>>>> +TRACE_EVENT(execlists_submit_context,
>>>> +	TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct intel_context *to),
>>>> +
>>>> +	TP_ARGS(ring, to_num, to),
>>>> +
>>>> +	TP_STRUCT__entry(
>>>> +			__field(u32, ring)
>>>> +			__field(u32, to_num)
>>>> +			__field(struct intel_context *, to)
>>>> +			__field(struct i915_address_space *, vm)
>>>> +			__field(u32, dev)
>>>> +	),
>>>> +
>>>> +	TP_fast_assign(
>>>> +			__entry->ring = ring->id;
>>>> +			__entry->to_num = to_num;
>>>> +			__entry->to = to;
>>>> +			__entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
>>>> +			__entry->dev = ring->dev->primary->index;
>>>> +	),
>>>> +
>>>> +	TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
>>>> +		  __entry->dev, __entry->ring,
>>>> +		  __entry->to_num, __entry->to, __entry->vm)
>>>> +);
>>>> +
>>>>   #endif /* _I915_TRACE_H_ */
>>>>
>>>>   /* This part must be outside protection */
>>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>>> index 6025ac7..e72759d 100644
>>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>> @@ -135,6 +135,7 @@
>>>>   #include <drm/drmP.h>
>>>>   #include <drm/i915_drm.h>
>>>>   #include "i915_drv.h"
>>>> +#include "i915_trace.h"
>>>>
>>>>   #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
>>>>   #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
>>>> @@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
>>>>   	BUG_ON(!ctx_obj0);
>>>>   	WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
>>>>
>>>> +	trace_execlists_submit_context(ring, 0, to0);
>>>>   	execlists_ctx_write_tail(ctx_obj0, tail0);
>>>
>>> This is very tenuous. This is not part of the context lifetime but of
>>> the request.
>>> -Chris
>>>
>>
>> The aim wasn't to track the context or the request but to track the
>> switch_mm. considering that in execlist mode that is automatically
>> done by the GPU when the ctx is moved on the ring, this looked like
>> a good place to put the trace. What were you exactly concerned
>> about?
>
> I have a different pattern for the lifetimes in my head. In terms of
> usage tracking, when the context is submitted is to hardware is more or
> less irrelevant to the lifetime per se - it is interesting, but only
> in the context of tracking the execlists/scheduler.
>
> For the context, the important point is when a new request (e.g.
> execbuffer) is created from that context, which will then keep that
> context alive until the request is complete. That's my
> switch_mm/switch_context equivalent. (I think I have refined my stance a
> lot since working with the contexts and requests).
>
> The other perspective, is that I want the context tracepoints to be
> actions on the contexts themselves, rather than actions on the hardware
> which deserve to be in a different domain. (Obviously in the overlap,
> there will be some arguing as to which domain it best fits in.)
> I am trying to keep the tracepoints somewhat logically organised. :|
> -Chris
>

+ intel-gfx, which I've inadvertently removed in my previous reply.

I now understand what you meant and I can see your point. However, since 
I'm still getting familiar with the handling of contexts in the driver 
(execlists etc), I'll have to dig a bit more to find the right places 
for tracepoints. Would it be ok for you if I were to drop the 
submit_context trace for now and just go on with the others? I'll get 
back to this part when I feel more comfortable with my understanding of 
the code.

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

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

* Re: [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime
  2014-11-10 12:28             ` Ceraolo Spurio, Daniele
@ 2014-11-10 12:34               ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2014-11-10 12:34 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele; +Cc: intel-gfx

On Mon, Nov 10, 2014 at 12:28:11PM +0000, Ceraolo Spurio, Daniele wrote:
> On 11/10/2014 11:54 AM, Chris Wilson wrote:
> >On Mon, Nov 10, 2014 at 11:40:40AM +0000, Ceraolo Spurio, Daniele wrote:
> >>On 11/8/2014 8:44 AM, Chris Wilson wrote:
> >>>On Fri, Nov 07, 2014 at 05:45:01PM +0000, daniele.ceraolospurio@intel.com wrote:
> >>>>+/**
> >>>>+ * DOC: execlist_submit_context tracepoint
> >>>>+ *
> >>>>+ * These tracepoint are used to track the contexts that are submitted to the
> >>>>+ * ring. An mm switch is automatically performed by the GPU during the context
> >>>>+ * switch. Given the fact that the mm switch is an important point in the
> >>>>+ * lifetime of a vm, the vm assigned to the context is also printed by the
> >>>>+ * tracepoint when full ppgtt is enabled.
> >>>>+ */
> >>>>+TRACE_EVENT(execlists_submit_context,
> >>>>+	TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct intel_context *to),
> >>>>+
> >>>>+	TP_ARGS(ring, to_num, to),
> >>>>+
> >>>>+	TP_STRUCT__entry(
> >>>>+			__field(u32, ring)
> >>>>+			__field(u32, to_num)
> >>>>+			__field(struct intel_context *, to)
> >>>>+			__field(struct i915_address_space *, vm)
> >>>>+			__field(u32, dev)
> >>>>+	),
> >>>>+
> >>>>+	TP_fast_assign(
> >>>>+			__entry->ring = ring->id;
> >>>>+			__entry->to_num = to_num;
> >>>>+			__entry->to = to;
> >>>>+			__entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
> >>>>+			__entry->dev = ring->dev->primary->index;
> >>>>+	),
> >>>>+
> >>>>+	TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
> >>>>+		  __entry->dev, __entry->ring,
> >>>>+		  __entry->to_num, __entry->to, __entry->vm)
> >>>>+);
> >>>>+
> >>>>  #endif /* _I915_TRACE_H_ */
> >>>>
> >>>>  /* This part must be outside protection */
> >>>>diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >>>>index 6025ac7..e72759d 100644
> >>>>--- a/drivers/gpu/drm/i915/intel_lrc.c
> >>>>+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >>>>@@ -135,6 +135,7 @@
> >>>>  #include <drm/drmP.h>
> >>>>  #include <drm/i915_drm.h>
> >>>>  #include "i915_drv.h"
> >>>>+#include "i915_trace.h"
> >>>>
> >>>>  #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
> >>>>  #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
> >>>>@@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
> >>>>  	BUG_ON(!ctx_obj0);
> >>>>  	WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
> >>>>
> >>>>+	trace_execlists_submit_context(ring, 0, to0);
> >>>>  	execlists_ctx_write_tail(ctx_obj0, tail0);
> >>>
> >>>This is very tenuous. This is not part of the context lifetime but of
> >>>the request.
> >>>-Chris
> >>>
> >>
> >>The aim wasn't to track the context or the request but to track the
> >>switch_mm. considering that in execlist mode that is automatically
> >>done by the GPU when the ctx is moved on the ring, this looked like
> >>a good place to put the trace. What were you exactly concerned
> >>about?
> >
> >I have a different pattern for the lifetimes in my head. In terms of
> >usage tracking, when the context is submitted is to hardware is more or
> >less irrelevant to the lifetime per se - it is interesting, but only
> >in the context of tracking the execlists/scheduler.
> >
> >For the context, the important point is when a new request (e.g.
> >execbuffer) is created from that context, which will then keep that
> >context alive until the request is complete. That's my
> >switch_mm/switch_context equivalent. (I think I have refined my stance a
> >lot since working with the contexts and requests).
> >
> >The other perspective, is that I want the context tracepoints to be
> >actions on the contexts themselves, rather than actions on the hardware
> >which deserve to be in a different domain. (Obviously in the overlap,
> >there will be some arguing as to which domain it best fits in.)
> >I am trying to keep the tracepoints somewhat logically organised. :|
> >-Chris
> >
> 
> + intel-gfx, which I've inadvertently removed in my previous reply.
> 
> I now understand what you meant and I can see your point. However,
> since I'm still getting familiar with the handling of contexts in
> the driver (execlists etc), I'll have to dig a bit more to find the
> right places for tracepoints. Would it be ok for you if I were to
> drop the submit_context trace for now and just go on with the
> others? I'll get back to this part when I feel more comfortable with
> my understanding of the code.

Yes, all the others looked good to me. Please just drop the contentious
tracepoint inside execlists, and slap on my

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

(we will end up one in execlists, just probably as part of a different
set of tracepoints ;)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v7] drm/i915: Add tracepoints to track a vm during its lifetime
  2014-11-07 17:45     ` [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime daniele.ceraolospurio
  2014-11-08  8:44       ` Chris Wilson
@ 2014-11-10 13:44       ` daniele.ceraolospurio
  2014-11-11  3:00         ` [PATCH v7] drm/i915: Add tracepoints to track a vm shuang.he
  2014-11-11  9:21         ` [PATCH v7] drm/i915: Add tracepoints to track a vm during its lifetime Daniel Vetter
  1 sibling, 2 replies; 16+ messages in thread
From: daniele.ceraolospurio @ 2014-11-10 13:44 UTC (permalink / raw)
  To: intel-gfx

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

- ppgtt init/release: these tracepoints are useful for observing the
  creation and destruction of Full PPGTTs.

- ctx create/free: we can use the ctx_free trace in combination with the
  ppgtt_release one to be sure that the ppgtt doesn't stay alive for too
  long after the ctx is destroyed. ctx_create is there for simmetry

- switch_mm: important point in the lifetime of the vm

v4: add DOC information
v5: pull the DOC in drm.tmpl
v6: clean ppgtt init/release traces + add ctx create/free and switch_mm
    tracepoints (Chris)
v7: drop execlist_submit_context tracepoint

Cc: Chris Wilson <chris@chris-wilson.co.uk>

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 Documentation/DocBook/drm.tmpl          |  21 +++++++
 drivers/gpu/drm/i915/i915_gem_context.c |   6 ++
 drivers/gpu/drm/i915/i915_gem_gtt.c     |   4 ++
 drivers/gpu/drm/i915/i915_trace.h       | 104 ++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7277a7f..0f485a0 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3994,6 +3994,27 @@ int num_ioctls;</synopsis>
 !Idrivers/gpu/drm/i915/intel_lrc.c
       </sect2>
     </sect1>
+
+    <sect1>
+      <title> Tracing </title>
+      <para>
+    This sections covers all things related to the tracepoints implemented in
+    the i915 driver.
+      </para>
+      <sect2>
+        <title> i915_ppgtt_create and i915_ppgtt_release </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_ppgtt_create and i915_ppgtt_release tracepoints
+      </sect2>
+      <sect2>
+        <title> i915_context_create and i915_context_free </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_context_create and i915_context_free tracepoints
+      </sect2>
+      <sect2>
+        <title> switch_mm </title>
+!Pdrivers/gpu/drm/i915/i915_trace.h switch_mm tracepoint
+      </sect2>
+    </sect1>
+
   </chapter>
 !Cdrivers/gpu/drm/i915/i915_irq.c
 </part>
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 7d32571..1fb0000 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -88,6 +88,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "i915_trace.h"
 
 /* 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
@@ -137,6 +138,8 @@ void i915_gem_context_free(struct kref *ctx_ref)
 	struct intel_context *ctx = container_of(ctx_ref,
 						 typeof(*ctx), ref);
 
+	trace_i915_context_free(ctx);
+
 	if (i915.enable_execlists)
 		intel_lr_context_free(ctx);
 
@@ -274,6 +277,8 @@ i915_gem_create_context(struct drm_device *dev,
 		ctx->ppgtt = ppgtt;
 	}
 
+	trace_i915_context_create(ctx);
+
 	return ctx;
 
 err_unpin:
@@ -549,6 +554,7 @@ static int do_switch(struct intel_engine_cs *ring,
 	from = ring->last_context;
 
 	if (to->ppgtt) {
+		trace_switch_mm(ring, to);
 		ret = to->ppgtt->switch_mm(to->ppgtt, ring);
 		if (ret)
 			goto unpin_out;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c1cf332..7737e55 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
 
 	ppgtt->file_priv = fpriv;
 
+	trace_i915_ppgtt_create(&ppgtt->base);
+
 	return ppgtt;
 }
 
@@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
 	struct i915_hw_ppgtt *ppgtt =
 		container_of(kref, struct i915_hw_ppgtt, ref);
 
+	trace_i915_ppgtt_release(&ppgtt->base);
+
 	/* vmas should already be unbound */
 	WARN_ON(!list_empty(&ppgtt->base.active_list));
 	WARN_ON(!list_empty(&ppgtt->base.inactive_list));
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..751d4ad 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -587,6 +587,110 @@ TRACE_EVENT(intel_gpu_freq_change,
 	    TP_printk("new_freq=%u", __entry->freq)
 );
 
+/**
+ * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints
+ *
+ * With full ppgtt enabled each process using drm will allocate at least one
+ * translation table. With these traces it is possible to keep track of the
+ * allocation and of the lifetime of the tables; this can be used during
+ * testing/debug to verify that we are not leaking ppgtts.
+ * These traces identify the ppgtt through the vm pointer, which is also printed
+ * by the i915_vma_bind and i915_vma_unbind tracepoints.
+ */
+DECLARE_EVENT_CLASS(i915_ppgtt,
+	TP_PROTO(struct i915_address_space *vm),
+	TP_ARGS(vm),
+
+	TP_STRUCT__entry(
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+			__entry->vm = vm;
+			__entry->dev = vm->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
+)
+
+DEFINE_EVENT(i915_ppgtt, i915_ppgtt_create,
+	TP_PROTO(struct i915_address_space *vm),
+	TP_ARGS(vm)
+);
+
+DEFINE_EVENT(i915_ppgtt, i915_ppgtt_release,
+	TP_PROTO(struct i915_address_space *vm),
+	TP_ARGS(vm)
+);
+
+/**
+ * DOC: i915_context_create and i915_context_free tracepoints
+ *
+ * These tracepoints are used to track creation and deletion of contexts.
+ * If full ppgtt is enabled, they also print the address of the vm assigned to
+ * the context.
+ */
+DECLARE_EVENT_CLASS(i915_context,
+	TP_PROTO(struct intel_context *ctx),
+	TP_ARGS(ctx),
+
+	TP_STRUCT__entry(
+			__field(u32, dev)
+			__field(struct intel_context *, ctx)
+			__field(struct i915_address_space *, vm)
+	),
+
+	TP_fast_assign(
+			__entry->ctx = ctx;
+			__entry->vm = ctx->ppgtt ? &ctx->ppgtt->base : NULL;
+			__entry->dev = ctx->file_priv->dev_priv->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, ctx=%p, ctx_vm=%p",
+		  __entry->dev, __entry->ctx, __entry->vm)
+)
+
+DEFINE_EVENT(i915_context, i915_context_create,
+	TP_PROTO(struct intel_context *ctx),
+	TP_ARGS(ctx)
+);
+
+DEFINE_EVENT(i915_context, i915_context_free,
+	TP_PROTO(struct intel_context *ctx),
+	TP_ARGS(ctx)
+);
+
+/**
+ * DOC: switch_mm tracepoint
+ *
+ * This tracepoint allows tracking of the mm switch, which is an important point
+ * in the lifetime of the vm in the legacy submission path. This tracepoint is
+ * called only if full ppgtt is enabled.
+ */
+TRACE_EVENT(switch_mm,
+	TP_PROTO(struct intel_engine_cs *ring, struct intel_context *to),
+
+	TP_ARGS(ring, to),
+
+	TP_STRUCT__entry(
+			__field(u32, ring)
+			__field(struct intel_context *, to)
+			__field(struct i915_address_space *, vm)
+			__field(u32, dev)
+	),
+
+	TP_fast_assign(
+			__entry->ring = ring->id;
+			__entry->to = to;
+			__entry->vm = to->ppgtt? &to->ppgtt->base : NULL;
+			__entry->dev = ring->dev->primary->index;
+	),
+
+	TP_printk("dev=%u, ring=%u, ctx=%p, ctx_vm=%p",
+		  __entry->dev, __entry->ring, __entry->to, __entry->vm)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
-- 
1.8.5.2

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

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

* Re: [PATCH v7] drm/i915: Add tracepoints to track a vm
  2014-11-10 13:44       ` [PATCH v7] " daniele.ceraolospurio
@ 2014-11-11  3:00         ` shuang.he
  2014-11-11  9:21         ` [PATCH v7] drm/i915: Add tracepoints to track a vm during its lifetime Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2014-11-11  3:00 UTC (permalink / raw)
  To: shuang.he, intel-gfx, daniele.ceraolospurio

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=277/348->276/348
PNV: pass/total=323/328->325/328
ILK: pass/total=328/330->329/330
IVB: pass/total=545/546->544/546
SNB: pass/total=558/563->559/563
HSW: pass/total=577/581->574/581
BDW: pass/total=435/435->434/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc-lut, NSPT(1, M29)PASS(9, M36M29) -> NSPT(1, M29)PASS(3, M29)
PNV: Intel_gpu_tools, igt_gem_mmap_offset_exhaustion, DMESG_WARN(2, M23M24)PASS(14, M24M23M7) -> PASS(4, M7)
PNV: Intel_gpu_tools, igt_gem_unref_active_buffers, DMESG_WARN(2, M23)PASS(14, M24M23M7) -> PASS(4, M7)
ILK: Intel_gpu_tools, igt_kms_render_gpu-blit, DMESG_WARN(1, M26)PASS(15, M6M26M37) -> PASS(4, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-rmfb, PASS(4, M26) -> DMESG_WARN(2, M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms-interruptible, DMESG_WARN(1, M26)PASS(15, M6M26M37) -> PASS(4, M26)
IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(4, M4M34)PASS(9, M34M4) -> NSPT(2, M34)PASS(2, M34)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(3, M35M22)PASS(7, M22M35) -> PASS(4, M22)
HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(4, M39M20M40)PASS(9, M40M39M20) -> NSPT(2, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-random, FAIL(1, M39)PASS(3, M40M39) -> FAIL(1, M39)DMESG_WARN(2, M39)PASS(1, M39)
HSW: Intel_gpu_tools, igt_kms_rotation_crc_primary-rotation, PASS(4, M40M39) -> DMESG_WARN(1, M39)PASS(3, M39)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(16, M30M28M42) -> DMESG_WARN(1, M42)PASS(3, M42)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v7] drm/i915: Add tracepoints to track a vm during its lifetime
  2014-11-10 13:44       ` [PATCH v7] " daniele.ceraolospurio
  2014-11-11  3:00         ` [PATCH v7] drm/i915: Add tracepoints to track a vm shuang.he
@ 2014-11-11  9:21         ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-11-11  9:21 UTC (permalink / raw)
  To: daniele.ceraolospurio; +Cc: intel-gfx

On Mon, Nov 10, 2014 at 01:44:31PM +0000, daniele.ceraolospurio@intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> - ppgtt init/release: these tracepoints are useful for observing the
>   creation and destruction of Full PPGTTs.
> 
> - ctx create/free: we can use the ctx_free trace in combination with the
>   ppgtt_release one to be sure that the ppgtt doesn't stay alive for too
>   long after the ctx is destroyed. ctx_create is there for simmetry
> 
> - switch_mm: important point in the lifetime of the vm
> 
> v4: add DOC information
> v5: pull the DOC in drm.tmpl
> v6: clean ppgtt init/release traces + add ctx create/free and switch_mm
>     tracepoints (Chris)
> v7: drop execlist_submit_context tracepoint
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2014-11-11  9:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-25 16:10 [PATCH v3] drm/i915: Add ppgtt create/release trace points daniele.ceraolospurio
2014-09-29 13:06 ` Daniel Vetter
2014-10-22 13:28 ` [PATCH v4] " daniele.ceraolospurio
2014-10-22 15:29   ` Daniel Vetter
2014-10-24 15:30   ` [PATCH v5] " daniele.ceraolospurio
2014-10-27  8:49     ` Chris Wilson
2014-10-27 11:03       ` Ceraolo Spurio, Daniele
2014-10-28  8:47         ` Chris Wilson
2014-11-07 17:45     ` [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime daniele.ceraolospurio
2014-11-08  8:44       ` Chris Wilson
     [not found]         ` <5460A438.5080804@intel.com>
2014-11-10 11:43           ` Ceraolo Spurio, Daniele
     [not found]           ` <20141110115431.GC22109@nuc-i3427.alporthouse.com>
2014-11-10 12:28             ` Ceraolo Spurio, Daniele
2014-11-10 12:34               ` Chris Wilson
2014-11-10 13:44       ` [PATCH v7] " daniele.ceraolospurio
2014-11-11  3:00         ` [PATCH v7] drm/i915: Add tracepoints to track a vm shuang.he
2014-11-11  9:21         ` [PATCH v7] drm/i915: Add tracepoints to track a vm during its lifetime Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox