All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Auld <matthew.auld@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	dri-devel@lists.freedesktop.org,
	"Nirmoy Das" <nirmoy.das@intel.com>
Subject: [Intel-gfx] [PATCH 2/6] drm/i915/ttm: audit remaining bo->resource
Date: Mon, 30 Jan 2023 10:12:26 +0000	[thread overview]
Message-ID: <20230130101230.25347-2-matthew.auld@intel.com> (raw)
In-Reply-To: <20230130101230.25347-1-matthew.auld@intel.com>

In the near future TTM will have NULL bo->resource when the object is
initially created, plus after calling into pipeline-gutting. Try to
handle the remaining cases. In practice NULL bo->resource should be
taken to mean swapped-out or purged object.

References: 516198d317d8 ("drm/i915: audit bo->resource usage v3")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Christian König <ckoenig.leichtzumerken@gmail.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c      | 12 +++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |  7 ++++++-
 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c   |  7 +++++--
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 4758f21c91e1..4ba1d7862ff9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -472,7 +472,7 @@ static int i915_ttm_shrink(struct drm_i915_gem_object *obj, unsigned int flags)
 	struct ttm_placement place = {};
 	int ret;
 
-	if (!bo->ttm || bo->resource->mem_type != TTM_PL_SYSTEM)
+	if (!bo->ttm || (bo->resource && bo->resource->mem_type != TTM_PL_SYSTEM))
 		return 0;
 
 	GEM_BUG_ON(!i915_tt->is_shmem);
