All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt
@ 2019-10-17  9:44 Tvrtko Ursulin
  2019-10-17  9:45 ` [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites Tvrtko Ursulin
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-10-17  9:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Medium term goal is to eliminate the i915->engine[] array and to get there
we have recently introduced equivalent array in intel_gt. Now we need to
migrate the code further towards this state.

This next step is to eliminate usage of i915->engines[] from the
for_each_engine_masked iterator.

For this to work we also need to use engine->id as index when populating
the gt->engine[] array and adjust the default engine set indexing to use
engine->legacy_idx instead of assuming gt->engines[] indexing.

v2:
 * Populate gt->engine[] earlier.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |  4 ++--
 drivers/gpu/drm/i915/gt/intel_engine_cs.c   |  3 +++
 drivers/gpu/drm/i915/gt/intel_engine_user.c |  2 --
 drivers/gpu/drm/i915/gt/intel_gt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_hangcheck.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_reset.c       | 12 ++++++------
 drivers/gpu/drm/i915/gvt/execlist.c         |  4 ++--
 drivers/gpu/drm/i915/gvt/scheduler.c        |  2 +-
 drivers/gpu/drm/i915/i915_active.c          |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h             |  6 +++---
 10 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 5d8221c7ba83..4e30cb974648 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -209,8 +209,8 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
 			return ERR_CAST(ce);
 		}
 
-		e->engines[id] = ce;
-		e->num_engines = id + 1;
+		e->engines[engine->legacy_idx] = ce;
+		e->num_engines = engine->legacy_idx + 1;
 	}
 
 	return e;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 5051a1fd2565..e93b5c679c4f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -330,7 +330,10 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	gt->engine_class[info->class][info->instance] = engine;
 
 	intel_engine_add_user(engine);
+	GEM_BUG_ON(id >= ARRAY_SIZE(gt->i915->engine));
 	gt->i915->engine[id] = engine;
+	GEM_BUG_ON(id >= ARRAY_SIZE(gt->engine));
+	gt->engine[id] = engine;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 77cd5de83930..e9f3fbaa74a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -183,8 +183,6 @@ static void add_legacy_ring(struct legacy_ring *ring,
 	if (unlikely(idx == -1))
 		return;
 
-	GEM_BUG_ON(idx >= ARRAY_SIZE(ring->gt->engine));
-	ring->gt->engine[idx] = engine;
 	ring->instance++;
 
 	engine->legacy_idx = idx;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index b3619a2a5d0e..c99b6b2f38c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -186,7 +186,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
-		for_each_engine_masked(engine, i915, engine_mask, id)
+		for_each_engine_masked(engine, gt, engine_mask, id)
 			gen8_clear_engine_error_register(engine);
 	}
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index c14dbeb3ccc3..b2af73984f93 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -237,7 +237,7 @@ static void hangcheck_declare_hang(struct intel_gt *gt,
 		hung &= ~stuck;
 	len = scnprintf(msg, sizeof(msg),
 			"%s on ", stuck == hung ? "no progress" : "hang");
-	for_each_engine_masked(engine, gt->i915, hung, tmp)
+	for_each_engine_masked(engine, gt, hung, tmp)
 		len += scnprintf(msg + len, sizeof(msg) - len,
 				 "%s, ", engine->name);
 	msg[len-2] = '\0';
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 477bfafdb103..b191b0745703 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -298,7 +298,7 @@ static int gen6_reset_engines(struct intel_gt *gt,
 		intel_engine_mask_t tmp;
 
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 		}
@@ -432,7 +432,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 		hw_mask = GEN11_GRDOM_FULL;
 	} else {
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 			ret = gen11_lock_sfc(engine, &hw_mask);
@@ -451,7 +451,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 	 * expiration).
 	 */
 	if (engine_mask != ALL_ENGINES)
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+		for_each_engine_masked(engine, gt, engine_mask, tmp)
 			gen11_unlock_sfc(engine);
 
 	return ret;
@@ -510,7 +510,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 	intel_engine_mask_t tmp;
 	int ret;
 
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+	for_each_engine_masked(engine, gt, engine_mask, tmp) {
 		ret = gen8_engine_reset_prepare(engine);
 		if (ret && !reset_non_ready)
 			goto skip_reset;
@@ -536,7 +536,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 		ret = gen6_reset_engines(gt, engine_mask, retry);
 
 skip_reset:
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+	for_each_engine_masked(engine, gt, engine_mask, tmp)
 		gen8_engine_reset_cancel(engine);
 
 	return ret;
@@ -1206,7 +1206,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 	 * single reset fails.
 	 */
 	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
 			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
 					     &gt->reset.flags))
diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
index f21b8fb5b37e..d6e7a1189bad 100644
--- a/drivers/gpu/drm/i915/gvt/execlist.c
+++ b/drivers/gpu/drm/i915/gvt/execlist.c
@@ -534,7 +534,7 @@ static void clean_execlist(struct intel_vgpu *vgpu,
 	struct intel_vgpu_submission *s = &vgpu->submission;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		kfree(s->ring_scan_buffer[engine->id]);
 		s->ring_scan_buffer[engine->id] = NULL;
 		s->ring_scan_buffer_size[engine->id] = 0;
@@ -548,7 +548,7 @@ static void reset_execlist(struct intel_vgpu *vgpu,
 	struct intel_engine_cs *engine;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp)
 		init_vgpu_execlist(vgpu, engine->id);
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 6850f1f40241..9ebb2534558b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -887,7 +887,7 @@ void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,
 	intel_engine_mask_t tmp;
 
 	/* free the unsubmited workloads in the queues. */
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		list_for_each_entry_safe(pos, n,
 			&s->workload_q_head[engine->id], list) {
 			list_del_init(&pos->list);
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index aa37c07004b9..7927b1a0c7a6 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -590,8 +590,8 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *i915 = engine->i915;
 	intel_engine_mask_t tmp, mask = engine->mask;
+	struct intel_gt *gt = engine->gt;
 	struct llist_node *pos, *next;
 	int err;
 
@@ -603,7 +603,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	 * We can then use the preallocated nodes in
 	 * i915_active_acquire_barrier()
 	 */
-	for_each_engine_masked(engine, i915, mask, tmp) {
+	for_each_engine_masked(engine, gt, mask, tmp) {
 		u64 idx = engine->kernel_context->timeline->fence_context;
 		struct active_node *node;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 88956f37d96c..40e923b0c2c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1415,10 +1415,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
 
 /* Iterator over subset of engines selected by mask */
-#define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
-	for ((tmp__) = (mask__) & INTEL_INFO(dev_priv__)->engine_mask; \
+#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
+	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
 	     (tmp__) ? \
-	     ((engine__) = (dev_priv__)->engine[__mask_next_bit(tmp__)]), 1 : \
+	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
 	     0;)
 
 #define rb_to_uabi_engine(rb) \
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites
  2019-10-17  9:44 [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt Tvrtko Ursulin
@ 2019-10-17  9:45 ` Tvrtko Ursulin
  2019-10-17 10:01   ` Chris Wilson
  2019-10-17  9:57 ` [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt Chris Wilson
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-10-17  9:45 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Where the function, or code segment, operates on intel_gt, we need to
start passing it instead of i915 to for_each_engine(_masked).

This is another partial step in migration of i915->engines[] to
gt->engines[].

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  4 +-
 drivers/gpu/drm/i915/gt/intel_gt.c            |  2 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm.c         |  8 ++--
 drivers/gpu/drm/i915/gt/intel_gt_requests.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_hangcheck.c     |  4 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         | 18 ++++-----
 drivers/gpu/drm/i915/gt/selftest_context.c    |  6 +--
 drivers/gpu/drm/i915/gt/selftest_engine_pm.c  |  2 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 20 +++++-----
 drivers/gpu/drm/i915/gt/selftest_lrc.c        | 38 +++++++++----------
 drivers/gpu/drm/i915/gt/selftest_reset.c      |  2 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  8 ++--
 .../gpu/drm/i915/gt/selftest_workarounds.c    | 10 ++---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  6 +--
 drivers/gpu/drm/i915/selftests/igt_reset.c    |  4 +-
 15 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index e93b5c679c4f..65ab4990604e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1121,7 +1121,7 @@ bool intel_engines_are_idle(struct intel_gt *gt)
 	if (!READ_ONCE(gt->awake))
 		return true;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (!intel_engine_is_idle(engine))
 			return false;
 	}
@@ -1134,7 +1134,7 @@ void intel_engines_reset_default_submission(struct intel_gt *gt)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		engine->set_default_submission(engine);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index c99b6b2f38c2..1c4b6c9642ad 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -197,7 +197,7 @@ static void gen6_check_faults(struct intel_gt *gt)
 	enum intel_engine_id id;
 	u32 fault;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		fault = GEN6_RING_FAULT_REG_READ(engine);
 		if (fault & RING_FAULT_VALID) {
 			DRM_DEBUG_DRIVER("Unexpected fault\n"
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index bd1bd3e00a94..b866d5b1eee0 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -136,16 +136,16 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force)
 
 	intel_uc_sanitize(&gt->uc);
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		if (engine->reset.prepare)
 			engine->reset.prepare(engine);
 
 	if (reset_engines(gt) || force) {
-		for_each_engine(engine, gt->i915, id)
+		for_each_engine(engine, gt, id)
 			__intel_engine_reset(engine, false);
 	}
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		if (engine->reset.finish)
 			engine->reset.finish(engine);
 }
