diff mbox

[05/19] drm/i915: align the vma start to the largest gtt page size

Message ID 20170621203345.26320-6-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthew Auld June 21, 2017, 8:33 p.m. UTC
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@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_vma.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/i915_vma.h |  1 +
 2 files changed, 16 insertions(+)

Comments

Chris Wilson June 21, 2017, 9:35 p.m. UTC | #1
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@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  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
diff mbox

Patch

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;
+
 	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;
 	void __iomem *iomap;
 	u64 size;
 	u64 display_alignment;