Context isolation

Context isolation

[Chris Wilson]

We know we had screwed up with legacy ringbuffer submission and contexts
not providing sufficient isolation and scrubbing of newly create
contexts (mostly due to the evolution from no contexts and everyone
cooperating). An early attempt to provide context-switch isolation
failed as we were corrupting the context across suspend/hibernate; as a
regression it had to be reverted and we expected the underlying problem
to be fixed first. That should now be fixed by commit 5ab57c702069
("drm/i915: Flush logical context image out to memory upon suspend")
Furthermore that patch did not address the issue raised most recently,
we were not scrubbing HW state for new contexts, to give each new user
context the same view of default HW state. We had presumed this was not
an issue for execlists either (which at least had proper isolation for
context switches) but were completely wrong.

In short, this series records the default HW state on load after a
reset, then uses that as the template image for all user contexts,
providing complete (insofar as HW support allows) isolation.

The one piece missing here is a param to indicate that we've moved pass
the screw up.
-Chris

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

[PATCH 1/6] drm/i915: Force the switch to the i915->kernel_context

[Chris Wilson]

In the next few patches, we will have a hard requirement that we emit a
context-switch to the perma-pinned i915->kernel_context (so that we can
save the HW state using that context-switch). As the first context
itself may be classed as a kernel context, we want to be explicit in our
comparison. For an extra-layer of finesse, we can check the last
unretired context on the engine; as well as the last retired context
when idle.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index ddbe5c9bf45a..c15e288bed02 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1587,8 +1587,20 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv)
 
 bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine)
 {
-	return (!engine->last_retired_context ||
-		i915_gem_context_is_kernel(engine->last_retired_context));
+	const struct i915_gem_context *kctx = engine->i915->kernel_context;
+	struct drm_i915_gem_request *rq;
+
+	lockdep_assert_held(&engine->i915->drm.struct_mutex);
+
+	/* An unretired request? */
+	rq = __i915_gem_active_peek(&engine->timeline->last_request);
+	if (rq)
+		return rq->ctx == kctx;
+
+	if (!engine->last_retired_context)
+		return true;
+
+	return engine->last_retired_context == kctx;
 }
 
 void intel_engines_reset_default_submission(struct drm_i915_private *i915)
-- 
2.15.0

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

[PATCH 2/6] drm/i915: Move GT powersaving init to i915_gem_init()

[Chris Wilson]

GT powersaving is tightly coupled to the request infrastructure. To
avoid complications with the order of initialisation in the next patch
(where we want to send requests to hw during GEM init) move the
powersaving initialisation into the purview of i915_gem_init().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c      | 7 ++++++-
 drivers/gpu/drm/i915/intel_display.c | 2 --
 drivers/gpu/drm/i915/intel_pm.c      | 2 --
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9470ba0c1930..e36a3a840552 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5017,6 +5017,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 		goto out_unlock;
 
 	ret = i915_gem_init_hw(dev_priv);
+	if (ret)
+		goto out_unlock;
+
+	intel_init_gt_powersave(dev_priv);
+
+out_unlock:
 	if (ret == -EIO) {
 		/* Allow engine initialisation to fail by marking the GPU as
 		 * wedged. But we only want to do this where the GPU is angry,
@@ -5029,7 +5035,6 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 		ret = 0;
 	}
 
-out_unlock:
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..c3bf87c2036c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15174,8 +15174,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	intel_init_gt_powersave(dev_priv);
-
 	intel_setup_overlay(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07118c0b69d3..6e1358d4e764 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7900,7 +7900,6 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 		intel_runtime_pm_get(dev_priv);
 	}
 
-	mutex_lock(&dev_priv->drm.struct_mutex);
 	mutex_lock(&dev_priv->pcu_lock);
 
 	/* Initialize RPS limits (for userspace) */
@@ -7942,7 +7941,6 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 	rps->boost_freq = rps->max_freq;
 
 	mutex_unlock(&dev_priv->pcu_lock);
-	mutex_unlock(&dev_priv->drm.struct_mutex);
 
 	intel_autoenable_gt_powersave(dev_priv);
 }
-- 
2.15.0

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

[PATCH 3/6] drm/i915: Inline intel_modeset_gem_init()

[Chris Wilson]

intel_modeset_gem_init() now only sets up the legacy overlay, so let's
remove the function and call the setup directly during driver load. This
should help us find a better point in the initialisation sequence for it
later.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c      | 2 +-
 drivers/gpu/drm/i915/i915_drv.h      | 1 -
 drivers/gpu/drm/i915/intel_display.c | 7 -------
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7e9e061073b..1b440f2b90a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -676,7 +676,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_uc;
 
-	intel_modeset_gem_init(dev);
+	intel_setup_overlay(dev_priv);
 
 	if (INTEL_INFO(dev_priv)->num_pipes == 0)
 		return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72bb5b51035a..593714fe5c5e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4118,7 +4118,6 @@ void intel_device_info_dump(struct drm_i915_private *dev_priv);
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern int intel_modeset_init(struct drm_device *dev);
-extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_connector_register(struct drm_connector *);
 extern void intel_connector_unregister(struct drm_connector *);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3bf87c2036c..5debb79540a2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15170,13 +15170,6 @@ void intel_display_resume(struct drm_device *dev)
 		drm_atomic_state_put(state);
 }
 
-void intel_modeset_gem_init(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	intel_setup_overlay(dev_priv);
-}
-
 int intel_connector_register(struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
-- 
2.15.0

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

[PATCH 4/6] drm/i915: Record the default hw state after reset upon load

[Chris Wilson]

Take a copy of the HW state after a reset upon module loading by
executing a context switch from a blank context to the kernel context,
thus saving the default hw state over the blank context image.
We can then use the default hw state to initialise any future context,
ensuring that each starts with the default view of hw state.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gvt/scheduler.c    |  2 -
 drivers/gpu/drm/i915/i915_debugfs.c     |  1 -
 drivers/gpu/drm/i915/i915_gem.c         | 69 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_context.c | 50 +++---------------------
 drivers/gpu/drm/i915/i915_gem_context.h |  4 +-
 drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
 drivers/gpu/drm/i915/intel_lrc.c        | 35 ++++++++++-------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++++++++++++++-------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
 9 files changed, 137 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index f6ded475bb2c..42cc61230ca7 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -723,8 +723,6 @@ int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
 	if (IS_ERR(vgpu->shadow_ctx))
 		return PTR_ERR(vgpu->shadow_ctx);
 
-	vgpu->shadow_ctx->engine[RCS].initialised = true;
-
 	bitmap_zero(vgpu->shadow_ctx_desc_updated, I915_NUM_ENGINES);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 39883cd915db..cfcef1899da8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1974,7 +1974,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
 			struct intel_context *ce = &ctx->engine[engine->id];
 
 			seq_printf(m, "%s: ", engine->name);
-			seq_putc(m, ce->initialised ? 'I' : 'i');
 			if (ce->state)
 				describe_obj(m, ce->state->obj);
 			if (ce->ring)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e36a3a840552..ed4b1708fec5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4967,6 +4967,71 @@ bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
 	return true;
 }
 
+static int __intel_engines_record_defaults(struct drm_i915_private *i915)
+{
+	struct i915_gem_context *ctx;
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+	int err;
+
+	/*
+	 * As we reset the gpu during very early sanitisation, the current
+	 * register state on the GPU should reflect its defaults values.
+	 * We load a context onto the hw (with restore-inhibit), then switch
+	 * over to a second context to save that default register state. We
+	 * can then prime every new context with that state so they all start
+	 * from defaults.
+	 */
+
+	ctx = i915_gem_context_create_kernel(i915, 0);
+	if (IS_ERR(ctx))
+		return PTR_ERR(ctx);
+
+	for_each_engine(engine, i915, id) {
+		struct drm_i915_gem_request *rq;
+
+		rq = i915_gem_request_alloc(engine, ctx);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			goto out_ctx;
+		}
+
+		err = i915_switch_context(rq);
+		if (engine->init_context)
+			err = engine->init_context(rq);
+
+		__i915_add_request(rq, true);
+		if (err)
+			goto out_ctx;
+	}
+
+	err = i915_gem_switch_to_kernel_context(i915);
+	if (err)
+		goto out_ctx;
+
+	err = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
+	if (err)
+		goto out_ctx;
+
+	for_each_engine(engine, i915, id) {
+		if (!ctx->engine[id].state)
+			continue;
+
+		engine->default_state =
+			i915_gem_object_get(ctx->engine[id].state->obj);
+
+		err = i915_gem_object_set_to_cpu_domain(engine->default_state,
+							false);
+		if (err)
+			goto out_ctx;
+	}
+
+out_ctx:
+	i915_gem_context_set_closed(ctx);
+	i915_gem_context_put(ctx);
+	return err;
+}
+
 int i915_gem_init(struct drm_i915_private *dev_priv)
 {
 	int ret;
@@ -5022,6 +5087,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 
 	intel_init_gt_powersave(dev_priv);
 
+	ret = __intel_engines_record_defaults(dev_priv);
+	if (ret)
+		goto out_unlock;
+
 out_unlock:
 	if (ret == -EIO) {
 		/* Allow engine initialisation to fail by marking the GPU as
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 10affb35ac56..499cc0f25653 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -343,7 +343,7 @@ static void __destroy_hw_context(struct i915_gem_context *ctx,
  * context state of the GPU for applications that don't utilize HW contexts, as
  * well as an idle case.
  */
-static struct i915_gem_context *
+struct i915_gem_context *
 i915_gem_create_context(struct drm_i915_private *dev_priv,
 			struct drm_i915_file_private *file_priv)
 {
@@ -418,8 +418,8 @@ i915_gem_context_create_gvt(struct drm_device *dev)
 	return ctx;
 }
 
-static struct i915_gem_context *
-create_kernel_context(struct drm_i915_private *i915, int prio)
+struct i915_gem_context *
+i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
 {
 	struct i915_gem_context *ctx;
 
@@ -473,7 +473,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 	ida_init(&dev_priv->contexts.hw_ida);
 
 	/* lowest priority; idle task */
-	ctx = create_kernel_context(dev_priv, I915_PRIORITY_MIN);
+	ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
 	if (IS_ERR(ctx)) {
 		DRM_ERROR("Failed to create default global context\n");
 		err = PTR_ERR(ctx);
@@ -487,7 +487,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 	dev_priv->kernel_context = ctx;
 
 	/* highest priority; preempting task */
-	ctx = create_kernel_context(dev_priv, INT_MAX);
+	ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
 	if (IS_ERR(ctx)) {
 		DRM_ERROR("Failed to create default preempt context\n");
 		err = PTR_ERR(ctx);
@@ -522,28 +522,6 @@ void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
 		engine->context_unpin(engine, engine->last_retired_context);
 		engine->last_retired_context = NULL;
 	}
-
-	/* Force the GPU state to be restored on enabling */
-	if (!i915_modparams.enable_execlists) {
-		struct i915_gem_context *ctx;
-
-		list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
-			if (!i915_gem_context_is_default(ctx))
-				continue;
-
-			for_each_engine(engine, dev_priv, id)
-				ctx->engine[engine->id].initialised = false;
-
-			ctx->remap_slice = ALL_L3_SLICES(dev_priv);
-		}
-
-		for_each_engine(engine, dev_priv, id) {
-			struct intel_context *kce =
-				&dev_priv->kernel_context->engine[engine->id];
-
-			kce->initialised = true;
-		}
-	}
 }
 
 void i915_gem_contexts_fini(struct drm_i915_private *i915)
@@ -718,9 +696,6 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
 	if (to->remap_slice)
 		return false;
 
-	if (!to->engine[RCS].initialised)
-		return false;
-
 	if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
 		return false;
 
@@ -795,11 +770,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 			return ret;
 	}
 
-	if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
-		/* NB: If we inhibit the restore, the context is not allowed to
-		 * die because future work may end up depending on valid address
-		 * space. This means we must enforce that a page table load
-		 * occur when this occurs. */
+	if (i915_gem_context_is_kernel(to))
 		hw_flags = MI_RESTORE_INHIBIT;
 	else if (ppgtt && intel_engine_flag(engine) & ppgtt->pd_dirty_rings)
 		hw_flags = MI_FORCE_RESTORE;
@@ -843,15 +814,6 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 		to->remap_slice &= ~(1<<i);
 	}
 