@@ -511,7 +511,13 @@ static void i915_ttm_delete_mem_notify(struct ttm_buffer_object *bo)
 {
 	struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
 
-	if (bo->resource && !i915_ttm_is_ghost_object(bo)) {
+	/*
+	 * This gets called twice by ttm, so long as we have a ttm resource or
+	 * ttm_tt then we can still safely call this. Due to pipeline-gutting,
+	 * we maybe have NULL bo->resource, but in that case we should always
+	 * have a ttm alive (like if the pages are swapped out).
+	 */
+	if ((bo->resource || bo->ttm) && !i915_ttm_is_ghost_object(bo)) {
 		__i915_gem_object_pages_fini(obj);
 		i915_ttm_free_cached_io_rsgt(obj);
 	}
@@ -1198,7 +1204,7 @@ static void i915_ttm_unmap_virtual(struct drm_i915_gem_object *obj)
 
 	assert_object_held_shared(obj);
 
-	if (i915_ttm_cpu_maps_iomem(bo->resource)) {
+	if (bo->resource && i915_ttm_cpu_maps_iomem(bo->resource)) {
 		wakeref = intel_runtime_pm_get(&to_i915(obj->base.dev)->runtime_pm);
 
 		/* userfault_count is protected by obj lock and rpm wakeref. */
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 76dd9e5e1a8b..72953ebadfd8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -83,7 +83,8 @@ void i915_ttm_adjust_domains_after_move(struct drm_i915_gem_object *obj)
 {
 	struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
 
-	if (i915_ttm_cpu_maps_iomem(bo->resource) || bo->ttm->caching != ttm_cached) {
+	if ((bo->resource && i915_ttm_cpu_maps_iomem(bo->resource)) ||
+	    bo->ttm->caching != ttm_cached) {
 		obj->write_domain = I915_GEM_DOMAIN_WC;
 		obj->read_domains = I915_GEM_DOMAIN_WC;
 	} else {
@@ -711,6 +712,10 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
 
 	assert_object_held(dst);
 	assert_object_held(src);
+
+	if (GEM_WARN_ON(!src_bo->resource || !dst_bo->resource))
+		return -EINVAL;
+
 	i915_deps_init(&deps, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
 
 	ret = dma_resv_reserve_fences(src_bo->base.resv, 1);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
index 7e67742bc65e..be44e7eed892 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
@@ -53,7 +53,7 @@ static int i915_ttm_backup(struct i915_gem_apply_to_region *apply,
 	unsigned int flags;
 	int err = 0;
 
-	if (bo->resource->mem_type == I915_PL_SYSTEM || obj->ttm.backup)
+	if (!bo->resource || bo->resource->mem_type == I915_PL_SYSTEM || obj->ttm.backup)
 		return 0;
 
 	if (pm_apply->allow_gpu && i915_gem_object_evictable(obj))
@@ -187,7 +187,10 @@ static int i915_ttm_restore(struct i915_gem_apply_to_region *apply,
 		return err;
 
 	/* Content may have been swapped. */
-	err = ttm_tt_populate(backup_bo->bdev, backup_bo->ttm, &ctx);
+	if (!backup_bo->resource)
+		err = ttm_bo_validate(backup_bo, i915_ttm_sys_placement(), &ctx);
+	if (!err)
+		err = ttm_tt_populate(backup_bo->bdev, backup_bo->ttm, &ctx);
 	if (!err) {
 		err = i915_gem_obj_copy_ttm(obj, backup, pm_apply->allow_gpu,
 					    false);
-- 
2.39.1


WARNING: multiple messages have this Message-ID (diff)
From: Matthew Auld <matthew.auld@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	dri-devel@lists.freedesktop.org,
	"Nirmoy Das" <nirmoy.das@intel.com>
Subject: [PATCH 2/6] drm/i915/ttm: audit remaining bo->resource
Date: Mon, 30 Jan 2023 10:12:26 +0000	[thread overview]
Message-ID: <20230130101230.25347-2-matthew.auld@intel.com> (raw)
In-Reply-To: <20230130101230.25347-1-matthew.auld@intel.com>

In the near future TTM will have NULL bo->resource when the object is
initially created, plus after calling into pipeline-gutting. Try to
handle the remaining cases. In practice NULL bo->resource should be
taken to mean swapped-out or purged object.

References: 516198d317d8 ("drm/i915: audit bo->resource usage v3")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Christian König <ckoenig.leichtzumerken@gmail.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c      | 12 +++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |  7 ++++++-
 drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c   |  7 +++++--
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 4758f21c91e1..4ba1d7862ff9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -472,7 +472,7 @@ static int i915_ttm_shrink(struct drm_i915_gem_object *obj, unsigned int flags)
 	struct ttm_placement place = {};
 	int ret;
 
-	if (!bo->ttm || bo->resource->mem_type != TTM_PL_SYSTEM)
+	if (!bo->ttm || (bo->resource && bo->resource->mem_type != TTM_PL_SYSTEM))
 		return 0;
 
 	GEM_BUG_ON(!i915_tt->is_shmem);
@@ -511,7 +511,13 @@ static void i915_ttm_delete_mem_notify(struct ttm_buffer_object *bo)
 {
 	struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
 
-	if (bo->resource && !i915_ttm_is_ghost_object(bo)) {
+	/*
+	 * This gets called twice by ttm, so long as we have a ttm resource or
+	 * ttm_tt then we can still safely call this. Due to pipeline-gutting,
+	 * we maybe have NULL bo->resource, but in that case we should always
+	 * have a ttm alive (like if the pages are swapped out).
+	 */
+	if ((bo->resource || bo->ttm) && !i915_ttm_is_ghost_object(bo)) {
 		__i915_gem_object_pages_fini(obj);
 		i915_ttm_free_cached_io_rsgt(obj);
 	}
@@ -1198,7 +1204,7 @@ static void i915_ttm_unmap_virtual(struct drm_i915_gem_object *obj)
 
 	assert_object_held_shared(obj);
 
-	if (i915_ttm_cpu_maps_iomem(bo->resource)) {
+	if (bo->resource && i915_ttm_cpu_maps_iomem(bo->resource)) {
 		wakeref = intel_runtime_pm_get(&to_i915(obj->base.dev)->runtime_pm);
 
 		/* userfault_count is protected by obj lock and rpm wakeref. */
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 76dd9e5e1a8b..72953ebadfd8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -83,7 +83,8 @@ void i915_ttm_adjust_domains_after_move(struct drm_i915_gem_object *obj)
 {
 	struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
 
-	if (i915_ttm_cpu_maps_iomem(bo->resource) || bo->ttm->caching != ttm_cached) {
+	if ((bo->resource && i915_ttm_cpu_maps_iomem(bo->resource)) ||
+	    bo->ttm->caching != ttm_cached) {
 		obj->write_domain = I915_GEM_DOMAIN_WC;
 		obj->read_domains = I915_GEM_DOMAIN_WC;
 	} else {
@@ -711,6 +712,10 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
 
 	assert_object_held(dst);
 	assert_object_held(src);
+
+	if (GEM_WARN_ON(!src_bo->resource || !dst_bo->resource))
+		return -EINVAL;
+
 	i915_deps_init(&deps, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
 
 	ret = dma_resv_reserve_fences(src_bo->base.resv, 1);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
index 7e67742bc65e..be44e7eed892 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_pm.c
@@ -53,7 +53,7 @@ static int i915_ttm_backup(struct i915_gem_apply_to_region *apply,
 	unsigned int flags;
 	int err = 0;
 
-	if (bo->resource->mem_type == I915_PL_SYSTEM || obj->ttm.backup)
+	if (!bo->resource || bo->resource->mem_type == I915_PL_SYSTEM || obj->ttm.backup)
 		return 0;
 
 	if (pm_apply->allow_gpu && i915_gem_object_evictable(obj))
@@ -187,7 +187,10 @@ static int i915_ttm_restore(struct i915_gem_apply_to_region *apply,
 		return err;
 
 	/* Content may have been swapped. */
-	err = ttm_tt_populate(backup_bo->bdev, backup_bo->ttm, &ctx);
+	if (!backup_bo->resource)
+		err = ttm_bo_validate(backup_bo, i915_ttm_sys_placement(), &ctx);
+	if (!err)
+		err = ttm_tt_populate(backup_bo->bdev, backup_bo->ttm, &ctx);
 	if (!err) {
 		err = i915_gem_obj_copy_ttm(obj, backup, pm_apply->allow_gpu,
 					    false);
-- 
2.39.1


  reply	other threads:[~2023-01-30 10:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-30 10:12 [Intel-gfx] [PATCH 1/6] drm/i915/ttm: fix sparse warning Matthew Auld
2023-01-30 10:12 ` Matthew Auld
2023-01-30 10:12 ` Matthew Auld [this message]
2023-01-30 10:12   ` [PATCH 2/6] drm/i915/ttm: audit remaining bo->resource Matthew Auld
2023-01-30 11:00   ` [Intel-gfx] " Andrzej Hajda
2023-01-30 11:40     ` Matthew Auld
2023-01-30 10:12 ` [Intel-gfx] [PATCH 3/6] drm/ttm: clear the ttm_tt when bo->resource is NULL Matthew Auld
2023-01-30 10:12   ` Matthew Auld
2023-01-30 10:12 ` [Intel-gfx] [PATCH 4/6] drm/ttm: stop allocating dummy resources during BO creation Matthew Auld
2023-01-30 10:12   ` Matthew Auld
2023-01-30 10:12 ` [Intel-gfx] [PATCH 5/6] drm/ttm: stop allocating a dummy resource for pipelined gutting Matthew Auld
2023-01-30 10:12   ` Matthew Auld
2023-01-30 10:12 ` [Intel-gfx] [PATCH 6/6] drm/ttm: prevent moving of pinned BOs Matthew Auld
2023-01-30 10:12   ` Matthew Auld
2023-01-30 10:51 ` [Intel-gfx] [PATCH 1/6] drm/i915/ttm: fix sparse warning Andrzej Hajda
2023-01-30 14:47 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/6] " Patchwork
2023-01-30 15:14 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-02-01  7:28 ` [PATCH 1/6] " Christian König
2023-02-01  7:28   ` [Intel-gfx] " Christian König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230130101230.25347-2-matthew.auld@intel.com \
    --to=matthew.auld@intel.com \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=nirmoy.das@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.