Re: [PATCH v6 02/64] drm/i915: Pin timeline map after first timeline pin, v3.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Reviewed-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxx>


On 1/5/21 4:34 PM, Maarten Lankhorst wrote:
We're starting to require the reservation lock for pinning,
so wait until we have that.

Update the selftests to handle this correctly, and ensure pin is
called in live_hwsp_rollover_user() and mock_hwsp_freelist().

Changes since v1:
- Fix NULL + XX arithmatic, use casts. (kbuild)
Changes since v2:
- Clear entire cacheline when pinning.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Reported-by: kernel test robot <lkp@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/intel_timeline.c    | 40 +++++++++----
  drivers/gpu/drm/i915/gt/intel_timeline.h    |  2 +
  drivers/gpu/drm/i915/gt/mock_engine.c       | 22 ++++++-
  drivers/gpu/drm/i915/gt/selftest_timeline.c | 63 +++++++++++----------
  drivers/gpu/drm/i915/i915_selftest.h        |  2 +
  5 files changed, 84 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 3d43f15f34f3..7509af471341 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -53,14 +53,29 @@ static int __timeline_active(struct i915_active *active)
  	return 0;
  }
+I915_SELFTEST_EXPORT int
+intel_timeline_pin_map(struct intel_timeline *timeline)
+{
+	struct drm_i915_gem_object *obj = timeline->hwsp_ggtt->obj;
+	u32 ofs = offset_in_page(timeline->hwsp_offset);
+	void *vaddr;
+
+	vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+	if (IS_ERR(vaddr))
+		return PTR_ERR(vaddr);
+
+	timeline->hwsp_map = vaddr;
+	timeline->hwsp_seqno = memset(vaddr + ofs, 0, CACHELINE_BYTES);
+	clflush(vaddr + ofs);
+
+	return 0;
+}
+
  static int intel_timeline_init(struct intel_timeline *timeline,
  			       struct intel_gt *gt,
  			       struct i915_vma *hwsp,
  			       unsigned int offset)
  {
-	void *vaddr;
-	u32 *seqno;
-
  	kref_init(&timeline->kref);
  	atomic_set(&timeline->pin_count, 0);
@@ -77,14 +92,8 @@ static int intel_timeline_init(struct intel_timeline *timeline,
  		timeline->hwsp_ggtt = hwsp;
  	}
- vaddr = i915_gem_object_pin_map(hwsp->obj, I915_MAP_WB);
-	if (IS_ERR(vaddr))
-		return PTR_ERR(vaddr);
-
-	timeline->hwsp_map = vaddr;
-	seqno = vaddr + timeline->hwsp_offset;
-	WRITE_ONCE(*seqno, 0);
-	timeline->hwsp_seqno = seqno;
+	timeline->hwsp_map = NULL;
+	timeline->hwsp_seqno = (void *)(long)timeline->hwsp_offset;
GEM_BUG_ON(timeline->hwsp_offset >= hwsp->size); @@ -114,7 +123,8 @@ static void intel_timeline_fini(struct rcu_head *rcu)
  	struct intel_timeline *timeline =
  		container_of(rcu, struct intel_timeline, rcu);
- i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
+	if (timeline->hwsp_map)
+		i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
i915_vma_put(timeline->hwsp_ggtt);
  	i915_active_fini(&timeline->active);
@@ -174,6 +184,12 @@ int intel_timeline_pin(struct intel_timeline *tl, struct i915_gem_ww_ctx *ww)
  	if (atomic_add_unless(&tl->pin_count, 1, 0))
  		return 0;
+ if (!tl->hwsp_map) {
+		err = intel_timeline_pin_map(tl);
+		if (err)
+			return err;
+	}
+
  	err = i915_ggtt_pin(tl->hwsp_ggtt, ww, 0, PIN_HIGH);
  	if (err)
  		return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h b/drivers/gpu/drm/i915/gt/intel_timeline.h
index f502a619843f..c50543d3ed5d 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
@@ -110,4 +110,6 @@ void intel_gt_show_timelines(struct intel_gt *gt,
  						  const char *prefix,
  						  int indent));
