On Thu, 15 Jul 2021 at 23:39, Jason Ekstrand <jason@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> Cc: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> 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
>
