[Intel-gfx] [PATCH 6/8] drm/i915/gem: Always call obj->ops->migrate unless can_migrate fails

[Matthew Auld]

On Fri, 23 Jul 2021 at 18:22, Jason Ekstrand <jason at jlekstrand.net> 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 at jlekstrand.net>

Thanks for fixing this,
Reviewed-by: Matthew Auld <matthew.auld at intel.com>

> ---
>  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx