[Intel-gfx] [RFC 03/13] drm/i915/xehpsdv: enforce min GTT alignment

[Matthew Auld]

On Sun, 22 Aug 2021 at 17:30, Ayaz A Siddiqui <ayaz.siddiqui at intel.com> wrote:
>
> From: Matthew Auld <matthew.auld at intel.com>
>
> For local-memory objects we need to align the GTT addresses to 64K, both
> for the ppgtt and ggtt.
>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Signed-off-by: Stuart Summers <stuart.summers at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vma.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 4b7fc4647e46..1ea1fa08efdf 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -670,8 +670,13 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
>         }
>
>         color = 0;
> -       if (vma->obj && i915_vm_has_cache_coloring(vma->vm))
> -               color = vma->obj->cache_level;
> +       if (vma->obj) {
> +               if (HAS_64K_PAGES(vma->vm->i915) && i915_gem_object_is_lmem(vma->obj))
> +                       alignment = max(alignment, I915_GTT_PAGE_SIZE_64K);

If we have an SMEM | LMEM object we should probably also apply the
alignment here? Userspace might be surprised if this suddenly starts
failing when the current object placement changes?

Maybe something like:

if (is_lmem() || obj->mm.n_placements > 1)
   min_alignment = 64K;

But thinking about this some more it might be quite strange for
userspace to have LMEM | SMEM objects not occupy the entire 2M range,
since flipping the color would likely mean evicting the entire 2M
range anyway? Maybe the kernel should try to prevent that?

if (mixed_placements(obj)) {
    min_alignment = 2M;
    min_padding = 2M;
} else if (is_lmem(obj)) {
   min_alignment = 64K;
}

Could maybe do this as part of gem_create_ext, when setting the
placements, and just store the min_alignment etc? Could also document
all the rules for this as part of the gem_create_ext kernel doc in the
uapi headers?

> +
> +               if (i915_vm_has_cache_coloring(vma->vm))
> +                       color = vma->obj->cache_level;
> +       }
>
>         if (flags & PIN_OFFSET_FIXED) {
>                 u64 offset = flags & PIN_OFFSET_MASK;
> --
> 2.26.2
>