@@ -177,7 +177,7 @@ int intel_gt_resume(struct intel_gt *gt)
 	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
 	intel_rc6_sanitize(&gt->rc6);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
 		intel_engine_pm_get(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index cbb4069b11e1..b73229a84d85 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -25,7 +25,7 @@ static void flush_submission(struct intel_gt *gt)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		intel_engine_flush_submission(engine);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index b2af73984f93..0fdef00af9e4 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -281,7 +281,7 @@ static void hangcheck_elapsed(struct work_struct *work)
 	 */
 	intel_uncore_arm_unclaimed_mmio_detection(gt->uncore);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct hangcheck hc;
 
 		intel_engine_breadcrumbs_irq(engine);
@@ -303,7 +303,7 @@ static void hangcheck_elapsed(struct work_struct *work)
 	if (GEM_SHOW_DEBUG() && (hung | stuck)) {
 		struct drm_printer p = drm_debug_printer("hangcheck");
 
-		for_each_engine(engine, gt->i915, id) {
+		for_each_engine(engine, gt, id) {
 			if (intel_engine_is_idle(engine))
 				continue;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b191b0745703..bf8d1ed4b1d8 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -682,7 +682,7 @@ static intel_engine_mask_t reset_prepare(struct intel_gt *gt)
 	intel_engine_mask_t awake = 0;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (intel_engine_pm_get_if_awake(engine))
 			awake |= engine->mask;
 		reset_prepare_engine(engine);
@@ -712,7 +712,7 @@ static int gt_reset(struct intel_gt *gt, intel_engine_mask_t stalled_mask)
 	if (err)
 		return err;
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		__intel_engine_reset(engine, stalled_mask & engine->mask);
 
 	i915_gem_restore_fences(gt->ggtt);
@@ -733,7 +733,7 @@ static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		reset_finish_engine(engine);
 		if (awake & engine->mask)
 			intel_engine_pm_put(engine);
@@ -769,7 +769,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt)
 	if (GEM_SHOW_DEBUG() && !intel_engines_are_idle(gt)) {
 		struct drm_printer p = drm_debug_printer(__func__);
 
-		for_each_engine(engine, gt->i915, id)
+		for_each_engine(engine, gt, id)
 			intel_engine_dump(engine, &p, "%s\n", engine->name);
 	}
 
@@ -786,7 +786,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt)
 	if (!INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
 		__intel_gt_reset(gt, ALL_ENGINES);
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		engine->submit_request = nop_submit_request;
 
 	/*
@@ -798,7 +798,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt)
 	set_bit(I915_WEDGED, &gt->reset.flags);
 
 	/* Mark all executing requests as skipped */
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		engine->cancel_requests(engine);
 
 	reset_finish(gt, awake);
@@ -934,7 +934,7 @@ static int resume(struct intel_gt *gt)
 	enum intel_engine_id id;
 	int ret;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		ret = engine->resume(engine);
 		if (ret)
 			return ret;
@@ -1234,7 +1234,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 	synchronize_rcu_expedited();
 
 	/* Prevent any other reset-engine attempt. */
-	for_each_engine(engine, gt->i915, tmp) {
+	for_each_engine(engine, gt, tmp) {
 		while (test_and_set_bit(I915_RESET_ENGINE + engine->id,
 					&gt->reset.flags))
 			wait_on_bit(&gt->reset.flags,
@@ -1244,7 +1244,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 
 	intel_gt_reset_global(gt, engine_mask, msg);
 
-	for_each_engine(engine, gt->i915, tmp)
+	for_each_engine(engine, gt, tmp)
 		clear_bit_unlock(I915_RESET_ENGINE + engine->id,
 				 &gt->reset.flags);
 	clear_bit_unlock(I915_RESET_BACKOFF, &gt->reset.flags);
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index 7c838a57e174..f63a26a3e620 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -159,7 +159,7 @@ static int live_context_size(void *arg)
 	if (IS_ERR(fixme))
 		return PTR_ERR(fixme);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct {
 			struct drm_i915_gem_object *state;
 			void *pinned;
@@ -305,7 +305,7 @@ static int live_active_context(void *arg)
 		goto out_file;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		err = __live_active_context(engine, fixme);
 		if (err)
 			break;
@@ -415,7 +415,7 @@ static int live_remote_context(void *arg)
 		goto out_file;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		err = __live_remote_context(engine, fixme);
 		if (err)
 			break;
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
index 3a1419376912..20b9c83f43ad 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
@@ -25,7 +25,7 @@ static int live_engine_pm(void *arg)
 	}
 
 	GEM_BUG_ON(intel_gt_pm_is_awake(gt));
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		const typeof(*igt_atomic_phases) *p;
 
 		for (p = igt_atomic_phases; p->name; p++) {
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 569a4105d49e..8e0016464325 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -323,7 +323,7 @@ static int igt_hang_sanitycheck(void *arg)
 	if (err)
 		return err;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct intel_wedge_me w;
 		long timeout;
 
@@ -400,7 +400,7 @@ static int igt_reset_nop(void *arg)
 	reset_count = i915_reset_count(global);
 	count = 0;
 	do {
-		for_each_engine(engine, gt->i915, id) {
+		for_each_engine(engine, gt, id) {
 			int i;
 
 			for (i = 0; i < 16; i++) {
@@ -471,7 +471,7 @@ static int igt_reset_nop_engine(void *arg)
 	}
 
 	i915_gem_context_clear_bannable(ctx);
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		unsigned int reset_count, reset_engine_count;
 		unsigned int count;
 		IGT_TIMEOUT(end_time);
@@ -560,7 +560,7 @@ static int __igt_reset_engine(struct intel_gt *gt, bool active)
 			return err;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		unsigned int reset_count, reset_engine_count;
 		IGT_TIMEOUT(end_time);
 
@@ -782,7 +782,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 			h.ctx->sched.priority = 1024;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct active_engine threads[I915_NUM_ENGINES] = {};
 		unsigned long device = i915_reset_count(global);
 		unsigned long count = 0, reported;
@@ -800,7 +800,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 		}
 
 		memset(threads, 0, sizeof(threads));
-		for_each_engine(other, gt->i915, tmp) {
+		for_each_engine(other, gt, tmp) {
 			struct task_struct *tsk;
 
 			threads[tmp].resets =
@@ -914,7 +914,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 		}
 
 unwind:
-		for_each_engine(other, gt->i915, tmp) {
+		for_each_engine(other, gt, tmp) {
 			int ret;
 
 			if (!threads[tmp].task)
@@ -1335,7 +1335,7 @@ static int wait_for_others(struct intel_gt *gt,
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (engine == exclude)
 			continue;
 
@@ -1363,7 +1363,7 @@ static int igt_reset_queue(void *arg)
 	if (err)
 		goto unlock;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_request *prev;
 		IGT_TIMEOUT(end_time);
 		unsigned int count;
@@ -1651,7 +1651,7 @@ static int igt_reset_engines_atomic(void *arg)
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
-		for_each_engine(engine, gt->i915, id) {
+		for_each_engine(engine, gt, id) {
 			err = igt_atomic_reset_engine(engine, p);
 			if (err)
 				goto out;
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 2868371c609e..007d6203fe0e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -123,7 +123,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio)
 		goto err_spin;
 
 	err = 0;
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct intel_context *ce[2] = {};
 		struct i915_request *rq[2];
 		struct igt_live_test t;
@@ -384,7 +384,7 @@ slice_semaphore_queue(struct intel_engine_cs *outer,
 		return PTR_ERR(head);
 
 	i915_request_get(head);
-	for_each_engine(engine, outer->i915, id) {
+	for_each_engine(engine, outer->gt, id) {
 		for (i = 0; i < count; i++) {
 			struct i915_request *rq;
 
@@ -456,7 +456,7 @@ static int live_timeslice_preempt(void *arg)
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
-		for_each_engine(engine, gt->i915, id) {
+		for_each_engine(engine, gt, id) {
 			if (!intel_engine_has_preemption(engine))
 				continue;
 
@@ -532,7 +532,7 @@ static int live_busywait_preempt(void *arg)
 	if (err)
 		goto err_map;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_request *lo, *hi;
 		struct igt_live_test t;
 		u32 *cs;
@@ -708,7 +708,7 @@ static int live_preempt(void *arg)
 	ctx_lo->sched.priority =
 		I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct igt_live_test t;
 		struct i915_request *rq;
 
@@ -804,7 +804,7 @@ static int live_late_preempt(void *arg)
 	/* Make sure ctx_lo stays before ctx_hi until we trigger preemption. */
 	ctx_lo->sched.priority = I915_USER_PRIORITY(1);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct igt_live_test t;
 		struct i915_request *rq;
 
@@ -929,7 +929,7 @@ static int live_nopreempt(void *arg)
 		goto err_client_a;
 	b.ctx->sched.priority = I915_USER_PRIORITY(I915_PRIORITY_MAX);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_request *rq_a, *rq_b;
 
 		if (!intel_engine_has_preemption(engine))
@@ -1040,7 +1040,7 @@ static int live_suppress_self_preempt(void *arg)
 	if (preempt_client_init(gt, &b))
 		goto err_client_a;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_request *rq_a, *rq_b;
 		int depth;
 
@@ -1207,7 +1207,7 @@ static int live_suppress_wait_preempt(void *arg)
 	if (preempt_client_init(gt, &client[3])) /* bystander */
 		goto err_client_2;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		int depth;
 
 		if (!intel_engine_has_preemption(engine))
@@ -1318,7 +1318,7 @@ static int live_chain_preempt(void *arg)
 	if (preempt_client_init(gt, &lo))
 		goto err_client_hi;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_sched_attr attr = {
 			.priority = I915_USER_PRIORITY(I915_PRIORITY_MAX),
 		};
@@ -1467,7 +1467,7 @@ static int live_preempt_hang(void *arg)
 	ctx_lo->sched.priority =
 		I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_request *rq;
 
 		if (!intel_engine_has_preemption(engine))
@@ -1656,7 +1656,7 @@ static int smoke_crescendo(struct preempt_smoke *smoke, unsigned int flags)
 	unsigned long count;
 	int err = 0;
 
-	for_each_engine(engine, smoke->gt->i915, id) {
+	for_each_engine(engine, smoke->gt, id) {
 		arg[id] = *smoke;
 		arg[id].engine = engine;
 		if (!(flags & BATCH))
@@ -1673,7 +1673,7 @@ static int smoke_crescendo(struct preempt_smoke *smoke, unsigned int flags)
 	}
 
 	count = 0;
-	for_each_engine(engine, smoke->gt->i915, id) {
+	for_each_engine(engine, smoke->gt, id) {
 		int status;
 
 		if (IS_ERR_OR_NULL(tsk[id]))
@@ -1702,7 +1702,7 @@ static int smoke_random(struct preempt_smoke *smoke, unsigned int flags)
 
 	count = 0;
 	do {
-		for_each_engine(smoke->engine, smoke->gt->i915, id) {
+		for_each_engine(smoke->engine, smoke->gt, id) {
 			struct i915_gem_context *ctx = smoke_context(smoke);
 			int err;
 
@@ -1937,7 +1937,7 @@ static int live_virtual_engine(void *arg)
 	if (USES_GUC_SUBMISSION(gt->i915))
 		return 0;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		err = nop_virtual_engine(gt, &engine, 1, 1, 0);
 		if (err) {
 			pr_err("Failed to wrap engine %s: err=%d\n",
@@ -2270,7 +2270,7 @@ static int bond_virtual_engine(struct intel_gt *gt,
 
 	err = 0;
 	rq[0] = ERR_PTR(-ENOMEM);
-	for_each_engine(master, gt->i915, id) {
+	for_each_engine(master, gt, id) {
 		struct i915_sw_fence fence = {};
 
 		if (master->class == class)
@@ -2508,7 +2508,7 @@ static int live_lrc_layout(void *arg)
 		return -ENOMEM;
 
 	err = 0;
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		u32 *hw, *lrc;
 		int dw;
 
@@ -2701,7 +2701,7 @@ static int live_lrc_state(void *arg)
 		goto out_close;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		err = __live_lrc_state(fixme, engine, scratch);
 		if (err)
 			break;
@@ -2844,7 +2844,7 @@ static int live_gpr_clear(void *arg)
 		goto out_close;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		err = __live_gpr_clear(fixme, engine, scratch);
 		if (err)
 			break;
diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index 419b38fa7828..6efb9221b7fa 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -125,7 +125,7 @@ static int igt_atomic_engine_reset(void *arg)
 	if (!igt_force_reset(gt))
 		goto out_unlock;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		tasklet_disable_nosync(&engine->execlists.tasklet);
 		intel_engine_pm_get(engine);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index 473a56b0ae46..dac86f699a4c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -519,7 +519,7 @@ static int live_hwsp_engine(void *arg)
 		return -ENOMEM;
 
 	count = 0;
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (!intel_engine_can_store_dword(engine))
 			continue;
 
@@ -594,7 +594,7 @@ static int live_hwsp_alternate(void *arg)
 
 	count = 0;
 	for (n = 0; n < NUM_TIMELINES; n++) {
-		for_each_engine(engine, gt->i915, id) {
+		for_each_engine(engine, gt, id) {
 			struct intel_timeline *tl;
 			struct i915_request *rq;
 
@@ -666,7 +666,7 @@ static int live_hwsp_wrap(void *arg)
 	if (err)
 		goto out_free;
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		const u32 *hwsp_seqno[2];
 		struct i915_request *rq;
 		u32 seqno[2];
@@ -766,7 +766,7 @@ static int live_hwsp_recycle(void *arg)
 	 */
 
 	count = 0;
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		IGT_TIMEOUT(end_time);
 
 		if (!intel_engine_can_store_dword(engine))
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index 8856c6c46cc4..ef02920cec29 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -69,7 +69,7 @@ reference_lists_init(struct intel_gt *gt, struct wa_lists *lists)
 	gt_init_workarounds(gt->i915, &lists->gt_wa_list);
 	wa_init_finish(&lists->gt_wa_list);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		struct i915_wa_list *wal = &lists->engine[id].wa_list;
 
 		wa_init_start(wal, "REF", engine->name);
@@ -88,7 +88,7 @@ reference_lists_fini(struct intel_gt *gt, struct wa_lists *lists)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		intel_wa_list_free(&lists->engine[id].wa_list);
 
 	intel_wa_list_free(&lists->gt_wa_list);
@@ -726,7 +726,7 @@ static int live_dirty_whitelist(void *arg)
 		goto out_file;
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (engine->whitelist.count == 0)
 			continue;
 
@@ -750,7 +750,7 @@ static int live_reset_whitelist(void *arg)
 	/* If we reset the gpu, we should not lose the RING_NONPRIV */
 	igt_global_reset_lock(gt);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (engine->whitelist.count == 0)
 			continue;
 
@@ -1048,7 +1048,7 @@ static int live_isolated_whitelist(void *arg)
 		i915_vm_put(vm);
 	}
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		if (!engine->kernel_context->vm)
 			continue;
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index f325d3dd564f..e9713a21779e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1014,7 +1014,7 @@ static void guc_interrupts_capture(struct intel_gt *gt)
 	 * to GuC
 	 */
 	irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
 
 	/* route USER_INTERRUPT to Host, all others are sent to GuC. */
@@ -1062,7 +1062,7 @@ static void guc_interrupts_release(struct intel_gt *gt)
 	 */
 	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
 	irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING);
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
 
 	/* route all GT interrupts to the host */
@@ -1145,7 +1145,7 @@ int intel_guc_submission_enable(struct intel_guc *guc)
 	/* Take over from manual control of ELSP (execlists) */
 	guc_interrupts_capture(gt);
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		engine->set_default_submission = guc_set_default_submission;
 		engine->set_default_submission(engine);
 	}
diff --git a/drivers/gpu/drm/i915/selftests/igt_reset.c b/drivers/gpu/drm/i915/selftests/igt_reset.c
index 7ec8f8b049c6..9f8590b868a9 100644
--- a/drivers/gpu/drm/i915/selftests/igt_reset.c
+++ b/drivers/gpu/drm/i915/selftests/igt_reset.c
@@ -22,7 +22,7 @@ void igt_global_reset_lock(struct intel_gt *gt)
 		wait_event(gt->reset.queue,
 			   !test_bit(I915_RESET_BACKOFF, &gt->reset.flags));
 
-	for_each_engine(engine, gt->i915, id) {
+	for_each_engine(engine, gt, id) {
 		while (test_and_set_bit(I915_RESET_ENGINE + id,
 					&gt->reset.flags))
 			wait_on_bit(&gt->reset.flags, I915_RESET_ENGINE + id,
@@ -35,7 +35,7 @@ void igt_global_reset_unlock(struct intel_gt *gt)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, gt->i915, id)
+	for_each_engine(engine, gt, id)
 		clear_bit(I915_RESET_ENGINE + id, &gt->reset.flags);
 
 	clear_bit(I915_RESET_BACKOFF, &gt->reset.flags);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17  9:44 [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt Tvrtko Ursulin
  2019-10-17  9:45 ` [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites Tvrtko Ursulin
@ 2019-10-17  9:57 ` Chris Wilson
  2019-10-17 10:33   ` Tvrtko Ursulin
  2019-10-17 13:45   ` [PATCH v3 1/2] " Tvrtko Ursulin
  2019-10-17 10:00 ` [PATCH " Chris Wilson
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2019-10-17  9:57 UTC (permalink / raw)
  To: intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Medium term goal is to eliminate the i915->engine[] array and to get there
we have recently introduced equivalent array in intel_gt. Now we need to
migrate the code further towards this state.

This next step is to eliminate usage of i915->engines[] from the
for_each_engine_masked iterator.

For this to work we also need to use engine->id as index when populating
the gt->engine[] array and adjust the default engine set indexing to use
engine->legacy_idx instead of assuming gt->engines[] indexing.

v2:
  * Populate gt->engine[] earlier.
  * Check that we don't duplicate engine->legacy_idx

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c  | 13 ++++++++++---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  4 ++++
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  1 +
 drivers/gpu/drm/i915/gt/intel_engine_user.c  | 14 +++-----------
 drivers/gpu/drm/i915/gt/intel_gt.c           |  2 +-
 drivers/gpu/drm/i915/gt/intel_hangcheck.c    |  2 +-
 drivers/gpu/drm/i915/gt/intel_reset.c        | 12 ++++++------
 drivers/gpu/drm/i915/gvt/execlist.c          |  4 ++--
 drivers/gpu/drm/i915/gvt/scheduler.c         |  2 +-
 drivers/gpu/drm/i915/i915_active.c           |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h              |  6 +++---
 11 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 5d8221c7ba83..5e1d89a5721b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -203,15 +203,22 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
+		if (engine->legacy_idx == INVALID_ENGINE)
+			continue;
+
+		GEM_BUG_ON(engine->legacy_idx > I915_NUM_ENGINES);
+		GEM_BUG_ON(e->engines[engine->legacy_idx]);
+
 		ce = intel_context_create(ctx, engine);
 		if (IS_ERR(ce)) {
-			__free_engines(e, id);
+			__free_engines(e, e->num_engines + 1);
 			return ERR_CAST(ce);
 		}
 
-		e->engines[id] = ce;
-		e->num_engines = id + 1;
+		e->engines[engine->legacy_idx] = ce;
+		e->num_engines = max(e->num_engines, engine->legacy_idx);
 	}
+	e->num_engines++;
 
 	return e;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 5051a1fd2565..4dcdb909b62d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -277,6 +277,9 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	BUILD_BUG_ON(MAX_ENGINE_CLASS >= BIT(GEN11_ENGINE_CLASS_WIDTH));
 	BUILD_BUG_ON(MAX_ENGINE_INSTANCE >= BIT(GEN11_ENGINE_INSTANCE_WIDTH));
 
+	if (GEM_DEBUG_WARN_ON(id >= ARRAY_SIZE(gt->engine)))
+		return -EINVAL;
+
 	if (GEM_DEBUG_WARN_ON(info->class > MAX_ENGINE_CLASS))
 		return -EINVAL;
 
@@ -328,6 +331,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	intel_engine_sanitize_mmio(engine);
 
 	gt->engine_class[info->class][info->instance] = engine;
+	gt->engine[id] = engine;
 
 	intel_engine_add_user(engine);
 	gt->i915->engine[id] = engine;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 6199064f332b..3451be034caf 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -148,6 +148,7 @@ enum intel_engine_id {
 	VECS1,
 #define _VECS(n) (VECS0 + (n))
 	I915_NUM_ENGINES
+#define INVALID_ENGINE ((enum intel_engine_id)-1)
 };
 
 struct st_preempt_hang {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 77cd5de83930..1e28bfeab519 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -171,23 +171,15 @@ static int legacy_ring_idx(const struct legacy_ring *ring)
 static void add_legacy_ring(struct legacy_ring *ring,
 			    struct intel_engine_cs *engine)
 {
-	int idx;
-
 	if (engine->gt != ring->gt || engine->class != ring->class) {
 		ring->gt = engine->gt;
 		ring->class = engine->class;
 		ring->instance = 0;
 	}
 
-	idx = legacy_ring_idx(ring);
-	if (unlikely(idx == -1))
-		return;
-
-	GEM_BUG_ON(idx >= ARRAY_SIZE(ring->gt->engine));
-	ring->gt->engine[idx] = engine;
-	ring->instance++;
-
-	engine->legacy_idx = idx;
+	engine->legacy_idx = legacy_ring_idx(ring);
+	if (engine->legacy_idx != INVALID_ENGINE)
+		ring->instance++;
 }
 
 void intel_engines_driver_register(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index b3619a2a5d0e..c99b6b2f38c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -186,7 +186,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
-		for_each_engine_masked(engine, i915, engine_mask, id)
+		for_each_engine_masked(engine, gt, engine_mask, id)
 			gen8_clear_engine_error_register(engine);
 	}
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index c14dbeb3ccc3..b2af73984f93 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -237,7 +237,7 @@ static void hangcheck_declare_hang(struct intel_gt *gt,
 		hung &= ~stuck;
 	len = scnprintf(msg, sizeof(msg),
 			"%s on ", stuck == hung ? "no progress" : "hang");
-	for_each_engine_masked(engine, gt->i915, hung, tmp)
+	for_each_engine_masked(engine, gt, hung, tmp)
 		len += scnprintf(msg + len, sizeof(msg) - len,
 				 "%s, ", engine->name);
 	msg[len-2] = '\0';
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 477bfafdb103..b191b0745703 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -298,7 +298,7 @@ static int gen6_reset_engines(struct intel_gt *gt,
 		intel_engine_mask_t tmp;
 
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 		}
@@ -432,7 +432,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 		hw_mask = GEN11_GRDOM_FULL;
 	} else {
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 			ret = gen11_lock_sfc(engine, &hw_mask);
@@ -451,7 +451,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 	 * expiration).
 	 */
 	if (engine_mask != ALL_ENGINES)
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+		for_each_engine_masked(engine, gt, engine_mask, tmp)
 			gen11_unlock_sfc(engine);
 
 	return ret;
@@ -510,7 +510,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 	intel_engine_mask_t tmp;
 	int ret;
 
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+	for_each_engine_masked(engine, gt, engine_mask, tmp) {
 		ret = gen8_engine_reset_prepare(engine);
 		if (ret && !reset_non_ready)
 			goto skip_reset;
@@ -536,7 +536,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 		ret = gen6_reset_engines(gt, engine_mask, retry);
 
 skip_reset:
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+	for_each_engine_masked(engine, gt, engine_mask, tmp)
 		gen8_engine_reset_cancel(engine);
 
 	return ret;
@@ -1206,7 +1206,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 	 * single reset fails.
 	 */
 	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
 			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
 					     &gt->reset.flags))
diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
index f21b8fb5b37e..d6e7a1189bad 100644
--- a/drivers/gpu/drm/i915/gvt/execlist.c
+++ b/drivers/gpu/drm/i915/gvt/execlist.c
@@ -534,7 +534,7 @@ static void clean_execlist(struct intel_vgpu *vgpu,
 	struct intel_vgpu_submission *s = &vgpu->submission;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		kfree(s->ring_scan_buffer[engine->id]);
 		s->ring_scan_buffer[engine->id] = NULL;
 		s->ring_scan_buffer_size[engine->id] = 0;
@@ -548,7 +548,7 @@ static void reset_execlist(struct intel_vgpu *vgpu,
 	struct intel_engine_cs *engine;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp)
 		init_vgpu_execlist(vgpu, engine->id);
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 6850f1f40241..9ebb2534558b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -887,7 +887,7 @@ void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,
 	intel_engine_mask_t tmp;
 
 	/* free the unsubmited workloads in the queues. */
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		list_for_each_entry_safe(pos, n,
 			&s->workload_q_head[engine->id], list) {
 			list_del_init(&pos->list);
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index aa37c07004b9..7927b1a0c7a6 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -590,8 +590,8 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *i915 = engine->i915;
 	intel_engine_mask_t tmp, mask = engine->mask;
+	struct intel_gt *gt = engine->gt;
 	struct llist_node *pos, *next;
 	int err;
 
@@ -603,7 +603,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	 * We can then use the preallocated nodes in
 	 * i915_active_acquire_barrier()
 	 */
-	for_each_engine_masked(engine, i915, mask, tmp) {
+	for_each_engine_masked(engine, gt, mask, tmp) {
 		u64 idx = engine->kernel_context->timeline->fence_context;
 		struct active_node *node;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 88956f37d96c..40e923b0c2c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1415,10 +1415,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
 
 /* Iterator over subset of engines selected by mask */
-#define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
-	for ((tmp__) = (mask__) & INTEL_INFO(dev_priv__)->engine_mask; \
+#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
+	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
 	     (tmp__) ? \
-	     ((engine__) = (dev_priv__)->engine[__mask_next_bit(tmp__)]), 1 : \
+	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
 	     0;)
 
 #define rb_to_uabi_engine(rb) \
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17  9:44 [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt Tvrtko Ursulin
  2019-10-17  9:45 ` [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites Tvrtko Ursulin
  2019-10-17  9:57 ` [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt Chris Wilson
@ 2019-10-17 10:00 ` Chris Wilson
  2019-10-17 22:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4) Patchwork
  2019-10-17 22:38 ` ✗ Fi.CI.BAT: failure " Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-10-17 10:00 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-10-17 10:44:59)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Medium term goal is to eliminate the i915->engine[] array and to get there
> we have recently introduced equivalent array in intel_gt. Now we need to
> migrate the code further towards this state.
> 
> This next step is to eliminate usage of i915->engines[] from the
> for_each_engine_masked iterator.
> 
> For this to work we also need to use engine->id as index when populating
> the gt->engine[] array and adjust the default engine set indexing to use
> engine->legacy_idx instead of assuming gt->engines[] indexing.
> 
> v2:
>  * Populate gt->engine[] earlier.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

I checked all the legacy_idx to make sure there wasn't anything nasty
there, and it all looked okay.

So once we have the early foible sorted out,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites
  2019-10-17  9:45 ` [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites Tvrtko Ursulin
@ 2019-10-17 10:01   ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-10-17 10:01 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-10-17 10:45:00)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Where the function, or code segment, operates on intel_gt, we need to
> start passing it instead of i915 to for_each_engine(_masked).
> 
> This is another partial step in migration of i915->engines[] to
> gt->engines[].
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17  9:57 ` [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt Chris Wilson
@ 2019-10-17 10:33   ` Tvrtko Ursulin
  2019-10-17 13:45   ` [PATCH v3 1/2] " Tvrtko Ursulin
  1 sibling, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-10-17 10:33 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


Now you're making me looks silly and reply to myself. :))

On 17/10/2019 10:57, Chris Wilson wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Medium term goal is to eliminate the i915->engine[] array and to get there
> we have recently introduced equivalent array in intel_gt. Now we need to
> migrate the code further towards this state.
> 
> This next step is to eliminate usage of i915->engines[] from the
> for_each_engine_masked iterator.
> 
> For this to work we also need to use engine->id as index when populating
> the gt->engine[] array and adjust the default engine set indexing to use
> engine->legacy_idx instead of assuming gt->engines[] indexing.
> 
> v2:
>    * Populate gt->engine[] earlier.
>    * Check that we don't duplicate engine->legacy_idx
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c  | 13 ++++++++++---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  4 ++++
>   drivers/gpu/drm/i915/gt/intel_engine_types.h |  1 +
>   drivers/gpu/drm/i915/gt/intel_engine_user.c  | 14 +++-----------
>   drivers/gpu/drm/i915/gt/intel_gt.c           |  2 +-
>   drivers/gpu/drm/i915/gt/intel_hangcheck.c    |  2 +-
>   drivers/gpu/drm/i915/gt/intel_reset.c        | 12 ++++++------
>   drivers/gpu/drm/i915/gvt/execlist.c          |  4 ++--
>   drivers/gpu/drm/i915/gvt/scheduler.c         |  2 +-
>   drivers/gpu/drm/i915/i915_active.c           |  4 ++--
>   drivers/gpu/drm/i915/i915_drv.h              |  6 +++---
>   11 files changed, 34 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 5d8221c7ba83..5e1d89a5721b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -203,15 +203,22 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
>   	for_each_engine(engine, gt, id) {
>   		struct intel_context *ce;
>   
> +		if (engine->legacy_idx == INVALID_ENGINE)
> +			continue;
> +
> +		GEM_BUG_ON(engine->legacy_idx > I915_NUM_ENGINES);

 >= ?

> +		GEM_BUG_ON(e->engines[engine->legacy_idx]);
> +
>   		ce = intel_context_create(ctx, engine);
>   		if (IS_ERR(ce)) {
> -			__free_engines(e, id);
> +			__free_engines(e, e->num_engines + 1);
>   			return ERR_CAST(ce);
>   		}
>   
> -		e->engines[id] = ce;
> -		e->num_engines = id + 1;
> +		e->engines[engine->legacy_idx] = ce;
> +		e->num_engines = max(e->num_engines, engine->legacy_idx);
>   	}
> +	e->num_engines++;
>   
>   	return e;
>   }
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 5051a1fd2565..4dcdb909b62d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -277,6 +277,9 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	BUILD_BUG_ON(MAX_ENGINE_CLASS >= BIT(GEN11_ENGINE_CLASS_WIDTH));
>   	BUILD_BUG_ON(MAX_ENGINE_INSTANCE >= BIT(GEN11_ENGINE_INSTANCE_WIDTH));
>   
> +	if (GEM_DEBUG_WARN_ON(id >= ARRAY_SIZE(gt->engine)))
> +		return -EINVAL;
> +
>   	if (GEM_DEBUG_WARN_ON(info->class > MAX_ENGINE_CLASS))
>   		return -EINVAL;
>   
> @@ -328,6 +331,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	intel_engine_sanitize_mmio(engine);
>   
>   	gt->engine_class[info->class][info->instance] = engine;
> +	gt->engine[id] = engine;
>   
>   	intel_engine_add_user(engine);
>   	gt->i915->engine[id] = engine;
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 6199064f332b..3451be034caf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -148,6 +148,7 @@ enum intel_engine_id {
>   	VECS1,
>   #define _VECS(n) (VECS0 + (n))
>   	I915_NUM_ENGINES
> +#define INVALID_ENGINE ((enum intel_engine_id)-1)
>   };
>   
>   struct st_preempt_hang {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> index 77cd5de83930..1e28bfeab519 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> @@ -171,23 +171,15 @@ static int legacy_ring_idx(const struct legacy_ring *ring)
>   static void add_legacy_ring(struct legacy_ring *ring,
>   			    struct intel_engine_cs *engine)
>   {
> -	int idx;
> -
>   	if (engine->gt != ring->gt || engine->class != ring->class) {
>   		ring->gt = engine->gt;
>   		ring->class = engine->class;
>   		ring->instance = 0;
>   	}
>   
> -	idx = legacy_ring_idx(ring);
> -	if (unlikely(idx == -1))
> -		return;
> -
> -	GEM_BUG_ON(idx >= ARRAY_SIZE(ring->gt->engine));
> -	ring->gt->engine[idx] = engine;
> -	ring->instance++;
> -
> -	engine->legacy_idx = idx;
> +	engine->legacy_idx = legacy_ring_idx(ring);
> +	if (engine->legacy_idx != INVALID_ENGINE)

Bonus points if legacy_ring_idx actually used the new INVALID_ENGINE define.

> +		ring->instance++;
>   }
>   
>   void intel_engines_driver_register(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index b3619a2a5d0e..c99b6b2f38c2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -186,7 +186,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
>   		struct intel_engine_cs *engine;
>   		enum intel_engine_id id;
>   
> -		for_each_engine_masked(engine, i915, engine_mask, id)
> +		for_each_engine_masked(engine, gt, engine_mask, id)
>   			gen8_clear_engine_error_register(engine);
>   	}
>   }
> diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> index c14dbeb3ccc3..b2af73984f93 100644
> --- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> @@ -237,7 +237,7 @@ static void hangcheck_declare_hang(struct intel_gt *gt,
>   		hung &= ~stuck;
>   	len = scnprintf(msg, sizeof(msg),
>   			"%s on ", stuck == hung ? "no progress" : "hang");
> -	for_each_engine_masked(engine, gt->i915, hung, tmp)
> +	for_each_engine_masked(engine, gt, hung, tmp)
>   		len += scnprintf(msg + len, sizeof(msg) - len,
>   				 "%s, ", engine->name);
>   	msg[len-2] = '\0';
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 477bfafdb103..b191b0745703 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -298,7 +298,7 @@ static int gen6_reset_engines(struct intel_gt *gt,
>   		intel_engine_mask_t tmp;
>   
>   		hw_mask = 0;
> -		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
> +		for_each_engine_masked(engine, gt, engine_mask, tmp) {
>   			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
>   			hw_mask |= hw_engine_mask[engine->id];
>   		}
> @@ -432,7 +432,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
>   		hw_mask = GEN11_GRDOM_FULL;
>   	} else {
>   		hw_mask = 0;
> -		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
> +		for_each_engine_masked(engine, gt, engine_mask, tmp) {
>   			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
>   			hw_mask |= hw_engine_mask[engine->id];
>   			ret = gen11_lock_sfc(engine, &hw_mask);
> @@ -451,7 +451,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
>   	 * expiration).
>   	 */
>   	if (engine_mask != ALL_ENGINES)
> -		for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
> +		for_each_engine_masked(engine, gt, engine_mask, tmp)
>   			gen11_unlock_sfc(engine);
>   
>   	return ret;
> @@ -510,7 +510,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
>   	intel_engine_mask_t tmp;
>   	int ret;
>   
> -	for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
> +	for_each_engine_masked(engine, gt, engine_mask, tmp) {
>   		ret = gen8_engine_reset_prepare(engine);
>   		if (ret && !reset_non_ready)
>   			goto skip_reset;
> @@ -536,7 +536,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
>   		ret = gen6_reset_engines(gt, engine_mask, retry);
>   
>   skip_reset:
> -	for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
> +	for_each_engine_masked(engine, gt, engine_mask, tmp)
>   		gen8_engine_reset_cancel(engine);
>   
>   	return ret;
> @@ -1206,7 +1206,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
>   	 * single reset fails.
>   	 */
>   	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
> -		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
> +		for_each_engine_masked(engine, gt, engine_mask, tmp) {
>   			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
>   			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
>   					     &gt->reset.flags))
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
> index f21b8fb5b37e..d6e7a1189bad 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -534,7 +534,7 @@ static void clean_execlist(struct intel_vgpu *vgpu,
>   	struct intel_vgpu_submission *s = &vgpu->submission;
>   	intel_engine_mask_t tmp;
>   
> -	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
> +	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
>   		kfree(s->ring_scan_buffer[engine->id]);
>   		s->ring_scan_buffer[engine->id] = NULL;
>   		s->ring_scan_buffer_size[engine->id] = 0;
> @@ -548,7 +548,7 @@ static void reset_execlist(struct intel_vgpu *vgpu,
>   	struct intel_engine_cs *engine;
>   	intel_engine_mask_t tmp;
>   
> -	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> +	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp)
>   		init_vgpu_execlist(vgpu, engine->id);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 6850f1f40241..9ebb2534558b 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -887,7 +887,7 @@ void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,
>   	intel_engine_mask_t tmp;
>   
>   	/* free the unsubmited workloads in the queues. */
> -	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
> +	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
>   		list_for_each_entry_safe(pos, n,
>   			&s->workload_q_head[engine->id], list) {
>   			list_del_init(&pos->list);
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index aa37c07004b9..7927b1a0c7a6 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -590,8 +590,8 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
>   int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   					    struct intel_engine_cs *engine)
>   {
> -	struct drm_i915_private *i915 = engine->i915;
>   	intel_engine_mask_t tmp, mask = engine->mask;
> +	struct intel_gt *gt = engine->gt;
>   	struct llist_node *pos, *next;
>   	int err;
>   
> @@ -603,7 +603,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   	 * We can then use the preallocated nodes in
>   	 * i915_active_acquire_barrier()
>   	 */
> -	for_each_engine_masked(engine, i915, mask, tmp) {
> +	for_each_engine_masked(engine, gt, mask, tmp) {
>   		u64 idx = engine->kernel_context->timeline->fence_context;
>   		struct active_node *node;
>   
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 88956f37d96c..40e923b0c2c8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1415,10 +1415,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>   		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
>   
>   /* Iterator over subset of engines selected by mask */
> -#define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
> -	for ((tmp__) = (mask__) & INTEL_INFO(dev_priv__)->engine_mask; \
> +#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
> +	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
>   	     (tmp__) ? \
> -	     ((engine__) = (dev_priv__)->engine[__mask_next_bit(tmp__)]), 1 : \
> +	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
>   	     0;)
>   
>   #define rb_to_uabi_engine(rb) \
> 

Regards,

Tvrtko

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17  9:57 ` [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt Chris Wilson
  2019-10-17 10:33   ` Tvrtko Ursulin
@ 2019-10-17 13:45   ` Tvrtko Ursulin
  2019-10-17 13:51     ` Chris Wilson
  2019-10-17 16:18     ` [PATCH v4 " Tvrtko Ursulin
  1 sibling, 2 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-10-17 13:45 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Medium term goal is to eliminate the i915->engine[] array and to get there
we have recently introduced equivalent array in intel_gt. Now we need to
migrate the code further towards this state.

This next step is to eliminate usage of i915->engines[] from the
for_each_engine_masked iterator.

For this to work we also need to use engine->id as index when populating
the gt->engine[] array and adjust the default engine set indexing to use
engine->legacy_idx instead of assuming gt->engines[] indexing.

v2:
 * Populate gt->engine[] earlier.

v3:
 * Work around the initialization order issue between default_engines()
   and intel_engines_driver_register() which sets engine->legacy_idx for
   now. It will be fixed properly later.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |  7 +++++--
 drivers/gpu/drm/i915/gt/intel_engine_cs.c   |  4 ++++
 drivers/gpu/drm/i915/gt/intel_engine_user.c |  2 --
 drivers/gpu/drm/i915/gt/intel_gt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_hangcheck.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_reset.c       | 12 ++++++------
 drivers/gpu/drm/i915/gvt/execlist.c         |  4 ++--
 drivers/gpu/drm/i915/gvt/scheduler.c        |  2 +-
 drivers/gpu/drm/i915/i915_active.c          |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h             |  6 +++---
 10 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 5d8221c7ba83..87d66d8faff1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -203,14 +203,17 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
+		if (engine->legacy_idx == -1)
+			continue;
+
 		ce = intel_context_create(ctx, engine);
 		if (IS_ERR(ce)) {
 			__free_engines(e, id);
 			return ERR_CAST(ce);
 		}
 
-		e->engines[id] = ce;
-		e->num_engines = id + 1;
+		e->engines[engine->legacy_idx] = ce;
+		e->num_engines = engine->legacy_idx + 1;
 	}
 
 	return e;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 5051a1fd2565..d1a752a664ec 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -293,6 +293,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	BUILD_BUG_ON(BITS_PER_TYPE(engine->mask) < I915_NUM_ENGINES);
 
 	engine->id = id;
+	engine->legacy_idx = -1;
 	engine->mask = BIT(id);
 	engine->i915 = gt->i915;
 	engine->gt = gt;
@@ -330,7 +331,10 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	gt->engine_class[info->class][info->instance] = engine;
 
 	intel_engine_add_user(engine);
+	GEM_BUG_ON(id >= ARRAY_SIZE(gt->i915->engine));
 	gt->i915->engine[id] = engine;
+	GEM_BUG_ON(id >= ARRAY_SIZE(gt->engine));
+	gt->engine[id] = engine;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 77cd5de83930..e9f3fbaa74a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -183,8 +183,6 @@ static void add_legacy_ring(struct legacy_ring *ring,
 	if (unlikely(idx == -1))
 		return;
 
-	GEM_BUG_ON(idx >= ARRAY_SIZE(ring->gt->engine));
-	ring->gt->engine[idx] = engine;
 	ring->instance++;
 
 	engine->legacy_idx = idx;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index b3619a2a5d0e..c99b6b2f38c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -186,7 +186,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
-		for_each_engine_masked(engine, i915, engine_mask, id)
+		for_each_engine_masked(engine, gt, engine_mask, id)
 			gen8_clear_engine_error_register(engine);
 	}
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index c14dbeb3ccc3..b2af73984f93 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -237,7 +237,7 @@ static void hangcheck_declare_hang(struct intel_gt *gt,
 		hung &= ~stuck;
 	len = scnprintf(msg, sizeof(msg),
 			"%s on ", stuck == hung ? "no progress" : "hang");
-	for_each_engine_masked(engine, gt->i915, hung, tmp)
+	for_each_engine_masked(engine, gt, hung, tmp)
 		len += scnprintf(msg + len, sizeof(msg) - len,
 				 "%s, ", engine->name);
 	msg[len-2] = '\0';
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 477bfafdb103..b191b0745703 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -298,7 +298,7 @@ static int gen6_reset_engines(struct intel_gt *gt,
 		intel_engine_mask_t tmp;
 
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 		}
@@ -432,7 +432,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 		hw_mask = GEN11_GRDOM_FULL;
 	} else {
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 			ret = gen11_lock_sfc(engine, &hw_mask);
@@ -451,7 +451,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 	 * expiration).
 	 */
 	if (engine_mask != ALL_ENGINES)
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+		for_each_engine_masked(engine, gt, engine_mask, tmp)
 			gen11_unlock_sfc(engine);
 
 	return ret;
@@ -510,7 +510,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 	intel_engine_mask_t tmp;
 	int ret;
 
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+	for_each_engine_masked(engine, gt, engine_mask, tmp) {
 		ret = gen8_engine_reset_prepare(engine);
 		if (ret && !reset_non_ready)
 			goto skip_reset;
@@ -536,7 +536,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 		ret = gen6_reset_engines(gt, engine_mask, retry);
 
 skip_reset:
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+	for_each_engine_masked(engine, gt, engine_mask, tmp)
 		gen8_engine_reset_cancel(engine);
 
 	return ret;
@@ -1206,7 +1206,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 	 * single reset fails.
 	 */
 	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
 			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
 					     &gt->reset.flags))
diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
index f21b8fb5b37e..d6e7a1189bad 100644
--- a/drivers/gpu/drm/i915/gvt/execlist.c
+++ b/drivers/gpu/drm/i915/gvt/execlist.c
@@ -534,7 +534,7 @@ static void clean_execlist(struct intel_vgpu *vgpu,
 	struct intel_vgpu_submission *s = &vgpu->submission;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		kfree(s->ring_scan_buffer[engine->id]);
 		s->ring_scan_buffer[engine->id] = NULL;
 		s->ring_scan_buffer_size[engine->id] = 0;
@@ -548,7 +548,7 @@ static void reset_execlist(struct intel_vgpu *vgpu,
 	struct intel_engine_cs *engine;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp)
 		init_vgpu_execlist(vgpu, engine->id);
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 6850f1f40241..9ebb2534558b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -887,7 +887,7 @@ void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,
 	intel_engine_mask_t tmp;
 
 	/* free the unsubmited workloads in the queues. */
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		list_for_each_entry_safe(pos, n,
 			&s->workload_q_head[engine->id], list) {
 			list_del_init(&pos->list);
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index aa37c07004b9..7927b1a0c7a6 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -590,8 +590,8 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *i915 = engine->i915;
 	intel_engine_mask_t tmp, mask = engine->mask;
+	struct intel_gt *gt = engine->gt;
 	struct llist_node *pos, *next;
 	int err;
 
@@ -603,7 +603,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	 * We can then use the preallocated nodes in
 	 * i915_active_acquire_barrier()
 	 */
-	for_each_engine_masked(engine, i915, mask, tmp) {
+	for_each_engine_masked(engine, gt, mask, tmp) {
 		u64 idx = engine->kernel_context->timeline->fence_context;
 		struct active_node *node;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 88956f37d96c..40e923b0c2c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1415,10 +1415,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
 
 /* Iterator over subset of engines selected by mask */
-#define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
-	for ((tmp__) = (mask__) & INTEL_INFO(dev_priv__)->engine_mask; \
+#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
+	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
 	     (tmp__) ? \
-	     ((engine__) = (dev_priv__)->engine[__mask_next_bit(tmp__)]), 1 : \
+	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
 	     0;)
 
 #define rb_to_uabi_engine(rb) \
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17 13:45   ` [PATCH v3 1/2] " Tvrtko Ursulin
@ 2019-10-17 13:51     ` Chris Wilson
  2019-10-17 16:18     ` [PATCH v4 " Tvrtko Ursulin
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-10-17 13:51 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-10-17 14:45:31)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Medium term goal is to eliminate the i915->engine[] array and to get there
> we have recently introduced equivalent array in intel_gt. Now we need to
> migrate the code further towards this state.
> 
> This next step is to eliminate usage of i915->engines[] from the
> for_each_engine_masked iterator.
> 
> For this to work we also need to use engine->id as index when populating
> the gt->engine[] array and adjust the default engine set indexing to use
> engine->legacy_idx instead of assuming gt->engines[] indexing.
> 
> v2:
>  * Populate gt->engine[] earlier.
> 
> v3:
>  * Work around the initialization order issue between default_engines()
>    and intel_engines_driver_register() which sets engine->legacy_idx for
>    now. It will be fixed properly later.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c |  7 +++++--
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c   |  4 ++++
>  drivers/gpu/drm/i915/gt/intel_engine_user.c |  2 --
>  drivers/gpu/drm/i915/gt/intel_gt.c          |  2 +-
>  drivers/gpu/drm/i915/gt/intel_hangcheck.c   |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c       | 12 ++++++------
>  drivers/gpu/drm/i915/gvt/execlist.c         |  4 ++--
>  drivers/gpu/drm/i915/gvt/scheduler.c        |  2 +-
>  drivers/gpu/drm/i915/i915_active.c          |  4 ++--
>  drivers/gpu/drm/i915/i915_drv.h             |  6 +++---
>  10 files changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 5d8221c7ba83..87d66d8faff1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -203,14 +203,17 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
>         for_each_engine(engine, gt, id) {
>                 struct intel_context *ce;
>  
> +               if (engine->legacy_idx == -1)
> +                       continue;
> +
>                 ce = intel_context_create(ctx, engine);
>                 if (IS_ERR(ce)) {
>                         __free_engines(e, id);
>                         return ERR_CAST(ce);
>                 }
>  
> -               e->engines[id] = ce;
> -               e->num_engines = id + 1;
> +               e->engines[engine->legacy_idx] = ce;
> +               e->num_engines = engine->legacy_idx + 1;

e->num_engines = max(e->num_engines, engine->legacy_idx + 1);

and remove id above.

Please take the v2.5, throw in the legacy_idx=-1 and remove bits until
you are happy :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 1/2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17 13:45   ` [PATCH v3 1/2] " Tvrtko Ursulin
  2019-10-17 13:51     ` Chris Wilson
@ 2019-10-17 16:18     ` Tvrtko Ursulin
  2019-10-17 20:54       ` Chris Wilson
  1 sibling, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-10-17 16:18 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Medium term goal is to eliminate the i915->engine[] array and to get there
we have recently introduced equivalent array in intel_gt. Now we need to
migrate the code further towards this state.

This next step is to eliminate usage of i915->engines[] from the
for_each_engine_masked iterator.

For this to work we also need to use engine->id as index when populating
the gt->engine[] array and adjust the default engine set indexing to use
engine->legacy_idx instead of assuming gt->engines[] indexing.

v2:
  * Populate gt->engine[] earlier.
  * Check that we don't duplicate engine->legacy_idx

v3:
  * Work around the initialization order issue between default_engines()
    and intel_engines_driver_register() which sets engine->legacy_idx for
    now. It will be fixed properly later.

v4:
  * Merge with forgotten v2.5.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c  | 13 ++++++++++---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  5 +++++
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  1 +
 drivers/gpu/drm/i915/gt/intel_engine_user.c  | 18 +++++-------------
 drivers/gpu/drm/i915/gt/intel_gt.c           |  2 +-
 drivers/gpu/drm/i915/gt/intel_hangcheck.c    |  2 +-
 drivers/gpu/drm/i915/gt/intel_reset.c        | 12 ++++++------
 drivers/gpu/drm/i915/gvt/execlist.c          |  4 ++--
 drivers/gpu/drm/i915/gvt/scheduler.c         |  2 +-
 drivers/gpu/drm/i915/i915_active.c           |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h              |  6 +++---
 11 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 5d8221c7ba83..7b01f4605f21 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -203,15 +203,22 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
+		if (engine->legacy_idx == INVALID_ENGINE)
+			continue;
+
+		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
+		GEM_BUG_ON(e->engines[engine->legacy_idx]);
+
 		ce = intel_context_create(ctx, engine);
 		if (IS_ERR(ce)) {
-			__free_engines(e, id);
+			__free_engines(e, e->num_engines + 1);
 			return ERR_CAST(ce);
 		}
 
-		e->engines[id] = ce;
-		e->num_engines = id + 1;
+		e->engines[engine->legacy_idx] = ce;
+		e->num_engines = max(e->num_engines, engine->legacy_idx);
 	}
+	e->num_engines++;
 
 	return e;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 5051a1fd2565..e514c68b0713 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -277,6 +277,9 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	BUILD_BUG_ON(MAX_ENGINE_CLASS >= BIT(GEN11_ENGINE_CLASS_WIDTH));
 	BUILD_BUG_ON(MAX_ENGINE_INSTANCE >= BIT(GEN11_ENGINE_INSTANCE_WIDTH));
 
+	if (GEM_DEBUG_WARN_ON(id >= ARRAY_SIZE(gt->engine)))
+		return -EINVAL;
+
 	if (GEM_DEBUG_WARN_ON(info->class > MAX_ENGINE_CLASS))
 		return -EINVAL;
 
@@ -293,6 +296,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	BUILD_BUG_ON(BITS_PER_TYPE(engine->mask) < I915_NUM_ENGINES);
 
 	engine->id = id;
+	engine->legacy_idx = INVALID_ENGINE;
 	engine->mask = BIT(id);
 	engine->i915 = gt->i915;
 	engine->gt = gt;
@@ -328,6 +332,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	intel_engine_sanitize_mmio(engine);
 
 	gt->engine_class[info->class][info->instance] = engine;
+	gt->engine[id] = engine;
 
 	intel_engine_add_user(engine);
 	gt->i915->engine[id] = engine;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 6199064f332b..3451be034caf 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -148,6 +148,7 @@ enum intel_engine_id {
 	VECS1,
 #define _VECS(n) (VECS0 + (n))
 	I915_NUM_ENGINES
+#define INVALID_ENGINE ((enum intel_engine_id)-1)
 };
 
 struct st_preempt_hang {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 77cd5de83930..7f7150a733f4 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -160,10 +160,10 @@ static int legacy_ring_idx(const struct legacy_ring *ring)
 	};
 
 	if (GEM_DEBUG_WARN_ON(ring->class >= ARRAY_SIZE(map)))
-		return -1;
+		return INVALID_ENGINE;
 
 	if (GEM_DEBUG_WARN_ON(ring->instance >= map[ring->class].max))
-		return -1;
+		return INVALID_ENGINE;
 
 	return map[ring->class].base + ring->instance;
 }
@@ -171,23 +171,15 @@ static int legacy_ring_idx(const struct legacy_ring *ring)
 static void add_legacy_ring(struct legacy_ring *ring,
 			    struct intel_engine_cs *engine)
 {
-	int idx;
-
 	if (engine->gt != ring->gt || engine->class != ring->class) {
 		ring->gt = engine->gt;
 		ring->class = engine->class;
 		ring->instance = 0;
 	}
 
-	idx = legacy_ring_idx(ring);
-	if (unlikely(idx == -1))
-		return;
-
-	GEM_BUG_ON(idx >= ARRAY_SIZE(ring->gt->engine));
-	ring->gt->engine[idx] = engine;
-	ring->instance++;
-
-	engine->legacy_idx = idx;
+	engine->legacy_idx = legacy_ring_idx(ring);
+	if (engine->legacy_idx != INVALID_ENGINE)
+		ring->instance++;
 }
 
 void intel_engines_driver_register(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index b3619a2a5d0e..c99b6b2f38c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -186,7 +186,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
-		for_each_engine_masked(engine, i915, engine_mask, id)
+		for_each_engine_masked(engine, gt, engine_mask, id)
 			gen8_clear_engine_error_register(engine);
 	}
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index c14dbeb3ccc3..b2af73984f93 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -237,7 +237,7 @@ static void hangcheck_declare_hang(struct intel_gt *gt,
 		hung &= ~stuck;
 	len = scnprintf(msg, sizeof(msg),
 			"%s on ", stuck == hung ? "no progress" : "hang");
-	for_each_engine_masked(engine, gt->i915, hung, tmp)
+	for_each_engine_masked(engine, gt, hung, tmp)
 		len += scnprintf(msg + len, sizeof(msg) - len,
 				 "%s, ", engine->name);
 	msg[len-2] = '\0';
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 477bfafdb103..b191b0745703 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -298,7 +298,7 @@ static int gen6_reset_engines(struct intel_gt *gt,
 		intel_engine_mask_t tmp;
 
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 		}
@@ -432,7 +432,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 		hw_mask = GEN11_GRDOM_FULL;
 	} else {
 		hw_mask = 0;
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
 			hw_mask |= hw_engine_mask[engine->id];
 			ret = gen11_lock_sfc(engine, &hw_mask);
@@ -451,7 +451,7 @@ static int gen11_reset_engines(struct intel_gt *gt,
 	 * expiration).
 	 */
 	if (engine_mask != ALL_ENGINES)
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+		for_each_engine_masked(engine, gt, engine_mask, tmp)
 			gen11_unlock_sfc(engine);
 
 	return ret;
@@ -510,7 +510,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 	intel_engine_mask_t tmp;
 	int ret;
 
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+	for_each_engine_masked(engine, gt, engine_mask, tmp) {
 		ret = gen8_engine_reset_prepare(engine);
 		if (ret && !reset_non_ready)
 			goto skip_reset;
@@ -536,7 +536,7 @@ static int gen8_reset_engines(struct intel_gt *gt,
 		ret = gen6_reset_engines(gt, engine_mask, retry);
 
 skip_reset:
-	for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
+	for_each_engine_masked(engine, gt, engine_mask, tmp)
 		gen8_engine_reset_cancel(engine);
 
 	return ret;
@@ -1206,7 +1206,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 	 * single reset fails.
 	 */
 	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
-		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
+		for_each_engine_masked(engine, gt, engine_mask, tmp) {
 			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
 			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
 					     &gt->reset.flags))
diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
index f21b8fb5b37e..d6e7a1189bad 100644
--- a/drivers/gpu/drm/i915/gvt/execlist.c
+++ b/drivers/gpu/drm/i915/gvt/execlist.c
@@ -534,7 +534,7 @@ static void clean_execlist(struct intel_vgpu *vgpu,
 	struct intel_vgpu_submission *s = &vgpu->submission;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		kfree(s->ring_scan_buffer[engine->id]);
 		s->ring_scan_buffer[engine->id] = NULL;
 		s->ring_scan_buffer_size[engine->id] = 0;
@@ -548,7 +548,7 @@ static void reset_execlist(struct intel_vgpu *vgpu,
 	struct intel_engine_cs *engine;
 	intel_engine_mask_t tmp;
 
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp)
 		init_vgpu_execlist(vgpu, engine->id);
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 6850f1f40241..9ebb2534558b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -887,7 +887,7 @@ void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,
 	intel_engine_mask_t tmp;
 
 	/* free the unsubmited workloads in the queues. */
-	for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
+	for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) {
 		list_for_each_entry_safe(pos, n,
 			&s->workload_q_head[engine->id], list) {
 			list_del_init(&pos->list);
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index aa37c07004b9..7927b1a0c7a6 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -590,8 +590,8 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *i915 = engine->i915;
 	intel_engine_mask_t tmp, mask = engine->mask;
+	struct intel_gt *gt = engine->gt;
 	struct llist_node *pos, *next;
 	int err;
 
@@ -603,7 +603,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	 * We can then use the preallocated nodes in
 	 * i915_active_acquire_barrier()
 	 */
-	for_each_engine_masked(engine, i915, mask, tmp) {
+	for_each_engine_masked(engine, gt, mask, tmp) {
 		u64 idx = engine->kernel_context->timeline->fence_context;
 		struct active_node *node;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 88956f37d96c..40e923b0c2c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1415,10 +1415,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
 
 /* Iterator over subset of engines selected by mask */
-#define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
-	for ((tmp__) = (mask__) & INTEL_INFO(dev_priv__)->engine_mask; \
+#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
+	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
 	     (tmp__) ? \
-	     ((engine__) = (dev_priv__)->engine[__mask_next_bit(tmp__)]), 1 : \
+	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
 	     0;)
 
 #define rb_to_uabi_engine(rb) \
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/2] drm/i915: Make for_each_engine_masked work on intel_gt
  2019-10-17 16:18     ` [PATCH v4 " Tvrtko Ursulin
@ 2019-10-17 20:54       ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-10-17 20:54 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-10-17 17:18:52)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Medium term goal is to eliminate the i915->engine[] array and to get there
> we have recently introduced equivalent array in intel_gt. Now we need to
> migrate the code further towards this state.
> 
> This next step is to eliminate usage of i915->engines[] from the
> for_each_engine_masked iterator.
> 
> For this to work we also need to use engine->id as index when populating
> the gt->engine[] array and adjust the default engine set indexing to use
> engine->legacy_idx instead of assuming gt->engines[] indexing.
> 
> v2:
>   * Populate gt->engine[] earlier.
>   * Check that we don't duplicate engine->legacy_idx
> 
> v3:
>   * Work around the initialization order issue between default_engines()
>     and intel_engines_driver_register() which sets engine->legacy_idx for
>     now. It will be fixed properly later.
> 
> v4:
>   * Merge with forgotten v2.5.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

* fingers crossed
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4)
  2019-10-17  9:44 [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2019-10-17 10:00 ` [PATCH " Chris Wilson
@ 2019-10-17 22:09 ` Patchwork
  2019-10-17 22:38 ` ✗ Fi.CI.BAT: failure " Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-10-17 22:09 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4)
URL   : https://patchwork.freedesktop.org/series/68142/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d956d416d5b5 drm/i915: Make for_each_engine_masked work on intel_gt
-:301: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'gt__' - possible side-effects?
#301: FILE: drivers/gpu/drm/i915/i915_drv.h:1418:
+#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
+	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
 	     (tmp__) ? \
+	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
 	     0;)

-:301: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'tmp__' - possible side-effects?
#301: FILE: drivers/gpu/drm/i915/i915_drv.h:1418:
+#define for_each_engine_masked(engine__, gt__, mask__, tmp__) \
+	for ((tmp__) = (mask__) & INTEL_INFO((gt__)->i915)->engine_mask; \
 	     (tmp__) ? \
+	     ((engine__) = (gt__)->engine[__mask_next_bit(tmp__)]), 1 : \
 	     0;)

total: 0 errors, 0 warnings, 2 checks, 211 lines checked
0fad377701e2 drm/i915: Pass in intel_gt at some for_each_engine sites

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4)
  2019-10-17  9:44 [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2019-10-17 22:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4) Patchwork
@ 2019-10-17 22:38 ` Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-10-17 22:38 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4)
URL   : https://patchwork.freedesktop.org/series/68142/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7124 -> Patchwork_14863
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14863 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14863, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14863:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_execlists:
    - fi-icl-u2:          NOTRUN -> [DMESG-FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-u2/igt@i915_selftest@live_execlists.html
    - fi-icl-u3:          [PASS][2] -> [DMESG-FAIL][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-icl-u3/igt@i915_selftest@live_execlists.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-u3/igt@i915_selftest@live_execlists.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live_execlists:
    - {fi-icl-dsi}:       [PASS][4] -> [DMESG-FAIL][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-icl-dsi/igt@i915_selftest@live_execlists.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-dsi/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_requests:
    - {fi-tgl-u}:         [PASS][6] -> [INCOMPLETE][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-tgl-u/igt@i915_selftest@live_requests.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-tgl-u/igt@i915_selftest@live_requests.html

  
Known issues
------------

  Here are the changes found in Patchwork_14863 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-icl-u3:          [PASS][8] -> [DMESG-WARN][9] ([fdo#107724]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][10] -> [FAIL][11] ([fdo#111045] / [fdo#111096])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_mmap@basic:
    - fi-icl-u3:          [DMESG-WARN][12] ([fdo#107724]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-icl-u3/igt@gem_mmap@basic.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-u3/igt@gem_mmap@basic.html

  * igt@i915_selftest@live_execlists:
    - {fi-icl-guc}:       [DMESG-FAIL][14] -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-icl-guc/igt@i915_selftest@live_execlists.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-guc/igt@i915_selftest@live_execlists.html
    - fi-skl-6770hq:      [DMESG-FAIL][16] -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-skl-6770hq/igt@i915_selftest@live_execlists.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-skl-6770hq/igt@i915_selftest@live_execlists.html
    - fi-cfl-8109u:       [DMESG-FAIL][18] -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-cfl-8109u/igt@i915_selftest@live_execlists.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-cfl-8109u/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u2:          [INCOMPLETE][20] ([fdo#107713] / [fdo#108569]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7124/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/fi-icl-u2/igt@i915_selftest@live_hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096


Participating hosts (53 -> 46)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7124 -> Patchwork_14863

  CI-20190529: 20190529
  CI_DRM_7124: ab91b089cfa3de7fa8c619a9ef8b476cc869dc74 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5232: bb5735423eaf6fdbf6b2f94ef0b8520e74eab993 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14863: 0fad377701e221b91b3cd66a4c96816e8fc9701d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0fad377701e2 drm/i915: Pass in intel_gt at some for_each_engine sites
d956d416d5b5 drm/i915: Make for_each_engine_masked work on intel_gt

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14863/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-10-17 22:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-17  9:44 [PATCH 1/2] drm/i915: Make for_each_engine_masked work on intel_gt Tvrtko Ursulin
2019-10-17  9:45 ` [PATCH 2/2] drm/i915: Pass in intel_gt at some for_each_engine sites Tvrtko Ursulin
2019-10-17 10:01   ` Chris Wilson
2019-10-17  9:57 ` [PATCH v2] drm/i915: Make for_each_engine_masked work on intel_gt Chris Wilson
2019-10-17 10:33   ` Tvrtko Ursulin
2019-10-17 13:45   ` [PATCH v3 1/2] " Tvrtko Ursulin
2019-10-17 13:51     ` Chris Wilson
2019-10-17 16:18     ` [PATCH v4 " Tvrtko Ursulin
2019-10-17 20:54       ` Chris Wilson
2019-10-17 10:00 ` [PATCH " Chris Wilson
2019-10-17 22:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/2] drm/i915: Make for_each_engine_masked work on intel_gt (rev4) Patchwork
2019-10-17 22:38 ` ✗ Fi.CI.BAT: failure " Patchwork

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.