On Fri, Jul 16, 2021 at 8:54 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Thu, 15 Jul 2021 at 23:39, Jason Ekstrand jason@jlekstrand.net wrote:
Whenever we had a user object (n_placements > 0), we were ignoring obj->mm.region and always putting obj->placements[0] as the requested region. For LMEM+SMEM objects, this was causing them to get shoved into LMEM on every i915_ttm_get_pages() even when SMEM was requested by, say, i915_gem_object_migrate().
i915_ttm_migrate calls i915_ttm_place_from_region() directly with the requested region, so there shouldn't be an issue with migration right? Do you have some more details?
With i915_ttm_migrate directly, no. But, in the last patch in the series, we're trying to migrate LMEM+SMEM buffers into SMEM on attach() and pin it there. This blows up in a very unexpected (IMO) way. The flow goes something like this:
- Client attempts a dma-buf import from another device - In attach() we call i915_gem_object_migrate() which calls i915_ttm_migrate() which migrates as requested. - Once the migration is complete, we call i915_gem_object_pin_pages() which calls i915_ttm_get_pages() which depends on i915_ttm_placement_from_obj() and so migrates it right back to LMEM.
Maybe the problem here is actually that our TTM code isn't respecting obj->mm.pages_pin_count?
In case you can't tell, I really have no clue what I'm doing here. I'm really stumbling around in the dark finding things that make my bug go away. I'm happy for the feedback.
--Jason
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Thomas Hellström thomas.hellstrom@linux.intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 +-- 1 file changed, 1 insertion(+), 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 d30f274c329c7..5985e994d56cf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -150,8 +150,7 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, unsigned int i;
placement->num_placement = 1;
i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] :obj->mm.region, requested, flags);
i915_ttm_place_from_region(obj->mm.region, requested, flags); /* Cache this on object? */ placement->num_busy_placement = num_allowed;-- 2.31.1