+I915_SELFTEST_DECLARE(int intel_timeline_pin_map(struct intel_timeline *tl));
+
  #endif
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index 2f830017c51d..effbac877eec 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -32,9 +32,20 @@
  #include "mock_engine.h"
  #include "selftests/mock_request.h"
-static void mock_timeline_pin(struct intel_timeline *tl)
+static int mock_timeline_pin(struct intel_timeline *tl)
  {
+	int err;
+
+	if (WARN_ON(!i915_gem_object_trylock(tl->hwsp_ggtt->obj)))
+		return -EBUSY;
+
+	err = intel_timeline_pin_map(tl);
+	i915_gem_object_unlock(tl->hwsp_ggtt->obj);
+	if (err)
+		return err;
+
  	atomic_inc(&tl->pin_count);
+	return 0;
  }
static void mock_timeline_unpin(struct intel_timeline *tl)
@@ -152,6 +163,8 @@ static void mock_context_destroy(struct kref *ref)
static int mock_context_alloc(struct intel_context *ce)
  {
+	int err;
+
  	ce->ring = mock_ring(ce->engine);
  	if (!ce->ring)
  		return -ENOMEM;
@@ -162,7 +175,12 @@ static int mock_context_alloc(struct intel_context *ce)
  		return PTR_ERR(ce->timeline);
  	}
- mock_timeline_pin(ce->timeline);
+	err = mock_timeline_pin(ce->timeline);
+	if (err) {
+		intel_timeline_put(ce->timeline);
+		ce->timeline = NULL;
+		return err;
+	}
return 0;
  }
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index 6e32e91cdab4..2e2bea7ac7a5 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -35,7 +35,7 @@ static unsigned long hwsp_cacheline(struct intel_timeline *tl)
  {
  	unsigned long address = (unsigned long)page_address(hwsp_page(tl));
- return (address + tl->hwsp_offset) / CACHELINE_BYTES;
+	return (address + offset_in_page(tl->hwsp_offset)) / CACHELINE_BYTES;
  }
#define CACHELINES_PER_PAGE (PAGE_SIZE / CACHELINE_BYTES)
@@ -59,6 +59,7 @@ static void __mock_hwsp_record(struct mock_hwsp_freelist *state,
  	tl = xchg(&state->history[idx], tl);
  	if (tl) {
  		radix_tree_delete(&state->cachelines, hwsp_cacheline(tl));
+		intel_timeline_unpin(tl);
  		intel_timeline_put(tl);
  	}
  }
@@ -78,6 +79,12 @@ static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
  		if (IS_ERR(tl))
  			return PTR_ERR(tl);
+ err = intel_timeline_pin(tl, NULL);
+		if (err) {
+			intel_timeline_put(tl);
+			return err;
+		}
+
  		cacheline = hwsp_cacheline(tl);
  		err = radix_tree_insert(&state->cachelines, cacheline, tl);
  		if (err) {
@@ -85,6 +92,7 @@ static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
  				pr_err("HWSP cacheline %lu already used; duplicate allocation!\n",
  				       cacheline);
  			}
+			intel_timeline_unpin(tl);
  			intel_timeline_put(tl);
  			return err;
  		}
@@ -452,7 +460,7 @@ static int emit_ggtt_store_dw(struct i915_request *rq, u32 addr, u32 value)
  }
static struct i915_request *
-tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
+checked_tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
  {
  	struct i915_request *rq;
  	int err;
@@ -463,6 +471,13 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
  		goto out;
  	}
+ if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
+		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
+		       *tl->hwsp_seqno, tl->seqno);
+		intel_timeline_unpin(tl);
+		return ERR_PTR(-EINVAL);
+	}
+
  	rq = intel_engine_create_kernel_request(engine);
  	if (IS_ERR(rq))
  		goto out_unpin;
@@ -484,25 +499,6 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
  	return rq;
  }