-	if (!to->engine[RCS].initialised) {
-		if (engine->init_context) {
-			ret = engine->init_context(req);
-			if (ret)
-				return ret;
-		}
-		to->engine[RCS].initialised = true;
-	}
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 44688e22a5c2..4bfb72f8e1cb 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -157,7 +157,6 @@ struct i915_gem_context {
 		u32 *lrc_reg_state;
 		u64 lrc_desc;
 		int pin_count;
-		bool initialised;
 	} engine[I915_NUM_ENGINES];
 
 	/** ring_size: size for allocating the per-engine ring buffer */
@@ -292,6 +291,9 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
 				       struct drm_file *file);
 
+struct i915_gem_context *
+i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio);
+
 static inline struct i915_gem_context *
 i915_gem_context_get(struct i915_gem_context *ctx)
 {
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index c15e288bed02..6b8519401d4d 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -679,6 +679,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 	intel_engine_cleanup_cmd_parser(engine);
 	i915_gem_batch_pool_fini(&engine->batch_pool);
 
+	if (engine->default_state)
+		i915_gem_object_put(engine->default_state);
+
 	if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
 		engine->context_unpin(engine, engine->i915->preempt_context);
 	engine->context_unpin(engine, engine->i915->kernel_context);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6840ec8db037..297ce0327273 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1147,7 +1147,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 	struct intel_engine_cs *engine = request->engine;
 	struct intel_context *ce = &request->ctx->engine[engine->id];
 	u32 *cs;
-	int ret;
 
 	GEM_BUG_ON(!ce->pin_count);
 
@@ -1161,14 +1160,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	if (!ce->initialised) {
-		ret = engine->init_context(request);
-		if (ret)
-			return ret;
-
-		ce->initialised = true;
-	}
-
 	/* Note that after this point, we have committed to using
 	 * this request as it is being used to both track the
 	 * state of engine initialisation and liveness of the
@@ -2100,7 +2091,6 @@ static void execlists_init_reg_state(u32 *regs,
 
 	CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine),
 		_MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH |
-				   CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
 				   (HAS_RESOURCE_STREAMER(dev_priv) ?
 				   CTX_CTRL_RS_CTX_ENABLE : 0)));
 	CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
@@ -2177,6 +2167,7 @@ populate_lr_context(struct i915_gem_context *ctx,
 		    struct intel_ring *ring)
 {
 	void *vaddr;
+	u32 *regs;
 	int ret;
 
 	ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
@@ -2193,11 +2184,28 @@ populate_lr_context(struct i915_gem_context *ctx,
 	}
 	ctx_obj->mm.dirty = true;
 
+	if (engine->default_state) {
+		void *defaults;
+
+		defaults = i915_gem_object_pin_map(engine->default_state,
+						   I915_MAP_WB);
+		if (IS_ERR(defaults))
+			return PTR_ERR(defaults);
+
+		memcpy(vaddr + LRC_HEADER_PAGES * PAGE_SIZE,
+		       defaults + LRC_HEADER_PAGES * PAGE_SIZE,
+		       engine->context_size);
+		i915_gem_object_unpin_map(engine->default_state);
+	}
+
 	/* The second page of the context object contains some fields which must
 	 * be set up prior to the first execution. */
-
-	execlists_init_reg_state(vaddr + LRC_STATE_PN * PAGE_SIZE,
-				 ctx, engine, ring);
+	regs = vaddr + LRC_STATE_PN * PAGE_SIZE;
+	execlists_init_reg_state(regs, ctx, engine, ring);
+	if (!engine->default_state) {
+		regs[CTX_CONTEXT_CONTROL+1] |=
+			_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
+	}
 
 	i915_gem_object_unpin_map(ctx_obj);
 
@@ -2250,7 +2258,6 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 
 	ce->ring = ring;
 	ce->state = vma;
-	ce->initialised |= engine->init_context == NULL;
 
 	return 0;
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 47fadf8da84e..3e5c296cd8bc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1368,7 +1368,7 @@ static int context_pin(struct i915_gem_context *ctx)
 	 * on an active context (which by nature is already on the GPU).
 	 */
 	if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
-		ret = i915_gem_object_set_to_gtt_domain(vma->obj, false);
+		ret = i915_gem_object_set_to_gtt_domain(vma->obj, true);
 		if (ret)
 			return ret;
 	}
@@ -1383,11 +1383,34 @@ alloc_context_vma(struct intel_engine_cs *engine)
 	struct drm_i915_private *i915 = engine->i915;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
+	int err;
 
 	obj = i915_gem_object_create(i915, engine->context_size);
 	if (IS_ERR(obj))
 		return ERR_CAST(obj);
 
+	if (engine->default_state) {
+		void *defaults, *vaddr;
+
+		vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+		if (IS_ERR(vaddr)) {
+			err = PTR_ERR(vaddr);
+			goto err_obj;
+		}
+
+		defaults = i915_gem_object_pin_map(engine->default_state,
+						   I915_MAP_WB);
+		if (IS_ERR(defaults)) {
+			err = PTR_ERR(defaults);
+			goto err_map;
+		}
+
+		memcpy(vaddr, defaults, engine->context_size);
+
+		i915_gem_object_unpin_map(engine->default_state);
+		i915_gem_object_unpin_map(obj);
+	}
+
 	/*
 	 * Try to make the context utilize L3 as well as LLC.
 	 *
@@ -1409,10 +1432,18 @@ alloc_context_vma(struct intel_engine_cs *engine)
 	}
 
 	vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);
-	if (IS_ERR(vma))
-		i915_gem_object_put(obj);
+	if (IS_ERR(vma)) {
+		err = PTR_ERR(vma);
+		goto err_obj;
+	}
 
 	return vma;
+
+err_map:
+	i915_gem_object_unpin_map(obj);
+err_obj:
+	i915_gem_object_put(obj);
+	return ERR_PTR(err);
 }
 
 static struct intel_ring *
@@ -1449,16 +1480,6 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
 		ce->state->obj->pin_global++;
 	}
 
-	/* The kernel context is only used as a placeholder for flushing the
-	 * active context. It is never used for submitting user rendering and
-	 * as such never requires the golden render context, and so we can skip
-	 * emitting it when we switch to the kernel context. This is required
-	 * as during eviction we cannot allocate and pin the renderstate in
-	 * order to initialise the context.
-	 */
-	if (i915_gem_context_is_kernel(ctx))
-		ce->initialised = true;
-
 	i915_gem_context_get(ctx);
 
 out:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 69ad875fd011..1d752b9a3065 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -303,6 +303,7 @@ struct intel_engine_cs {
 	struct intel_ring *buffer;
 	struct intel_timeline *timeline;
 
+	struct drm_i915_gem_object *default_state;
 	struct intel_render_state *render_state;
 
 	atomic_t irq_count;
-- 
2.15.0

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

[PATCH 5/6] drm/i915: Remove redundant intel_autoenable_gt_powersave()

[Chris Wilson]

Now that we always execute a context switch upon module load, there is
no need to queue a delayed task for doing so. The purpose of the delayed
task is to enable GT powersaving, for which we need the HW state to be
valid (i.e. having loaded a context and initialised basic state). We
used to defer this operation as historically it was slow (due to slow
register polling, fixed with commit 1758b90e38f5 ("drm/i915: Use a hybrid
scheme for fast register waits")) but now we have a requirement to save
the default HW state.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c  |  2 --
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 drivers/gpu/drm/i915/intel_pm.c  | 66 ----------------------------------------
 4 files changed, 70 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1b440f2b90a5..2c8636a387a8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1743,8 +1743,6 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	intel_opregion_notify_adapter(dev_priv, PCI_D0);
 
-	intel_autoenable_gt_powersave(dev_priv);
-
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 593714fe5c5e..466ee055030b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1388,7 +1388,6 @@ struct intel_gen6_power_mgmt {
 	struct intel_rps rps;
 	struct intel_rc6 rc6;
 	struct intel_llc_pstate llc_pstate;
-	struct delayed_work autoenable_work;
 };
 
 /* defined intel_pm.c */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 00b488688042..76b12fb1a35f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1881,7 +1881,6 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_enable_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_autoenable_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_disable_gt_powersave(struct drm_i915_private *dev_priv);
 void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv);
 void gen6_rps_busy(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6e1358d4e764..308439dd89d4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7941,8 +7941,6 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 	rps->boost_freq = rps->max_freq;
 
 	mutex_unlock(&dev_priv->pcu_lock);
-
-	intel_autoenable_gt_powersave(dev_priv);
 }
 
 void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
@@ -7967,9 +7965,6 @@ void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) < 6)
 		return;
 
-	if (cancel_delayed_work_sync(&dev_priv->gt_pm.autoenable_work))
-		intel_runtime_pm_put(dev_priv);
-
 	/* gen6_rps_idle() will be called later to disable interrupts */
 }
 
@@ -8128,65 +8123,6 @@ void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
-static void __intel_autoenable_gt_powersave(struct work_struct *work)
-{
-	struct drm_i915_private *dev_priv =
-		container_of(work,
-			     typeof(*dev_priv),
-			     gt_pm.autoenable_work.work);
-	struct intel_engine_cs *rcs;
-	struct drm_i915_gem_request *req;
-
-	rcs = dev_priv->engine[RCS];
-	if (rcs->last_retired_context)
-		goto out;
-
-	if (!rcs->init_context)
-		goto out;
-
-	mutex_lock(&dev_priv->drm.struct_mutex);
-
-	req = i915_gem_request_alloc(rcs, dev_priv->kernel_context);
-	if (IS_ERR(req))
-		goto unlock;
-
-	if (!i915_modparams.enable_execlists && i915_switch_context(req) == 0)
-		rcs->init_context(req);
-
-	/* Mark the device busy, calling intel_enable_gt_powersave() */
-	i915_add_request(req);
-
-unlock:
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-out:
-	intel_runtime_pm_put(dev_priv);
-}
-
-void intel_autoenable_gt_powersave(struct drm_i915_private *dev_priv)
-{
-	if (IS_IRONLAKE_M(dev_priv)) {
-		ironlake_enable_drps(dev_priv);
-		intel_init_emon(dev_priv);
-	} else if (INTEL_INFO(dev_priv)->gen >= 6) {
-		/*
-		 * PCU communication is slow and this doesn't need to be
-		 * done at any specific time, so do this out of our fast path
-		 * to make resume and init faster.
-		 *
-		 * We depend on the HW RC6 power context save/restore
-		 * mechanism when entering D3 through runtime PM suspend. So
-		 * disable RPM until RPS/RC6 is properly setup. We can only
-		 * get here via the driver load/system resume/runtime resume
-		 * paths, so the _noresume version is enough (and in case of
-		 * runtime resume it's necessary).
-		 */
-		if (queue_delayed_work(dev_priv->wq,
-				       &dev_priv->gt_pm.autoenable_work,
-				       round_jiffies_up_relative(HZ)))
-			intel_runtime_pm_get_noresume(dev_priv);
-	}
-}
-
 static void ibx_init_clock_gating(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -9443,8 +9379,6 @@ void intel_pm_setup(struct drm_i915_private *dev_priv)
 {
 	mutex_init(&dev_priv->pcu_lock);
 
-	INIT_DELAYED_WORK(&dev_priv->gt_pm.autoenable_work,
-			  __intel_autoenable_gt_powersave);
 	atomic_set(&dev_priv->gt_pm.rps.num_waiters, 0);
 
 	dev_priv->runtime_pm.suspended = false;
-- 
2.15.0

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

[PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Chris Wilson]

As we now record the default HW state and so only emit the "golden"
renderstate once to prepare the HW, there is no advantage in keeping the
renderstate batch around as it will never be used again.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h              |   1 -
 drivers/gpu/drm/i915/i915_gem_render_state.c | 134 +++++++++------------------
 drivers/gpu/drm/i915/i915_gem_render_state.h |   4 +-
 drivers/gpu/drm/i915/intel_engine_cs.c       |   9 +-
 drivers/gpu/drm/i915/intel_lrc.c             |   1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c      |   5 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h      |   2 -
 7 files changed, 50 insertions(+), 106 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 466ee055030b..71747131c444 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -67,7 +67,6 @@
 #include "i915_gem_fence_reg.h"
 #include "i915_gem_object.h"
 #include "i915_gem_gtt.h"
-#include "i915_gem_render_state.h"
 #include "i915_gem_request.h"
 #include "i915_gem_timeline.h"
 
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 3703dc91eeda..cb2ea98676bf 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -30,6 +30,7 @@
 
 struct intel_render_state {
 	const struct intel_renderstate_rodata *rodata;
+	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
 	u32 batch_offset;
 	u32 batch_size;
@@ -74,17 +75,16 @@ static int render_state_setup(struct intel_render_state *so,
 			      struct drm_i915_private *i915)
 {
 	const struct intel_renderstate_rodata *rodata = so->rodata;
-	struct drm_i915_gem_object *obj = so->vma->obj;
 	unsigned int i = 0, reloc_index = 0;
 	unsigned int needs_clflush;
 	u32 *d;
 	int ret;
 
-	ret = i915_gem_obj_prepare_shmem_write(obj, &needs_clflush);
+	ret = i915_gem_obj_prepare_shmem_write(so->obj, &needs_clflush);
 	if (ret)
 		return ret;
 
-	d = kmap_atomic(i915_gem_object_get_dirty_page(obj, 0));
+	d = kmap_atomic(i915_gem_object_get_dirty_page(so->obj, 0));
 
 	while (i < rodata->batch_items) {
 		u32 s = rodata->batch[i];
@@ -112,7 +112,7 @@ static int render_state_setup(struct intel_render_state *so,
 		goto err;
 	}
 
-	so->batch_offset = so->vma->node.start;
+	so->batch_offset = i915_ggtt_offset(so->vma);
 	so->batch_size = rodata->batch_items * sizeof(u32);
 
 	while (i % CACHELINE_DWORDS)
@@ -160,9 +160,9 @@ static int render_state_setup(struct intel_render_state *so,
 		drm_clflush_virt_range(d, i * sizeof(u32));
 	kunmap_atomic(d);
 
-	ret = i915_gem_object_set_to_gtt_domain(obj, false);
+	ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
 out:
-	i915_gem_obj_finish_shmem_access(obj);
+	i915_gem_obj_finish_shmem_access(so->obj);
 	return ret;
 
 err:
@@ -173,112 +173,64 @@ static int render_state_setup(struct intel_render_state *so,
 
 #undef OUT_BATCH
 
-int i915_gem_render_state_init(struct intel_engine_cs *engine)
+int i915_gem_render_state_emit(struct drm_i915_gem_request *rq)
 {
-	struct intel_render_state *so;
-	const struct intel_renderstate_rodata *rodata;
-	struct drm_i915_gem_object *obj;
-	int ret;
+	struct intel_engine_cs *engine = rq->engine;
+	struct intel_render_state so;
+	int err;
 
 	if (engine->id != RCS)
 		return 0;
 
-	rodata = render_state_get_rodata(engine);
-	if (!rodata)
+	so.rodata = render_state_get_rodata(engine);
+	if (!so.rodata)
 		return 0;
 
-	if (rodata->batch_items * 4 > PAGE_SIZE)
+	if (so.rodata->batch_items * 4 > PAGE_SIZE)
 		return -EINVAL;
 
-	so = kmalloc(sizeof(*so), GFP_KERNEL);
-	if (!so)
-		return -ENOMEM;
+	so.obj = i915_gem_object_create_internal(engine->i915, PAGE_SIZE);
+	if (IS_ERR(so.obj))
+		return PTR_ERR(so.obj);
 
-	obj = i915_gem_object_create_internal(engine->i915, PAGE_SIZE);
-	if (IS_ERR(obj)) {
-		ret = PTR_ERR(obj);
-		goto err_free;
-	}
-
-	so->vma = i915_vma_instance(obj, &engine->i915->ggtt.base, NULL);
-	if (IS_ERR(so->vma)) {
-		ret = PTR_ERR(so->vma);
+	so.vma = i915_vma_instance(so.obj, &engine->i915->ggtt.base, NULL);
+	if (IS_ERR(so.vma)) {
+		err = PTR_ERR(so.vma);
 		goto err_obj;
 	}
 
-	so->rodata = rodata;
-	engine->render_state = so;
-	return 0;
-
-err_obj:
-	i915_gem_object_put(obj);
-err_free:
-	kfree(so);
-	return ret;
-}
-
-int i915_gem_render_state_emit(struct drm_i915_gem_request *req)
-{
-	struct intel_render_state *so;
-	int ret;
-
-	lockdep_assert_held(&req->i915->drm.struct_mutex);
+	err = i915_vma_pin(so.vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
+	if (err)
+		goto err_vma;
 
-	so = req->engine->render_state;
-	if (!so)
-		return 0;
-
-	/* Recreate the page after shrinking */
-	if (!i915_gem_object_has_pages(so->vma->obj))
-		so->batch_offset = -1;
-
-	ret = i915_vma_pin(so->vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
-	if (ret)
-		return ret;
-
-	if (so->vma->node.start != so->batch_offset) {
-		ret = render_state_setup(so, req->i915);
-		if (ret)
-			goto err_unpin;
-	}
+	err = render_state_setup(&so, rq->i915);
+	if (err)
+		goto err_unpin;
 
-	ret = req->engine->emit_flush(req, EMIT_INVALIDATE);
-	if (ret)
+	err = engine->emit_flush(rq, EMIT_INVALIDATE);
+	if (err)
 		goto err_unpin;
 
-	ret = req->engine->emit_bb_start(req,
-					 so->batch_offset, so->batch_size,
-					 I915_DISPATCH_SECURE);
-	if (ret)
+	err = engine->emit_bb_start(rq,
+				    so.batch_offset, so.batch_size,
+				    I915_DISPATCH_SECURE);
+	if (err)
 		goto err_unpin;
 
-	if (so->aux_size > 8) {
-		ret = req->engine->emit_bb_start(req,
-						 so->aux_offset, so->aux_size,
-						 I915_DISPATCH_SECURE);
-		if (ret)
+	if (so.aux_size > 8) {
+		err = engine->emit_bb_start(rq,
+					    so.aux_offset, so.aux_size,
+					    I915_DISPATCH_SECURE);
+		if (err)
 			goto err_unpin;
 	}
 
-	i915_vma_move_to_active(so->vma, req, 0);
+	i915_vma_move_to_active(so.vma, rq, 0);
 err_unpin:
-	i915_vma_unpin(so->vma);
-	return ret;
-}
-
-void i915_gem_render_state_fini(struct intel_engine_cs *engine)
-{
-	struct intel_render_state *so;
-	struct drm_i915_gem_object *obj;
-
-	so = fetch_and_zero(&engine->render_state);
-	if (!so)
-		return;
-
-	obj = so->vma->obj;
-
-	i915_vma_close(so->vma);
-	__i915_gem_object_release_unless_active(obj);
-
-	kfree(so);
+	i915_vma_unpin(so.vma);
+err_vma:
+	i915_vma_close(so.vma);
+err_obj:
+	__i915_gem_object_release_unless_active(so.obj);
+	return err;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.h b/drivers/gpu/drm/i915/i915_gem_render_state.h
index 87481845799d..86369520482e 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.h
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.h
@@ -26,8 +26,6 @@
 
 struct drm_i915_gem_request;
 
-int i915_gem_render_state_init(struct intel_engine_cs *engine);
-int i915_gem_render_state_emit(struct drm_i915_gem_request *req);
-void i915_gem_render_state_fini(struct intel_engine_cs *engine);
+int i915_gem_render_state_emit(struct drm_i915_gem_request *rq);
 
 #endif /* _I915_GEM_RENDER_STATE_H_ */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 6b8519401d4d..6f8154007396 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -633,21 +633,15 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
 	if (ret)
 		goto err_unpin_preempt;
 
-	ret = i915_gem_render_state_init(engine);
-	if (ret)
-		goto err_breadcrumbs;
-
 	if (HWS_NEEDS_PHYSICAL(engine->i915))
 		ret = init_phys_status_page(engine);
 	else
 		ret = init_status_page(engine);
 	if (ret)
-		goto err_rs_fini;
+		goto err_breadcrumbs;
 
 	return 0;
 
-err_rs_fini:
-	i915_gem_render_state_fini(engine);
 err_breadcrumbs:
 	intel_engine_fini_breadcrumbs(engine);
 err_unpin_preempt:
@@ -674,7 +668,6 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 	else
 		cleanup_status_page(engine);
 
-	i915_gem_render_state_fini(engine);
 	intel_engine_fini_breadcrumbs(engine);
 	intel_engine_cleanup_cmd_parser(engine);
 	i915_gem_batch_pool_fini(&engine->batch_pool);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 297ce0327273..1d968f0f4044 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -136,6 +136,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "i915_gem_render_state.h"
 #include "intel_mocs.h"
 
 #define RING_EXECLIST_QFULL		(1 << 0x2)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3e5c296cd8bc..ac279d0eb894 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -28,9 +28,12 @@
  */
 
 #include <linux/log2.h>
+
 #include <drm/drmP.h>
-#include "i915_drv.h"
 #include <drm/i915_drm.h>
+
+#include "i915_drv.h"
+#include "i915_gem_render_state.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1d752b9a3065..0ada71ab09ff 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -165,7 +165,6 @@ struct i915_ctx_workarounds {
 };
 
 struct drm_i915_gem_request;
-struct intel_render_state;
 
 /*
  * Engine IDs definitions.
@@ -304,7 +303,6 @@ struct intel_engine_cs {
 	struct intel_timeline *timeline;
 
 	struct drm_i915_gem_object *default_state;
-	struct intel_render_state *render_state;
 
 	atomic_t irq_count;
 	unsigned long irq_posted;
-- 
2.15.0

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

✗ Fi.CI.BAT: failure for series starting with [1/6] drm/i915: Force the switch to the i915->kernel_context

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915: Force the switch to the i915->kernel_context
URL   : https://patchwork.freedesktop.org/series/33046/
State : failure

== Summary ==

Series 33046v1 series starting with [1/6] drm/i915: Force the switch to the i915->kernel_context
https://patchwork.freedesktop.org/api/1.0/series/33046/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test gem_exec_reloc:
        Subgroup basic-cpu-active:
                fail       -> PASS       (fi-gdg-551) fdo#102582 +1
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> INCOMPLETE (fi-glk-dsi)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                incomplete -> PASS       (fi-cnl-y) fdo#103339
Test kms_frontbuffer_tracking:
        Subgroup basic:
                dmesg-warn -> PASS       (fi-bdw-5557u) fdo#102473

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#103339 https://bugs.freedesktop.org/show_bug.cgi?id=103339
fdo#102473 https://bugs.freedesktop.org/show_bug.cgi?id=102473

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:455s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:379s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:507s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:507s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:492s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:600s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:427s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:264s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:582s
fi-glk-dsi       total:206  pass:183  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:429s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:426s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:498s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:485s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:573s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:584s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:570s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:588s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:652s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:504s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:463s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:575s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:422s

dcf7d008f606b543fef53609a6e336097e6b5694 drm-tip: 2017y-11m-02d-08h-43m-13s UTC integration manifest
028a928e8aeb drm/i915: Stop caching the "golden" renderstate
b5b2ad90612f drm/i915: Remove redundant intel_autoenable_gt_powersave()
6bb7725f5687 drm/i915: Record the default hw state after reset upon load
dd82c688f3f9 drm/i915: Inline intel_modeset_gem_init()
a3899988bc8e drm/i915: Move GT powersaving init to i915_gem_init()
5325edbb2409 drm/i915: Force the switch to the i915->kernel_context

== Logs ==

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

Re: [PATCH 5/6] drm/i915: Remove redundant intel_autoenable_gt_powersave()

[Joonas Lahtinen]

On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> Now that we always execute a context switch upon module load, there is
> no need to queue a delayed task for doing so. The purpose of the delayed
> task is to enable GT powersaving, for which we need the HW state to be
> valid (i.e. having loaded a context and initialised basic state). We
> used to defer this operation as historically it was slow (due to slow
> register polling, fixed with commit 1758b90e38f5 ("drm/i915: Use a hybrid
> scheme for fast register waits")) but now we have a requirement to save
> the default HW state.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Force the switch to the i915->kernel_context

[Joonas Lahtinen]

On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> In the next few patches, we will have a hard requirement that we emit a
> context-switch to the perma-pinned i915->kernel_context (so that we can
> save the HW state using that context-switch). As the first context
> itself may be classed as a kernel context, we want to be explicit in our
> comparison. For an extra-layer of finesse, we can check the last
> unretired context on the engine; as well as the last retired context
> when idle.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -1587,8 +1587,20 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv)
>  
>  bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine)
>  {
> -	return (!engine->last_retired_context ||
> -		i915_gem_context_is_kernel(engine->last_retired_context));
> +	const struct i915_gem_context *kctx = engine->i915->kernel_context;

*kernel_context = ...

> +	struct drm_i915_gem_request *rq;
> +
> +	lockdep_assert_held(&engine->i915->drm.struct_mutex);
> +
> +	/* An unretired request? */
> +	rq = __i915_gem_active_peek(&engine->timeline->last_request);
> +	if (rq)
> +		return rq->ctx == kctx;
> +
> +	if (!engine->last_retired_context)
> +		return true;
> +
> +	return engine->last_retired_context == kctx;

It might be worthy dropping a comment that a nonexistent context is
considered kernel context to the *cough*nonexistent*cough* kerneldoc.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Move GT powersaving init to i915_gem_init()

[Joonas Lahtinen]

On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> GT powersaving is tightly coupled to the request infrastructure. To
> avoid complications with the order of initialisation in the next patch
> (where we want to send requests to hw during GEM init) move the
> powersaving initialisation into the purview of i915_gem_init().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/i915: Inline intel_modeset_gem_init()

[Joonas Lahtinen]

On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> intel_modeset_gem_init() now only sets up the legacy overlay, so let's
> remove the function and call the setup directly during driver load. This
> should help us find a better point in the initialisation sequence for it
> later.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Record the default hw state after reset upon load

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Take a copy of the HW state after a reset upon module loading by
> executing a context switch from a blank context to the kernel context,
> thus saving the default hw state over the blank context image.
> We can then use the default hw state to initialise any future context,
> ensuring that each starts with the default view of hw state.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c    |  2 -
>  drivers/gpu/drm/i915/i915_debugfs.c     |  1 -
>  drivers/gpu/drm/i915/i915_gem.c         | 69 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_context.c | 50 +++---------------------
>  drivers/gpu/drm/i915/i915_gem_context.h |  4 +-
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
>  drivers/gpu/drm/i915/intel_lrc.c        | 35 ++++++++++-------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++++++++++++++-------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>  9 files changed, 137 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index f6ded475bb2c..42cc61230ca7 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -723,8 +723,6 @@ int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
>  	if (IS_ERR(vgpu->shadow_ctx))
>  		return PTR_ERR(vgpu->shadow_ctx);
>  
> -	vgpu->shadow_ctx->engine[RCS].initialised = true;
> -
>  	bitmap_zero(vgpu->shadow_ctx_desc_updated, I915_NUM_ENGINES);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 39883cd915db..cfcef1899da8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1974,7 +1974,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
>  			struct intel_context *ce = &ctx->engine[engine->id];
>  
>  			seq_printf(m, "%s: ", engine->name);
> -			seq_putc(m, ce->initialised ? 'I' : 'i');
>  			if (ce->state)
>  				describe_obj(m, ce->state->obj);
>  			if (ce->ring)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e36a3a840552..ed4b1708fec5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4967,6 +4967,71 @@ bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
>  	return true;
>  }
>  
> +static int __intel_engines_record_defaults(struct drm_i915_private *i915)
> +{
> +	struct i915_gem_context *ctx;
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +	int err;
> +
> +	/*
> +	 * As we reset the gpu during very early sanitisation, the current
> +	 * register state on the GPU should reflect its defaults values.
> +	 * We load a context onto the hw (with restore-inhibit), then switch
> +	 * over to a second context to save that default register state. We
> +	 * can then prime every new context with that state so they all start
> +	 * from defaults.
> +	 */
> +
> +	ctx = i915_gem_context_create_kernel(i915, 0);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	for_each_engine(engine, i915, id) {
> +		struct drm_i915_gem_request *rq;
> +
> +		rq = i915_gem_request_alloc(engine, ctx);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			goto out_ctx;
> +		}
> +
> +		err = i915_switch_context(rq);
> +		if (engine->init_context)
> +			err = engine->init_context(rq);
> +
> +		__i915_add_request(rq, true);
> +		if (err)
> +			goto out_ctx;
> +	}
> +
> +	err = i915_gem_switch_to_kernel_context(i915);
> +	if (err)
> +		goto out_ctx;
> +
> +	err = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
> +	if (err)
> +		goto out_ctx;
> +
> +	for_each_engine(engine, i915, id) {
> +		if (!ctx->engine[id].state)
> +			continue;
> +
> +		engine->default_state =
> +			i915_gem_object_get(ctx->engine[id].state->obj);
> +
> +		err = i915_gem_object_set_to_cpu_domain(engine->default_state,
> +							false);
> +		if (err)
> +			goto out_ctx;
> +	}
> +
> +out_ctx:
> +	i915_gem_context_set_closed(ctx);
> +	i915_gem_context_put(ctx);
> +	return err;
> +}
> +
>  int i915_gem_init(struct drm_i915_private *dev_priv)
>  {
>  	int ret;
> @@ -5022,6 +5087,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>  
>  	intel_init_gt_powersave(dev_priv);
>  
> +	ret = __intel_engines_record_defaults(dev_priv);
> +	if (ret)
> +		goto out_unlock;
> +
>  out_unlock:
>  	if (ret == -EIO) {
>  		/* Allow engine initialisation to fail by marking the GPU as
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 10affb35ac56..499cc0f25653 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -343,7 +343,7 @@ static void __destroy_hw_context(struct i915_gem_context *ctx,
>   * context state of the GPU for applications that don't utilize HW contexts, as
>   * well as an idle case.
>   */
> -static struct i915_gem_context *
> +struct i915_gem_context *
>  i915_gem_create_context(struct drm_i915_private *dev_priv,
>  			struct drm_i915_file_private *file_priv)
>  {
> @@ -418,8 +418,8 @@ i915_gem_context_create_gvt(struct drm_device *dev)
>  	return ctx;
>  }
>  
> -static struct i915_gem_context *
> -create_kernel_context(struct drm_i915_private *i915, int prio)
> +struct i915_gem_context *
> +i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
>  {
>  	struct i915_gem_context *ctx;
>  
> @@ -473,7 +473,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
>  	ida_init(&dev_priv->contexts.hw_ida);
>  
>  	/* lowest priority; idle task */
> -	ctx = create_kernel_context(dev_priv, I915_PRIORITY_MIN);
> +	ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
>  	if (IS_ERR(ctx)) {
>  		DRM_ERROR("Failed to create default global context\n");
>  		err = PTR_ERR(ctx);
> @@ -487,7 +487,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
>  	dev_priv->kernel_context = ctx;
>  
>  	/* highest priority; preempting task */
> -	ctx = create_kernel_context(dev_priv, INT_MAX);
> +	ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
>  	if (IS_ERR(ctx)) {
>  		DRM_ERROR("Failed to create default preempt context\n");
>  		err = PTR_ERR(ctx);
> @@ -522,28 +522,6 @@ void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
>  		engine->context_unpin(engine, engine->last_retired_context);
>  		engine->last_retired_context = NULL;
>  	}
> -
> -	/* Force the GPU state to be restored on enabling */
> -	if (!i915_modparams.enable_execlists) {
> -		struct i915_gem_context *ctx;
> -
> -		list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> -			if (!i915_gem_context_is_default(ctx))
> -				continue;
> -
> -			for_each_engine(engine, dev_priv, id)
> -				ctx->engine[engine->id].initialised = false;
> -
> -			ctx->remap_slice = ALL_L3_SLICES(dev_priv);
> -		}
> -
> -		for_each_engine(engine, dev_priv, id) {
> -			struct intel_context *kce =
> -				&dev_priv->kernel_context->engine[engine->id];
> -
> -			kce->initialised = true;
> -		}
> -	}
>  }
>  
>  void i915_gem_contexts_fini(struct drm_i915_private *i915)
> @@ -718,9 +696,6 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
>  	if (to->remap_slice)
>  		return false;
>  
> -	if (!to->engine[RCS].initialised)
> -		return false;
> -
>  	if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
>  		return false;
>  
> @@ -795,11 +770,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>  			return ret;
>  	}
>  
> -	if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
> -		/* NB: If we inhibit the restore, the context is not allowed to
> -		 * die because future work may end up depending on valid address
> -		 * space. This means we must enforce that a page table load
> -		 * occur when this occurs. */
> +	if (i915_gem_context_is_kernel(to))
>  		hw_flags = MI_RESTORE_INHIBIT;
>  	else if (ppgtt && intel_engine_flag(engine) & ppgtt->pd_dirty_rings)
>  		hw_flags = MI_FORCE_RESTORE;
> @@ -843,15 +814,6 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>  		to->remap_slice &= ~(1<<i);
>  	}
>  
> -	if (!to->engine[RCS].initialised) {
> -		if (engine->init_context) {
> -			ret = engine->init_context(req);
> -			if (ret)
> -				return ret;
> -		}
> -		to->engine[RCS].initialised = true;
> -	}
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 44688e22a5c2..4bfb72f8e1cb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -157,7 +157,6 @@ struct i915_gem_context {
>  		u32 *lrc_reg_state;
>  		u64 lrc_desc;
>  		int pin_count;
> -		bool initialised;
>  	} engine[I915_NUM_ENGINES];
>  
>  	/** ring_size: size for allocating the per-engine ring buffer */
> @@ -292,6 +291,9 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>  int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
>  				       struct drm_file *file);
>  
> +struct i915_gem_context *
> +i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio);
> +
>  static inline struct i915_gem_context *
>  i915_gem_context_get(struct i915_gem_context *ctx)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index c15e288bed02..6b8519401d4d 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -679,6 +679,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>  	intel_engine_cleanup_cmd_parser(engine);
>  	i915_gem_batch_pool_fini(&engine->batch_pool);
>  
> +	if (engine->default_state)
> +		i915_gem_object_put(engine->default_state);
> +
>  	if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
>  		engine->context_unpin(engine, engine->i915->preempt_context);
>  	engine->context_unpin(engine, engine->i915->kernel_context);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6840ec8db037..297ce0327273 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1147,7 +1147,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
>  	struct intel_engine_cs *engine = request->engine;
>  	struct intel_context *ce = &request->ctx->engine[engine->id];
>  	u32 *cs;
> -	int ret;
>  
>  	GEM_BUG_ON(!ce->pin_count);
>  
> @@ -1161,14 +1160,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	if (!ce->initialised) {
> -		ret = engine->init_context(request);
> -		if (ret)
> -			return ret;
> -
> -		ce->initialised = true;
> -	}
> -
>  	/* Note that after this point, we have committed to using
>  	 * this request as it is being used to both track the
>  	 * state of engine initialisation and liveness of the
> @@ -2100,7 +2091,6 @@ static void execlists_init_reg_state(u32 *regs,
>  
>  	CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine),
>  		_MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH |
> -				   CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
>  				   (HAS_RESOURCE_STREAMER(dev_priv) ?
>  				   CTX_CTRL_RS_CTX_ENABLE : 0)));
>  	CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
> @@ -2177,6 +2167,7 @@ populate_lr_context(struct i915_gem_context *ctx,
>  		    struct intel_ring *ring)
>  {
>  	void *vaddr;
> +	u32 *regs;
>  	int ret;
>  
>  	ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
> @@ -2193,11 +2184,28 @@ populate_lr_context(struct i915_gem_context *ctx,
>  	}
>  	ctx_obj->mm.dirty = true;
>  
> +	if (engine->default_state) {
> +		void *defaults;
> +
> +		defaults = i915_gem_object_pin_map(engine->default_state,
> +						   I915_MAP_WB);
> +		if (IS_ERR(defaults))
> +			return PTR_ERR(defaults);
> +
> +		memcpy(vaddr + LRC_HEADER_PAGES * PAGE_SIZE,
> +		       defaults + LRC_HEADER_PAGES * PAGE_SIZE,
> +		       engine->context_size);
> +		i915_gem_object_unpin_map(engine->default_state);
> +	}
> +
>  	/* The second page of the context object contains some fields which must
>  	 * be set up prior to the first execution. */
> -
> -	execlists_init_reg_state(vaddr + LRC_STATE_PN * PAGE_SIZE,
> -				 ctx, engine, ring);
> +	regs = vaddr + LRC_STATE_PN * PAGE_SIZE;
> +	execlists_init_reg_state(regs, ctx, engine, ring);
> +	if (!engine->default_state) {
> +		regs[CTX_CONTEXT_CONTROL+1] |=
> +			_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
> +	}

We will get the default state we copy from now with restore inhibit set
for all copied context. Where do we clear it?
-Mika


>  
>  	i915_gem_object_unpin_map(ctx_obj);
>  
> @@ -2250,7 +2258,6 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>  
>  	ce->ring = ring;
>  	ce->state = vma;
> -	ce->initialised |= engine->init_context == NULL;
>  
>  	return 0;
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 47fadf8da84e..3e5c296cd8bc 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1368,7 +1368,7 @@ static int context_pin(struct i915_gem_context *ctx)
>  	 * on an active context (which by nature is already on the GPU).
>  	 */
>  	if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
> -		ret = i915_gem_object_set_to_gtt_domain(vma->obj, false);
> +		ret = i915_gem_object_set_to_gtt_domain(vma->obj, true);
>  		if (ret)
>  			return ret;
>  	}
> @@ -1383,11 +1383,34 @@ alloc_context_vma(struct intel_engine_cs *engine)
>  	struct drm_i915_private *i915 = engine->i915;
>  	struct drm_i915_gem_object *obj;
>  	struct i915_vma *vma;
> +	int err;
>  
>  	obj = i915_gem_object_create(i915, engine->context_size);
>  	if (IS_ERR(obj))
>  		return ERR_CAST(obj);
>  
> +	if (engine->default_state) {
> +		void *defaults, *vaddr;
> +
> +		vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
> +		if (IS_ERR(vaddr)) {
> +			err = PTR_ERR(vaddr);
> +			goto err_obj;
> +		}
> +
> +		defaults = i915_gem_object_pin_map(engine->default_state,
> +						   I915_MAP_WB);
> +		if (IS_ERR(defaults)) {
> +			err = PTR_ERR(defaults);
> +			goto err_map;
> +		}
> +
> +		memcpy(vaddr, defaults, engine->context_size);
> +
> +		i915_gem_object_unpin_map(engine->default_state);
> +		i915_gem_object_unpin_map(obj);
> +	}
> +
>  	/*
>  	 * Try to make the context utilize L3 as well as LLC.
>  	 *
> @@ -1409,10 +1432,18 @@ alloc_context_vma(struct intel_engine_cs *engine)
>  	}
>  
>  	vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);
> -	if (IS_ERR(vma))
> -		i915_gem_object_put(obj);
> +	if (IS_ERR(vma)) {
> +		err = PTR_ERR(vma);
> +		goto err_obj;
> +	}
>  
>  	return vma;
> +
> +err_map:
> +	i915_gem_object_unpin_map(obj);
> +err_obj:
> +	i915_gem_object_put(obj);
> +	return ERR_PTR(err);
>  }
>  
>  static struct intel_ring *
> @@ -1449,16 +1480,6 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
>  		ce->state->obj->pin_global++;
>  	}
>  
> -	/* The kernel context is only used as a placeholder for flushing the
> -	 * active context. It is never used for submitting user rendering and
> -	 * as such never requires the golden render context, and so we can skip
> -	 * emitting it when we switch to the kernel context. This is required
> -	 * as during eviction we cannot allocate and pin the renderstate in
> -	 * order to initialise the context.
> -	 */
> -	if (i915_gem_context_is_kernel(ctx))
> -		ce->initialised = true;
> -
>  	i915_gem_context_get(ctx);
>  
>  out:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 69ad875fd011..1d752b9a3065 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -303,6 +303,7 @@ struct intel_engine_cs {
>  	struct intel_ring *buffer;
>  	struct intel_timeline *timeline;
>  
> +	struct drm_i915_gem_object *default_state;
>  	struct intel_render_state *render_state;
>  
>  	atomic_t irq_count;
> -- 
> 2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Record the default hw state after reset upon load

[Chris Wilson]

Quoting Mika Kuoppala (2017-11-02 13:56:50)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> > -
> > -     execlists_init_reg_state(vaddr + LRC_STATE_PN * PAGE_SIZE,
> > -                              ctx, engine, ring);
> > +     regs = vaddr + LRC_STATE_PN * PAGE_SIZE;
> > +     execlists_init_reg_state(regs, ctx, engine, ring);
> > +     if (!engine->default_state) {
> > +             regs[CTX_CONTEXT_CONTROL+1] |=
> > +                     _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
> > +     }
> 
> We will get the default state we copy from now with restore inhibit set
> for all copied context. Where do we clear it?

It's one of those magic bits that only affect the next context-switch.
When the context is saved again, the bit it cleared, so the default
state we inherit doesn't have that bit set.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Record the default hw state after reset upon load

[Ville Syrjälä]

On Thu, Nov 02, 2017 at 12:42:24PM +0000, Chris Wilson wrote:
> Take a copy of the HW state after a reset upon module loading by
> executing a context switch from a blank context to the kernel context,
> thus saving the default hw state over the blank context image.
> We can then use the default hw state to initialise any future context,
> ensuring that each starts with the default view of hw state.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c    |  2 -
>  drivers/gpu/drm/i915/i915_debugfs.c     |  1 -
>  drivers/gpu/drm/i915/i915_gem.c         | 69 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_context.c | 50 +++---------------------
>  drivers/gpu/drm/i915/i915_gem_context.h |  4 +-
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
>  drivers/gpu/drm/i915/intel_lrc.c        | 35 ++++++++++-------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++++++++++++++-------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>  9 files changed, 137 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index f6ded475bb2c..42cc61230ca7 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -723,8 +723,6 @@ int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
>  	if (IS_ERR(vgpu->shadow_ctx))
>  		return PTR_ERR(vgpu->shadow_ctx);
>  
> -	vgpu->shadow_ctx->engine[RCS].initialised = true;
> -
>  	bitmap_zero(vgpu->shadow_ctx_desc_updated, I915_NUM_ENGINES);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 39883cd915db..cfcef1899da8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1974,7 +1974,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
>  			struct intel_context *ce = &ctx->engine[engine->id];
>  
>  			seq_printf(m, "%s: ", engine->name);
> -			seq_putc(m, ce->initialised ? 'I' : 'i');
>  			if (ce->state)
>  				describe_obj(m, ce->state->obj);
>  			if (ce->ring)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e36a3a840552..ed4b1708fec5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4967,6 +4967,71 @@ bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
>  	return true;
>  }
>  
> +static int __intel_engines_record_defaults(struct drm_i915_private *i915)
> +{
> +	struct i915_gem_context *ctx;
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +	int err;
> +
> +	/*
> +	 * As we reset the gpu during very early sanitisation, the current
> +	 * register state on the GPU should reflect its defaults values.
> +	 * We load a context onto the hw (with restore-inhibit), then switch
> +	 * over to a second context to save that default register state. We
> +	 * can then prime every new context with that state so they all start
> +	 * from defaults.
> +	 */
> +
> +	ctx = i915_gem_context_create_kernel(i915, 0);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	for_each_engine(engine, i915, id) {
> +		struct drm_i915_gem_request *rq;
> +
> +		rq = i915_gem_request_alloc(engine, ctx);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			goto out_ctx;
> +		}
> +
> +		err = i915_switch_context(rq);
> +		if (engine->init_context)
> +			err = engine->init_context(rq);
> +
> +		__i915_add_request(rq, true);
> +		if (err)
> +			goto out_ctx;
> +	}
> +
> +	err = i915_gem_switch_to_kernel_context(i915);
> +	if (err)
> +		goto out_ctx;
> +
> +	err = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
> +	if (err)
> +		goto out_ctx;
> +
> +	for_each_engine(engine, i915, id) {
> +		if (!ctx->engine[id].state)
> +			continue;
> +
> +		engine->default_state =
> +			i915_gem_object_get(ctx->engine[id].state->obj);
> +
> +		err = i915_gem_object_set_to_cpu_domain(engine->default_state,
> +							false);
> +		if (err)
> +			goto out_ctx;

Should we unmap the default context from the GTT here to make sure
the GPU never gets a chance to clobber it?

I also had the notion that context might save a copy of its CCID, and
that we'd potentially have to adjust it in every new context image. But
now that I look at the docs it seems pre-HSW CCID was part of the
runlist part of the context which we don't use, and on HSW it seems to
be saved in the power context. And I presume it must also be part of
the power context on pre-HSW in ringbuffer mode, otherwise it would get
lost when entering rc6.

And indeed not having CCID in the context makes perfect sense since
MI_SET_CONTEXT provides the new value. Thus also getting it from
the context itself would be redundant, and potentially dangerous if
the context has moved to another location within the GTT since the
last time it was used.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Record the default hw state after reset upon load

[Joonas Lahtinen]

On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> Take a copy of the HW state after a reset upon module loading by
> executing a context switch from a blank context to the kernel context,
> thus saving the default hw state over the blank context image.
> We can then use the default hw state to initialise any future context,
> ensuring that each starts with the default view of hw state.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -795,11 +770,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>  			return ret;
>  	}
>  
> -	if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
> -		/* NB: If we inhibit the restore, the context is not allowed to
> -		 * die because future work may end up depending on valid address
> -		 * space. This means we must enforce that a page table load
> -		 * occur when this occurs. */

Maybe add a comment explaining that we never ever assume execution on
the kernel context, so we don't care about the register state.

> +	if (i915_gem_context_is_kernel(to))
>  		hw_flags = MI_RESTORE_INHIBIT;
>  	else if (ppgtt && intel_engine_flag(engine) & ppgtt->pd_dirty_rings)
>  		hw_flags = MI_FORCE_RESTORE;

<SNIP>

> @@ -2193,11 +2184,28 @@ populate_lr_context(struct i915_gem_context *ctx,
>  	}
>  	ctx_obj->mm.dirty = true;
>  
> +	if (engine->default_state) {
> +		void *defaults;
> +
> +		defaults = i915_gem_object_pin_map(engine->default_state,
> +						   I915_MAP_WB);
> +		if (IS_ERR(defaults))
> +			return PTR_ERR(defaults);
> +
> +		memcpy(vaddr + LRC_HEADER_PAGES * PAGE_SIZE,
> +		       defaults + LRC_HEADER_PAGES * PAGE_SIZE,

Could use somethingsomething_offset variable.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Move GT powersaving init to i915_gem_init()

[Sagar Arun Kamble]

On 11/2/2017 6:12 PM, Chris Wilson wrote:
> GT powersaving is tightly coupled to the request infrastructure. To
> avoid complications with the order of initialisation in the next patch
> (where we want to send requests to hw during GEM init) move the
> powersaving initialisation into the purview of i915_gem_init().
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem.c      | 7 ++++++-
>   drivers/gpu/drm/i915/intel_display.c | 2 --
>   drivers/gpu/drm/i915/intel_pm.c      | 2 --
>   3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9470ba0c1930..e36a3a840552 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5017,6 +5017,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>   		goto out_unlock;
>   
>   	ret = i915_gem_init_hw(dev_priv);
> +	if (ret)
> +		goto out_unlock;
> +
> +	intel_init_gt_powersave(dev_priv);
Can this be moved before gem_init_hw. That way SLPC can get the initial 
platform RP configuration during uc_init.
> +
> +out_unlock:
>   	if (ret == -EIO) {
>   		/* Allow engine initialisation to fail by marking the GPU as
>   		 * wedged. But we only want to do this where the GPU is angry,
> @@ -5029,7 +5035,6 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>   		ret = 0;
>   	}
>   
> -out_unlock:
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   	mutex_unlock(&dev_priv->drm.struct_mutex);
>   
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 737de251d0f8..c3bf87c2036c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15174,8 +15174,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = to_i915(dev);
>   
> -	intel_init_gt_powersave(dev_priv);
> -
>   	intel_setup_overlay(dev_priv);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 07118c0b69d3..6e1358d4e764 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7900,7 +7900,6 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>   		intel_runtime_pm_get(dev_priv);
>   	}
>   
> -	mutex_lock(&dev_priv->drm.struct_mutex);
>   	mutex_lock(&dev_priv->pcu_lock);
>   
>   	/* Initialize RPS limits (for userspace) */
> @@ -7942,7 +7941,6 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>   	rps->boost_freq = rps->max_freq;
>   
>   	mutex_unlock(&dev_priv->pcu_lock);
> -	mutex_unlock(&dev_priv->drm.struct_mutex);
>   
>   	intel_autoenable_gt_powersave(dev_priv);
>   }

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

Re: [PATCH 4/6] drm/i915: Record the default hw state after reset upon load

[Chris Wilson]

Quoting Ville Syrjälä (2017-11-02 14:23:00)
> On Thu, Nov 02, 2017 at 12:42:24PM +0000, Chris Wilson wrote:
> > Take a copy of the HW state after a reset upon module loading by
> > executing a context switch from a blank context to the kernel context,
> > thus saving the default hw state over the blank context image.
> > We can then use the default hw state to initialise any future context,
> > ensuring that each starts with the default view of hw state.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gvt/scheduler.c    |  2 -
> >  drivers/gpu/drm/i915/i915_debugfs.c     |  1 -
> >  drivers/gpu/drm/i915/i915_gem.c         | 69 +++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_gem_context.c | 50 +++---------------------
> >  drivers/gpu/drm/i915/i915_gem_context.h |  4 +-
> >  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
> >  drivers/gpu/drm/i915/intel_lrc.c        | 35 ++++++++++-------
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++++++++++++++-------
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
> >  9 files changed, 137 insertions(+), 75 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index f6ded475bb2c..42cc61230ca7 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -723,8 +723,6 @@ int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
> >       if (IS_ERR(vgpu->shadow_ctx))
> >               return PTR_ERR(vgpu->shadow_ctx);
> >  
> > -     vgpu->shadow_ctx->engine[RCS].initialised = true;
> > -
> >       bitmap_zero(vgpu->shadow_ctx_desc_updated, I915_NUM_ENGINES);
> >  
> >       return 0;
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 39883cd915db..cfcef1899da8 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1974,7 +1974,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
> >                       struct intel_context *ce = &ctx->engine[engine->id];
> >  
> >                       seq_printf(m, "%s: ", engine->name);
> > -                     seq_putc(m, ce->initialised ? 'I' : 'i');
> >                       if (ce->state)
> >                               describe_obj(m, ce->state->obj);
> >                       if (ce->ring)
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index e36a3a840552..ed4b1708fec5 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4967,6 +4967,71 @@ bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
> >       return true;
> >  }
> >  
> > +static int __intel_engines_record_defaults(struct drm_i915_private *i915)
> > +{
> > +     struct i915_gem_context *ctx;
> > +     struct intel_engine_cs *engine;
> > +     enum intel_engine_id id;
> > +     int err;
> > +
> > +     /*
> > +      * As we reset the gpu during very early sanitisation, the current
> > +      * register state on the GPU should reflect its defaults values.
> > +      * We load a context onto the hw (with restore-inhibit), then switch
> > +      * over to a second context to save that default register state. We
> > +      * can then prime every new context with that state so they all start
> > +      * from defaults.
> > +      */
> > +
> > +     ctx = i915_gem_context_create_kernel(i915, 0);
> > +     if (IS_ERR(ctx))
> > +             return PTR_ERR(ctx);
> > +
> > +     for_each_engine(engine, i915, id) {
> > +             struct drm_i915_gem_request *rq;
> > +
> > +             rq = i915_gem_request_alloc(engine, ctx);
> > +             if (IS_ERR(rq)) {
> > +                     err = PTR_ERR(rq);
> > +                     goto out_ctx;
> > +             }
> > +
> > +             err = i915_switch_context(rq);
> > +             if (engine->init_context)
> > +                     err = engine->init_context(rq);
> > +
> > +             __i915_add_request(rq, true);
> > +             if (err)
> > +                     goto out_ctx;
> > +     }
> > +
> > +     err = i915_gem_switch_to_kernel_context(i915);
> > +     if (err)
> > +             goto out_ctx;
> > +
> > +     err = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
> > +     if (err)
> > +             goto out_ctx;
> > +
> > +     for_each_engine(engine, i915, id) {
> > +             if (!ctx->engine[id].state)
> > +                     continue;
> > +
> > +             engine->default_state =
> > +                     i915_gem_object_get(ctx->engine[id].state->obj);
> > +
> > +             err = i915_gem_object_set_to_cpu_domain(engine->default_state,
> > +                                                     false);
> > +             if (err)
> > +                     goto out_ctx;
> 
> Should we unmap the default context from the GTT here to make sure
> the GPU never gets a chance to clobber it?

Ah. I was thinking it would be unmapped as part of its teardown of the
context at the end of the function. But no, since we keep the object
around, and closing the context doesn't actually call i915_vma_close()
but just reaps the object as soon as possible. Whilst this is the only
place stealing the logical state object from the context, there isn't
much point in placing the fixup logic inside context-close, and we might
as well apply it here.

> I also had the notion that context might save a copy of its CCID, and
> that we'd potentially have to adjust it in every new context image. But
> now that I look at the docs it seems pre-HSW CCID was part of the
> runlist part of the context which we don't use, and on HSW it seems to
> be saved in the power context. And I presume it must also be part of
> the power context on pre-HSW in ringbuffer mode, otherwise it would get
> lost when entering rc6.

The CCID being part of the context would be scary, since it is just its
ggtt address. That would mean we would have to reload each ctx back into
its old location each time...

> And indeed not having CCID in the context makes perfect sense since
> MI_SET_CONTEXT provides the new value. Thus also getting it from
> the context itself would be redundant, and potentially dangerous if
> the context has moved to another location within the GTT since the
> last time it was used.
 
Nods. What is that say about great minds? Fools never differ. ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Joonas Lahtinen]

On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> As we now record the default HW state and so only emit the "golden"
> renderstate once to prepare the HW, there is no advantage in keeping the
> renderstate batch around as it will never be used again.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Rodrigo Vivi]

On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
> On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> > As we now record the default HW state and so only emit the "golden"
> > renderstate once to prepare the HW, there is no advantage in keeping the
> > renderstate batch around as it will never be used again.

So, with this in place we really don't need that null context for CNL.
to fullfill all Mesa needs, right?!

> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Regards, Joonas
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Move GT powersaving init to i915_gem_init()

[Chris Wilson]

Quoting Sagar Arun Kamble (2017-11-02 14:35:17)
> 
> 
> On 11/2/2017 6:12 PM, Chris Wilson wrote:
> > GT powersaving is tightly coupled to the request infrastructure. To
> > avoid complications with the order of initialisation in the next patch
> > (where we want to send requests to hw during GEM init) move the
> > powersaving initialisation into the purview of i915_gem_init().
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_gem.c      | 7 ++++++-
> >   drivers/gpu/drm/i915/intel_display.c | 2 --
> >   drivers/gpu/drm/i915/intel_pm.c      | 2 --
> >   3 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 9470ba0c1930..e36a3a840552 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -5017,6 +5017,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
> >               goto out_unlock;
> >   
> >       ret = i915_gem_init_hw(dev_priv);
> > +     if (ret)
> > +             goto out_unlock;
> > +
> > +     intel_init_gt_powersave(dev_priv);
> Can this be moved before gem_init_hw. That way SLPC can get the initial 
> platform RP configuration during uc_init.

Not at this point in the series, I would argue. Once we remove
intel_autoenable_gt_powersave(), it looks free to be moved before
i915_gem_init().

Or we split out the autoenable to here (or just after) and then remove
it. Or you just move init_gt_powersave() a bit earlier in a jiffie.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Joonas Lahtinen]

On Thu, 2017-11-02 at 07:54 -0700, Rodrigo Vivi wrote:
> On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
> > On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> > > As we now record the default HW state and so only emit the "golden"
> > > renderstate once to prepare the HW, there is no advantage in keeping the
> > > renderstate batch around as it will never be used again.
> 
> So, with this in place we really don't need that null context for CNL.
> to fullfill all Mesa needs, right?!

Separate issue, this only fixes isolation. This patch just releases it
from memory after it's been applied to the default context states to be
stored.

But yes, we also decided not to have null context for new platforms.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Move GT powersaving init to i915_gem_init()

[Sagar Arun Kamble]

On 11/2/2017 8:25 PM, Chris Wilson wrote:
> Quoting Sagar Arun Kamble (2017-11-02 14:35:17)
>>
>> On 11/2/2017 6:12 PM, Chris Wilson wrote:
>>> GT powersaving is tightly coupled to the request infrastructure. To
>>> avoid complications with the order of initialisation in the next patch
>>> (where we want to send requests to hw during GEM init) move the
>>> powersaving initialisation into the purview of i915_gem_init().
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    drivers/gpu/drm/i915/i915_gem.c      | 7 ++++++-
>>>    drivers/gpu/drm/i915/intel_display.c | 2 --
>>>    drivers/gpu/drm/i915/intel_pm.c      | 2 --
>>>    3 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>> index 9470ba0c1930..e36a3a840552 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -5017,6 +5017,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>>>                goto out_unlock;
>>>    
>>>        ret = i915_gem_init_hw(dev_priv);
>>> +     if (ret)
>>> +             goto out_unlock;
>>> +
>>> +     intel_init_gt_powersave(dev_priv);
>> Can this be moved before gem_init_hw. That way SLPC can get the initial
>> platform RP configuration during uc_init.
> Not at this point in the series, I would argue. Once we remove
> intel_autoenable_gt_powersave(), it looks free to be moved before
> i915_gem_init().
>
> Or we split out the autoenable to here (or just after) and then remove
> it. Or you just move init_gt_powersave() a bit earlier in a jiffie.
> -Chris
Ah. right. Will move later.

Thanks,
Sagar

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

Re: [PATCH 1/6] drm/i915: Force the switch to the i915->kernel_context

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In the next few patches, we will have a hard requirement that we emit a
> context-switch to the perma-pinned i915->kernel_context (so that we can
> save the HW state using that context-switch). As the first context
> itself may be classed as a kernel context, we want to be explicit in our
> comparison. For an extra-layer of finesse, we can check the last
> unretired context on the engine; as well as the last retired context
> when idle.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index ddbe5c9bf45a..c15e288bed02 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1587,8 +1587,20 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv)
>  
>  bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine)
>  {
> -	return (!engine->last_retired_context ||
> -		i915_gem_context_is_kernel(engine->last_retired_context));
> +	const struct i915_gem_context *kctx = engine->i915->kernel_context;
> +	struct drm_i915_gem_request *rq;
> +
> +	lockdep_assert_held(&engine->i915->drm.struct_mutex);
> +
> +	/* An unretired request? */
> +	rq = __i915_gem_active_peek(&engine->timeline->last_request);
> +	if (rq)
> +		return rq->ctx == kctx;
> +
> +	if (!engine->last_retired_context)
> +		return true;
> +
> +	return engine->last_retired_context == kctx;


Conside that we would have intel_engine_loaded_context(engine)
{
        rq = __i915_gem_active_peek(&engine->timeline->last_request);
        if (rq)
           return rq->ctx;

        if (engine->last_retired_context)
                return engine->last_retired_context;

        return NULL;        
}

and then have simple intel_engine_has_kernel_context_loaded()

Also further on the patch series we do switch to kernel context
on init, to grab the hw state. So would it be possible to completely
get rid of notion that !engine->last_retired_context means kernel
context? I would rather assert that we have always last retired
context after init and resume.

-Mika

>  }
>  
>  void intel_engines_reset_default_submission(struct drm_i915_private *i915)
> -- 
> 2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Force the switch to the i915->kernel_context

[Chris Wilson]

Quoting Mika Kuoppala (2017-11-02 15:40:50)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > In the next few patches, we will have a hard requirement that we emit a
> > context-switch to the perma-pinned i915->kernel_context (so that we can
> > save the HW state using that context-switch). As the first context
> > itself may be classed as a kernel context, we want to be explicit in our
> > comparison. For an extra-layer of finesse, we can check the last
> > unretired context on the engine; as well as the last retired context
> > when idle.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_engine_cs.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index ddbe5c9bf45a..c15e288bed02 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -1587,8 +1587,20 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv)
> >  
> >  bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine)
> >  {
> > -     return (!engine->last_retired_context ||
> > -             i915_gem_context_is_kernel(engine->last_retired_context));
> > +     const struct i915_gem_context *kctx = engine->i915->kernel_context;
> > +     struct drm_i915_gem_request *rq;
> > +
> > +     lockdep_assert_held(&engine->i915->drm.struct_mutex);
> > +
> > +     /* An unretired request? */
> > +     rq = __i915_gem_active_peek(&engine->timeline->last_request);
> > +     if (rq)
> > +             return rq->ctx == kctx;
> > +
> > +     if (!engine->last_retired_context)
> > +             return true;
> > +
> > +     return engine->last_retired_context == kctx;
> 
> 
> Conside that we would have intel_engine_loaded_context(engine)
> {
>         rq = __i915_gem_active_peek(&engine->timeline->last_request);
>         if (rq)
>            return rq->ctx;
> 
>         if (engine->last_retired_context)
>                 return engine->last_retired_context;
> 
>         return NULL;        
> }
> 
> and then have simple intel_engine_has_kernel_context_loaded()

Could, just needs refinement on the name first. I'll keep one clumsy
name for the moment ;)
 
> Also further on the patch series we do switch to kernel context
> on init, to grab the hw state. So would it be possible to completely
> get rid of notion that !engine->last_retired_context means kernel
> context? I would rather assert that we have always last retired
> context after init and resume.

Yup, that seems possible.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Oscar Mateo]

On 11/02/2017 07:56 AM, Joonas Lahtinen wrote:
> On Thu, 2017-11-02 at 07:54 -0700, Rodrigo Vivi wrote:
>> On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
>>> On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
>>>> As we now record the default HW state and so only emit the "golden"
>>>> renderstate once to prepare the HW, there is no advantage in keeping the
>>>> renderstate batch around as it will never be used again.
>> So, with this in place we really don't need that null context for CNL.
>> to fullfill all Mesa needs, right?!
> Separate issue, this only fixes isolation. This patch just releases it
> from memory after it's been applied to the default context states to be
> stored.
>
> But yes, we also decided not to have null context for new platforms.

At last until, two years from now, we find out that there is a very 
subtle reason why we need it :)

> Regards, Joonas

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

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Rodrigo Vivi]

On Thu, Nov 02, 2017 at 10:36:26PM +0000, Oscar Mateo wrote:
> 
> 
> On 11/02/2017 07:56 AM, Joonas Lahtinen wrote:
> > On Thu, 2017-11-02 at 07:54 -0700, Rodrigo Vivi wrote:
> > > On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
> > > > On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> > > > > As we now record the default HW state and so only emit the "golden"
> > > > > renderstate once to prepare the HW, there is no advantage in keeping the
> > > > > renderstate batch around as it will never be used again.
> > > So, with this in place we really don't need that null context for CNL.
> > > to fullfill all Mesa needs, right?!
> > Separate issue, this only fixes isolation. This patch just releases it
> > from memory after it's been applied to the default context states to be
> > stored.
> > 
> > But yes, we also decided not to have null context for new platforms.
> 
> At last until, two years from now, we find out that there is a very subtle
> reason why we need it :)

:( - Yeap, for me just to be on the safe side and start with a clean context
would be a good reason already...

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

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Chris Wilson]

Quoting Rodrigo Vivi (2017-11-02 23:34:52)
> On Thu, Nov 02, 2017 at 10:36:26PM +0000, Oscar Mateo wrote:
> > 
> > 
> > On 11/02/2017 07:56 AM, Joonas Lahtinen wrote:
> > > On Thu, 2017-11-02 at 07:54 -0700, Rodrigo Vivi wrote:
> > > > On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
> > > > > On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> > > > > > As we now record the default HW state and so only emit the "golden"
> > > > > > renderstate once to prepare the HW, there is no advantage in keeping the
> > > > > > renderstate batch around as it will never be used again.
> > > > So, with this in place we really don't need that null context for CNL.
> > > > to fullfill all Mesa needs, right?!
> > > Separate issue, this only fixes isolation. This patch just releases it
> > > from memory after it's been applied to the default context states to be
> > > stored.
> > > 
> > > But yes, we also decided not to have null context for new platforms.
> > 
> > At last until, two years from now, we find out that there is a very subtle
> > reason why we need it :)
> 
> :( - Yeap, for me just to be on the safe side and start with a clean context
> would be a good reason already...

That is not what golden renderstate does.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Joonas Lahtinen]

