On Fri, Nov 20, 2015 at 12:43:50PM +0000, Chris Wilson wrote: > The multiple levels of indirect do nothing but hinder the compiler and > the pointer chasing turns to be quite painful but painless to fix. > > Signed-off-by: Chris Wilson <[email protected]>
vma->is_ggtt = vm->is_ggtt is robust enough for me to carry it twice. Reviewed-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++++------- > drivers/gpu/drm/i915/i915_drv.h | 9 +-------- > drivers/gpu/drm/i915/i915_gem.c | 32 > +++++++++++++----------------- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++--- > drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++++++++----------- > drivers/gpu/drm/i915/i915_gem_gtt.h | 3 +++ > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > drivers/gpu/drm/i915/i915_trace.h | 27 ++++++++----------------- > 8 files changed, 44 insertions(+), 69 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 630717fec688..d83f35c8df34 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -123,8 +123,7 @@ static u64 i915_gem_obj_total_ggtt_size(struct > drm_i915_gem_object *obj) > struct i915_vma *vma; > > list_for_each_entry(vma, &obj->vma_list, vma_link) { > - if (i915_is_ggtt(vma->vm) && > - drm_mm_node_allocated(&vma->node)) > + if (vma->is_ggtt && drm_mm_node_allocated(&vma->node)) > size += vma->node.size; > } > > @@ -171,12 +170,11 @@ describe_obj(struct seq_file *m, struct > drm_i915_gem_object *obj) > seq_printf(m, " (fence: %d)", obj->fence_reg); > list_for_each_entry(vma, &obj->vma_list, vma_link) { > seq_printf(m, " (%sgtt offset: %08llx, size: %08llx", > - i915_is_ggtt(vma->vm) ? "g" : "pp", > + vma->is_ggtt ? "g" : "pp", > vma->node.start, vma->node.size); > - if (i915_is_ggtt(vma->vm)) > - seq_printf(m, ", type: %u)", vma->ggtt_view.type); > - else > - seq_puts(m, ")"); > + if (vma->is_ggtt) > + seq_printf(m, ", type: %u", vma->ggtt_view.type); > + seq_puts(m, ")"); > } > if (obj->stolen) > seq_printf(m, " (stolen: %08llx)", obj->stolen->start); > @@ -348,7 +346,7 @@ static int per_file_stats(int id, void *ptr, void *data) > if (!drm_mm_node_allocated(&vma->node)) > continue; > > - if (i915_is_ggtt(vma->vm)) { > + if (vma->is_ggtt) { > stats->global += obj->base.size; > continue; > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index d8bd58cbb727..e86e1188deb0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3023,18 +3023,11 @@ bool i915_gem_obj_is_pinned(struct > drm_i915_gem_object *obj); > /* Some GGTT VM helpers */ > #define i915_obj_to_ggtt(obj) \ > (&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base) > -static inline bool i915_is_ggtt(struct i915_address_space *vm) > -{ > - struct i915_address_space *ggtt = > - &((struct drm_i915_private *)(vm)->dev->dev_private)->gtt.base; > - return vm == ggtt; > -} > > static inline struct i915_hw_ppgtt * > i915_vm_to_ppgtt(struct i915_address_space *vm) > { > - WARN_ON(i915_is_ggtt(vm)); > - > + WARN_ON(vm->is_ggtt); > return container_of(vm, struct i915_hw_ppgtt, base); > } > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index d6706dd4117c..390cb9c15bad 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3300,8 +3300,7 @@ int i915_vma_unbind(struct i915_vma *vma) > * cause memory corruption through use-after-free. > */ > > - if (i915_is_ggtt(vma->vm) && > - vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { > + if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { > i915_gem_object_finish_gtt(obj); > > /* release the fence reg _after_ flushing */ > @@ -3316,7 +3315,7 @@ int i915_vma_unbind(struct i915_vma *vma) > vma->bound = 0; > > list_del_init(&vma->mm_list); > - if (i915_is_ggtt(vma->vm)) { > + if (vma->is_ggtt) { > if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) { > obj->map_and_fenceable = false; > } else if (vma->ggtt_view.pages) { > @@ -3427,7 +3426,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object > *obj, > struct i915_vma *vma; > int ret; > > - if (i915_is_ggtt(vm)) { > + if (vm->is_ggtt) { > u32 fence_size, fence_alignment, unfenced_alignment; > u64 view_size; > > @@ -4177,13 +4176,13 @@ i915_gem_object_do_pin(struct drm_i915_gem_object > *obj, > if (WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base)) > return -ENODEV; > > - if (WARN_ON(flags & (PIN_GLOBAL | PIN_MAPPABLE) && !i915_is_ggtt(vm))) > + if (WARN_ON(flags & (PIN_GLOBAL | PIN_MAPPABLE) && !vm->is_ggtt)) > return -EINVAL; > > if (WARN_ON((flags & (PIN_MAPPABLE | PIN_GLOBAL)) == PIN_MAPPABLE)) > return -EINVAL; > > - if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view)) > + if (WARN_ON(vm->is_ggtt != !!ggtt_view)) > return -EINVAL; > > vma = ggtt_view ? i915_gem_obj_to_ggtt_view(obj, ggtt_view) : > @@ -4245,7 +4244,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, > uint64_t flags) > { > return i915_gem_object_do_pin(obj, vm, > - i915_is_ggtt(vm) ? &i915_ggtt_view_normal > : NULL, > + vm->is_ggtt ? &i915_ggtt_view_normal : > NULL, > size, alignment, flags); > } > > @@ -4550,7 +4549,7 @@ struct i915_vma *i915_gem_obj_to_vma(struct > drm_i915_gem_object *obj, > { > struct i915_vma *vma; > list_for_each_entry(vma, &obj->vma_list, vma_link) { > - if (i915_is_ggtt(vma->vm) && > + if (vma->is_ggtt && > vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) > continue; > if (vma->vm == vm) > @@ -4577,17 +4576,14 @@ struct i915_vma *i915_gem_obj_to_ggtt_view(struct > drm_i915_gem_object *obj, > > void i915_gem_vma_destroy(struct i915_vma *vma) > { > - struct i915_address_space *vm = NULL; > WARN_ON(vma->node.allocated); > > /* Keep the vma as a placeholder in the execbuffer reservation lists */ > if (!list_empty(&vma->exec_list)) > return; > > - vm = vma->vm; > - > - if (!i915_is_ggtt(vm)) > - i915_ppgtt_put(i915_vm_to_ppgtt(vm)); > + if (!vma->is_ggtt) > + i915_ppgtt_put(i915_vm_to_ppgtt(vma->vm)); > > list_del(&vma->vma_link); > > @@ -4984,7 +4980,7 @@ init_ring_lists(struct intel_engine_cs *ring) > void i915_init_vm(struct drm_i915_private *dev_priv, > struct i915_address_space *vm) > { > - if (!i915_is_ggtt(vm)) > + if (!vm->is_ggtt) > drm_mm_init(&vm->mm, vm->start, vm->total); > vm->dev = dev_priv->dev; > INIT_LIST_HEAD(&vm->active_list); > @@ -5148,7 +5144,7 @@ u64 i915_gem_obj_offset(struct drm_i915_gem_object *o, > WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base); > > list_for_each_entry(vma, &o->vma_list, vma_link) { > - if (i915_is_ggtt(vma->vm) && > + if (vma->is_ggtt && > vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) > continue; > if (vma->vm == vm) > @@ -5156,7 +5152,7 @@ u64 i915_gem_obj_offset(struct drm_i915_gem_object *o, > } > > WARN(1, "%s vma for this object not found.\n", > - i915_is_ggtt(vm) ? "global" : "ppgtt"); > + vm->is_ggtt ? "global" : "ppgtt"); > return -1; > } > > @@ -5181,7 +5177,7 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o, > struct i915_vma *vma; > > list_for_each_entry(vma, &o->vma_list, vma_link) { > - if (i915_is_ggtt(vma->vm) && > + if (vma->is_ggtt && > vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) > continue; > if (vma->vm == vm && drm_mm_node_allocated(&vma->node)) > @@ -5228,7 +5224,7 @@ unsigned long i915_gem_obj_size(struct > drm_i915_gem_object *o, > BUG_ON(list_empty(&o->vma_list)); > > list_for_each_entry(vma, &o->vma_list, vma_link) { > - if (i915_is_ggtt(vma->vm) && > + if (vma->is_ggtt && > vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) > continue; > if (vma->vm == vm) > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index afa930dab632..46907f5da4f2 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -652,7 +652,7 @@ need_reloc_mappable(struct i915_vma *vma) > if (entry->relocation_count == 0) > return false; > > - if (!i915_is_ggtt(vma->vm)) > + if (!vma->is_ggtt) > return false; > > /* See also use_cpu_reloc() */ > @@ -671,8 +671,7 @@ eb_vma_misplaced(struct i915_vma *vma) > struct drm_i915_gem_exec_object2 *entry = vma->exec_entry; > struct drm_i915_gem_object *obj = vma->obj; > > - WARN_ON(entry->flags & __EXEC_OBJECT_NEEDS_MAP && > - !i915_is_ggtt(vma->vm)); > + WARN_ON(entry->flags & __EXEC_OBJECT_NEEDS_MAP && !vma->is_ggtt); > > if (entry->alignment && > vma->node.start & (entry->alignment - 1)) > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index b1ee6f89e70b..8bb1102608fd 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2456,7 +2456,6 @@ static int ggtt_bind_vma(struct i915_vma *vma, > if (obj->gt_ro) > pte_flags |= PTE_READ_ONLY; > > - > if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { > vma->vm->insert_entries(vma->vm, pages, > vma->node.start, > @@ -3027,6 +3026,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > } > > gtt->base.dev = dev; > + gtt->base.is_ggtt = true; > > ret = gtt->gtt_probe(dev, >t->base.total, >t->stolen_size, > >t->mappable_base, >t->mappable_end); > @@ -3105,7 +3105,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device > *dev) > container_of(vm, struct i915_hw_ppgtt, > base); > > - if (i915_is_ggtt(vm)) > + if (vm->is_ggtt) > ppgtt = dev_priv->mm.aliasing_ppgtt; > > gen6_write_page_range(dev_priv, &ppgtt->pd, > @@ -3123,7 +3123,7 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj, > { > struct i915_vma *vma; > > - if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view)) > + if (WARN_ON(vm->is_ggtt != !!ggtt_view)) > return ERR_PTR(-EINVAL); > > vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL); > @@ -3135,13 +3135,14 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj, > INIT_LIST_HEAD(&vma->exec_list); > vma->vm = vm; > vma->obj = obj; > + vma->is_ggtt = vm->is_ggtt; > > - if (i915_is_ggtt(vm)) > + if (vm->is_ggtt) > vma->ggtt_view = *ggtt_view; > + else > + i915_ppgtt_get(i915_vm_to_ppgtt(vm)); > > list_add_tail(&vma->vma_link, &obj->vma_list); > - if (!i915_is_ggtt(vm)) > - i915_ppgtt_get(i915_vm_to_ppgtt(vm)); > > return vma; > } > @@ -3155,7 +3156,7 @@ i915_gem_obj_lookup_or_create_vma(struct > drm_i915_gem_object *obj, > vma = i915_gem_obj_to_vma(obj, vm); > if (!vma) > vma = __i915_gem_vma_create(obj, vm, > - i915_is_ggtt(vm) ? > &i915_ggtt_view_normal : NULL); > + vm->is_ggtt ? > &i915_ggtt_view_normal : NULL); > > return vma; > } > @@ -3381,13 +3382,9 @@ int i915_vma_bind(struct i915_vma *vma, enum > i915_cache_level cache_level, > return 0; > > if (vma->bound == 0 && vma->vm->allocate_va_range) { > - trace_i915_va_alloc(vma->vm, > - vma->node.start, > - vma->node.size, > - VM_TO_TRACE_NAME(vma->vm)); > - > /* XXX: i915_vma_pin() will fix this +- hack */ > vma->pin_count++; > + trace_i915_va_alloc(vma); > ret = vma->vm->allocate_va_range(vma->vm, > vma->node.start, > vma->node.size); > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 82750073d5b3..9b506ec6b90c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -183,6 +183,7 @@ struct i915_vma { > #define GLOBAL_BIND (1<<0) > #define LOCAL_BIND (1<<1) > unsigned int bound : 4; > + unsigned is_ggtt : 1; > > /** > * Support different GGTT views into the same object. > @@ -275,6 +276,8 @@ struct i915_address_space { > u64 start; /* Start offset always 0 for dri2 */ > u64 total; /* size addr space maps (ex. 2GB for ggtt) */ > > + bool is_ggtt; > + > struct i915_page_scratch *scratch_page; > struct i915_page_table *scratch_pt; > struct i915_page_directory *scratch_pd; > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > b/drivers/gpu/drm/i915/i915_gpu_error.c > index 8b37f72bd91f..8afc49600835 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -612,7 +612,7 @@ i915_error_object_create(struct drm_i915_private > *dev_priv, > dst->gtt_offset = -1; > > reloc_offset = dst->gtt_offset; > - if (i915_is_ggtt(vm)) > + if (vm->is_ggtt) > vma = i915_gem_obj_to_ggtt(src); > use_ggtt = (src->cache_level == I915_CACHE_NONE && > vma && (vma->bound & GLOBAL_BIND) && > diff --git a/drivers/gpu/drm/i915/i915_trace.h > b/drivers/gpu/drm/i915/i915_trace.h > index 533b6a87fd1d..58ef4e0ccea1 100644 > --- a/drivers/gpu/drm/i915/i915_trace.h > +++ b/drivers/gpu/drm/i915/i915_trace.h > @@ -175,35 +175,24 @@ TRACE_EVENT(i915_vma_unbind, > __entry->obj, __entry->offset, __entry->size, __entry->vm) > ); > > -#define VM_TO_TRACE_NAME(vm) \ > - (i915_is_ggtt(vm) ? "G" : \ > - "P") > - > -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), > +TRACE_EVENT(i915_va_alloc, > + TP_PROTO(struct i915_vma *vma), > + TP_ARGS(vma), > > 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 - 1; > - __assign_str(name, name); > + __entry->vm = vma->vm; > + __entry->start = vma->node.start; > + __entry->end = vma->node.start + vma->node.size - 1; > ), > > - 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) > + TP_printk("vm=%p (%c), 0x%llx-0x%llx", > + __entry->vm, __entry->vm->is_ggtt ? 'G' : 'P', > __entry->start, __entry->end) > ); > > DECLARE_EVENT_CLASS(i915_px_entry, > -- > 2.6.2 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
