[Intel-gfx] [PATCH v6 10/32] drm/i915: Add dynamic page trace events

[Mika Kuoppala]

Michel Thierry <michel.thierry at intel.com> writes:

> Traces for page directories and tables allocation and map.
>
> v2: Removed references to teardown.
> v3: bitmap_scnprintf has been deprecated.
>
> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c     |  2 +
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  5 ++
>  drivers/gpu/drm/i915/i915_trace.h   | 95 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 312b7d2..4e51275 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3601,6 +3601,8 @@ search_free:
>  
>  	/*  allocate before insert / bind */
>  	if (vma->vm->allocate_va_range) {
> +		trace_i915_va_alloc(vma->vm, vma->node.start, vma->node.size,
> +				VM_TO_TRACE_NAME(vma->vm));
>  		ret = vma->vm->allocate_va_range(vma->vm,
>  						vma->node.start,
>  						vma->node.size);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 29cda58..94cdd99 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1210,6 +1210,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
>  
>  		ppgtt->pd.page_table[pde] = pt;
>  		set_bit(pde, new_page_tables);
> +		trace_i915_page_table_entry_alloc(vm, pde, start, GEN6_PDE_SHIFT);
>  	}
>  
>  	start = start_save;
> @@ -1225,6 +1226,10 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
>  		if (test_and_clear_bit(pde, new_page_tables))
>  			gen6_write_pde(&ppgtt->pd, pde, pt);
>  
> +		trace_i915_page_table_entry_map(vm, pde, pt,
> +					 gen6_pte_index(start),
> +					 gen6_pte_count(start, length),
> +					 I915_PPGTT_PT_ENTRIES);
>  		bitmap_or(pt->used_ptes, tmp_bitmap, pt->used_ptes,
>  				I915_PPGTT_PT_ENTRIES);
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index f004d3d..0038dc2 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -156,6 +156,101 @@ TRACE_EVENT(i915_vma_unbind,
>  		      __entry->obj, __entry->offset, __entry->size, __entry->vm)
>  );
>  
> +#define VM_TO_TRACE_NAME(vm) \
> +	(i915_is_ggtt(vm) ? "GGTT" : \
> +		      "Private VM")

"G" and "P" would suffice but it is matter of taste.

> +DECLARE_EVENT_CLASS(i915_va,
> +	TP_PROTO(struct i915_address_space *vm, u64 start, u64 length, const char *name),
> +	TP_ARGS(vm, start, length, name),
> +
> +	TP_STRUCT__entry(
> +		__field(struct i915_address_space *, vm)
> +		__field(u64, start)
> +		__field(u64, end)
> +		__string(name, name)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vm = vm;
> +		__entry->start = start;
> +		__entry->end = start + length;

__entry->end = start + length - 1;

> +		__assign_str(name, name);
> +	),
> +
> +	TP_printk("vm=%p (%s), 0x%llx-0x%llx",
> +		  __entry->vm, __get_str(name),  __entry->start, __entry->end)
> +);
> +
> +DEFINE_EVENT(i915_va, i915_va_alloc,
> +	     TP_PROTO(struct i915_address_space *vm, u64 start, u64 length, const char *name),
> +	     TP_ARGS(vm, start, length, name)
> +);
> +
> +DECLARE_EVENT_CLASS(i915_page_table_entry,
> +	TP_PROTO(struct i915_address_space *vm, u32 pde, u64 start, u64 pde_shift),
> +	TP_ARGS(vm, pde, start, pde_shift),
> +
> +	TP_STRUCT__entry(
> +		__field(struct i915_address_space *, vm)
> +		__field(u32, pde)
> +		__field(u64, start)
> +		__field(u64, end)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vm = vm;
> +		__entry->pde = pde;
> +		__entry->start = start;
> +		__entry->end = (start + (1ULL << pde_shift)) & ~((1ULL
> << pde_shift)-1);

-1 for her also?

> +	),
> +
> +	TP_printk("vm=%p, pde=%d (0x%llx-0x%llx)",
> +		  __entry->vm, __entry->pde, __entry->start, __entry->end)
> +);
> +
> +DEFINE_EVENT(i915_page_table_entry, i915_page_table_entry_alloc,
> +	     TP_PROTO(struct i915_address_space *vm, u32 pde, u64 start, u64 pde_shift),
> +	     TP_ARGS(vm, pde, start, pde_shift)
> +);
> +
> +/* Avoid extra math because we only support two sizes. The format is defined by
> + * bitmap_scnprintf. Each 32 bits is 8 HEX digits followed by comma */
> +#define TRACE_PT_SIZE(bits) \
> +	((((bits) == 1024) ? 288 : 144) + 1)
> +

This seems to be calculating the length of the output string, not the
bitmap size.

> +DECLARE_EVENT_CLASS(i915_page_table_entry_update,
> +	TP_PROTO(struct i915_address_space *vm, u32 pde,
> +		 struct i915_page_table_entry *pt, u32 first, u32 len, size_t bits),
> +	TP_ARGS(vm, pde, pt, first, len, bits),
> +
> +	TP_STRUCT__entry(
> +		__field(struct i915_address_space *, vm)
> +		__field(u32, pde)
> +		__field(u32, first)
> +		__field(u32, last)
> +		__bitmask(cur_ptes, TRACE_PT_SIZE(bits))

..and this seems wrong.

__bitmask(cur_ptes, bits);

And I would replace size_t with u32 throughout the patch.

> +	),
> +
> +	TP_fast_assign(
> +		__entry->vm = vm;
> +		__entry->pde = pde;
> +		__entry->first = first;
> +		__entry->last = first + len;

It is not len that we are getting but count. So please rename
the argument and

__entry->last = first + count - 1;

-Mika

> +		__assign_bitmask(cur_ptes, pt->used_ptes, bits);
> +	),
> +
> +	TP_printk("vm=%p, pde=%d, updating %u:%u\t%s",
> +		  __entry->vm, __entry->pde, __entry->last, __entry->first,
> +		  __get_bitmask(cur_ptes))
> +);
> +
> +DEFINE_EVENT(i915_page_table_entry_update, i915_page_table_entry_map,
> +	TP_PROTO(struct i915_address_space *vm, u32 pde,
> +		 struct i915_page_table_entry *pt, u32 first, u32 len, size_t bits),
> +	TP_ARGS(vm, pde, pt, first, len, bits)
> +);
> +
>  TRACE_EVENT(i915_gem_object_change_domain,
>  	    TP_PROTO(struct drm_i915_gem_object *obj, u32 old_read, u32 old_write),
>  	    TP_ARGS(obj, old_read, old_write),
> -- 
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx