On Fri, Jul 16, 2021 at 8:18 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Thu, 15 Jul 2021 at 23:39, Jason Ekstrand jason@jlekstrand.net 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.
I'm happy to drop this patch. It doesn't actually fix anything AFAIK. It makes the behavior more what I expected but my expectations are not to be trusted here.
The other TTM patch does fix a real bug AFAIK.
--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 | 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