[Intel-gfx] [PATCH 03/12] drm/i915/bdw: Add dynamic page trace events
Daniel Vetter
daniel at ffwll.ch
Tue Feb 24 02:59:28 PST 2015
On Fri, Feb 20, 2015 at 05:45:57PM +0000, Michel Thierry wrote:
> From: Ben Widawsky <benjamin.widawsky at intel.com>
>
> The dynamic page allocation patch series added it for GEN6, this patch
> adds them for GEN8.
>
> v2: Consolidate pagetable/page_directory events
> v3: Multiple rebases.
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thierry at intel.com> (v3)
On second thought I wonder how useful these tracepoints really are. We
already have tracepoints for binding/unbinding, that should tell us all
about vm usage. Actual pt allocation itself seems fairly boring tbh.
What's the practical application of these tracepoints? Same goes ofc for
the corresponding gen6 patch. Note that tracepoints are somewhat
considered abi (or at least can become abi), we need a solid justification
for them. "Seemed useful and cheap to add" isn't enough imo.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 23 +++++++++++++++--------
> drivers/gpu/drm/i915/i915_trace.h | 16 ++++++++++++++++
> 2 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d3ad517..ecfb62a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -673,19 +673,24 @@ static void __gen8_do_map_pt(gen8_ppgtt_pde_t * const pde,
> /* It's likely we'll map more than one pagetable at a time. This function will
> * save us unnecessary kmap calls, but do no more functionally than multiple
> * calls to map_pt. */
> -static void gen8_map_pagetable_range(struct i915_page_directory_entry *pd,
> +static void gen8_map_pagetable_range(struct i915_address_space *vm,
> + struct i915_page_directory_entry *pd,
> uint64_t start,
> - uint64_t length,
> - struct drm_device *dev)
> + uint64_t length)
> {
> gen8_ppgtt_pde_t * const page_directory = kmap_atomic(pd->page);
> struct i915_page_table_entry *pt;
> uint64_t temp, pde;
>
> - gen8_for_each_pde(pt, pd, start, length, temp, pde)
> - __gen8_do_map_pt(page_directory + pde, pt, dev);
> + gen8_for_each_pde(pt, pd, start, length, temp, pde) {
> + __gen8_do_map_pt(page_directory + pde, pt, vm->dev);
> + trace_i915_page_table_entry_map(vm, pde, pt,
> + gen8_pte_index(start),
> + gen8_pte_count(start, length),
> + GEN8_PTES_PER_PAGE);
> + }
>
> - if (!HAS_LLC(dev))
> + if (!HAS_LLC(vm->dev))
> drm_clflush_virt_range(page_directory, PAGE_SIZE);
>
> kunmap_atomic(page_directory);
> @@ -815,6 +820,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_address_space *vm,
>
> pd->page_tables[pde] = pt;
> set_bit(pde, new_pts);
> + trace_i915_page_table_entry_alloc(vm, pde, start, GEN8_PDE_SHIFT);
> }
>
> return 0;
> @@ -876,6 +882,7 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_address_space *vm,
>
> pdp->page_directory[pdpe] = pd;
> set_bit(pdpe, new_pds);
> + trace_i915_page_directory_entry_alloc(vm, pdpe, start, GEN8_PDPE_SHIFT);
> }
>
> return 0;
> @@ -1014,7 +1021,7 @@ static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm,
> }
>
> set_bit(pdpe, pdp->used_pdpes);
> - gen8_map_pagetable_range(pd, start, length, dev);
> + gen8_map_pagetable_range(vm, pd, start, length);
> }
>
> free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> @@ -1115,7 +1122,7 @@ static int gen8_aliasing_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> }
>
> gen8_for_each_pdpe(pd, pdp, start, size, temp, pdpe)
> - gen8_map_pagetable_range(pd, start, size, dev);
> + gen8_map_pagetable_range(&ppgtt->base, pd,start, size);
>
> ppgtt->base.allocate_va_range = NULL;
> ppgtt->base.clear_range = gen8_ppgtt_clear_range;
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 3a657e4..6c20f76 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -214,6 +214,22 @@ DEFINE_EVENT(i915_page_table_entry, i915_page_table_entry_alloc,
> TP_ARGS(vm, pde, start, pde_shift)
> );
>
> +DEFINE_EVENT_PRINT(i915_page_table_entry, i915_page_directory_entry_alloc,
> + TP_PROTO(struct i915_address_space *vm, u32 pdpe, u64 start, u64 pdpe_shift),
> + TP_ARGS(vm, pdpe, start, pdpe_shift),
> +
> + TP_printk("vm=%p, pdpe=%d (0x%llx-0x%llx)",
> + __entry->vm, __entry->pde, __entry->start, __entry->end)
> +);
> +
> +DEFINE_EVENT_PRINT(i915_page_table_entry, i915_page_directory_pointer_entry_alloc,
> + TP_PROTO(struct i915_address_space *vm, u32 pml4e, u64 start, u64 pml4e_shift),
> + TP_ARGS(vm, pml4e, start, pml4e_shift),
> +
> + TP_printk("vm=%p, pml4e=%d (0x%llx-0x%llx)",
> + __entry->vm, __entry->pde, __entry->start, __entry->end)
> +);
> +
> /* 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) \
> --
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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
More information about the Intel-gfx
mailing list