On Thu, 2017-11-02 at 16:34 -0700, Rodrigo Vivi wrote:
> On Thu, Nov 02, 2017 at 10:36:26PM +0000, Oscar Mateo wrote:
> > 
> > 
> > On 11/02/2017 07:56 AM, Joonas Lahtinen wrote:
> > > On Thu, 2017-11-02 at 07:54 -0700, Rodrigo Vivi wrote:
> > > > On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
> > > > > On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> > > > > > As we now record the default HW state and so only emit the "golden"
> > > > > > renderstate once to prepare the HW, there is no advantage in keeping the
> > > > > > renderstate batch around as it will never be used again.
> > > > 
> > > > So, with this in place we really don't need that null context for CNL.
> > > > to fullfill all Mesa needs, right?!
> > > 
> > > Separate issue, this only fixes isolation. This patch just releases it
> > > from memory after it's been applied to the default context states to be
> > > stored.
> > > 
> > > But yes, we also decided not to have null context for new platforms.
> > 
> > At last until, two years from now, we find out that there is a very subtle
> > reason why we need it :)
> 
> :( - Yeap, for me just to be on the safe side and start with a clean context
> would be a good reason already...

Indeed, we are starting with a clean context now. If the HW division
messes a new design so badly that it won't be able to execute without
null render state, we shall reiterate :) But I have high hopes that we
don't need to.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Stop caching the "golden" renderstate

