diff mbox series

[5/8] drm/i915/gem/ttm: Only call __i915_gem_object_set_pages if needed

Message ID 20210723172142.3273510-6-jason@jlekstrand.net (mailing list archive)
State New, archived
Headers show
Series drm/i915: Migrate memory to SMEM when imported cross-device (v8) | expand

Commit Message

Jason Ekstrand July 23, 2021, 5:21 p.m. UTC
__i915_ttm_get_pages does two things.  First, it calls ttm_bo_validate()
to check the given placement and migrate the BO if needed.  Then, it
updates the GEM object to match, in case the object was migrated.  If
no migration occured, however, we might still have pages on the GEM
object in which case we don't need to fetch them from TTM and call
__i915_gem_object_set_pages.  This hasn't been a problem before because
the primary user of __i915_ttm_get_pages is __i915_gem_object_get_pages
which only calls it if the GEM object doesn't have pages.

However, i915_ttm_migrate also uses __i915_ttm_get_pages to do the
migration so this meant it was unsafe to call on an already populated
object.  This patch checks i915_gem_object_has_pages() before trying to
__i915_gem_object_set_pages so i915_ttm_migrate is safe to call, even on
populated objects.

Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Matthew Auld July 26, 2021, 8:08 a.m. UTC | #1
On Fri, 23 Jul 2021 at 18:22, Jason Ekstrand <jason@jlekstrand.net> wrote:
>
> __i915_ttm_get_pages does two things.  First, it calls ttm_bo_validate()
> to check the given placement and migrate the BO if needed.  Then, it
> updates the GEM object to match, in case the object was migrated.  If
> no migration occured, however, we might still have pages on the GEM
> object in which case we don't need to fetch them from TTM and call
> __i915_gem_object_set_pages.  This hasn't been a problem before because
> the primary user of __i915_ttm_get_pages is __i915_gem_object_get_pages
> which only calls it if the GEM object doesn't have pages.
>
> However, i915_ttm_migrate also uses __i915_ttm_get_pages to do the
> migration so this meant it was unsafe to call on an already populated
> object.  This patch checks i915_gem_object_has_pages() before trying to
> __i915_gem_object_set_pages so i915_ttm_migrate is safe to call, even on
> populated objects.
>
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index f253b11e9e367..771eb2963123f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -662,13 +662,14 @@  static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj,
 		i915_ttm_adjust_gem_after_move(obj);
 	}
 
-	GEM_WARN_ON(obj->mm.pages);
-	/* Object either has a page vector or is an iomem object */
-	st = bo->ttm ? i915_ttm_tt_get_st(bo->ttm) : obj->ttm.cached_io_st;
-	if (IS_ERR(st))
-		return PTR_ERR(st);
+	if (!i915_gem_object_has_pages(obj)) {
+		/* Object either has a page vector or is an iomem object */
+		st = bo->ttm ? i915_ttm_tt_get_st(bo->ttm) : obj->ttm.cached_io_st;
+		if (IS_ERR(st))
+			return PTR_ERR(st);
 
-	__i915_gem_object_set_pages(obj, st, i915_sg_dma_sizes(st->sgl));
+		__i915_gem_object_set_pages(obj, st, i915_sg_dma_sizes(st->sgl));
+	}
 
 	return ret;
 }