[Intel-gfx] [PATCH 4/7] drm/i915/gem/ttm: Place new BOs in the requested region

[Matthew Auld]

On Thu, 15 Jul 2021 at 23:39, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> __i915_gem_ttm_object_init() was ignoring the placement requests coming
> from the client and always placing all BOs in SMEM upon creation.
> Instead, compute the requested placement set from the object and pass
> that into ttm_bo_init_reserved().

AFAIK this is intentional, where we use SYS as a safe placeholder
specifically for object creation, since that avoids allocating any
actual pages. Later at get_pages() time we do the real allocation,
where we need to consider the placements.

If we set the real placements here, the ttm_bo_validate() call in
init_reserved() might allocate the backing pages here for the
lmem-only case, which is not what we want in i915.

>
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Cc: Matthew Auld <matthew.auld at intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index 6589411396d3f..d30f274c329c7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -898,6 +898,8 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
>  {
>         static struct lock_class_key lock_class;
>         struct drm_i915_private *i915 = mem->i915;
> +       struct ttm_place requested, busy[I915_TTM_MAX_PLACEMENTS];
> +       struct ttm_placement placement;
>         struct ttm_operation_ctx ctx = {
>                 .interruptible = true,
>                 .no_wait_gpu = false,
> @@ -919,6 +921,9 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
>         /* Forcing the page size is kernel internal only */
>         GEM_BUG_ON(page_size && obj->mm.n_placements);
>
> +       GEM_BUG_ON(obj->mm.n_placements > I915_TTM_MAX_PLACEMENTS);
> +       i915_ttm_placement_from_obj(obj, &requested, busy, &placement);
> +
>         /*
>          * If this function fails, it will call the destructor, but
>          * our caller still owns the object. So no freeing in the
> @@ -927,8 +932,7 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
>          * until successful initialization.
>          */
>         ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), size,
> -                                  bo_type, &i915_sys_placement,
> -                                  page_size >> PAGE_SHIFT,
> +                                  bo_type, &placement, page_size >> PAGE_SHIFT,
>                                    &ctx, NULL, NULL, i915_ttm_bo_destroy);
>         if (ret)
>                 return i915_ttm_err_to_gem(ret);
> --
> 2.31.1
>