[Rodrigo Vivi]

On Fri, Nov 03, 2017 at 08:39:21AM +0000, Joonas Lahtinen wrote:
> On Thu, 2017-11-02 at 16:34 -0700, Rodrigo Vivi wrote:
> > On Thu, Nov 02, 2017 at 10:36:26PM +0000, Oscar Mateo wrote:
> > > 
> > > 
> > > On 11/02/2017 07:56 AM, Joonas Lahtinen wrote:
> > > > On Thu, 2017-11-02 at 07:54 -0700, Rodrigo Vivi wrote:
> > > > > On Thu, Nov 02, 2017 at 02:43:16PM +0000, Joonas Lahtinen wrote:
> > > > > > On Thu, 2017-11-02 at 12:42 +0000, Chris Wilson wrote:
> > > > > > > As we now record the default HW state and so only emit the "golden"
> > > > > > > renderstate once to prepare the HW, there is no advantage in keeping the
> > > > > > > renderstate batch around as it will never be used again.
> > > > > 
> > > > > So, with this in place we really don't need that null context for CNL.
> > > > > to fullfill all Mesa needs, right?!
> > > > 
> > > > Separate issue, this only fixes isolation. This patch just releases it
> > > > from memory after it's been applied to the default context states to be
> > > > stored.
> > > > 
> > > > But yes, we also decided not to have null context for new platforms.
> > > 
> > > At last until, two years from now, we find out that there is a very subtle
> > > reason why we need it :)
> > 
> > :( - Yeap, for me just to be on the safe side and start with a clean context
> > would be a good reason already...
> 
> Indeed, we are starting with a clean context now. If the HW division
> messes a new design so badly that it won't be able to execute without
> null render state, we shall reiterate :) But I have high hopes that we
> don't need to.

Well, yeah!
That makes sense and also it makes the platform enabling easier.
Thanks for the work and explanations.

> 
> Regards, Joonas
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx