[PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Avoid a global idle barrier by reconfiguring each context by rewriting
them with MI_STORE_DWORD from the kernel context.

v2: We only need to determine the desired register values once, they are
the same for all contexts.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
 drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
 drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
 3 files changed, 195 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index e367dce2a696..1f0d10bb88c1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
 	ctx->sched.priority = I915_USER_PRIORITY(prio);
 	ctx->ring_size = PAGE_SIZE;
 
+	/* Isolate the kernel context from prying eyes and sticky fingers */
 	GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
+	list_del_init(&ctx->link);
 
 	return ctx;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e1ae1399c72b..9cc5374401e1 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1552,9 +1552,12 @@ __execlists_update_reg_state(struct intel_context *ce,
 	regs[CTX_RING_TAIL + 1] = ring->tail;
 
 	/* RPCS */
-	if (engine->class == RENDER_CLASS)
+	if (engine->class == RENDER_CLASS) {
 		regs[CTX_R_PWR_CLK_STATE + 1] =
 			intel_sseu_make_rpcs(engine->i915, &ce->sseu);
+
+		i915_oa_init_reg_state(engine, ce, regs);
+	}
 }
 
 static int
@@ -2966,8 +2969,6 @@ static void execlists_init_reg_state(u32 *regs,
 	if (rcs) {
 		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
 		CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, 0);
-
-		i915_oa_init_reg_state(engine, ce, regs);
 	}
 
 	regs[CTX_END] = MI_BATCH_BUFFER_END;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 357e63beb373..25c34a0b521e 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1630,6 +1630,27 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv)
 				      ~GT_NOA_ENABLE));
 }
 
+static u32 oa_config_flex_reg(const struct i915_oa_config *oa_config,
+			      i915_reg_t reg)
+{
+	u32 mmio = i915_mmio_reg_offset(reg);
+	int i;
+
+	/*
+	 * This arbitrary default will select the 'EU FPU0 Pipeline
+	 * Active' event. In the future it's anticipated that there
+	 * will be an explicit 'No Event' we can select, but not yet...
+	 */
+	if (!oa_config)
+		return 0;
+
+	for (i = 0; i < oa_config->flex_regs_len; i++) {
+		if (i915_mmio_reg_offset(oa_config->flex_regs[i].addr) == mmio)
+			return oa_config->flex_regs[i].value;
+	}
+
+	return 0;
+}
 /*
  * NB: It must always remain pointer safe to run this even if the OA unit
  * has been disabled.
@@ -1663,28 +1684,8 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
 		GEN8_OA_COUNTER_RESUME);
 
 	for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
-		u32 state_offset = ctx_flexeu0 + i * 2;
-		u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
-
-		/*
-		 * This arbitrary default will select the 'EU FPU0 Pipeline
-		 * Active' event. In the future it's anticipated that there
-		 * will be an explicit 'No Event' we can select, but not yet...
-		 */
-		u32 value = 0;
-
-		if (oa_config) {
-			u32 j;
-
-			for (j = 0; j < oa_config->flex_regs_len; j++) {
-				if (i915_mmio_reg_offset(oa_config->flex_regs[j].addr) == mmio) {
-					value = oa_config->flex_regs[j].value;
-					break;
-				}
-			}
-		}
-
-		CTX_REG(reg_state, state_offset, flex_regs[i], value);
+		CTX_REG(reg_state, ctx_flexeu0 + i * 2, flex_regs[i],
+			oa_config_flex_reg(oa_config, flex_regs[i]));
 	}
 
 	CTX_REG(reg_state,
@@ -1692,6 +1693,107 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
 		intel_sseu_make_rpcs(i915, &ce->sseu));
 }
 
+struct flex {
+	i915_reg_t reg;
+	u32 offset;
+	u32 value;
+};
+
+static int
+gen8_store_flex(struct i915_request *rq,
+		struct intel_context *ce,
+		const struct flex *flex, unsigned int count)
+{
+	u32 offset;
+	u32 *cs;
+
+	cs = intel_ring_begin(rq, 4 * count);
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
+
+	offset = i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
+	do {
+		*cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
+		*cs++ = offset + (flex->offset + 1) * sizeof(u32);
+		*cs++ = 0;
+		*cs++ = flex->value;
+	} while (flex++, --count);
+
+	intel_ring_advance(rq, cs);
+
+	return 0;
+}
+
+static int
+gen8_load_flex(struct i915_request *rq,
+	       struct intel_context *ce,
+	       const struct flex *flex, unsigned int count)
+{
+	u32 *cs;
+
+	GEM_BUG_ON(!count || count > 63);
+
+	cs = intel_ring_begin(rq, 2 * count + 2);
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
+
+	*cs++ = MI_LOAD_REGISTER_IMM(count);
+	do {
+		*cs++ = i915_mmio_reg_offset(flex->reg);
+		*cs++ = flex->value;
+	} while (flex++, --count);
+	*cs++ = MI_NOOP;
+
+	intel_ring_advance(rq, cs);
+
+	return 0;
+}
+
+static int gen8_modify_context(struct intel_context *ce,
+			       const struct flex *flex, unsigned int count)
+{
+	struct i915_request *rq;
+	int err;
+
+	lockdep_assert_held(&ce->pin_mutex);
+
+	rq = i915_request_create(ce->engine->kernel_context);
+	if (IS_ERR(rq))
+		return PTR_ERR(rq);
+
+	/* Serialise with the remote context */
+	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
+	if (err)
+		goto out_add;
+
+	/* Keep the remote context alive until after we finish editing */
+	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+	if (err)
+		goto out_add;
+
+	err = gen8_store_flex(rq, ce, flex, count);
+
+out_add:
+	i915_request_add(rq);
+	return err;
+}
+
+static int gen8_modify_self(struct intel_context *ce,
+			    const struct flex *flex, unsigned int count)
+{
+	struct i915_request *rq;
+	int err;
+
+	rq = i915_request_create(ce);
+	if (IS_ERR(rq))
+		return PTR_ERR(rq);
+
+	err = gen8_load_flex(rq, ce, flex, count);
+
+	i915_request_add(rq);
+	return err;
+}
+
 /*
  * Manages updating the per-context aspects of the OA stream
  * configuration across all contexts.
@@ -1716,15 +1818,43 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
  *
  * Note: it's only the RCS/Render context that has any OA state.
  */
-static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
+static int gen8_configure_all_contexts(struct drm_i915_private *i915,
 				       const struct i915_oa_config *oa_config)
 {
-	unsigned int map_type = i915_coherent_map_type(dev_priv);
+	/* The MMIO offsets for Flex EU registers aren't contiguous */
+	const u32 ctx_flexeu0 = i915->perf.oa.ctx_flexeu0_offset;
+#define ctx_flexeuN(N) (ctx_flexeu0 + 2 * (N))
+	struct flex regs[] = {
+		{
+			GEN8_R_PWR_CLK_STATE,
+			CTX_R_PWR_CLK_STATE,
+		},
+		{
+			GEN8_OACTXCONTROL,
+			i915->perf.oa.ctx_oactxctrl_offset,
+			((i915->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
+			 (i915->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
+			 GEN8_OA_COUNTER_RESUME)
+		},
+		{ EU_PERF_CNTL0, ctx_flexeuN(0) },
+		{ EU_PERF_CNTL1, ctx_flexeuN(1) },
+		{ EU_PERF_CNTL2, ctx_flexeuN(2) },
+		{ EU_PERF_CNTL3, ctx_flexeuN(3) },
+		{ EU_PERF_CNTL4, ctx_flexeuN(4) },
+		{ EU_PERF_CNTL5, ctx_flexeuN(5) },
+		{ EU_PERF_CNTL6, ctx_flexeuN(6) },
+	};
+#undef ctx_flexeuN
+	struct intel_engine_cs *engine;
 	struct i915_gem_context *ctx;
-	struct i915_request *rq;
-	int ret;
+	enum intel_engine_id id;
+	int err;
+	int i;
+
+	for (i = 2; i < ARRAY_SIZE(regs); i++)
+		regs[i].value = oa_config_flex_reg(oa_config, regs[i].reg);
 
-	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+	lockdep_assert_held(&i915->drm.struct_mutex);
 
 	/*
 	 * The OA register config is setup through the context image. This image
@@ -1736,58 +1866,58 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
 	 * this might leave small interval of time where the OA unit is
 	 * configured at an invalid sampling period.
 	 *
-	 * So far the best way to work around this issue seems to be draining
-	 * the GPU from any submitted work.
+	 * Note that since we emit all requests from a single ring, there
+	 * is still an implicit global barrier here that may cause a high
+	 * priority context to wait for an otherwise independent low priority
+	 * context. Contexts idle at the time of reconfiguration are not
+	 * trapped behind the barrier.
 	 */
-	ret = i915_gem_wait_for_idle(dev_priv,
-				     I915_WAIT_LOCKED,
-				     MAX_SCHEDULE_TIMEOUT);
-	if (ret)
-		return ret;
-
-	/* Update all contexts now that we've stalled the submission. */
-	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
+	list_for_each_entry(ctx, &i915->contexts.list, link) {
 		struct i915_gem_engines_iter it;
 		struct intel_context *ce;
 
 		for_each_gem_engine(ce,
 				    i915_gem_context_lock_engines(ctx),
 				    it) {
-			u32 *regs;
-
 			if (ce->engine->class != RENDER_CLASS)
 				continue;
 
-			/* OA settings will be set upon first use */
-			if (!ce->state)
-				continue;
-
-			regs = i915_gem_object_pin_map(ce->state->obj,
-						       map_type);
-			if (IS_ERR(regs)) {
-				i915_gem_context_unlock_engines(ctx);
-				return PTR_ERR(regs);
-			}
+			err = intel_context_lock_pinned(ce);
+			if (err)
+				break;
 
-			ce->state->obj->mm.dirty = true;
-			regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
+			regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu);
 
-			gen8_update_reg_state_unlocked(ce, regs, oa_config);
+			/* Otherwise OA settings will be set upon first use */
+			if (intel_context_is_pinned(ce))
+				err = gen8_modify_context(ce, regs, ARRAY_SIZE(regs));
 
-			i915_gem_object_unpin_map(ce->state->obj);
+			intel_context_unlock_pinned(ce);
+			if (err)
+				break;
 		}
 		i915_gem_context_unlock_engines(ctx);
+		if (err)
+			return err;
 	}
 
 	/*
-	 * Apply the configuration by doing one context restore of the edited
-	 * context image.
+	 * After updating all other contexts, we need to modify ourselves.
+	 * If we don't modify the kernel_context, we do not get events while
+	 * idle.
 	 */
-	rq = i915_request_create(dev_priv->engine[RCS0]->kernel_context);
-	if (IS_ERR(rq))
-		return PTR_ERR(rq);
+	for_each_engine(engine, i915, id) {
+		struct intel_context *ce = engine->kernel_context;
 
-	i915_request_add(rq);
+		if (engine->class != RENDER_CLASS)
+			continue;
+
+		regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu);
+
+		err = gen8_modify_self(ce, regs, ARRAY_SIZE(regs));
+		if (err)
+			return err;
+	}
 
 	return 0;
 }