-static struct intel_timeline *
-checked_intel_timeline_create(struct intel_gt *gt)
-{
-	struct intel_timeline *tl;
-
-	tl = intel_timeline_create(gt);
-	if (IS_ERR(tl))
-		return tl;
-
-	if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
-		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
-		       *tl->hwsp_seqno, tl->seqno);
-		intel_timeline_put(tl);
-		return ERR_PTR(-EINVAL);
-	}
-
-	return tl;
-}
-
  static int live_hwsp_engine(void *arg)
  {
  #define NUM_TIMELINES 4096
@@ -535,13 +531,13 @@ static int live_hwsp_engine(void *arg)
  			struct intel_timeline *tl;
  			struct i915_request *rq;
- tl = checked_intel_timeline_create(gt);
+			tl = intel_timeline_create(gt);
  			if (IS_ERR(tl)) {
  				err = PTR_ERR(tl);
  				break;
  			}
- rq = tl_write(tl, engine, count);
+			rq = checked_tl_write(tl, engine, count);
  			if (IS_ERR(rq)) {
  				intel_timeline_put(tl);
  				err = PTR_ERR(rq);
@@ -608,14 +604,14 @@ static int live_hwsp_alternate(void *arg)
  			if (!intel_engine_can_store_dword(engine))
  				continue;
- tl = checked_intel_timeline_create(gt);
+			tl = intel_timeline_create(gt);
  			if (IS_ERR(tl)) {
  				err = PTR_ERR(tl);
  				goto out;
  			}
intel_engine_pm_get(engine);
-			rq = tl_write(tl, engine, count);
+			rq = checked_tl_write(tl, engine, count);
  			intel_engine_pm_put(engine);
  			if (IS_ERR(rq)) {
  				intel_timeline_put(tl);
@@ -1258,6 +1254,10 @@ static int live_hwsp_rollover_user(void *arg)
  		if (!tl->has_initial_breadcrumb)
  			goto out;
+ err = intel_context_pin(ce);
+		if (err)
+			goto out;
+
  		tl->seqno = -4u;
  		WRITE_ONCE(*(u32 *)tl->hwsp_seqno, tl->seqno);
@@ -1267,7 +1267,7 @@ static int live_hwsp_rollover_user(void *arg)
  			this = intel_context_create_request(ce);
  			if (IS_ERR(this)) {
  				err = PTR_ERR(this);
-				goto out;
+				goto out_unpin;
  			}
pr_debug("%s: create fence.seqnp:%d\n",
@@ -1286,17 +1286,18 @@ static int live_hwsp_rollover_user(void *arg)
  		if (i915_request_wait(rq[2], 0, HZ / 5) < 0) {
  			pr_err("Wait for timeline wrap timed out!\n");
  			err = -EIO;
-			goto out;
+			goto out_unpin;
  		}
for (i = 0; i < ARRAY_SIZE(rq); i++) {
  			if (!i915_request_completed(rq[i])) {
  				pr_err("Pre-wrap request not completed!\n");
  				err = -EINVAL;
-				goto out;
+				goto out_unpin;
  			}
  		}
-
+out_unpin:
+		intel_context_unpin(ce);
  out:
  		for (i = 0; i < ARRAY_SIZE(rq); i++)
  			i915_request_put(rq[i]);
@@ -1338,13 +1339,13 @@ static int live_hwsp_recycle(void *arg)
  			struct intel_timeline *tl;
  			struct i915_request *rq;
- tl = checked_intel_timeline_create(gt);
+			tl = intel_timeline_create(gt);
  			if (IS_ERR(tl)) {
  				err = PTR_ERR(tl);
  				break;
  			}
- rq = tl_write(tl, engine, count);
+			rq = checked_tl_write(tl, engine, count);
  			if (IS_ERR(rq)) {
  				intel_timeline_put(tl);
  				err = PTR_ERR(rq);
diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
index d53d207ab6eb..f54de0499be7 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -107,6 +107,7 @@ int __i915_subtests(const char *caller,
#define I915_SELFTEST_DECLARE(x) x
  #define I915_SELFTEST_ONLY(x) unlikely(x)
+#define I915_SELFTEST_EXPORT
#else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */ @@ -116,6 +117,7 @@ static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; } #define I915_SELFTEST_DECLARE(x)
  #define I915_SELFTEST_ONLY(x) 0
+#define I915_SELFTEST_EXPORT static
#endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux