On Fri, 23 Jul 2021 at 18:22, Jason Ekstrand <jason@xxxxxxxxxxxxxx> wrote:
>
> Without TTM, we have no such hook so we exit early but this is fine
> because we use TTM on all LMEM platforms and, on integrated platforms,
> there is no real migration. If we do have the hook, it's better to just
> let TTM handle the migration because it knows where things are actually
> placed.
>
> This fixes a bug where i915_gem_object_migrate fails to migrate newly
> created LMEM objects. In that scenario, the object has obj->mm.region
> set to LMEM but TTM has it in SMEM because that's where all new objects
> are placed there prior to getting actual pages. When we invoke
> i915_gem_object_migrate, it exits early because, from the point of view
> of the GEM object, it's already in LMEM and no migration is needed.
> Then, when we try to pin the pages, __i915_ttm_get_pages is called
> which, unaware of our failed attempt at a migration, places the object
> in SMEM. This only happens on newly created objects because they have
> this weird state where TTM thinks they're in SMEM, GEM thinks they're in
> LMEM, and the reality is that they don't exist at all.
>
> It's better if GEM just always calls into TTM and let's TTM handle
> things. That way the lies stay better contained. Once the migration is
> complete, the object will have pages, obj->mm.region will be correct,
> and we're done lying.
>
> Signed-off-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
Thanks for fixing this,
Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index d09bd9bdb38ac..9d3497e1235a0 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -607,12 +607,15 @@ int i915_gem_object_migrate(struct drm_i915_gem_object *obj,
> mr = i915->mm.regions[id];
> GEM_BUG_ON(!mr);
>
> - if (obj->mm.region == mr)
> - return 0;
> -
> if (!i915_gem_object_can_migrate(obj, id))
> return -EINVAL;
>
> + if (!obj->ops->migrate) {
> + if (GEM_WARN_ON(obj->mm.region != mr))
> + return -EINVAL;
> + return 0;
> + }
> +
> return obj->ops->migrate(obj, mr);
> }
>
> --
> 2.31.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