-- 
2.20.1

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

[PATCH 2/2] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Modifying a remote context requires careful serialisation with requests
on that context, and that serialisation requires us to take their
timeline->mutex. Make it so.

Note that while struct_mutex rules, we can't create more than one
request in parallel, but that age is soon coming to an end.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 16 ++---------
 drivers/gpu/drm/i915/gt/intel_context.c     | 30 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_context.h     |  3 +++
 drivers/gpu/drm/i915/i915_perf.c            |  7 +----
 4 files changed, 36 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 1f0d10bb88c1..6000177472ee 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1192,20 +1192,8 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
-	/* Queue this switch after all other activity by this context. */
-	ret = i915_active_request_set(&ce->ring->timeline->last_request, rq);
-	if (ret)
-		goto out_add;
-
-	/*
-	 * Guarantee context image and the timeline remains pinned until the
-	 * modifying request is retired by setting the ce activity tracker.
-	 *
-	 * But we only need to take one pin on the account of it. Or in other
-	 * words transfer the pinned ce object to tracked active request.
-	 */
-	GEM_BUG_ON(i915_active_is_idle(&ce->active));
-	ret = i915_active_ref(&ce->active, rq->fence.context, rq);
+	/* Serialise with the remote context */
+	ret = intel_context_prepare_remote_request(ce, rq);
 	if (ret)
 		goto out_add;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 1110fc8f657a..975c78ac151a 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -239,6 +239,36 @@ void intel_context_exit_engine(struct intel_context *ce)
 	intel_engine_pm_put(ce->engine);
 }
 
+int intel_context_prepare_remote_request(struct intel_context *ce,
+					 struct i915_request *rq)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err;
+
+	err = mutex_lock_interruptible_nested(&tl->mutex, SINGLE_DEPTH_NESTING);
+	if (err)
+		return err;
+
+	/* Queue this switch after all other activity by this context. */
+	err = i915_active_request_set(&tl->last_request, rq);
+	if (err)
+		goto unlock;
+
+	/*
+	 * Guarantee context image and the timeline remains pinned until the
+	 * modifying request is retired by setting the ce activity tracker.
+	 *
+	 * But we only need to take one pin on the account of it. Or in other
+	 * words transfer the pinned ce object to tracked active request.
+	 */
+	GEM_BUG_ON(i915_active_is_idle(&ce->active));
+	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+
+unlock:
+	mutex_unlock(&tl->mutex);
+	return err;
+}
+
 struct i915_request *intel_context_create_request(struct intel_context *ce)
 {
 	struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 40cd8320fcc3..b41c610c2ce6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -139,6 +139,9 @@ static inline void intel_context_timeline_unlock(struct intel_context *ce)
 	mutex_unlock(&ce->ring->timeline->mutex);
 }
 
+int intel_context_prepare_remote_request(struct intel_context *ce,
+					 struct i915_request *rq);
+
 struct i915_request *intel_context_create_request(struct intel_context *ce);
 
 #endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 25c34a0b521e..6f77014640b8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1762,12 +1762,7 @@ static int gen8_modify_context(struct intel_context *ce,
 		return PTR_ERR(rq);
 
 	/* Serialise with the remote context */
-	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
-	if (err)
-		goto out_add;
-
-	/* Keep the remote context alive until after we finish editing */
-	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+	err = intel_context_prepare_remote_request(ce, rq);
 	if (err)
 		goto out_add;
 
-- 
2.20.1

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

[PATCH v2] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Modifying a remote context requires careful serialisation with requests
on that context, and that serialisation requires us to take their
timeline->mutex. Make it so.

Note that while struct_mutex rules, we can't create more than one
request in parallel, but that age is soon coming to an end.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 16 ++--------
 drivers/gpu/drm/i915/gt/intel_context.c     | 33 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_context.h     |  3 ++
 drivers/gpu/drm/i915/i915_perf.c            |  7 +----
 4 files changed, 39 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 1f0d10bb88c1..6000177472ee 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1192,20 +1192,8 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
-	/* Queue this switch after all other activity by this context. */
-	ret = i915_active_request_set(&ce->ring->timeline->last_request, rq);
-	if (ret)
-		goto out_add;
-
-	/*
-	 * Guarantee context image and the timeline remains pinned until the
-	 * modifying request is retired by setting the ce activity tracker.
-	 *
-	 * But we only need to take one pin on the account of it. Or in other
-	 * words transfer the pinned ce object to tracked active request.
-	 */
-	GEM_BUG_ON(i915_active_is_idle(&ce->active));
-	ret = i915_active_ref(&ce->active, rq->fence.context, rq);
+	/* Serialise with the remote context */
+	ret = intel_context_prepare_remote_request(ce, rq);
 	if (ret)
 		goto out_add;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 1110fc8f657a..f6805f443e8e 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -239,6 +239,39 @@ void intel_context_exit_engine(struct intel_context *ce)
 	intel_engine_pm_put(ce->engine);
 }
 
+int intel_context_prepare_remote_request(struct intel_context *ce,
+					 struct i915_request *rq)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err;
+
+	/* Only suitable for use in remotely modifying this context */
+	GEM_BUG_ON(rq->hw_context == ce);
+
+	err = mutex_lock_interruptible_nested(&tl->mutex, SINGLE_DEPTH_NESTING);
+	if (err)
+		return err;
+
+	/* Queue this switch after all other activity by this context. */
+	err = i915_active_request_set(&tl->last_request, rq);
+	if (err)
+		goto unlock;
+
+	/*
+	 * Guarantee context image and the timeline remains pinned until the
+	 * modifying request is retired by setting the ce activity tracker.
+	 *
+	 * But we only need to take one pin on the account of it. Or in other
+	 * words transfer the pinned ce object to tracked active request.
+	 */
+	GEM_BUG_ON(i915_active_is_idle(&ce->active));
+	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+
+unlock:
+	mutex_unlock(&tl->mutex);
+	return err;
+}
+
 struct i915_request *intel_context_create_request(struct intel_context *ce)
 {
 	struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 40cd8320fcc3..b41c610c2ce6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -139,6 +139,9 @@ static inline void intel_context_timeline_unlock(struct intel_context *ce)
 	mutex_unlock(&ce->ring->timeline->mutex);
 }
 
+int intel_context_prepare_remote_request(struct intel_context *ce,
+					 struct i915_request *rq);
+
 struct i915_request *intel_context_create_request(struct intel_context *ce);
 
 #endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 25c34a0b521e..6f77014640b8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1762,12 +1762,7 @@ static int gen8_modify_context(struct intel_context *ce,
 		return PTR_ERR(rq);
 
 	/* Serialise with the remote context */
-	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
-	if (err)
-		goto out_add;
-
-	/* Keep the remote context alive until after we finish editing */
-	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+	err = intel_context_prepare_remote_request(ce, rq);
 	if (err)
 		goto out_add;
 
-- 
2.20.1

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

[PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Modifying a remote context requires careful serialisation with requests
on that context, and that serialisation requires us to take their
timeline->mutex. Make it so.

Note that while struct_mutex rules, we can't create more than one
request in parallel, but that age is soon coming to an end.

v2: Though it doesn't affect the current users, contexts may share
timelines so check if we already hold the right mutex.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 16 ++-------
 drivers/gpu/drm/i915/gt/intel_context.c     | 38 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_context.h     |  3 ++
 drivers/gpu/drm/i915/i915_perf.c            |  7 +---
 4 files changed, 44 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 1f0d10bb88c1..6000177472ee 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1192,20 +1192,8 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
-	/* Queue this switch after all other activity by this context. */
-	ret = i915_active_request_set(&ce->ring->timeline->last_request, rq);
-	if (ret)
-		goto out_add;
-
-	/*
-	 * Guarantee context image and the timeline remains pinned until the
-	 * modifying request is retired by setting the ce activity tracker.
-	 *
-	 * But we only need to take one pin on the account of it. Or in other
-	 * words transfer the pinned ce object to tracked active request.
-	 */
-	GEM_BUG_ON(i915_active_is_idle(&ce->active));
-	ret = i915_active_ref(&ce->active, rq->fence.context, rq);
+	/* Serialise with the remote context */
+	ret = intel_context_prepare_remote_request(ce, rq);
 	if (ret)
 		goto out_add;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 1110fc8f657a..b1e2e4b60027 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -239,6 +239,44 @@ void intel_context_exit_engine(struct intel_context *ce)
 	intel_engine_pm_put(ce->engine);
 }
 
+int intel_context_prepare_remote_request(struct intel_context *ce,
+					 struct i915_request *rq)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err;
+
+	/* Only suitable for use in remotely modifying this context */
+	GEM_BUG_ON(rq->hw_context == ce);
+
+	if (rq->timeline != tl) { /* beware timeline sharing */
+		err = mutex_lock_interruptible_nested(&tl->mutex,
+						      SINGLE_DEPTH_NESTING);
+		if (err)
+			return err;
+	}
+	lockdep_assert_held(&tl->mutex);
+
+	/* Queue this switch after all other activity by this context. */
+	err = i915_active_request_set(&tl->last_request, rq);
+	if (err)
+		goto unlock;
+
+	/*
+	 * Guarantee context image and the timeline remains pinned until the
+	 * modifying request is retired by setting the ce activity tracker.
+	 *
+	 * But we only need to take one pin on the account of it. Or in other
+	 * words transfer the pinned ce object to tracked active request.
+	 */
+	GEM_BUG_ON(i915_active_is_idle(&ce->active));
+	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+
+unlock:
+	if (rq->timeline != tl)
+		mutex_unlock(&tl->mutex);
+	return err;
+}
+
 struct i915_request *intel_context_create_request(struct intel_context *ce)
 {
 	struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 40cd8320fcc3..b41c610c2ce6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -139,6 +139,9 @@ static inline void intel_context_timeline_unlock(struct intel_context *ce)
 	mutex_unlock(&ce->ring->timeline->mutex);
 }
 
+int intel_context_prepare_remote_request(struct intel_context *ce,
+					 struct i915_request *rq);
+
 struct i915_request *intel_context_create_request(struct intel_context *ce);
 
 #endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 25c34a0b521e..6f77014640b8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1762,12 +1762,7 @@ static int gen8_modify_context(struct intel_context *ce,
 		return PTR_ERR(rq);
 
 	/* Serialise with the remote context */
-	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
-	if (err)
-		goto out_add;
-
-	/* Keep the remote context alive until after we finish editing */
-	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+	err = intel_context_prepare_remote_request(ce, rq);
 	if (err)
 		goto out_add;
 
-- 
2.20.1

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/oa: Reconfigure contexts on the fly (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/oa: Reconfigure contexts on the fly (rev2)
URL   : https://patchwork.freedesktop.org/series/63362/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6429 -> Patchwork_13559
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-icl-u3/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-icl-u3/igt@debugfs_test@read_all_entries.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-dsi:         [PASS][3] -> [FAIL][4] ([fdo#103167])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
    - fi-hsw-peppy:       [PASS][5] -> [DMESG-WARN][6] ([fdo#102614])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_basic@bad-close:
    - fi-icl-u3:          [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-icl-u3/igt@gem_basic@bad-close.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-icl-u3/igt@gem_basic@bad-close.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [INCOMPLETE][9] ([fdo#107718]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-x1275:       [DMESG-WARN][11] ([fdo#111074]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-kbl-x1275/igt@i915_selftest@live_hangcheck.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-kbl-x1275/igt@i915_selftest@live_hangcheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([fdo#109485]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Warnings ####

  * igt@gem_exec_parse@basic-allowed:
    - fi-cml-u:           [SKIP][15] ([fdo#109289] / [fdo#110514]) -> [SKIP][16] ([fdo#109289]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-cml-u/igt@gem_exec_parse@basic-allowed.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-cml-u/igt@gem_exec_parse@basic-allowed.html

  * igt@gem_exec_parse@basic-rejected:
    - fi-cml-u2:          [SKIP][17] ([fdo#109289] / [fdo#110514]) -> [SKIP][18] ([fdo#109289]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-cml-u2/igt@gem_exec_parse@basic-rejected.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/fi-cml-u2/igt@gem_exec_parse@basic-rejected.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#110514]: https://bugs.freedesktop.org/show_bug.cgi?id=110514
  [fdo#111074]: https://bugs.freedesktop.org/show_bug.cgi?id=111074


Participating hosts (52 -> 47)
------------------------------

  Additional (4): fi-icl-u4 fi-skl-iommu fi-icl-u2 fi-skl-6700k2 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6429 -> Patchwork_13559

  CI_DRM_6429: ec3f2457cb852af756bf9022496370f4a22b0fee @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5089: d2a10fb734c34b72eb17c58d9dae813321112ee6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13559: 3b496e82b42132a3db848e58075f9aa8fe2dc71e @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13559/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

3b496e82b421 drm/i915: Add to timeline requires the timeline mutex
89478c5e67e7 drm/i915/oa: Reconfigure contexts on the fly

== Logs ==

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

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Tvrtko Ursulin]

On 08/07/2019 12:19, Chris Wilson wrote:
> Avoid a global idle barrier by reconfiguring each context by rewriting
> them with MI_STORE_DWORD from the kernel context.
> 
> v2: We only need to determine the desired register values once, they are
> the same for all contexts.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
>   drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
>   drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
>   3 files changed, 195 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index e367dce2a696..1f0d10bb88c1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
>   	ctx->sched.priority = I915_USER_PRIORITY(prio);
>   	ctx->ring_size = PAGE_SIZE;
>   
> +	/* Isolate the kernel context from prying eyes and sticky fingers */
>   	GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> +	list_del_init(&ctx->link);

Why exactly?

Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you 
won't care about?

Should adding to i915->contexts.list be done from gem_context_register? 
What remains not a kernel context, but on a list?

Won't preempt context be missed in re-configuration?

Regards,

Tvrtko

>   
>   	return ctx;
>   }
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index e1ae1399c72b..9cc5374401e1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1552,9 +1552,12 @@ __execlists_update_reg_state(struct intel_context *ce,
>   	regs[CTX_RING_TAIL + 1] = ring->tail;
>   
>   	/* RPCS */
> -	if (engine->class == RENDER_CLASS)
> +	if (engine->class == RENDER_CLASS) {
>   		regs[CTX_R_PWR_CLK_STATE + 1] =
>   			intel_sseu_make_rpcs(engine->i915, &ce->sseu);
> +
> +		i915_oa_init_reg_state(engine, ce, regs);
> +	}
>   }
>   
>   static int
> @@ -2966,8 +2969,6 @@ static void execlists_init_reg_state(u32 *regs,
>   	if (rcs) {
>   		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
>   		CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, 0);
> -
> -		i915_oa_init_reg_state(engine, ce, regs);
>   	}
>   
>   	regs[CTX_END] = MI_BATCH_BUFFER_END;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 357e63beb373..25c34a0b521e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1630,6 +1630,27 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv)
>   				      ~GT_NOA_ENABLE));
>   }
>   
> +static u32 oa_config_flex_reg(const struct i915_oa_config *oa_config,
> +			      i915_reg_t reg)
> +{
> +	u32 mmio = i915_mmio_reg_offset(reg);
> +	int i;
> +
> +	/*
> +	 * This arbitrary default will select the 'EU FPU0 Pipeline
> +	 * Active' event. In the future it's anticipated that there
> +	 * will be an explicit 'No Event' we can select, but not yet...
> +	 */
> +	if (!oa_config)
> +		return 0;
> +
> +	for (i = 0; i < oa_config->flex_regs_len; i++) {
> +		if (i915_mmio_reg_offset(oa_config->flex_regs[i].addr) == mmio)
> +			return oa_config->flex_regs[i].value;
> +	}
> +
> +	return 0;
> +}
>   /*
>    * NB: It must always remain pointer safe to run this even if the OA unit
>    * has been disabled.
> @@ -1663,28 +1684,8 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
>   		GEN8_OA_COUNTER_RESUME);
>   
>   	for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
> -		u32 state_offset = ctx_flexeu0 + i * 2;
> -		u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
> -
> -		/*
> -		 * This arbitrary default will select the 'EU FPU0 Pipeline
> -		 * Active' event. In the future it's anticipated that there
> -		 * will be an explicit 'No Event' we can select, but not yet...
> -		 */
> -		u32 value = 0;
> -
> -		if (oa_config) {
> -			u32 j;
> -
> -			for (j = 0; j < oa_config->flex_regs_len; j++) {
> -				if (i915_mmio_reg_offset(oa_config->flex_regs[j].addr) == mmio) {
> -					value = oa_config->flex_regs[j].value;
> -					break;
> -				}
> -			}
> -		}
> -
> -		CTX_REG(reg_state, state_offset, flex_regs[i], value);
> +		CTX_REG(reg_state, ctx_flexeu0 + i * 2, flex_regs[i],
> +			oa_config_flex_reg(oa_config, flex_regs[i]));
>   	}
>   
>   	CTX_REG(reg_state,
> @@ -1692,6 +1693,107 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
>   		intel_sseu_make_rpcs(i915, &ce->sseu));
>   }
>   
> +struct flex {
> +	i915_reg_t reg;
> +	u32 offset;
> +	u32 value;
> +};
> +
> +static int
> +gen8_store_flex(struct i915_request *rq,
> +		struct intel_context *ce,
> +		const struct flex *flex, unsigned int count)
> +{
> +	u32 offset;
> +	u32 *cs;
> +
> +	cs = intel_ring_begin(rq, 4 * count);
> +	if (IS_ERR(cs))
> +		return PTR_ERR(cs);
> +
> +	offset = i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
> +	do {
> +		*cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
> +		*cs++ = offset + (flex->offset + 1) * sizeof(u32);
> +		*cs++ = 0;
> +		*cs++ = flex->value;
> +	} while (flex++, --count);
> +
> +	intel_ring_advance(rq, cs);
> +
> +	return 0;
> +}
> +
> +static int
> +gen8_load_flex(struct i915_request *rq,
> +	       struct intel_context *ce,
> +	       const struct flex *flex, unsigned int count)
> +{
> +	u32 *cs;
> +
> +	GEM_BUG_ON(!count || count > 63);
> +
> +	cs = intel_ring_begin(rq, 2 * count + 2);
> +	if (IS_ERR(cs))
> +		return PTR_ERR(cs);
> +
> +	*cs++ = MI_LOAD_REGISTER_IMM(count);
> +	do {
> +		*cs++ = i915_mmio_reg_offset(flex->reg);
> +		*cs++ = flex->value;
> +	} while (flex++, --count);
> +	*cs++ = MI_NOOP;
> +
> +	intel_ring_advance(rq, cs);
> +
> +	return 0;
> +}
> +
> +static int gen8_modify_context(struct intel_context *ce,
> +			       const struct flex *flex, unsigned int count)
> +{
> +	struct i915_request *rq;
> +	int err;
> +
> +	lockdep_assert_held(&ce->pin_mutex);
> +
> +	rq = i915_request_create(ce->engine->kernel_context);
> +	if (IS_ERR(rq))
> +		return PTR_ERR(rq);
> +
> +	/* Serialise with the remote context */
> +	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
> +	if (err)
> +		goto out_add;
> +
> +	/* Keep the remote context alive until after we finish editing */
> +	err = i915_active_ref(&ce->active, rq->fence.context, rq);
> +	if (err)
> +		goto out_add;
> +
> +	err = gen8_store_flex(rq, ce, flex, count);
> +
> +out_add:
> +	i915_request_add(rq);
> +	return err;
> +}
> +
> +static int gen8_modify_self(struct intel_context *ce,
> +			    const struct flex *flex, unsigned int count)
> +{
> +	struct i915_request *rq;
> +	int err;
> +
> +	rq = i915_request_create(ce);
> +	if (IS_ERR(rq))
> +		return PTR_ERR(rq);
> +
> +	err = gen8_load_flex(rq, ce, flex, count);
> +
> +	i915_request_add(rq);
> +	return err;
> +}
> +
>   /*
>    * Manages updating the per-context aspects of the OA stream
>    * configuration across all contexts.
> @@ -1716,15 +1818,43 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
>    *
>    * Note: it's only the RCS/Render context that has any OA state.
>    */
> -static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
> +static int gen8_configure_all_contexts(struct drm_i915_private *i915,
>   				       const struct i915_oa_config *oa_config)
>   {
> -	unsigned int map_type = i915_coherent_map_type(dev_priv);
> +	/* The MMIO offsets for Flex EU registers aren't contiguous */
> +	const u32 ctx_flexeu0 = i915->perf.oa.ctx_flexeu0_offset;
> +#define ctx_flexeuN(N) (ctx_flexeu0 + 2 * (N))
> +	struct flex regs[] = {
> +		{
> +			GEN8_R_PWR_CLK_STATE,
> +			CTX_R_PWR_CLK_STATE,
> +		},
> +		{
> +			GEN8_OACTXCONTROL,
> +			i915->perf.oa.ctx_oactxctrl_offset,
> +			((i915->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
> +			 (i915->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> +			 GEN8_OA_COUNTER_RESUME)
> +		},
> +		{ EU_PERF_CNTL0, ctx_flexeuN(0) },
> +		{ EU_PERF_CNTL1, ctx_flexeuN(1) },
> +		{ EU_PERF_CNTL2, ctx_flexeuN(2) },
> +		{ EU_PERF_CNTL3, ctx_flexeuN(3) },
> +		{ EU_PERF_CNTL4, ctx_flexeuN(4) },
> +		{ EU_PERF_CNTL5, ctx_flexeuN(5) },
> +		{ EU_PERF_CNTL6, ctx_flexeuN(6) },
> +	};
> +#undef ctx_flexeuN
> +	struct intel_engine_cs *engine;
>   	struct i915_gem_context *ctx;
> -	struct i915_request *rq;
> -	int ret;
> +	enum intel_engine_id id;
> +	int err;
> +	int i;
> +
> +	for (i = 2; i < ARRAY_SIZE(regs); i++)
> +		regs[i].value = oa_config_flex_reg(oa_config, regs[i].reg);
>   
> -	lockdep_assert_held(&dev_priv->drm.struct_mutex);
> +	lockdep_assert_held(&i915->drm.struct_mutex);
>   
>   	/*
>   	 * The OA register config is setup through the context image. This image
> @@ -1736,58 +1866,58 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
>   	 * this might leave small interval of time where the OA unit is
>   	 * configured at an invalid sampling period.
>   	 *
> -	 * So far the best way to work around this issue seems to be draining
> -	 * the GPU from any submitted work.
> +	 * Note that since we emit all requests from a single ring, there
> +	 * is still an implicit global barrier here that may cause a high
> +	 * priority context to wait for an otherwise independent low priority
> +	 * context. Contexts idle at the time of reconfiguration are not
> +	 * trapped behind the barrier.
>   	 */
> -	ret = i915_gem_wait_for_idle(dev_priv,
> -				     I915_WAIT_LOCKED,
> -				     MAX_SCHEDULE_TIMEOUT);
> -	if (ret)
> -		return ret;
> -
> -	/* Update all contexts now that we've stalled the submission. */
> -	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> +	list_for_each_entry(ctx, &i915->contexts.list, link) {
>   		struct i915_gem_engines_iter it;
>   		struct intel_context *ce;
>   
>   		for_each_gem_engine(ce,
>   				    i915_gem_context_lock_engines(ctx),
>   				    it) {
> -			u32 *regs;
> -
>   			if (ce->engine->class != RENDER_CLASS)
>   				continue;
>   
> -			/* OA settings will be set upon first use */
> -			if (!ce->state)
> -				continue;
> -
> -			regs = i915_gem_object_pin_map(ce->state->obj,
> -						       map_type);
> -			if (IS_ERR(regs)) {
> -				i915_gem_context_unlock_engines(ctx);
> -				return PTR_ERR(regs);
> -			}
> +			err = intel_context_lock_pinned(ce);
> +			if (err)
> +				break;
>   
> -			ce->state->obj->mm.dirty = true;
> -			regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
> +			regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu);
>   
> -			gen8_update_reg_state_unlocked(ce, regs, oa_config);
> +			/* Otherwise OA settings will be set upon first use */
> +			if (intel_context_is_pinned(ce))
> +				err = gen8_modify_context(ce, regs, ARRAY_SIZE(regs));
>   
> -			i915_gem_object_unpin_map(ce->state->obj);
> +			intel_context_unlock_pinned(ce);
> +			if (err)
> +				break;
>   		}
>   		i915_gem_context_unlock_engines(ctx);
> +		if (err)
> +			return err;
>   	}
>   
>   	/*
> -	 * Apply the configuration by doing one context restore of the edited
> -	 * context image.
> +	 * After updating all other contexts, we need to modify ourselves.
> +	 * If we don't modify the kernel_context, we do not get events while
> +	 * idle.
>   	 */
> -	rq = i915_request_create(dev_priv->engine[RCS0]->kernel_context);
> -	if (IS_ERR(rq))
> -		return PTR_ERR(rq);
> +	for_each_engine(engine, i915, id) {
> +		struct intel_context *ce = engine->kernel_context;
>   
> -	i915_request_add(rq);
> +		if (engine->class != RENDER_CLASS)
> +			continue;
> +
> +		regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu);
> +
> +		err = gen8_modify_self(ce, regs, ARRAY_SIZE(regs));
> +		if (err)
> +			return err;
> +	}
>   
>   	return 0;
>   }
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
> 
> On 08/07/2019 12:19, Chris Wilson wrote:
> > Avoid a global idle barrier by reconfiguring each context by rewriting
> > them with MI_STORE_DWORD from the kernel context.
> > 
> > v2: We only need to determine the desired register values once, they are
> > the same for all contexts.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
> >   drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
> >   drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
> >   3 files changed, 195 insertions(+), 62 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index e367dce2a696..1f0d10bb88c1 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
> >       ctx->sched.priority = I915_USER_PRIORITY(prio);
> >       ctx->ring_size = PAGE_SIZE;
> >   
> > +     /* Isolate the kernel context from prying eyes and sticky fingers */
> >       GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> > +     list_del_init(&ctx->link);
> 
> Why exactly?

Otherwise we recursively try to modify the context.

> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you 
> won't care about?

No, because what matters for those is user contexts.
 
> Should adding to i915->contexts.list be done from gem_context_register? 
> What remains not a kernel context, but on a list?

And I also plan to pull it under its own mutex.
 
> Won't preempt context be missed in re-configuration?

What preempt-context? :-p And I would skip kernel_context if I could, but
for whatever reason oa events are being sent to userspace even while the
GPU is idle and igt expects the regular tick.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Tvrtko Ursulin]

On 08/07/2019 12:39, Chris Wilson wrote:
> Modifying a remote context requires careful serialisation with requests
> on that context, and that serialisation requires us to take their
> timeline->mutex. Make it so.
> 
> Note that while struct_mutex rules, we can't create more than one
> request in parallel, but that age is soon coming to an end.
> 
> v2: Though it doesn't affect the current users, contexts may share
> timelines so check if we already hold the right mutex.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c | 16 ++-------
>   drivers/gpu/drm/i915/gt/intel_context.c     | 38 +++++++++++++++++++++
>   drivers/gpu/drm/i915/gt/intel_context.h     |  3 ++
>   drivers/gpu/drm/i915/i915_perf.c            |  7 +---
>   4 files changed, 44 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 1f0d10bb88c1..6000177472ee 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1192,20 +1192,8 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> -	/* Queue this switch after all other activity by this context. */
> -	ret = i915_active_request_set(&ce->ring->timeline->last_request, rq);
> -	if (ret)
> -		goto out_add;
> -
> -	/*
> -	 * Guarantee context image and the timeline remains pinned until the
> -	 * modifying request is retired by setting the ce activity tracker.
> -	 *
> -	 * But we only need to take one pin on the account of it. Or in other
> -	 * words transfer the pinned ce object to tracked active request.
> -	 */
> -	GEM_BUG_ON(i915_active_is_idle(&ce->active));
> -	ret = i915_active_ref(&ce->active, rq->fence.context, rq);
> +	/* Serialise with the remote context */
> +	ret = intel_context_prepare_remote_request(ce, rq);
>   	if (ret)
>   		goto out_add;
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 1110fc8f657a..b1e2e4b60027 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -239,6 +239,44 @@ void intel_context_exit_engine(struct intel_context *ce)
>   	intel_engine_pm_put(ce->engine);
>   }
>   
> +int intel_context_prepare_remote_request(struct intel_context *ce,
> +					 struct i915_request *rq)
> +{
> +	struct intel_timeline *tl = ce->ring->timeline;
> +	int err;
> +
> +	/* Only suitable for use in remotely modifying this context */
> +	GEM_BUG_ON(rq->hw_context == ce);
> +
> +	if (rq->timeline != tl) { /* beware timeline sharing */
> +		err = mutex_lock_interruptible_nested(&tl->mutex,
> +						      SINGLE_DEPTH_NESTING);

Which caller is holding tl->mutex?

Regards,

Tvrtko

> +		if (err)
> +			return err;
> +	}
> +	lockdep_assert_held(&tl->mutex);
> +
> +	/* Queue this switch after all other activity by this context. */
> +	err = i915_active_request_set(&tl->last_request, rq);
> +	if (err)
> +		goto unlock;
> +
> +	/*
> +	 * Guarantee context image and the timeline remains pinned until the
> +	 * modifying request is retired by setting the ce activity tracker.
> +	 *
> +	 * But we only need to take one pin on the account of it. Or in other
> +	 * words transfer the pinned ce object to tracked active request.
> +	 */
> +	GEM_BUG_ON(i915_active_is_idle(&ce->active));
> +	err = i915_active_ref(&ce->active, rq->fence.context, rq);
> +
> +unlock:
> +	if (rq->timeline != tl)
> +		mutex_unlock(&tl->mutex);
> +	return err;
> +}
> +
>   struct i915_request *intel_context_create_request(struct intel_context *ce)
>   {
>   	struct i915_request *rq;
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> index 40cd8320fcc3..b41c610c2ce6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -139,6 +139,9 @@ static inline void intel_context_timeline_unlock(struct intel_context *ce)
>   	mutex_unlock(&ce->ring->timeline->mutex);
>   }
>   
> +int intel_context_prepare_remote_request(struct intel_context *ce,
> +					 struct i915_request *rq);
> +
>   struct i915_request *intel_context_create_request(struct intel_context *ce);
>   
>   #endif /* __INTEL_CONTEXT_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 25c34a0b521e..6f77014640b8 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1762,12 +1762,7 @@ static int gen8_modify_context(struct intel_context *ce,
>   		return PTR_ERR(rq);
>   
>   	/* Serialise with the remote context */
> -	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
> -	if (err)
> -		goto out_add;
> -
> -	/* Keep the remote context alive until after we finish editing */
> -	err = i915_active_ref(&ce->active, rq->fence.context, rq);
> +	err = intel_context_prepare_remote_request(ce, rq);
>   	if (err)
>   		goto out_add;
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-08 13:18:02)
> 
> On 08/07/2019 12:39, Chris Wilson wrote:
> > v2: Though it doesn't affect the current users, contexts may share
> > timelines so check if we already hold the right mutex.

> > +int intel_context_prepare_remote_request(struct intel_context *ce,
> > +                                      struct i915_request *rq)
> > +{
> > +     struct intel_timeline *tl = ce->ring->timeline;
> > +     int err;
> > +
> > +     /* Only suitable for use in remotely modifying this context */
> > +     GEM_BUG_ON(rq->hw_context == ce);
> > +
> > +     if (rq->timeline != tl) { /* beware timeline sharing */
> > +             err = mutex_lock_interruptible_nested(&tl->mutex,
> > +                                                   SINGLE_DEPTH_NESTING);
> 
> Which caller is holding tl->mutex?

None today, but it is conceivable. So rather than have a mysterious
deadlock not reported by lockdep in the future, nip the problem in the
bud.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Tvrtko Ursulin]

On 08/07/2019 13:16, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
>>
>> On 08/07/2019 12:19, Chris Wilson wrote:
>>> Avoid a global idle barrier by reconfiguring each context by rewriting
>>> them with MI_STORE_DWORD from the kernel context.
>>>
>>> v2: We only need to determine the desired register values once, they are
>>> the same for all contexts.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
>>>    drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
>>>    drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
>>>    3 files changed, 195 insertions(+), 62 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> index e367dce2a696..1f0d10bb88c1 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
>>>        ctx->sched.priority = I915_USER_PRIORITY(prio);
>>>        ctx->ring_size = PAGE_SIZE;
>>>    
>>> +     /* Isolate the kernel context from prying eyes and sticky fingers */
>>>        GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
>>> +     list_del_init(&ctx->link);
>>
>> Why exactly?
> 
> Otherwise we recursively try to modify the context.

 From gen8_configure_all_contexts, twice, or really recursively? If 
former isn't that solvable by simply skipping kernel contexts in the 
first loop?

>> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you
>> won't care about?
> 
> No, because what matters for those is user contexts.

There isn't some time cost associated with l3_remap calls when switching 
contexts?

>   
>> Should adding to i915->contexts.list be done from gem_context_register?
>> What remains not a kernel context, but on a list?
> 
> And I also plan to pull it under its own mutex.
>   
>> Won't preempt context be missed in re-configuration?
> 
> What preempt-context? :-p And I would skip kernel_context if I could, but
> for whatever reason oa events are being sent to userspace even while the
> GPU is idle and igt expects the regular tick.

My tree still has it? Is it out of date? Solution with keeping it on the 
list and skipping sounds more future proof if doable.

Regards,

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/oa: Reconfigure contexts on the fly (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/oa: Reconfigure contexts on the fly (rev3)
URL   : https://patchwork.freedesktop.org/series/63362/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6429 -> Patchwork_13561
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-u3:          [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#109100])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-icl-u3/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-icl-u3/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-7500u:       [PASS][5] -> [DMESG-WARN][6] ([fdo#111074])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html
    - fi-kbl-8809g:       [PASS][7] -> [DMESG-WARN][8] ([fdo#111074])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-kbl-8809g/igt@i915_selftest@live_hangcheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-kbl-8809g/igt@i915_selftest@live_hangcheck.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-x1275:       [DMESG-WARN][9] ([fdo#111074]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-kbl-x1275/igt@i915_selftest@live_hangcheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-kbl-x1275/igt@i915_selftest@live_hangcheck.html

  
#### Warnings ####

  * igt@gem_exec_parse@basic-allowed:
    - fi-cml-u:           [SKIP][11] ([fdo#109289] / [fdo#110514]) -> [SKIP][12] ([fdo#109289]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-cml-u/igt@gem_exec_parse@basic-allowed.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-cml-u/igt@gem_exec_parse@basic-allowed.html

  * igt@gem_exec_parse@basic-rejected:
    - fi-cml-u2:          [SKIP][13] ([fdo#109289] / [fdo#110514]) -> [SKIP][14] ([fdo#109289]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/fi-cml-u2/igt@gem_exec_parse@basic-rejected.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/fi-cml-u2/igt@gem_exec_parse@basic-rejected.html

  
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#110514]: https://bugs.freedesktop.org/show_bug.cgi?id=110514
  [fdo#111074]: https://bugs.freedesktop.org/show_bug.cgi?id=111074


Participating hosts (52 -> 47)
------------------------------

  Additional (4): fi-icl-u4 fi-skl-iommu fi-icl-u2 fi-skl-6700k2 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6429 -> Patchwork_13561

  CI_DRM_6429: ec3f2457cb852af756bf9022496370f4a22b0fee @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5089: d2a10fb734c34b72eb17c58d9dae813321112ee6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13561: dc2e47ac383044ca2b8c884ee51843d111c852f6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

dc2e47ac3830 drm/i915: Add to timeline requires the timeline mutex
9185a9575da5 drm/i915/oa: Reconfigure contexts on the fly

== Logs ==

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

Re: [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Quoting Chris Wilson (2019-07-08 13:22:01)
> Quoting Tvrtko Ursulin (2019-07-08 13:18:02)
> > 
> > On 08/07/2019 12:39, Chris Wilson wrote:
> > > v2: Though it doesn't affect the current users, contexts may share
> > > timelines so check if we already hold the right mutex.
> 
> > > +int intel_context_prepare_remote_request(struct intel_context *ce,
> > > +                                      struct i915_request *rq)
> > > +{
> > > +     struct intel_timeline *tl = ce->ring->timeline;
> > > +     int err;
> > > +
> > > +     /* Only suitable for use in remotely modifying this context */
> > > +     GEM_BUG_ON(rq->hw_context == ce);
> > > +
> > > +     if (rq->timeline != tl) { /* beware timeline sharing */
> > > +             err = mutex_lock_interruptible_nested(&tl->mutex,
> > > +                                                   SINGLE_DEPTH_NESTING);
> > 
> > Which caller is holding tl->mutex?
> 
> None today, but it is conceivable. So rather than have a mysterious
> deadlock not reported by lockdep in the future, nip the problem in the
> bud.

Just in case, creating a request takes the timeline->mutex and holds it
until submitted. So the scenario I considered was if one user context
wanted to modify another one (which could work), but they happen to be
on a common timeline.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-08 13:24:39)
> 
> On 08/07/2019 13:16, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
> >>
> >> On 08/07/2019 12:19, Chris Wilson wrote:
> >>> Avoid a global idle barrier by reconfiguring each context by rewriting
> >>> them with MI_STORE_DWORD from the kernel context.
> >>>
> >>> v2: We only need to determine the desired register values once, they are
> >>> the same for all contexts.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
> >>>    drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
> >>>    drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
> >>>    3 files changed, 195 insertions(+), 62 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> index e367dce2a696..1f0d10bb88c1 100644
> >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
> >>>        ctx->sched.priority = I915_USER_PRIORITY(prio);
> >>>        ctx->ring_size = PAGE_SIZE;
> >>>    
> >>> +     /* Isolate the kernel context from prying eyes and sticky fingers */
> >>>        GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> >>> +     list_del_init(&ctx->link);
> >>
> >> Why exactly?
> > 
> > Otherwise we recursively try to modify the context.
> 
>  From gen8_configure_all_contexts, twice, or really recursively? If 
> former isn't that solvable by simply skipping kernel contexts in the 
> first loop?
> 
> >> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you
> >> won't care about?
> > 
> > No, because what matters for those is user contexts.
> 
> There isn't some time cost associated with l3_remap calls when switching 
> contexts?

No, it's even weirder than that as it is not a context register (at
least on hsw where we support it). I guess, we should just unlazy and emit
a request from the sysfs.

> >> Should adding to i915->contexts.list be done from gem_context_register?
> >> What remains not a kernel context, but on a list?
> > 
> > And I also plan to pull it under its own mutex.
> >   
> >> Won't preempt context be missed in re-configuration?
> > 
> > What preempt-context? :-p And I would skip kernel_context if I could, but
> > for whatever reason oa events are being sent to userspace even while the
> > GPU is idle and igt expects the regular tick.
> 
> My tree still has it? Is it out of date? Solution with keeping it on the 
> list and skipping sounds more future proof if doable.

It's only used by guc for the guc command to preempt-to-idle. It should
not be recorded as an oa event but an inter-context blip (which is
exactly what it is).

Similarly, I also think oa should not prying into the kernel context
(even though it's not used for much at present).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Tvrtko Ursulin]

On 08/07/2019 14:32, Chris Wilson wrote:
> Quoting Chris Wilson (2019-07-08 13:22:01)
>> Quoting Tvrtko Ursulin (2019-07-08 13:18:02)
>>>
>>> On 08/07/2019 12:39, Chris Wilson wrote:
>>>> v2: Though it doesn't affect the current users, contexts may share
>>>> timelines so check if we already hold the right mutex.
>>
>>>> +int intel_context_prepare_remote_request(struct intel_context *ce,
>>>> +                                      struct i915_request *rq)
>>>> +{
>>>> +     struct intel_timeline *tl = ce->ring->timeline;
>>>> +     int err;
>>>> +
>>>> +     /* Only suitable for use in remotely modifying this context */
>>>> +     GEM_BUG_ON(rq->hw_context == ce);
>>>> +
>>>> +     if (rq->timeline != tl) { /* beware timeline sharing */
>>>> +             err = mutex_lock_interruptible_nested(&tl->mutex,
>>>> +                                                   SINGLE_DEPTH_NESTING);
>>>
>>> Which caller is holding tl->mutex?
>>
>> None today, but it is conceivable. So rather than have a mysterious
>> deadlock not reported by lockdep in the future, nip the problem in the
>> bud.
> 
> Just in case, creating a request takes the timeline->mutex and holds it
> until submitted. So the scenario I considered was if one user context
> wanted to modify another one (which could work), but they happen to be
> on a common timeline.

Maybe my forward vision is too limited. :) But since code consolidation 
is worth it, how about you keep that aspect in this patch, and leave the 
tl->mutex taking for later?

Regards,

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

Re: [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-08 14:40:18)
> 
> On 08/07/2019 14:32, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-07-08 13:22:01)
> >> Quoting Tvrtko Ursulin (2019-07-08 13:18:02)
> >>>
> >>> On 08/07/2019 12:39, Chris Wilson wrote:
> >>>> v2: Though it doesn't affect the current users, contexts may share
> >>>> timelines so check if we already hold the right mutex.
> >>
> >>>> +int intel_context_prepare_remote_request(struct intel_context *ce,
> >>>> +                                      struct i915_request *rq)
> >>>> +{
> >>>> +     struct intel_timeline *tl = ce->ring->timeline;
> >>>> +     int err;
> >>>> +
> >>>> +     /* Only suitable for use in remotely modifying this context */
> >>>> +     GEM_BUG_ON(rq->hw_context == ce);
> >>>> +
> >>>> +     if (rq->timeline != tl) { /* beware timeline sharing */
> >>>> +             err = mutex_lock_interruptible_nested(&tl->mutex,
> >>>> +                                                   SINGLE_DEPTH_NESTING);
> >>>
> >>> Which caller is holding tl->mutex?
> >>
> >> None today, but it is conceivable. So rather than have a mysterious
> >> deadlock not reported by lockdep in the future, nip the problem in the
> >> bud.
> > 
> > Just in case, creating a request takes the timeline->mutex and holds it
> > until submitted. So the scenario I considered was if one user context
> > wanted to modify another one (which could work), but they happen to be
> > on a common timeline.
> 
> Maybe my forward vision is too limited. :) But since code consolidation 
> is worth it, how about you keep that aspect in this patch, and leave the 
> tl->mutex taking for later?

In the current set of patches on the list for removing struct_mutex
around requests, it is already illegal to do the operation on
tl->last_request without holding the tl->mutex.

I'm considering how best to add lockdep to highlight that. I think if I
add a struct mutex *lock to i915_active_request, and make it only exist
under debug?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Add to timeline requires the timeline mutex

[Chris Wilson]

Quoting Chris Wilson (2019-07-08 14:47:05)
> I'm considering how best to add lockdep to highlight that. I think if I
> add a struct mutex *lock to i915_active_request, and make it only exist
> under debug?

The problem with that is that I need to find some way to communicate
when I'm deliberately lying... (Because we have the global engine-pm
barrier)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/oa: Reconfigure contexts on the fly (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/oa: Reconfigure contexts on the fly (rev3)
URL   : https://patchwork.freedesktop.org/series/63362/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6429_full -> Patchwork_13561_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-apl7/igt@gem_ctx_isolation@rcs0-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          [PASS][3] -> [SKIP][4] ([fdo#109271])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-kbl7/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-kbl7/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108] / [fdo#107807])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl9/igt@i915_pm_rpm@system-suspend-execbuf.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl3/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([fdo#105363])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl6/igt@kms_flip@flip-vs-expired-vblank.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl10/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#108566]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109441]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-iclb7/igt@kms_psr@psr2_sprite_plane_move.html

  
#### Possible fixes ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][15] ([fdo#110854]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-iclb3/igt@gem_exec_balancer@smoke.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-iclb4/igt@gem_exec_balancer@smoke.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-glk:          [INCOMPLETE][17] ([fdo#103359] / [k.org#198133]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-glk2/igt@kms_flip@flip-vs-suspend-interruptible.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-glk6/igt@kms_flip@flip-vs-suspend-interruptible.html
    - shard-apl:          [DMESG-WARN][19] ([fdo#108566]) -> [PASS][20] +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-apl5/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][21] ([fdo#103167]) -> [PASS][22] +4 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [INCOMPLETE][23] ([fdo#104108] / [fdo#106978]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl7/igt@kms_frontbuffer_tracking@psr-suspend.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl3/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][25] ([fdo#108145]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][27] ([fdo#108145] / [fdo#110403]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][29] ([fdo#109441]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-iclb3/igt@kms_psr@psr2_no_drrs.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][31] ([fdo#99912]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-apl7/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-apl5/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-skl:          [INCOMPLETE][33] ([fdo#104108]) -> [PASS][34] +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl3/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl4/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][35] ([fdo#110728]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-skl5/igt@perf@blocking.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-skl3/igt@perf@blocking.html

  
#### Warnings ####

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [DMESG-WARN][37] ([fdo#107724]) -> [SKIP][38] ([fdo#109349])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6429/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13561/shard-iclb7/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * Linux: CI_DRM_6429 -> Patchwork_13561

  CI_DRM_6429: ec3f2457cb852af756bf9022496370f4a22b0fee @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5089: d2a10fb734c34b72eb17c58d9dae813321112ee6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13561: dc2e47ac383044ca2b8c884ee51843d111c852f6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Tvrtko Ursulin]

On 08/07/2019 14:39, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-08 13:24:39)
>>
>> On 08/07/2019 13:16, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
>>>>
>>>> On 08/07/2019 12:19, Chris Wilson wrote:
>>>>> Avoid a global idle barrier by reconfiguring each context by rewriting
>>>>> them with MI_STORE_DWORD from the kernel context.
>>>>>
>>>>> v2: We only need to determine the desired register values once, they are
>>>>> the same for all contexts.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>> ---
>>>>>     drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
>>>>>     drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
>>>>>     drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
>>>>>     3 files changed, 195 insertions(+), 62 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>>> index e367dce2a696..1f0d10bb88c1 100644
>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>>> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
>>>>>         ctx->sched.priority = I915_USER_PRIORITY(prio);
>>>>>         ctx->ring_size = PAGE_SIZE;
>>>>>     
>>>>> +     /* Isolate the kernel context from prying eyes and sticky fingers */
>>>>>         GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
>>>>> +     list_del_init(&ctx->link);
>>>>
>>>> Why exactly?
>>>
>>> Otherwise we recursively try to modify the context.
>>
>>   From gen8_configure_all_contexts, twice, or really recursively? If
>> former isn't that solvable by simply skipping kernel contexts in the
>> first loop?
>>
>>>> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you
>>>> won't care about?
>>>
>>> No, because what matters for those is user contexts.
>>
>> There isn't some time cost associated with l3_remap calls when switching
>> contexts?
> 
> No, it's even weirder than that as it is not a context register (at
> least on hsw where we support it). I guess, we should just unlazy and emit
> a request from the sysfs.
> 
>>>> Should adding to i915->contexts.list be done from gem_context_register?
>>>> What remains not a kernel context, but on a list?
>>>
>>> And I also plan to pull it under its own mutex.
>>>    
>>>> Won't preempt context be missed in re-configuration?
>>>
>>> What preempt-context? :-p And I would skip kernel_context if I could, but
>>> for whatever reason oa events are being sent to userspace even while the
>>> GPU is idle and igt expects the regular tick.
>>
>> My tree still has it? Is it out of date? Solution with keeping it on the
>> list and skipping sounds more future proof if doable.
> 
> It's only used by guc for the guc command to preempt-to-idle. It should
> not be recorded as an oa event but an inter-context blip (which is
> exactly what it is).
> 
> Similarly, I also think oa should not prying into the kernel context
> (even though it's not used for much at present).

Will there be a separate blitter context?

OA can handle the changing config between kernel and other contexts?

Regards,

Tvrtko



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

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-11 08:23:37)
> 
> On 08/07/2019 14:39, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-07-08 13:24:39)
> >>
> >> On 08/07/2019 13:16, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
> >>>>
> >>>> On 08/07/2019 12:19, Chris Wilson wrote:
> >>>>> Avoid a global idle barrier by reconfiguring each context by rewriting
> >>>>> them with MI_STORE_DWORD from the kernel context.
> >>>>>
> >>>>> v2: We only need to determine the desired register values once, they are
> >>>>> the same for all contexts.
> >>>>>
> >>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>>>> ---
> >>>>>     drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
> >>>>>     drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
> >>>>>     drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
> >>>>>     3 files changed, 195 insertions(+), 62 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>>>> index e367dce2a696..1f0d10bb88c1 100644
> >>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>>>> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
> >>>>>         ctx->sched.priority = I915_USER_PRIORITY(prio);
> >>>>>         ctx->ring_size = PAGE_SIZE;
> >>>>>     
> >>>>> +     /* Isolate the kernel context from prying eyes and sticky fingers */
> >>>>>         GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> >>>>> +     list_del_init(&ctx->link);
> >>>>
> >>>> Why exactly?
> >>>
> >>> Otherwise we recursively try to modify the context.
> >>
> >>   From gen8_configure_all_contexts, twice, or really recursively? If
> >> former isn't that solvable by simply skipping kernel contexts in the
> >> first loop?
> >>
> >>>> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you
> >>>> won't care about?
> >>>
> >>> No, because what matters for those is user contexts.
> >>
> >> There isn't some time cost associated with l3_remap calls when switching
> >> contexts?
> > 
> > No, it's even weirder than that as it is not a context register (at
> > least on hsw where we support it). I guess, we should just unlazy and emit
> > a request from the sysfs.
> > 
> >>>> Should adding to i915->contexts.list be done from gem_context_register?
> >>>> What remains not a kernel context, but on a list?
> >>>
> >>> And I also plan to pull it under its own mutex.
> >>>    
> >>>> Won't preempt context be missed in re-configuration?
> >>>
> >>> What preempt-context? :-p And I would skip kernel_context if I could, but
> >>> for whatever reason oa events are being sent to userspace even while the
> >>> GPU is idle and igt expects the regular tick.
> >>
> >> My tree still has it? Is it out of date? Solution with keeping it on the
> >> list and skipping sounds more future proof if doable.
> > 
> > It's only used by guc for the guc command to preempt-to-idle. It should
> > not be recorded as an oa event but an inter-context blip (which is
> > exactly what it is).
> > 
> > Similarly, I also think oa should not prying into the kernel context
> > (even though it's not used for much at present).
> 
> Will there be a separate blitter context?

Yes, no. I will try to push as much of the migration into the culprit,
but there will always be a need for kswapd/kcompactd for background
cleanup and a get-out-of-jail. That should be the kernel_context as far
as I am concerned.

> OA can handle the changing config between kernel and other contexts?

Imo, the less spying on things it is not privileged to spy on the better.
I have no idea how stable it is, the code gives a very underwhelming
impression.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Quoting Chris Wilson (2019-07-08 14:39:35)
> Quoting Tvrtko Ursulin (2019-07-08 13:24:39)
> > 
> > On 08/07/2019 13:16, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
> > >>
> > >> On 08/07/2019 12:19, Chris Wilson wrote:
> > >>> Avoid a global idle barrier by reconfiguring each context by rewriting
> > >>> them with MI_STORE_DWORD from the kernel context.
> > >>>
> > >>> v2: We only need to determine the desired register values once, they are
> > >>> the same for all contexts.
> > >>>
> > >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > >>> ---
> > >>>    drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
> > >>>    drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
> > >>>    drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
> > >>>    3 files changed, 195 insertions(+), 62 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > >>> index e367dce2a696..1f0d10bb88c1 100644
> > >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > >>> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
> > >>>        ctx->sched.priority = I915_USER_PRIORITY(prio);
> > >>>        ctx->ring_size = PAGE_SIZE;
> > >>>    
> > >>> +     /* Isolate the kernel context from prying eyes and sticky fingers */
> > >>>        GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> > >>> +     list_del_init(&ctx->link);
> > >>
> > >> Why exactly?
> > > 
> > > Otherwise we recursively try to modify the context.
> > 
> >  From gen8_configure_all_contexts, twice, or really recursively? If 
> > former isn't that solvable by simply skipping kernel contexts in the 
> > first loop?
> > 
> > >> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you
> > >> won't care about?
> > > 
> > > No, because what matters for those is user contexts.
> > 
> > There isn't some time cost associated with l3_remap calls when switching 
> > contexts?
> 
> No, it's even weirder than that as it is not a context register (at
> least on hsw where we support it). I guess, we should just unlazy and emit
> a request from the sysfs.

Actually, I am wrong here. The L3 remapping registers do appear to be
saved in the context image. At least that is what my testing indicates.

I can still argue that they don't apply to the kernel context. But my
plan to remove the walk over GEM contexts and the lazy update fails.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Avoid a global idle barrier by reconfiguring each context by rewriting
them with MI_STORE_DWORD from the kernel context.

v2: We only need to determine the desired register values once, they are
the same for all contexts.
v3: Don't remove the kernel context from the list of known GEM contexts;
the world is not ready for that yet.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c |   7 +-
 drivers/gpu/drm/i915/i915_perf.c    | 251 +++++++++++++++++++++-------
 2 files changed, 197 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 270ef417dd1a..fa918c19a6af 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1570,9 +1570,12 @@ __execlists_update_reg_state(struct intel_context *ce,
 	regs[CTX_RING_TAIL + 1] = ring->tail;
 
 	/* RPCS */
-	if (engine->class == RENDER_CLASS)
+	if (engine->class == RENDER_CLASS) {
 		regs[CTX_R_PWR_CLK_STATE + 1] =
 			intel_sseu_make_rpcs(engine->i915, &ce->sseu);
+
+		i915_oa_init_reg_state(engine, ce, regs);
+	}
 }
 
 static int
@@ -2992,8 +2995,6 @@ static void execlists_init_reg_state(u32 *regs,
 	if (rcs) {
 		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
 		CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, 0);
-
-		i915_oa_init_reg_state(engine, ce, regs);
 	}
 
 	regs[CTX_END] = MI_BATCH_BUFFER_END;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 007826ded9b3..78cb9ec8866c 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1636,6 +1636,27 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv)
 				      ~GT_NOA_ENABLE));
 }
 
+static u32 oa_config_flex_reg(const struct i915_oa_config *oa_config,
+			      i915_reg_t reg)
+{
+	u32 mmio = i915_mmio_reg_offset(reg);
+	int i;
+
+	/*
+	 * This arbitrary default will select the 'EU FPU0 Pipeline
+	 * Active' event. In the future it's anticipated that there
+	 * will be an explicit 'No Event' we can select, but not yet...
+	 */
+	if (!oa_config)
+		return 0;
+
+	for (i = 0; i < oa_config->flex_regs_len; i++) {
+		if (i915_mmio_reg_offset(oa_config->flex_regs[i].addr) == mmio)
+			return oa_config->flex_regs[i].value;
+	}
+
+	return 0;
+}
 /*
  * NB: It must always remain pointer safe to run this even if the OA unit
  * has been disabled.
@@ -1669,28 +1690,8 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
 		GEN8_OA_COUNTER_RESUME);
 
 	for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
-		u32 state_offset = ctx_flexeu0 + i * 2;
-		u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
-
-		/*
-		 * This arbitrary default will select the 'EU FPU0 Pipeline
-		 * Active' event. In the future it's anticipated that there
-		 * will be an explicit 'No Event' we can select, but not yet...
-		 */
-		u32 value = 0;
-
-		if (oa_config) {
-			u32 j;
-
-			for (j = 0; j < oa_config->flex_regs_len; j++) {
-				if (i915_mmio_reg_offset(oa_config->flex_regs[j].addr) == mmio) {
-					value = oa_config->flex_regs[j].value;
-					break;
-				}
-			}
-		}
-
-		CTX_REG(reg_state, state_offset, flex_regs[i], value);
+		CTX_REG(reg_state, ctx_flexeu0 + i * 2, flex_regs[i],
+			oa_config_flex_reg(oa_config, flex_regs[i]));
 	}
 
 	CTX_REG(reg_state,
@@ -1698,6 +1699,107 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
 		intel_sseu_make_rpcs(i915, &ce->sseu));
 }
 
+struct flex {
+	i915_reg_t reg;
+	u32 offset;
+	u32 value;
+};
+
+static int
+gen8_store_flex(struct i915_request *rq,
+		struct intel_context *ce,
+		const struct flex *flex, unsigned int count)
+{
+	u32 offset;
+	u32 *cs;
+
+	cs = intel_ring_begin(rq, 4 * count);
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
+
+	offset = i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
+	do {
+		*cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
+		*cs++ = offset + (flex->offset + 1) * sizeof(u32);
+		*cs++ = 0;
+		*cs++ = flex->value;
+	} while (flex++, --count);
+
+	intel_ring_advance(rq, cs);
+
+	return 0;
+}
+
+static int
+gen8_load_flex(struct i915_request *rq,
+	       struct intel_context *ce,
+	       const struct flex *flex, unsigned int count)
+{
+	u32 *cs;
+
+	GEM_BUG_ON(!count || count > 63);
+
+	cs = intel_ring_begin(rq, 2 * count + 2);
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
+
+	*cs++ = MI_LOAD_REGISTER_IMM(count);
+	do {
+		*cs++ = i915_mmio_reg_offset(flex->reg);
+		*cs++ = flex->value;
+	} while (flex++, --count);
+	*cs++ = MI_NOOP;
+
+	intel_ring_advance(rq, cs);
+
+	return 0;
+}
+
+static int gen8_modify_context(struct intel_context *ce,
+			       const struct flex *flex, unsigned int count)
+{
+	struct i915_request *rq;
+	int err;
+
+	lockdep_assert_held(&ce->pin_mutex);
+
+	rq = i915_request_create(ce->engine->kernel_context);
+	if (IS_ERR(rq))
+		return PTR_ERR(rq);
+
+	/* Serialise with the remote context */
+	err = i915_active_request_set(&ce->ring->timeline->last_request, rq);
+	if (err)
+		goto out_add;
+
+	/* Keep the remote context alive until after we finish editing */
+	err = i915_active_ref(&ce->active, rq->fence.context, rq);
+	if (err)
+		goto out_add;
+
+	err = gen8_store_flex(rq, ce, flex, count);
+
+out_add:
+	i915_request_add(rq);
+	return err;
+}
+
+static int gen8_modify_self(struct intel_context *ce,
+			    const struct flex *flex, unsigned int count)
+{
+	struct i915_request *rq;
+	int err;
+
+	rq = i915_request_create(ce);
+	if (IS_ERR(rq))
+		return PTR_ERR(rq);
+
+	err = gen8_load_flex(rq, ce, flex, count);
+
+	i915_request_add(rq);
+	return err;
+}
+
 /*
  * Manages updating the per-context aspects of the OA stream
  * configuration across all contexts.
@@ -1722,15 +1824,43 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
  *
  * Note: it's only the RCS/Render context that has any OA state.
  */
-static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
+static int gen8_configure_all_contexts(struct drm_i915_private *i915,
 				       const struct i915_oa_config *oa_config)
 {
-	unsigned int map_type = i915_coherent_map_type(dev_priv);
+	/* The MMIO offsets for Flex EU registers aren't contiguous */
+	const u32 ctx_flexeu0 = i915->perf.oa.ctx_flexeu0_offset;
+#define ctx_flexeuN(N) (ctx_flexeu0 + 2 * (N))
+	struct flex regs[] = {
+		{
+			GEN8_R_PWR_CLK_STATE,
+			CTX_R_PWR_CLK_STATE,
+		},
+		{
+			GEN8_OACTXCONTROL,
+			i915->perf.oa.ctx_oactxctrl_offset,
+			((i915->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
+			 (i915->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
+			 GEN8_OA_COUNTER_RESUME)
+		},
+		{ EU_PERF_CNTL0, ctx_flexeuN(0) },
+		{ EU_PERF_CNTL1, ctx_flexeuN(1) },
+		{ EU_PERF_CNTL2, ctx_flexeuN(2) },
+		{ EU_PERF_CNTL3, ctx_flexeuN(3) },
+		{ EU_PERF_CNTL4, ctx_flexeuN(4) },
+		{ EU_PERF_CNTL5, ctx_flexeuN(5) },
+		{ EU_PERF_CNTL6, ctx_flexeuN(6) },
+	};
+#undef ctx_flexeuN
+	struct intel_engine_cs *engine;
 	struct i915_gem_context *ctx;
-	struct i915_request *rq;
-	int ret;
+	enum intel_engine_id id;
+	int err;
+	int i;
 
-	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+	for (i = 2; i < ARRAY_SIZE(regs); i++)
+		regs[i].value = oa_config_flex_reg(oa_config, regs[i].reg);
+
+	lockdep_assert_held(&i915->drm.struct_mutex);
 
 	/*
 	 * The OA register config is setup through the context image. This image
@@ -1742,58 +1872,63 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
 	 * this might leave small interval of time where the OA unit is
 	 * configured at an invalid sampling period.
 	 *
-	 * So far the best way to work around this issue seems to be draining
-	 * the GPU from any submitted work.
+	 * Note that since we emit all requests from a single ring, there
+	 * is still an implicit global barrier here that may cause a high
+	 * priority context to wait for an otherwise independent low priority
+	 * context. Contexts idle at the time of reconfiguration are not
+	 * trapped behind the barrier.
 	 */
-	ret = i915_gem_wait_for_idle(dev_priv,
-				     I915_WAIT_LOCKED,
-				     MAX_SCHEDULE_TIMEOUT);
-	if (ret)
-		return ret;
-
-	/* Update all contexts now that we've stalled the submission. */
-	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
+	list_for_each_entry(ctx, &i915->contexts.list, link) {
 		struct i915_gem_engines_iter it;
 		struct intel_context *ce;
 
+		if (ctx == i915->kernel_context)
+			continue;
+
 		for_each_gem_engine(ce,
 				    i915_gem_context_lock_engines(ctx),
 				    it) {
-			u32 *regs;
+			GEM_BUG_ON(ce == ce->engine->kernel_context);
 
 			if (ce->engine->class != RENDER_CLASS)
 				continue;
 
-			/* OA settings will be set upon first use */
-			if (!ce->state)
-				continue;
-
-			regs = i915_gem_object_pin_map(ce->state->obj,
-						       map_type);
-			if (IS_ERR(regs)) {
-				i915_gem_context_unlock_engines(ctx);
-				return PTR_ERR(regs);
-			}
+			err = intel_context_lock_pinned(ce);
+			if (err)
+				break;
 
-			ce->state->obj->mm.dirty = true;
-			regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
+			regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu);
 
-			gen8_update_reg_state_unlocked(ce, regs, oa_config);
+			/* Otherwise OA settings will be set upon first use */
+			if (intel_context_is_pinned(ce))
+				err = gen8_modify_context(ce, regs, ARRAY_SIZE(regs));
 
-			i915_gem_object_unpin_map(ce->state->obj);
+			intel_context_unlock_pinned(ce);
+			if (err)
+				break;
 		}
 		i915_gem_context_unlock_engines(ctx);
+		if (err)
+			return err;
 	}
 
 	/*
-	 * Apply the configuration by doing one context restore of the edited
-	 * context image.
+	 * After updating all other contexts, we need to modify ourselves.
+	 * If we don't modify the kernel_context, we do not get events while
+	 * idle.
 	 */
-	rq = i915_request_create(dev_priv->engine[RCS0]->kernel_context);
-	if (IS_ERR(rq))
-		return PTR_ERR(rq);
+	for_each_engine(engine, i915, id) {
+		struct intel_context *ce = engine->kernel_context;
 
-	i915_request_add(rq);
+		if (engine->class != RENDER_CLASS)
+			continue;
+
+		regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu);
+
+		err = gen8_modify_self(ce, regs, ARRAY_SIZE(regs));
+		if (err)
+			return err;
+	}
 
 	return 0;
 }
-- 
2.22.0

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

✓ Fi.CI.BAT: success for series starting with [v3] drm/i915/oa: Reconfigure contexts on the fly (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [v3] drm/i915/oa: Reconfigure contexts on the fly (rev4)
URL   : https://patchwork.freedesktop.org/series/63362/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6460 -> Patchwork_13625
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [PASS][1] -> [DMESG-FAIL][2] ([fdo#111108])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-dsi:         [PASS][3] -> [INCOMPLETE][4] ([fdo#107713] / [fdo#108569])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - {fi-icl-u4}:        [INCOMPLETE][5] ([fdo#107713]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/fi-icl-u4/igt@gem_close_race@basic-threads.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/fi-icl-u4/igt@gem_close_race@basic-threads.html

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

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108


Participating hosts (50 -> 46)
------------------------------

  Additional (2): fi-icl-u3 fi-apl-guc 
  Missing    (6): fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6460 -> Patchwork_13625

  CI_DRM_6460: 32d52650873db547f6702f12ebdbae3e8e052dad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5094: d7f140b5b02d054183a74842b4579cf7f5533927 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13625: 09112e115f02cc9dab9ec22c932915912c83c3cb @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

09112e115f02 drm/i915: Add to timeline requires the timeline mutex
9495a1c4b913 drm/i915/oa: Reconfigure contexts on the fly

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [v3] drm/i915/oa: Reconfigure contexts on the fly (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [v3] drm/i915/oa: Reconfigure contexts on the fly (rev4)
URL   : https://patchwork.freedesktop.org/series/63362/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6460_full -> Patchwork_13625_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2] ([fdo#104108])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-skl8/igt@gem_softpin@noreloc-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-skl10/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_rpm@i2c:
    - shard-hsw:          [PASS][3] -> [FAIL][4] ([fdo#104097])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-hsw6/igt@i915_pm_rpm@i2c.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-hsw8/igt@i915_pm_rpm@i2c.html

  * igt@i915_selftest@mock_fence:
    - shard-iclb:         [PASS][5] -> [INCOMPLETE][6] ([fdo#107713])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb3/igt@i915_selftest@mock_fence.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb7/igt@i915_selftest@mock_fence.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][7] -> [FAIL][8] ([fdo#103167]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#108566]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-apl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#109642] / [fdo#111068])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb1/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#108341])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb4/igt@kms_psr@no_drrs.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb6/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][17] -> [FAIL][18] ([fdo#99912])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-apl4/igt@kms_setmode@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-apl3/igt@kms_setmode@basic.html
    - shard-kbl:          [PASS][19] -> [FAIL][20] ([fdo#99912])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-kbl6/igt@kms_setmode@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-kbl7/igt@kms_setmode@basic.html

  * igt@perf@polling:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#110728])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-skl1/igt@perf@polling.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-skl4/igt@perf@polling.html

  * igt@perf@short-reads:
    - shard-apl:          [PASS][23] -> [FAIL][24] ([fdo#103183])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-apl7/igt@perf@short-reads.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-apl6/igt@perf@short-reads.html

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack:
    - shard-iclb:         [INCOMPLETE][25] ([fdo#107713]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render:
    - shard-iclb:         [FAIL][27] ([fdo#103167]) -> [PASS][28] +2 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][29] ([fdo#108566]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-apl5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][31] ([fdo#109441]) -> [PASS][32] +3 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6460/shard-iclb7/igt@kms_psr@psr2_basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13625/shard-iclb2/igt@kms_psr@psr2_basic.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103183]: https://bugs.freedesktop.org/show_bug.cgi?id=103183
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (11 -> 10)
------------------------------

  Missing    (1): pig-snb-2600 


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

  * Linux: CI_DRM_6460 -> Patchwork_13625

  CI_DRM_6460: 32d52650873db547f6702f12ebdbae3e8e052dad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5094: d7f140b5b02d054183a74842b4579cf7f5533927 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13625: 09112e115f02cc9dab9ec22c932915912c83c3cb @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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