Re: [Intel-gfx] [PATCH v7 2/3] drm/i915/ttm: Adjust gem flags and caching settings after a move

On Tue, 22 Jun 2021 at 10:34, Thomas Hellström thomas.hellstrom@linux.intel.com wrote:

After a TTM move or object init we need to update the i915 gem flags and caching settings to reflect the new placement. Currently caching settings are not changed during the lifetime of an object, although that might change moving forward if we run into performance issues or issues with WC system page allocations. Also introduce gpu_binds_iomem() and cpu_maps_iomem() to clean up the various ways we previously used to detect this. Finally, initialize the TTM object reserved to be able to update flags and caching before anyone else gets hold of the object.

Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Reviewed-by: Matthew Auld matthew.auld@intel.com

---

v6:

• Rebase on accelerated ttm moves.

---

<snip>

@@ -775,14 +845,13 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, i915_gem_object_init(obj, &i915_gem_ttm_obj_ops, &lock_class, flags); i915_gem_object_init_memory_region(obj, mem); i915_gem_object_make_unshrinkable(obj);

•   obj->read_domains = I915_GEM_DOMAIN_WC | I915_GEM_DOMAIN_GTT;
  

•   obj->mem_flags |= I915_BO_FLAG_IOMEM;
  

•   i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE);
    INIT_RADIX_TREE(&obj->ttm.get_io_page.radix, GFP_KERNEL | __GFP_NOWARN);
    mutex_init(&obj->ttm.get_io_page.lock);
    bo_type = (obj->flags & I915_BO_ALLOC_USER) ? ttm_bo_type_device :
            ttm_bo_type_kernel;
  
  

•   obj->base.vma_node.driver_private = i915_gem_to_ttm(obj);
  

•   /*
     * If this function fails, it will call the destructor, but
     * our caller still owns the object. So no freeing in the
  

@@ -790,14 +859,16 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, * Similarly, in delayed_destroy, we can't call ttm_bo_put() * until successful initialization. */

•   obj->base.vma_node.driver_private = i915_gem_to_ttm(obj);
  

•   ret = ttm_bo_init(&i915->bdev, i915_gem_to_ttm(obj), size,
  

•                     bo_type, &i915_sys_placement,
  

•                     mem->min_page_size >> PAGE_SHIFT,
  

•                     true, NULL, NULL, i915_ttm_bo_destroy);
  

•   if (!ret)
  

•           obj->ttm.created = true;
  

•   /* i915 wants -ENXIO when out of memory region space. */
  

•   return i915_ttm_err_to_gem(ret);
  

•   ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), size,
  

•                              bo_type, &i915_sys_placement, 1,
  

mem->min_page_size >> PAGE_SHIFT? Although just realised that looks iffy since it only considers the current region, when it should consider all future placements. I wonder if it makes sense to make page_alignment part of ttm_place? Anyway, it doesn't matter for this series.

•                              &ctx, NULL, NULL, i915_ttm_bo_destroy);
  

•   if (ret)
  

•           return i915_ttm_err_to_gem(ret);
  

•   obj->ttm.created = true;
  

•   i915_ttm_adjust_domains_after_move(obj);
  

•   i915_ttm_adjust_gem_after_move(obj);
  

•   i915_gem_object_unlock(obj);
  

•   return 0;
  

}

2.31.1

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx