[PATCH 070/131] drm/i915: Use VMA as the primary object for context state

Chris Wilson chris at chris-wilson.co.uk
Sat Aug 6 07:36:37 UTC 2016


When working with contexts, we most frequently want the GGTT VMA for the
context state, first and foremost. Since the object is available via the
VMA, we need only then store the VMA.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 12 ++---
 drivers/gpu/drm/i915/i915_drv.h            |  3 +-
 drivers/gpu/drm/i915/i915_gem_context.c    | 75 ++++++++++++++++++------------
 drivers/gpu/drm/i915/i915_gpu_error.c      |  2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  8 ++--
 drivers/gpu/drm/i915/intel_lrc.c           | 59 +++++++++++------------
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 14 ++----
 7 files changed, 89 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ab41ac50d188..498cdd2605fc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -393,7 +393,7 @@ static int per_file_ctx_stats(int id, void *ptr, void *data)
 
 	for (n = 0; n < ARRAY_SIZE(ctx->engine); n++) {
 		if (ctx->engine[n].state)
-			per_file_stats(0, ctx->engine[n].state, data);
+			per_file_stats(0, ctx->engine[n].state->obj, data);
 		if (ctx->engine[n].ring)
 			per_file_stats(0, ctx->engine[n].ring->obj, data);
 	}
@@ -2073,7 +2073,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
 			seq_printf(m, "%s: ", engine->name);
 			seq_putc(m, ce->initialised ? 'I' : 'i');
 			if (ce->state)
-				describe_obj(m, ce->state);
+				describe_obj(m, ce->state->obj);
 			if (ce->ring)
 				describe_ctx_ring(m, ce->ring);
 			seq_putc(m, '\n');
@@ -2096,8 +2096,7 @@ static void i915_dump_lrc_obj(struct seq_file *m,
 			      struct i915_gem_context *ctx,
 			      struct intel_engine_cs *engine)
 {
-	struct drm_i915_gem_object *obj = ctx->engine[engine->id].state;
-	struct i915_vma *vma = ctx->engine[engine->id].vma;
+	struct i915_vma *vma = ctx->engine[engine->id].state;
 	struct page *page;
 	int j;
 
@@ -2110,14 +2109,15 @@ static void i915_dump_lrc_obj(struct seq_file *m,
 			   lower_32_bits(vma->node.start));
 	}
 
-	if (i915_gem_object_get_pages(obj)) {
+	if (i915_gem_object_get_pages(vma->obj)) {
 		seq_puts(m, "\tFailed to get pages for context object\n\n");
 		return;
 	}
 
-	page = i915_gem_object_get_page(obj, LRC_STATE_PN);
+	page = i915_gem_object_get_page(vma->obj, LRC_STATE_PN);
 	if (page) {
 		u32 *reg_state = kmap_atomic(page);
+
 		for (j = 0; j < 0x600 / sizeof(u32) / 4; j += 4) {
 			seq_printf(m,
 				   "\t[0x%08x] 0x%08x 0x%08x 0x%08x 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7b652d10a60..6abd7c34e003 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -907,8 +907,7 @@ struct i915_gem_context {
 	u32 ggtt_alignment;
 
 	struct intel_context {
-		struct drm_i915_gem_object *state;
-		struct i915_vma *vma;
+		struct i915_vma *state;
 		struct intel_ring *ring;
 		uint32_t *lrc_reg_state;
 		u64 lrc_desc;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index f735992b11da..e530176edacf 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -218,7 +218,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
 		if (ce->ring)
 			intel_ring_free(ce->ring);
 
-		__i915_gem_object_release_unless_active(ce->state);
+		__i915_gem_object_release_unless_active(ce->state->obj);
 	}
 
 	put_pid(ctx->pid);
@@ -355,13 +355,26 @@ __create_hw_context(struct drm_device *dev,
 	INIT_WORK(&ctx->vma.resize, resize_vma_ht);
 
 	if (dev_priv->hw_context_size) {
-		struct drm_i915_gem_object *obj =
-				i915_gem_alloc_context_obj(dev, dev_priv->hw_context_size);
+		struct drm_i915_gem_object *obj;
+		struct i915_vma *vma;
+
+		obj = i915_gem_alloc_context_obj(dev,
+						 dev_priv->hw_context_size);
 		if (IS_ERR(obj)) {
 			ret = PTR_ERR(obj);
 			goto err_out;
 		}
-		ctx->engine[RCS].state = obj;
+
+		vma = i915_gem_obj_lookup_or_create_vma(obj,
+							&dev_priv->ggtt.base,
+							NULL);
+		if (IS_ERR(vma)) {
+			i915_gem_object_put(obj);
+			ret = PTR_ERR(vma);
+			goto err_out;
+		}
+
+		ctx->engine[RCS].state = vma;
 	}
 
 	/* Default context will never have a file_priv */
@@ -475,8 +488,8 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 	} else {
 		struct intel_context *ce = &ctx->engine[engine->id];
 
-		if (ce->vma)
-			i915_vma_unpin(ce->vma);
+		if (ce->state)
+			i915_vma_unpin(ce->state);
 
 		i915_gem_context_put(ctx);
 	}
@@ -697,7 +710,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
 
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_emit(ring, MI_SET_CONTEXT);
-	intel_ring_emit(ring, req->ctx->engine[RCS].vma->node.start | flags);
+	intel_ring_emit(ring, req->ctx->engine[RCS].state->node.start | flags);
 	/*
 	 * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
 	 * WaMiSetContext_Hang:snb,ivb,vlv
@@ -825,13 +838,26 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
 	return false;
 }
 
+static void flush_cpu_writes(struct drm_i915_gem_object *obj)
+{
+	if (obj->base.write_domain == 0)
+		return;
+
+	if (obj->base.write_domain & I915_GEM_DOMAIN_CPU) {
+		if (i915_gem_clflush_object(obj, false))
+			i915_gem_chipset_flush(to_i915(obj->base.dev));
+	}
+
+	wmb();
+	obj->base.write_domain = 0;
+}
+
 static int do_rcs_switch(struct drm_i915_gem_request *req)
 {
 	struct i915_gem_context *to = req->ctx;
 	struct intel_engine_cs *engine = req->engine;
 	struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
 	struct i915_gem_context *from;
-	struct i915_vma *vma;
 	u32 hw_flags;
 	int ret, i;
 
@@ -839,17 +865,11 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 		return 0;
 
 	/* Trying to pin first makes error handling easier. */
-	vma = i915_gem_object_ggtt_pin(to->engine[RCS].state, NULL, 0,
-				       to->ggtt_alignment, 0);
-	if (IS_ERR(vma))
-		return PTR_ERR(vma);
-
-	to->engine[RCS].vma = vma;
-
-	if (WARN_ON(!(vma->flags & I915_VMA_GLOBAL_BIND))) {
-		ret = -ENODEV;
-		goto unpin_vma;
-	}
+	ret = i915_vma_pin(to->engine[RCS].state,
+			   0, to->ggtt_alignment,
+			   PIN_GLOBAL);
+	if (ret)
+		return ret;
 
 	/*
 	 * Pin can switch back to the default context if we end up calling into
@@ -861,16 +881,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 	from = engine->last_context;
 
 	/*
-	 * Clear this page out of any CPU caches for coherent swap-in/out. Note
-	 * that thanks to write = false in this call and us not setting any gpu
-	 * write domains when putting a context object onto the active list
-	 * (when switching away from it), this won't block.
-	 *
-	 * XXX: We need a real interface to do this instead of trickery.
+	 * Clear this page out of any CPU caches for coherent swap-in/out.
 	 */
-	ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state, false);
-	if (ret)
-		goto unpin_vma;
+	flush_cpu_writes(to->engine[RCS].state->obj);
 
 	if (needs_pd_load_pre(ppgtt, engine, to)) {
 		/* Older GENs and non render rings still want the load first,
@@ -914,9 +927,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 		 * able to defer doing this until we know the object would be
 		 * swapped, but there is no way to do that yet.
 		 */
-		i915_vma_move_to_active(from->engine[RCS].vma, req, 0);
+		i915_vma_move_to_active(from->engine[RCS].state, req, 0);
 		/* obj is kept alive until the next request by its active ref */
-		i915_vma_unpin(from->engine[RCS].vma);
+		i915_vma_unpin(from->engine[RCS].state);
 
 		i915_gem_context_put(from);
 	}
@@ -963,7 +976,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 	return 0;
 
 unpin_vma:
-	i915_vma_unpin(vma);
+	i915_vma_unpin(to->engine[RCS].state);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 11882f58c8d5..8553d3227227 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1131,7 +1131,7 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
 
 			ee->ctx =
 				i915_error_object_create(dev_priv,
-							 request->ctx->engine[i].vma);
+							 request->ctx->engine[i].state);
 
 			pid = request->ctx->pid;
 			if (pid) {
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index c74ebf5aa0e9..e70d50fede55 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -351,13 +351,13 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
 		 * for now who owns a GuC client. But for future owner of GuC
 		 * client, need to make sure lrc is pinned prior to enter here.
 		 */
-		if (!ce->vma)
+		if (!ce->state)
 			break;	/* XXX: continue? */
 
 		lrc->context_desc = lower_32_bits(ce->lrc_desc);
 
 		/* The state page is after PPHWSP */
-		gfx_addr = ce->vma->node.start;
+		gfx_addr = ce->state->node.start;
 		lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
 		lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
 				(engine->guc_id << GUC_ELC_ENGINE_OFFSET);
@@ -1052,7 +1052,7 @@ int intel_guc_suspend(struct drm_device *dev)
 	/* any value greater than GUC_POWER_D0 */
 	data[1] = GUC_POWER_D1;
 	/* first page is shared data with GuC */
-	data[2] = ctx->engine[RCS].vma->node.start;
+	data[2] = ctx->engine[RCS].state->node.start;
 
 	return host2guc_action(guc, data, ARRAY_SIZE(data));
 }
@@ -1077,7 +1077,7 @@ int intel_guc_resume(struct drm_device *dev)
 	data[0] = HOST2GUC_ACTION_EXIT_S_STATE;
 	data[1] = GUC_POWER_D0;
 	/* first page is shared data with GuC */
-	data[2] = ctx->engine[RCS].vma->node.start;
+	data[2] = ctx->engine[RCS].state->node.start;
 
 	return host2guc_action(guc, data, ARRAY_SIZE(data));
 }
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 14704c53d68a..aebf6c7599ed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -315,7 +315,7 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 
 	desc = ctx->desc_template;				/* bits  3-4  */
 	desc |= engine->ctx_desc_template;			/* bits  0-11 */
-	desc |= ce->vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
+	desc |= ce->state->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
 								/* bits 12-31 */
 	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
 
@@ -763,9 +763,7 @@ void intel_execlists_cancel_requests(struct intel_engine_cs *engine)
 static int intel_lr_context_pin(struct i915_gem_context *ctx,
 				struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = ctx->i915;
 	struct intel_context *ce = &ctx->engine[engine->id];
-	struct i915_vma *vma;
 	void *vaddr;
 	u32 *lrc_reg_state;
 	int ret;
@@ -775,15 +773,12 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (ce->pin_count++)
 		return 0;
 
-	vma = i915_gem_object_ggtt_pin(ce->state, NULL,
-				       0, GEN8_LR_CONTEXT_ALIGN,
-				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-	if (IS_ERR(vma)) {
-		ret = PTR_ERR(vma);
+	ret = i915_vma_pin(ce->state, 0, GEN8_LR_CONTEXT_ALIGN,
+			   PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL);
+	if (ret)
 		goto err;
-	}
 
-	vaddr = i915_gem_object_pin_map(vma->obj);
+	vaddr = i915_gem_object_pin_map(ce->state->obj);
 	if (IS_ERR(vaddr)) {
 		ret = PTR_ERR(vaddr);
 		goto unpin_vma;
@@ -795,25 +790,25 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (ret)
 		goto unpin_map;
 
-	ce->vma = vma;
-
 	intel_lr_context_descriptor_update(ctx, engine);
 
 	lrc_reg_state[CTX_RING_BUFFER_START+1] = ce->ring->vma->node.start;
 	ce->lrc_reg_state = lrc_reg_state;
-	i915_gem_object_set_dirty(vma->obj);
+	i915_gem_object_set_dirty(ce->state->obj);
 
 	/* Invalidate GuC TLB. */
-	if (i915.enable_guc_submission)
+	if (i915.enable_guc_submission) {
+		struct drm_i915_private *dev_priv = ctx->i915;
 		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+	}
 
 	i915_gem_context_get(ctx);
 	return 0;
 
 unpin_map:
-	i915_gem_object_unpin_map(vma->obj);
+	i915_gem_object_unpin_map(ce->state->obj);
 unpin_vma:
-	__i915_vma_unpin(vma);
+	__i915_vma_unpin(ce->state);
 err:
 	ce->pin_count = 0;
 	return ret;
@@ -832,12 +827,8 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
 
 	intel_ring_unpin(ce->ring);
 
-	i915_gem_object_unpin_map(ce->state);
-	i915_vma_unpin(ce->vma);
-
-	ce->vma = NULL;
-	ce->lrc_desc = 0;
-	ce->lrc_reg_state = NULL;
+	i915_gem_object_unpin_map(ce->state->obj);
+	i915_vma_unpin(ce->state);
 
 	i915_gem_context_put(ctx);
 }
@@ -1820,7 +1811,7 @@ logical_ring_init(struct intel_engine_cs *engine)
 	}
 
 	/* And setup the hardware status page. */
-	ret = lrc_setup_hws(engine, dctx->engine[engine->id].vma);
+	ret = lrc_setup_hws(engine, dctx->engine[engine->id].state);
 	if (ret) {
 		DRM_ERROR("Failed to set up hws %s: %d\n", engine->name, ret);
 		goto error;
@@ -2131,6 +2122,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 {
 	struct drm_i915_gem_object *ctx_obj;
 	struct intel_context *ce = &ctx->engine[engine->id];
+	struct i915_vma *vma;
 	uint32_t context_size;
 	struct intel_ring *ring;
 	int ret;
@@ -2148,6 +2140,14 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 		return PTR_ERR(ctx_obj);
 	}
 
+	vma = i915_gem_obj_lookup_or_create_vma(ctx_obj,
+						&engine->i915->ggtt.base,
+						NULL);
+	if (IS_ERR(vma)) {
+		ret = PTR_ERR(vma);
+		goto error_deref_obj;
+	}
+
 	ring = intel_engine_create_ring(engine, ctx->ring_size);
 	if (IS_ERR(ring)) {
 		ret = PTR_ERR(ring);
@@ -2161,7 +2161,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 	}
 
 	ce->ring = ring;
-	ce->state = ctx_obj;
+	ce->state = vma;
 	ce->initialised = engine->init_context == NULL;
 
 	return 0;
@@ -2170,8 +2170,6 @@ error_ring_free:
 	intel_ring_free(ring);
 error_deref_obj:
 	i915_gem_object_put(ctx_obj);
-	ce->ring = NULL;
-	ce->state = NULL;
 	return ret;
 }
 
@@ -2182,14 +2180,13 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
 
 	for_each_engine(engine, dev_priv) {
 		struct intel_context *ce = &ctx->engine[engine->id];
-		struct drm_i915_gem_object *ctx_obj = ce->state;
 		void *vaddr;
 		uint32_t *reg_state;
 
-		if (!ctx_obj)
+		if (!ce->state)
 			continue;
 
-		vaddr = i915_gem_object_pin_map(ctx_obj);
+		vaddr = i915_gem_object_pin_map(ce->state->obj);
 		if (WARN_ON(IS_ERR(vaddr)))
 			continue;
 
@@ -2198,8 +2195,8 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
 		reg_state[CTX_RING_HEAD+1] = 0;
 		reg_state[CTX_RING_TAIL+1] = 0;
 
-		i915_gem_object_set_dirty(ce->state);
-		i915_gem_object_unpin_map(ctx_obj);
+		i915_gem_object_set_dirty(ce->state->obj);
+		i915_gem_object_unpin_map(ce->state->obj);
 
 		ce->ring->head = 0;
 		ce->ring->tail = 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 17dae2ceb692..130ba99be9d1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2098,14 +2098,10 @@ static int intel_ring_context_pin(struct i915_gem_context *ctx,
 		return 0;
 
 	if (ce->state) {
-		struct i915_vma *vma;
-
-		vma = i915_gem_object_ggtt_pin(ce->state, NULL, 0,
-					       ctx->ggtt_alignment, PIN_HIGH);
-		if (vma)
+		ret = i915_vma_pin(ce->state, 0, ctx->ggtt_alignment,
+				   PIN_GLOBAL | PIN_HIGH);
+		if (ret)
 			goto error;
-
-		ce->vma = vma;
 	}
 
 	/* The kernel context is only used as a placeholder for flushing the
@@ -2136,8 +2132,8 @@ static void intel_ring_context_unpin(struct i915_gem_context *ctx,
 	if (--ce->pin_count)
 		return;
 
-	if (ce->vma)
-		i915_vma_unpin(ce->vma);
+	if (ce->state)
+		i915_vma_unpin(ce->state);
 
 	i915_gem_context_put(ctx);
 }
-- 
2.8.1



More information about the Intel-gfx-trybot mailing list