Quoting Matthew Auld (2017-06-21 21:33:31)
> For the 48b PPGTT try to align the vma start address to the required
> page size boundary to guarantee we use said page size in the gtt. If we
> are dealing with multiple page sizes, we can't guarantee anything and
> just align to the largest. For soft pinning and objects which need to be
> tightly packed into the lower 32bits we don't force any alignment.
>
> v2: various improvements suggested by Chris
>
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_vma.c | 15 +++++++++++++++
> drivers/gpu/drm/i915/i915_vma.h | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 958be0a95960..cee1d00dc085 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -471,6 +471,9 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
> if (ret)
> return ret;
>
> + vma->page_sizes.phys = obj->mm.page_sizes.phys;
> + vma->page_sizes.sg = obj->mm.page_sizes.sg;
I expected this to be in the same place as where we assigned vma->pages.
That'll take a bit of rejigging to make it look neat. First thought is a
if (!vma->pages) vma->vm->set_pages(vma);
> +
> if (flags & PIN_OFFSET_FIXED) {
> u64 offset = flags & PIN_OFFSET_MASK;
> if (!IS_ALIGNED(offset, alignment) ||
> @@ -485,6 +488,18 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
> if (ret)
> goto err_unpin;
> } else {
> + /* We only support huge gtt pages through the 48b PPGTT,
> + * however we also don't want to force any alignment for
> + * objects which need to be tightly packed into the low 32bits.
> + */
> + if (end > (1ULL << 32) &&
> + vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
> + u64 page_alignment =
> + rounddown_pow_of_two(vma->page_sizes.sg);
> +
> + alignment = max(alignment, page_alignment);
> + }
> +
> ret = i915_gem_gtt_insert(vma->vm, &vma->node,
> size, alignment, obj->cache_level,
> start, end, flags);
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 4a673fc1a432..834f7ca2ada2 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -52,6 +52,7 @@ struct i915_vma {
> struct drm_i915_fence_reg *fence;
> struct reservation_object *resv; /** Alias of obj->resv */
> struct sg_table *pages;
> + struct i915_page_sizes page_sizes;
In the middle of a bunch of pointers! Have some decency please!
> void __iomem *iomap;
> u64 size;
> u64 display_alignment;
Especially when there are some related variables right here :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
