On ma, 2016-08-01 at 10:11 +0100, Chris Wilson wrote:
> Tracking the size of the VMA as allocated allows us to dramatically
> reduce the complexity of later functions (like inserting the VMA in to
> the drm_mm range manager).
>
I assume this new revision only removed the extra hunk of code, right?
If so,
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 103 ++++++++++++++----------------------
> drivers/gpu/drm/i915/i915_gem_gtt.c | 67 ++++++++---------------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +-
> 3 files changed, 63 insertions(+), 112 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index f11bf6c4f86a..d89fbee5c48a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2976,53 +2976,36 @@ i915_gem_object_insert_into_vm(struct drm_i915_gem_object *obj,
> u64 alignment,
> u64 flags)
> {
> - struct drm_device *dev = obj->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - u64 start, end;
> - u32 search_flag, alloc_flag;
> + struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> struct i915_vma *vma;
> + u64 start, end;
> + u64 min_alignment;
> int ret;
>
> - if (i915_is_ggtt(vm)) {
> - u32 fence_size, fence_alignment, unfenced_alignment;
> - u64 view_size;
> -
> - if (WARN_ON(!ggtt_view))
> - return ERR_PTR(-EINVAL);
> -
> - view_size = i915_ggtt_view_size(obj, ggtt_view);
> -
> - fence_size = i915_gem_get_ggtt_size(dev_priv,
> - view_size,
> - obj->tiling_mode);
> - fence_alignment = i915_gem_get_ggtt_alignment(dev_priv,
> - view_size,
> - obj->tiling_mode,
> - true);
> - unfenced_alignment = i915_gem_get_ggtt_alignment(dev_priv,
> - view_size,
> - obj->tiling_mode,
> - false);
> - size = max(size, view_size);
> - if (flags & PIN_MAPPABLE)
> - size = max_t(u64, size, fence_size);
> -
> - if (alignment == 0)
> - alignment = flags & PIN_MAPPABLE ? fence_alignment :
> - unfenced_alignment;
> - if (flags & PIN_MAPPABLE && alignment & (fence_alignment - 1)) {
> - DRM_DEBUG("Invalid object (view type=%u) alignment requested %llx\n",
> - ggtt_view ? ggtt_view->type : 0,
> - (long long)alignment);
> - return ERR_PTR(-EINVAL);
> - }
> - } else {
> - size = max_t(u64, size, obj->base.size);
> - alignment = 4096;
> + vma = ggtt_view ?
> + i915_gem_obj_lookup_or_create_ggtt_vma(obj, ggtt_view) :
> + i915_gem_obj_lookup_or_create_vma(obj, vm);
> + if (IS_ERR(vma))
> + return vma;
> +
> + size = max(size, vma->size);
> + if (flags & PIN_MAPPABLE)
> + size = i915_gem_get_ggtt_size(dev_priv, size, obj->tiling_mode);
> +
> + min_alignment =
> + i915_gem_get_ggtt_alignment(dev_priv, size, obj->tiling_mode,
> + flags & PIN_MAPPABLE);
> + if (alignment == 0)
> + alignment = min_alignment;
> + if (alignment & (min_alignment - 1)) {
> + DRM_DEBUG("Invalid object alignment requested %llu, minimum %llu\n",
> + alignment, min_alignment);
> + return ERR_PTR(-EINVAL);
> }
>
> start = flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
> - end = vm->total;
> +
> + end = vma->vm->total;
> if (flags & PIN_MAPPABLE)
> end = min_t(u64, end, dev_priv->ggtt.mappable_end);
> if (flags & PIN_ZONE_4G)
> @@ -3033,8 +3016,7 @@ i915_gem_object_insert_into_vm(struct drm_i915_gem_object *obj,
> * attempt to find space.
> */
> if (size > end) {
> - DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: request=%llu [object=%zd] > %s aperture=%llu\n",
> - ggtt_view ? ggtt_view->type : 0,
> + DRM_DEBUG("Attempting to bind an object larger than the aperture: request=%llu [object=%zd] > %s aperture=%llu\n",
> size, obj->base.size,
> flags & PIN_MAPPABLE ? "mappable" : "total",
> end);
> @@ -3047,31 +3029,27 @@ i915_gem_object_insert_into_vm(struct drm_i915_gem_object *obj,
>
> i915_gem_object_pin_pages(obj);
>
> - vma = ggtt_view ? i915_gem_obj_lookup_or_create_ggtt_vma(obj, ggtt_view) :
> - i915_gem_obj_lookup_or_create_vma(obj, vm);
> -
> - if (IS_ERR(vma))
> - goto err_unpin;
> -
> if (flags & PIN_OFFSET_FIXED) {
> uint64_t offset = flags & PIN_OFFSET_MASK;
> -
> - if (offset & (alignment - 1) || offset + size > end) {
> + if (offset & (alignment - 1) || offset > end - size) {
> ret = -EINVAL;
> - goto err_vma;
> + goto err_unpin;
> }
> +
> vma->node.start = offset;
> vma->node.size = size;
> vma->node.color = obj->cache_level;
> - ret = drm_mm_reserve_node(&vm->mm, &vma->node);
> + ret = drm_mm_reserve_node(&vma->vm->mm, &vma->node);
> if (ret) {
> ret = i915_gem_evict_for_vma(vma);
> if (ret == 0)
> - ret = drm_mm_reserve_node(&vm->mm, &vma->node);
> + ret = drm_mm_reserve_node(&vma->vm->mm, &vma->node);
> + if (ret)
> + goto err_unpin;
> }
> - if (ret)
> - goto err_vma;
> } else {
> + u32 search_flag, alloc_flag;
> +
> if (flags & PIN_HIGH) {
> search_flag = DRM_MM_SEARCH_BELOW;
> alloc_flag = DRM_MM_CREATE_TOP;
> @@ -3090,36 +3068,35 @@ i915_gem_object_insert_into_vm(struct drm_i915_gem_object *obj,
> alignment = 0;
>
> search_free:
> - ret = drm_mm_insert_node_in_range_generic(&vm->mm, &vma->node,
> + ret = drm_mm_insert_node_in_range_generic(&vma->vm->mm,
> + &vma->node,
> size, alignment,
> obj->cache_level,
> start, end,
> search_flag,
> alloc_flag);
> if (ret) {
> - ret = i915_gem_evict_something(vm, size, alignment,
> + ret = i915_gem_evict_something(vma->vm, size, alignment,
> obj->cache_level,
> start, end,
> flags);
> if (ret == 0)
> goto search_free;
>
> - goto err_vma;
> + goto err_unpin;
> }
> }
> GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
>
> list_move_tail(&obj->global_list, &dev_priv->mm.bound_list);
> - list_move_tail(&vma->vm_link, &vm->inactive_list);
> + list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
> obj->bind_count++;
>
> return vma;
>
> -err_vma:
> - vma = ERR_PTR(ret);
> err_unpin:
> i915_gem_object_unpin_pages(obj);
> - return vma;
> + return ERR_PTR(ret);
> }
>
> bool
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 31d13aea5461..ae6426e83ee0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -184,7 +184,7 @@ static void ppgtt_unbind_vma(struct i915_vma *vma)
> {
> vma->vm->clear_range(vma->vm,
> vma->node.start,
> - vma->obj->base.size,
> + vma->size,
> true);
> }
>
> @@ -2697,28 +2697,18 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
>
> static void ggtt_unbind_vma(struct i915_vma *vma)
> {
> - struct drm_device *dev = vma->vm->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct drm_i915_gem_object *obj = vma->obj;
> - const uint64_t size = min_t(uint64_t,
> - obj->base.size,
> - vma->node.size);
> + struct i915_hw_ppgtt *appgtt = to_i915(vma->vm->dev)->mm.aliasing_ppgtt;
> + const u64 size = min(vma->size, vma->node.size);
>
> - if (vma->bound & GLOBAL_BIND) {
> + if (vma->bound & GLOBAL_BIND)
> vma->vm->clear_range(vma->vm,
> - vma->node.start,
> - size,
> + vma->node.start, size,
> true);
> - }
> -
> - if (dev_priv->mm.aliasing_ppgtt && vma->bound & LOCAL_BIND) {
> - struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt;
>
> + if (vma->bound & LOCAL_BIND && appgtt)
> appgtt->base.clear_range(&appgtt->base,
> - vma->node.start,
> - size,
> + vma->node.start, size,
> true);
> - }
> }
>
> void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
> @@ -3367,14 +3357,14 @@ void i915_vma_close(struct i915_vma *vma)
> static struct i915_vma *
> __i915_gem_vma_create(struct drm_i915_gem_object *obj,
> struct i915_address_space *vm,
> - const struct i915_ggtt_view *ggtt_view)
> + const struct i915_ggtt_view *view)
> {
> struct i915_vma *vma;
> int i;
>
> GEM_BUG_ON(vm->closed);
>
> - if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view))
> + if (WARN_ON(i915_is_ggtt(vm) != !!view))
> return ERR_PTR(-EINVAL);
>
> vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL);
> @@ -3388,12 +3378,22 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj,
> list_add(&vma->vm_link, &vm->unbound_list);
> vma->vm = vm;
> vma->obj = obj;
> + vma->size = obj->base.size;
> vma->is_ggtt = i915_is_ggtt(vm);
>
> - if (i915_is_ggtt(vm))
> - vma->ggtt_view = *ggtt_view;
> - else
> + if (i915_is_ggtt(vm)) {
> + vma->ggtt_view = *view;
> + if (view->type == I915_GGTT_VIEW_PARTIAL) {
> + vma->size = view->params.partial.size;
> + vma->size <<= PAGE_SHIFT;
> + } else if (view->type == I915_GGTT_VIEW_ROTATED) {
> + vma->size =
> + intel_rotation_info_size(&view->params.rotated);
> + vma->size <<= PAGE_SHIFT;
> + }
> + } else {
> i915_ppgtt_get(i915_vm_to_ppgtt(vm));
> + }
>
> list_add_tail(&vma->obj_link, &obj->vma_list);
>
> @@ -3678,29 +3678,6 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
> return 0;
> }
>
> -/**
> - * i915_ggtt_view_size - Get the size of a GGTT view.
> - * @obj: Object the view is of.
> - * @view: The view in question.
> - *
> - * @return The size of the GGTT view in bytes.
> - */
> -size_t
> -i915_ggtt_view_size(struct drm_i915_gem_object *obj,
> - const struct i915_ggtt_view *view)
> -{
> - if (view->type == I915_GGTT_VIEW_NORMAL) {
> - return obj->base.size;
> - } else if (view->type == I915_GGTT_VIEW_ROTATED) {
> - return intel_rotation_info_size(&view->params.rotated) << PAGE_SHIFT;
> - } else if (view->type == I915_GGTT_VIEW_PARTIAL) {
> - return view->params.partial.size << PAGE_SHIFT;
> - } else {
> - WARN_ONCE(1, "GGTT view %u not implemented!\n", view->type);
> - return obj->base.size;
> - }
> -}
> -
> void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
> {
> void __iomem *ptr;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 2ebf07defa4e..2de6f613ac1e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -180,6 +180,7 @@ struct i915_vma {
> struct drm_i915_gem_object *obj;
> struct i915_address_space *vm;
> void __iomem *iomap;
> + u64 size;
>
> unsigned int active;
> struct i915_gem_active last_read[I915_NUM_ENGINES];
> @@ -610,10 +611,6 @@ i915_ggtt_view_equal(const struct i915_ggtt_view *a,
> return true;
> }
>
> -size_t
> -i915_ggtt_view_size(struct drm_i915_gem_object *obj,
> - const struct i915_ggtt_view *view);
> -
> /**
> * i915_vma_pin_iomap - calls ioremap_wc to map the GGTT VMA via the aperture
> * @vma: VMA to iomap
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
