public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Split alloc from init for lrc
@ 2015-08-18 14:23 Nick Hoath
  2015-08-18 14:31 ` Chris Wilson
  2015-08-28  8:12 ` shuang.he
  0 siblings, 2 replies; 17+ messages in thread
From: Nick Hoath @ 2015-08-18 14:23 UTC (permalink / raw)
  To: intel-gfx; +Cc: daniel.vetter

Extend init/init_hw split to context init.
   - Move context initialisation in to i915_gem_init_hw
   - Move one off initialisation for render ring to
        i915_gem_validate_context
   - Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources & initialisation
to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
    Left ->init_context int intel_lr_context_deferred_alloc (Daniel Vetter)
    Remove unnecessary init flag & ring type test. (Daniel Vetter)
    Improve commit message (Daniel Vetter)

Issue: VIZ-4798
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h            |   1 -
 drivers/gpu/drm/i915/i915_gem.c            |  12 +--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
 drivers/gpu/drm/i915/intel_lrc.c           | 147 ++++++++++++++---------------
 drivers/gpu/drm/i915/intel_lrc.h           |   4 +-
 5 files changed, 80 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f7fd519..844ccf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -880,7 +880,6 @@ struct intel_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	bool rcs_initialized;
 	struct {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..3ccef2d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4603,14 +4603,8 @@ int i915_gem_init_rings(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -4629,6 +4623,7 @@ i915_gem_init_hw(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *ring;
 	int ret, i, j;
+	struct drm_i915_gem_request *req;
 
 	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
 		return -EIO;
@@ -4680,9 +4675,12 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto out;
 	}
 
+	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
+	if (ret)
+		goto out;
+
 	/* Now it is safe to go back round and do everything else: */
 	for_each_ring(ring, dev_priv, i) {
-		struct drm_i915_gem_request *req;
 
 		WARN_ON(!ring->default_context);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 923a3c4..95f1a0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -994,6 +994,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 {
 	struct intel_context *ctx = NULL;
 	struct i915_ctx_hang_stats *hs;
+	int ret;
 
 	if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
 		return ERR_PTR(-EINVAL);
@@ -1009,7 +1010,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 	}
 
 	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
-		int ret = intel_lr_context_deferred_create(ctx, ring);
+		ret = intel_lr_context_deferred_alloc(ctx, ring);
 		if (ret) {
 			DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
 			return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 138964a..d0dc6b5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1426,11 +1426,31 @@ out:
 	return ret;
 }
 
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *default_ctx_obj)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+	/* The status page is offset 0 from the default context object
+	 * in LRC mode. */
+	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj);
+	ring->status_page.page_addr =
+			kmap(sg_page(default_ctx_obj->pages->sgl));
+	ring->status_page.obj = default_ctx_obj;
+
+	I915_WRITE(RING_HWS_PGA(ring->mmio_base),
+			(u32)ring->status_page.gfx_addr);
+	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
+}
+
 static int gen8_init_common_ring(struct intel_engine_cs *ring)
 {
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lrc_setup_hardware_status_page(ring,
+				ring->default_context->engine[ring->id].state);
+
 	I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
 	I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff);
 
@@ -1841,8 +1861,33 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	if (ret)
 		return ret;
 
-	ret = intel_lr_context_deferred_create(ring->default_context, ring);
+	ret = intel_lr_context_deferred_alloc(ring->default_context, ring);
+	if (ret)
+		return ret;
 
+	ret = i915_gem_obj_ggtt_pin(
+		ring->default_context->engine[ring->id].state,
+		GEN8_LR_CONTEXT_ALIGN, 0);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = intel_pin_and_map_ringbuffer_obj(dev,
+		ring->default_context->engine[ring->id].ringbuf);
+	if (ret) {
+		DRM_ERROR(
+			"Failed to pin and map ringbuffer %s: %d\n",
+			ring->name, ret);
+		goto error_unpin_ggtt;
+	}
+
+	return ret;
+
+error_unpin_ggtt:
+	i915_gem_object_ggtt_unpin(
+		ring->default_context->engine[ring->id].state);
 	return ret;
 }
 
@@ -2044,14 +2089,8 @@ int intel_logical_rings_init(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_logical_ring_cleanup(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_logical_ring_cleanup(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -2303,25 +2342,8 @@ static uint32_t get_lr_context_size(struct intel_engine_cs *ring)
 	return ret;
 }
 
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
-		struct drm_i915_gem_object *default_ctx_obj)
-{
-	struct drm_i915_private *dev_priv = ring->dev->dev_private;
-
-	/* The status page is offset 0 from the default context object
-	 * in LRC mode. */
-	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj);
-	ring->status_page.page_addr =
-			kmap(sg_page(default_ctx_obj->pages->sgl));
-	ring->status_page.obj = default_ctx_obj;
-
-	I915_WRITE(RING_HWS_PGA(ring->mmio_base),
-			(u32)ring->status_page.gfx_addr);
-	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
-}
-
 /**
- * intel_lr_context_deferred_create() - create the LRC specific bits of a context
+ * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context
  * @ctx: LR context to create.
  * @ring: engine to be used with the context.
  *
@@ -2333,10 +2355,10 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
  *
  * Return: non-zero on error.
  */
-int intel_lr_context_deferred_create(struct intel_context *ctx,
+
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
-	const bool is_global_default_ctx = (ctx == ring->default_context);
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
@@ -2354,22 +2376,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, 0);
-		if (ret) {
-			DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
-					ret);
-			drm_gem_object_unreference(&ctx_obj->base);
-			return ret;
-		}
-	}
-
 	ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL);
 	if (!ringbuf) {
 		DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n",
 				ring->name);
 		ret = -ENOMEM;
-		goto error_unpin_ctx;
+		goto error_deref_obj;
 	}
 
 	ringbuf->ring = ring;
@@ -2389,66 +2401,49 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 				ring->name, ret);
 			goto error_free_rbuf;
 		}
-
-		if (is_global_default_ctx) {
-			ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-			if (ret) {
-				DRM_ERROR(
-					"Failed to pin and map ringbuffer %s: %d\n",
-					ring->name, ret);
-				goto error_destroy_rbuf;
-			}
-		}
-
 	}
 
 	ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret);
-		goto error;
+		goto error_destroy_rbuf;
 	}
 
 	ctx->engine[ring->id].ringbuf = ringbuf;
 	ctx->engine[ring->id].state = ctx_obj;
 
-	if (ctx == ring->default_context)
-		lrc_setup_hardware_status_page(ring, ctx_obj);
-	else if (ring->id == RCS && !ctx->rcs_initialized) {
-		if (ring->init_context) {
-			struct drm_i915_gem_request *req;
+	if (ctx != ring->default_context && ring->init_context) {
+		struct drm_i915_gem_request *req;
 
-			ret = i915_gem_request_alloc(ring, ctx, &req);
-			if (ret)
-				return ret;
-
-			ret = ring->init_context(req);
-			if (ret) {
-				DRM_ERROR("ring init context: %d\n", ret);
-				i915_gem_request_cancel(req);
-				ctx->engine[ring->id].ringbuf = NULL;
-				ctx->engine[ring->id].state = NULL;
-				goto error;
-			}
-
-			i915_add_request_no_flush(req);
+		ret = i915_gem_request_alloc(ring,
+			ring->default_context, &req);
+		if (ret) {
+			DRM_ERROR("ring create req: %d\n",
+				  ret);
+			i915_gem_request_cancel(req);
+			goto error_destroy_rbuf;
 		}
 
-		ctx->rcs_initialized = true;
+		ret = ring->init_context(req);
+		if (ret) {
+			DRM_ERROR("ring init context: %d\n",
+				  ret);
+			i915_gem_request_cancel(req);
+			goto error_destroy_rbuf;
+		}
+		i915_add_request_no_flush(req);
 	}
 
 	return 0;
 
-error:
-	if (is_global_default_ctx)
-		intel_unpin_ringbuffer_obj(ringbuf);
 error_destroy_rbuf:
 	intel_destroy_ringbuffer_obj(ringbuf);
 error_free_rbuf:
 	kfree(ringbuf);
-error_unpin_ctx:
-	if (is_global_default_ctx)
-		i915_gem_object_ggtt_unpin(ctx_obj);
+error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
+	ctx->engine[ring->id].ringbuf = NULL;
+	ctx->engine[ring->id].state = NULL;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 64f89f99..109147e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -69,8 +69,8 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 
 /* Logical Ring Contexts */
 void intel_lr_context_free(struct intel_context *ctx);
-int intel_lr_context_deferred_create(struct intel_context *ctx,
-				     struct intel_engine_cs *ring);
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
+				    struct intel_engine_cs *ring);
 void intel_lr_context_unpin(struct drm_i915_gem_request *req);
 void intel_lr_context_reset(struct drm_device *dev,
 			struct intel_context *ctx);
-- 
2.1.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH] drm/i915: Split alloc from init for lrc
@ 2015-08-19 12:24 Nick Hoath
  2015-08-19 12:37 ` Chris Wilson
  2015-08-28 13:25 ` shuang.he
  0 siblings, 2 replies; 17+ messages in thread
From: Nick Hoath @ 2015-08-19 12:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Extend init/init_hw split to context init.
   - Move context initialisation in to i915_gem_init_hw
   - Move one off initialisation for render ring to
        i915_gem_validate_context
   - Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources & initialisation
to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
    Left ->init_context int intel_lr_context_deferred_alloc (Daniel Vetter)
    Remove unnecessary init flag & ring type test. (Daniel Vetter)
    Improve commit message (Daniel Vetter)
v3: On init/reinit, set the hw next sequence number to the sw next sequence
    number. This is set to 1 at driver load time. This prevents the seqno
    being reset on reinit (Chris Wilson)

Issue: VIZ-4798
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h            |   1 -
 drivers/gpu/drm/i915/i915_gem.c            |  18 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
 drivers/gpu/drm/i915/intel_lrc.c           | 147 ++++++++++++++---------------
 drivers/gpu/drm/i915/intel_lrc.h           |   4 +-
 5 files changed, 86 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f7fd519..844ccf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -880,7 +880,6 @@ struct intel_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	bool rcs_initialized;
 	struct {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..eb7c1f2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4603,14 +4603,8 @@ int i915_gem_init_rings(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -4629,6 +4623,7 @@ i915_gem_init_hw(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *ring;
 	int ret, i, j;
+	struct drm_i915_gem_request *req;
 
 	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
 		return -EIO;
@@ -4680,9 +4675,12 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto out;
 	}
 
+	ret = i915_gem_set_seqno(dev, dev_priv->next_seqno);
+	if (ret)
+		goto out;
+
 	/* Now it is safe to go back round and do everything else: */
 	for_each_ring(ring, dev_priv, i) {
-		struct drm_i915_gem_request *req;
 
 		WARN_ON(!ring->default_context);
 
@@ -4881,6 +4879,12 @@ i915_gem_load(struct drm_device *dev)
 		dev_priv->num_fence_regs =
 				I915_READ(vgtif_reg(avail_rs.fence_num));
 
+	/*
+	 * Set initial sequence number for requests.
+	  */
+	dev_priv->next_seqno = 1;
+	dev_priv->last_seqno = ~((uint32_t)0);
+
 	/* Initialize fence registers to zero */
 	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
 	i915_gem_restore_fences(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 923a3c4..95f1a0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -994,6 +994,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 {
 	struct intel_context *ctx = NULL;
 	struct i915_ctx_hang_stats *hs;
+	int ret;
 
 	if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
 		return ERR_PTR(-EINVAL);
@@ -1009,7 +1010,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 	}
 
 	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
-		int ret = intel_lr_context_deferred_create(ctx, ring);
+		ret = intel_lr_context_deferred_alloc(ctx, ring);
 		if (ret) {
 			DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
 			return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 138964a..d0dc6b5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1426,11 +1426,31 @@ out:
 	return ret;
 }
 
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *default_ctx_obj)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+	/* The status page is offset 0 from the default context object
+	 * in LRC mode. */
+	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj);
+	ring->status_page.page_addr =
+			kmap(sg_page(default_ctx_obj->pages->sgl));
+	ring->status_page.obj = default_ctx_obj;
+
+	I915_WRITE(RING_HWS_PGA(ring->mmio_base),
+			(u32)ring->status_page.gfx_addr);
+	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
+}
+
 static int gen8_init_common_ring(struct intel_engine_cs *ring)
 {
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lrc_setup_hardware_status_page(ring,
+				ring->default_context->engine[ring->id].state);
+
 	I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
 	I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff);
 
@@ -1841,8 +1861,33 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	if (ret)
 		return ret;
 
-	ret = intel_lr_context_deferred_create(ring->default_context, ring);
+	ret = intel_lr_context_deferred_alloc(ring->default_context, ring);
+	if (ret)
+		return ret;
 
+	ret = i915_gem_obj_ggtt_pin(
+		ring->default_context->engine[ring->id].state,
+		GEN8_LR_CONTEXT_ALIGN, 0);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = intel_pin_and_map_ringbuffer_obj(dev,
+		ring->default_context->engine[ring->id].ringbuf);
+	if (ret) {
+		DRM_ERROR(
+			"Failed to pin and map ringbuffer %s: %d\n",
+			ring->name, ret);
+		goto error_unpin_ggtt;
+	}
+
+	return ret;
+
+error_unpin_ggtt:
+	i915_gem_object_ggtt_unpin(
+		ring->default_context->engine[ring->id].state);
 	return ret;
 }
 
@@ -2044,14 +2089,8 @@ int intel_logical_rings_init(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_logical_ring_cleanup(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_logical_ring_cleanup(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -2303,25 +2342,8 @@ static uint32_t get_lr_context_size(struct intel_engine_cs *ring)
 	return ret;
 }
 
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
-		struct drm_i915_gem_object *default_ctx_obj)
-{
-	struct drm_i915_private *dev_priv = ring->dev->dev_private;
-
-	/* The status page is offset 0 from the default context object
-	 * in LRC mode. */
-	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj);
-	ring->status_page.page_addr =
-			kmap(sg_page(default_ctx_obj->pages->sgl));
-	ring->status_page.obj = default_ctx_obj;
-
-	I915_WRITE(RING_HWS_PGA(ring->mmio_base),
-			(u32)ring->status_page.gfx_addr);
-	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
-}
-
 /**
- * intel_lr_context_deferred_create() - create the LRC specific bits of a context
+ * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context
  * @ctx: LR context to create.
  * @ring: engine to be used with the context.
  *
@@ -2333,10 +2355,10 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
  *
  * Return: non-zero on error.
  */
-int intel_lr_context_deferred_create(struct intel_context *ctx,
+
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
-	const bool is_global_default_ctx = (ctx == ring->default_context);
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
@@ -2354,22 +2376,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, 0);
-		if (ret) {
-			DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
-					ret);
-			drm_gem_object_unreference(&ctx_obj->base);
-			return ret;
-		}
-	}
-
 	ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL);
 	if (!ringbuf) {
 		DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n",
 				ring->name);
 		ret = -ENOMEM;
-		goto error_unpin_ctx;
+		goto error_deref_obj;
 	}
 
 	ringbuf->ring = ring;
@@ -2389,66 +2401,49 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 				ring->name, ret);
 			goto error_free_rbuf;
 		}
-
-		if (is_global_default_ctx) {
-			ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-			if (ret) {
-				DRM_ERROR(
-					"Failed to pin and map ringbuffer %s: %d\n",
-					ring->name, ret);
-				goto error_destroy_rbuf;
-			}
-		}
-
 	}
 
 	ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret);
-		goto error;
+		goto error_destroy_rbuf;
 	}
 
 	ctx->engine[ring->id].ringbuf = ringbuf;
 	ctx->engine[ring->id].state = ctx_obj;
 
-	if (ctx == ring->default_context)
-		lrc_setup_hardware_status_page(ring, ctx_obj);
-	else if (ring->id == RCS && !ctx->rcs_initialized) {
-		if (ring->init_context) {
-			struct drm_i915_gem_request *req;
+	if (ctx != ring->default_context && ring->init_context) {
+		struct drm_i915_gem_request *req;
 
-			ret = i915_gem_request_alloc(ring, ctx, &req);
-			if (ret)
-				return ret;
-
-			ret = ring->init_context(req);
-			if (ret) {
-				DRM_ERROR("ring init context: %d\n", ret);
-				i915_gem_request_cancel(req);
-				ctx->engine[ring->id].ringbuf = NULL;
-				ctx->engine[ring->id].state = NULL;
-				goto error;
-			}
-
-			i915_add_request_no_flush(req);
+		ret = i915_gem_request_alloc(ring,
+			ring->default_context, &req);
+		if (ret) {
+			DRM_ERROR("ring create req: %d\n",
+				  ret);
+			i915_gem_request_cancel(req);
+			goto error_destroy_rbuf;
 		}
 
-		ctx->rcs_initialized = true;
+		ret = ring->init_context(req);
+		if (ret) {
+			DRM_ERROR("ring init context: %d\n",
+				  ret);
+			i915_gem_request_cancel(req);
+			goto error_destroy_rbuf;
+		}
+		i915_add_request_no_flush(req);
 	}
 
 	return 0;
 
-error:
-	if (is_global_default_ctx)
-		intel_unpin_ringbuffer_obj(ringbuf);
 error_destroy_rbuf:
 	intel_destroy_ringbuffer_obj(ringbuf);
 error_free_rbuf:
 	kfree(ringbuf);
-error_unpin_ctx:
-	if (is_global_default_ctx)
-		i915_gem_object_ggtt_unpin(ctx_obj);
+error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
+	ctx->engine[ring->id].ringbuf = NULL;
+	ctx->engine[ring->id].state = NULL;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 64f89f99..109147e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -69,8 +69,8 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 
 /* Logical Ring Contexts */
 void intel_lr_context_free(struct intel_context *ctx);
-int intel_lr_context_deferred_create(struct intel_context *ctx,
-				     struct intel_engine_cs *ring);
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
+				    struct intel_engine_cs *ring);
 void intel_lr_context_unpin(struct drm_i915_gem_request *req);
 void intel_lr_context_reset(struct drm_device *dev,
 			struct intel_context *ctx);
-- 
2.1.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH] drm/i915: Split alloc from init for lrc
@ 2015-09-04 13:49 Nick Hoath
  2015-09-09 13:54 ` Daniel, Thomas
  0 siblings, 1 reply; 17+ messages in thread
From: Nick Hoath @ 2015-09-04 13:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Extend init/init_hw split to context init.
   - Move context initialisation in to i915_gem_init_hw
   - Move one off initialisation for render ring to
        i915_gem_validate_context
   - Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources &
initialisation to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
    Left ->init_context int intel_lr_context_deferred_alloc
    (Daniel Vetter)
    Remove unnecessary init flag & ring type test. (Daniel Vetter)
    Improve commit message (Daniel Vetter)
v3: On init/reinit, set the hw next sequence number to the sw next
    sequence number. This is set to 1 at driver load time. This prevents
    the seqno being reset on reinit (Chris Wilson)
v4: Set seqno back to ~0 - 0x1000 at start-of-day, and increment by 0x100
    on reset.
    This makes it obvious which bbs are which after a reset. (David Gordon
    & John Harrison)
    Rebase.

Issue: VIZ-4798
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: David Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |   1 -
 drivers/gpu/drm/i915/i915_gem.c            |  24 +++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
 drivers/gpu/drm/i915/intel_lrc.c           | 155 ++++++++++++++---------------
 drivers/gpu/drm/i915/intel_lrc.h           |   4 +-
 5 files changed, 93 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1287007..ded7158 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -888,7 +888,6 @@ struct intel_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	bool rcs_initialized;
 	struct {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 41263cd..c8125a5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4613,14 +4613,8 @@ int i915_gem_init_rings(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -4639,6 +4633,7 @@ i915_gem_init_hw(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *ring;
 	int ret, i, j;
+	struct drm_i915_gem_request *req;
 
 	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
 		return -EIO;
@@ -4706,9 +4701,16 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto out;
 	}
 
+	/*
+	 * Increment the next seqno by 0x100 so we have a visible break
+	 * on re-initialisation
+	 */
+	ret = i915_gem_set_seqno(dev, dev_priv->next_seqno+0x100);
+	if (ret)
+		goto out;
+
 	/* Now it is safe to go back round and do everything else: */
 	for_each_ring(ring, dev_priv, i) {
-		struct drm_i915_gem_request *req;
 
 		WARN_ON(!ring->default_context);
 
@@ -4907,6 +4909,14 @@ i915_gem_load(struct drm_device *dev)
 		dev_priv->num_fence_regs =
 				I915_READ(vgtif_reg(avail_rs.fence_num));
 
+	/*
+	 * Set initial sequence number for requests.
+	 * Using this number allows the wraparound to happen early,
+	 * catching any obvious problems.
+	 */
+	dev_priv->next_seqno = ((u32)~0 - 0x1100);
+	dev_priv->last_seqno = ((u32)~0 - 0x1101);
+
 	/* Initialize fence registers to zero */
 	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
 	i915_gem_restore_fences(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a953d49..64674dc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -994,6 +994,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 {
 	struct intel_context *ctx = NULL;
 	struct i915_ctx_hang_stats *hs;
+	int ret;
 
 	if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
 		return ERR_PTR(-EINVAL);
@@ -1009,7 +1010,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 	}
 
 	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
-		int ret = intel_lr_context_deferred_create(ctx, ring);
+		ret = intel_lr_context_deferred_alloc(ctx, ring);
 		if (ret) {
 			DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
 			return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 28a712e..ebe30a9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1445,11 +1445,32 @@ out:
 	return ret;
 }
 
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *default_ctx_obj)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+	struct page *page;
+
+	/* The HWSP is part of the default context object in LRC mode. */
+	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
+			+ LRC_PPHWSP_PN * PAGE_SIZE;
+	page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
+	ring->status_page.page_addr = kmap(page);
+	ring->status_page.obj = default_ctx_obj;
+
+	I915_WRITE(RING_HWS_PGA(ring->mmio_base),
+			(u32)ring->status_page.gfx_addr);
+	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
+}
+
 static int gen8_init_common_ring(struct intel_engine_cs *ring)
 {
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lrc_setup_hardware_status_page(ring,
+				ring->default_context->engine[ring->id].state);
+
 	I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
 	I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff);
 
@@ -1889,8 +1910,33 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	if (ret)
 		return ret;
 
-	ret = intel_lr_context_deferred_create(ring->default_context, ring);
+	ret = intel_lr_context_deferred_alloc(ring->default_context, ring);
+	if (ret)
+		return ret;
+
+	ret = i915_gem_obj_ggtt_pin(
+		ring->default_context->engine[ring->id].state,
+		GEN8_LR_CONTEXT_ALIGN, 0);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = intel_pin_and_map_ringbuffer_obj(dev,
+		ring->default_context->engine[ring->id].ringbuf);
+	if (ret) {
+		DRM_ERROR(
+			"Failed to pin and map ringbuffer %s: %d\n",
+			ring->name, ret);
+		goto error_unpin_ggtt;
+	}
+
+	return ret;
 
+error_unpin_ggtt:
+	i915_gem_object_ggtt_unpin(
+		ring->default_context->engine[ring->id].state);
 	return ret;
 }
 
@@ -2112,14 +2158,8 @@ int intel_logical_rings_init(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_logical_ring_cleanup(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_logical_ring_cleanup(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -2370,26 +2410,8 @@ static uint32_t get_lr_context_size(struct intel_engine_cs *ring)
 	return ret;
 }
 
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
-		struct drm_i915_gem_object *default_ctx_obj)
-{
-	struct drm_i915_private *dev_priv = ring->dev->dev_private;
-	struct page *page;
-
-	/* The HWSP is part of the default context object in LRC mode. */
-	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
-			+ LRC_PPHWSP_PN * PAGE_SIZE;
-	page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
-	ring->status_page.page_addr = kmap(page);
-	ring->status_page.obj = default_ctx_obj;
-
-	I915_WRITE(RING_HWS_PGA(ring->mmio_base),
-			(u32)ring->status_page.gfx_addr);
-	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
-}
-
 /**
- * intel_lr_context_deferred_create() - create the LRC specific bits of a context
+ * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context
  * @ctx: LR context to create.
  * @ring: engine to be used with the context.
  *
@@ -2401,12 +2423,11 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
  *
  * Return: non-zero on error.
  */
-int intel_lr_context_deferred_create(struct intel_context *ctx,
+
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
-	const bool is_global_default_ctx = (ctx == ring->default_context);
 	struct drm_device *dev = ring->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
 	struct intel_ringbuffer *ringbuf;
@@ -2426,82 +2447,50 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-		if (ret) {
-			DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
-					ret);
-			drm_gem_object_unreference(&ctx_obj->base);
-			return ret;
-		}
-
-		/* Invalidate GuC TLB. */
-		if (i915.enable_guc_submission)
-			I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-	}
-
 	ringbuf = intel_engine_create_ringbuffer(ring, 4 * PAGE_SIZE);
 	if (IS_ERR(ringbuf)) {
 		ret = PTR_ERR(ringbuf);
-		goto error_unpin_ctx;
-	}
-
-	if (is_global_default_ctx) {
-		ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-		if (ret) {
-			DRM_ERROR(
-				  "Failed to pin and map ringbuffer %s: %d\n",
-				  ring->name, ret);
-			goto error_ringbuf;
-		}
+		goto error_deref_obj;
 	}
 
 	ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret);
-		goto error;
+		goto error_ringbuf;
 	}
 
 	ctx->engine[ring->id].ringbuf = ringbuf;
 	ctx->engine[ring->id].state = ctx_obj;
 
-	if (ctx == ring->default_context)
-		lrc_setup_hardware_status_page(ring, ctx_obj);
-	else if (ring->id == RCS && !ctx->rcs_initialized) {
-		if (ring->init_context) {
-			struct drm_i915_gem_request *req;
-
-			ret = i915_gem_request_alloc(ring, ctx, &req);
-			if (ret)
-				return ret;
+	if (ctx != ring->default_context && ring->init_context) {
+		struct drm_i915_gem_request *req;
 
-			ret = ring->init_context(req);
-			if (ret) {
-				DRM_ERROR("ring init context: %d\n", ret);
-				i915_gem_request_cancel(req);
-				ctx->engine[ring->id].ringbuf = NULL;
-				ctx->engine[ring->id].state = NULL;
-				goto error;
-			}
-
-			i915_add_request_no_flush(req);
+		ret = i915_gem_request_alloc(ring,
+			ring->default_context, &req);
+		if (ret) {
+			DRM_ERROR("ring create req: %d\n",
+				ret);
+			i915_gem_request_cancel(req);
+			goto error_ringbuf;
 		}
 
-		ctx->rcs_initialized = true;
+		ret = ring->init_context(req);
+		if (ret) {
+			DRM_ERROR("ring init context: %d\n",
+				ret);
+			i915_gem_request_cancel(req);
+			goto error_ringbuf;
+		}
+		i915_add_request_no_flush(req);
 	}
-
 	return 0;
 
-error:
-	if (is_global_default_ctx)
-		intel_unpin_ringbuffer_obj(ringbuf);
 error_ringbuf:
 	intel_ringbuffer_free(ringbuf);
-error_unpin_ctx:
-	if (is_global_default_ctx)
-		i915_gem_object_ggtt_unpin(ctx_obj);
+error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
+	ctx->engine[ring->id].ringbuf = NULL;
+	ctx->engine[ring->id].state = NULL;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 4cc54b3..69d99f0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -75,8 +75,8 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 #define LRC_STATE_PN	(LRC_PPHWSP_PN + 1)
 
 void intel_lr_context_free(struct intel_context *ctx);
-int intel_lr_context_deferred_create(struct intel_context *ctx,
-				     struct intel_engine_cs *ring);
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
+				    struct intel_engine_cs *ring);
 void intel_lr_context_unpin(struct drm_i915_gem_request *req);
 void intel_lr_context_reset(struct drm_device *dev,
 			struct intel_context *ctx);
-- 
2.1.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH] drm/i915: Split alloc from init for lrc
@ 2015-09-10 13:32 Nick Hoath
  0 siblings, 0 replies; 17+ messages in thread
From: Nick Hoath @ 2015-09-10 13:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Extend init/init_hw split to context init.
   - Move context initialisation in to i915_gem_init_hw
   - Move one off initialisation for render ring to
        i915_gem_validate_context
   - Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources &
initialisation to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
    Left ->init_context int intel_lr_context_deferred_alloc
    (Daniel Vetter)
    Remove unnecessary init flag & ring type test. (Daniel Vetter)
    Improve commit message (Daniel Vetter)
v3: On init/reinit, set the hw next sequence number to the sw next
    sequence number. This is set to 1 at driver load time. This prevents
    the seqno being reset on reinit (Chris Wilson)
v4: Set seqno back to ~0 - 0x1000 at start-of-day, and increment by 0x100
    on reset.
    This makes it obvious which bbs are which after a reset. (David Gordon
    & John Harrison)
    Rebase.
v5: Rebase. Fixed rebase breakage. Put context pinning in separate
    function. Removed code churn. (Thomas Daniel)

Issue: VIZ-4798
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: David Gordon <david.s.gordon@intel.com>
Cc: Thomas Daniel <thomas.daniel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |   1 -
 drivers/gpu/drm/i915/i915_gem.c            |  22 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   2 +-
 drivers/gpu/drm/i915/intel_lrc.c           | 164 ++++++++++++++---------------
 drivers/gpu/drm/i915/intel_lrc.h           |   4 +-
 5 files changed, 99 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2ef83c5..ed62eae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -890,7 +890,6 @@ struct intel_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	bool rcs_initialized;
 	struct {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 41263cd..c8302e5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4613,14 +4613,8 @@ int i915_gem_init_rings(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -4706,6 +4700,14 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto out;
 	}
 
+	/*
+	 * Increment the next seqno by 0x100 so we have a visible break
+	 * on re-initialisation
+	 */
+	ret = i915_gem_set_seqno(dev, dev_priv->next_seqno+0x100);
+	if (ret)
+		goto out;
+
 	/* Now it is safe to go back round and do everything else: */
 	for_each_ring(ring, dev_priv, i) {
 		struct drm_i915_gem_request *req;
@@ -4907,6 +4909,14 @@ i915_gem_load(struct drm_device *dev)
 		dev_priv->num_fence_regs =
 				I915_READ(vgtif_reg(avail_rs.fence_num));
 
+	/*
+	 * Set initial sequence number for requests.
+	 * Using this number allows the wraparound to happen early,
+	 * catching any obvious problems.
+	 */
+	dev_priv->next_seqno = ((u32)~0 - 0x1100);
+	dev_priv->last_seqno = ((u32)~0 - 0x1101);
+
 	/* Initialize fence registers to zero */
 	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
 	i915_gem_restore_fences(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a953d49..67ef118 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1009,7 +1009,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 	}
 
 	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
-		int ret = intel_lr_context_deferred_create(ctx, ring);
+		int ret = intel_lr_context_deferred_alloc(ctx, ring);
 		if (ret) {
 			DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
 			return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 28a712e..0248d11 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1008,39 +1008,54 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
 	return 0;
 }
 
-static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
+static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *ctx_obj,
+		struct intel_ringbuffer *ringbuf)
 {
-	struct drm_i915_private *dev_priv = rq->i915;
-	struct intel_engine_cs *ring = rq->ring;
-	struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
-	struct intel_ringbuffer *ringbuf = rq->ringbuf;
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret = 0;
 
 	WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
-	if (rq->ctx->engine[ring->id].pin_count++ == 0) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-		if (ret)
-			goto reset_pin_count;
+	ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
+			PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	if (ret)
+		return ret;
 
-		ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
-		if (ret)
-			goto unpin_ctx_obj;
+	ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
+	if (ret)
+		goto unpin_ctx_obj;
 
-		ctx_obj->dirty = true;
+	ctx_obj->dirty = true;
 
-		/* Invalidate GuC TLB. */
-		if (i915.enable_guc_submission)
-			I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-	}
+	/* Invalidate GuC TLB. */
+	if (i915.enable_guc_submission)
+		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
 
 	return ret;
 
 unpin_ctx_obj:
 	i915_gem_object_ggtt_unpin(ctx_obj);
+
+	return ret;
+}
+
+static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
+{
+	int ret = 0;
+	struct intel_engine_cs *ring = rq->ring;
+	struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+	struct intel_ringbuffer *ringbuf = rq->ringbuf;
+
+	if (rq->ctx->engine[ring->id].pin_count++ == 0) {
+		ret = intel_lr_context_do_pin(ring, ctx_obj, ringbuf);
+		if (ret)
+			goto reset_pin_count;
+	}
+	return ret;
+
 reset_pin_count:
 	rq->ctx->engine[ring->id].pin_count = 0;
-
 	return ret;
 }
 
@@ -1445,11 +1460,17 @@ out:
 	return ret;
 }
 
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *default_ctx_obj);
+
 static int gen8_init_common_ring(struct intel_engine_cs *ring)
 {
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lrc_setup_hardware_status_page(ring,
+				ring->default_context->engine[ring->id].state);
+
 	I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
 	I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff);
 
@@ -1889,7 +1910,21 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	if (ret)
 		return ret;
 
-	ret = intel_lr_context_deferred_create(ring->default_context, ring);
+	ret = intel_lr_context_deferred_alloc(ring->default_context, ring);
+	if (ret)
+		return ret;
+
+	/* As this is the default context, always pin it */
+	ret = intel_lr_context_do_pin(
+			ring,
+			ring->default_context->engine[ring->id].state,
+			ring->default_context->engine[ring->id].ringbuf);
+	if (ret) {
+		DRM_ERROR(
+			"Failed to pin and map ringbuffer %s: %d\n",
+			ring->name, ret);
+		return ret;
+	}
 
 	return ret;
 }
@@ -2112,14 +2147,8 @@ int intel_logical_rings_init(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_logical_ring_cleanup(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_logical_ring_cleanup(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -2389,7 +2418,7 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
 }
 
 /**
- * intel_lr_context_deferred_create() - create the LRC specific bits of a context
+ * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context
  * @ctx: LR context to create.
  * @ring: engine to be used with the context.
  *
@@ -2401,12 +2430,11 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
  *
  * Return: non-zero on error.
  */
-int intel_lr_context_deferred_create(struct intel_context *ctx,
+
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
-	const bool is_global_default_ctx = (ctx == ring->default_context);
 	struct drm_device *dev = ring->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
 	struct intel_ringbuffer *ringbuf;
@@ -2426,82 +2454,50 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-		if (ret) {
-			DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
-					ret);
-			drm_gem_object_unreference(&ctx_obj->base);
-			return ret;
-		}
-
-		/* Invalidate GuC TLB. */
-		if (i915.enable_guc_submission)
-			I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-	}
-
 	ringbuf = intel_engine_create_ringbuffer(ring, 4 * PAGE_SIZE);
 	if (IS_ERR(ringbuf)) {
 		ret = PTR_ERR(ringbuf);
-		goto error_unpin_ctx;
-	}
-
-	if (is_global_default_ctx) {
-		ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-		if (ret) {
-			DRM_ERROR(
-				  "Failed to pin and map ringbuffer %s: %d\n",
-				  ring->name, ret);
-			goto error_ringbuf;
-		}
+		goto error_deref_obj;
 	}
 
 	ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret);
-		goto error;
+		goto error_ringbuf;
 	}
 
 	ctx->engine[ring->id].ringbuf = ringbuf;
 	ctx->engine[ring->id].state = ctx_obj;
 
-	if (ctx == ring->default_context)
-		lrc_setup_hardware_status_page(ring, ctx_obj);
-	else if (ring->id == RCS && !ctx->rcs_initialized) {
-		if (ring->init_context) {
-			struct drm_i915_gem_request *req;
-
-			ret = i915_gem_request_alloc(ring, ctx, &req);
-			if (ret)
-				return ret;
-
-			ret = ring->init_context(req);
-			if (ret) {
-				DRM_ERROR("ring init context: %d\n", ret);
-				i915_gem_request_cancel(req);
-				ctx->engine[ring->id].ringbuf = NULL;
-				ctx->engine[ring->id].state = NULL;
-				goto error;
-			}
+	if (ctx != ring->default_context && ring->init_context) {
+		struct drm_i915_gem_request *req;
 
-			i915_add_request_no_flush(req);
+		ret = i915_gem_request_alloc(ring,
+			ring->default_context, &req);
+		if (ret) {
+			DRM_ERROR("ring create req: %d\n",
+				ret);
+			i915_gem_request_cancel(req);
+			goto error_ringbuf;
 		}
 
-		ctx->rcs_initialized = true;
+		ret = ring->init_context(req);
+		if (ret) {
+			DRM_ERROR("ring init context: %d\n",
+				ret);
+			i915_gem_request_cancel(req);
+			goto error_ringbuf;
+		}
+		i915_add_request_no_flush(req);
 	}
-
 	return 0;
 
-error:
-	if (is_global_default_ctx)
-		intel_unpin_ringbuffer_obj(ringbuf);
 error_ringbuf:
 	intel_ringbuffer_free(ringbuf);
-error_unpin_ctx:
-	if (is_global_default_ctx)
-		i915_gem_object_ggtt_unpin(ctx_obj);
+error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
+	ctx->engine[ring->id].ringbuf = NULL;
+	ctx->engine[ring->id].state = NULL;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 4cc54b3..69d99f0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -75,8 +75,8 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 #define LRC_STATE_PN	(LRC_PPHWSP_PN + 1)
 
 void intel_lr_context_free(struct intel_context *ctx);
-int intel_lr_context_deferred_create(struct intel_context *ctx,
-				     struct intel_engine_cs *ring);
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
+				    struct intel_engine_cs *ring);
 void intel_lr_context_unpin(struct drm_i915_gem_request *req);
 void intel_lr_context_reset(struct drm_device *dev,
 			struct intel_context *ctx);
-- 
2.1.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH] drm/i915: Split alloc from init for lrc
@ 2015-09-11 11:53 Nick Hoath
  2015-09-11 13:09 ` Daniel, Thomas
  2015-09-21 10:25 ` Tvrtko Ursulin
  0 siblings, 2 replies; 17+ messages in thread
From: Nick Hoath @ 2015-09-11 11:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Extend init/init_hw split to context init.
   - Move context initialisation in to i915_gem_init_hw
   - Move one off initialisation for render ring to
        i915_gem_validate_context
   - Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources &
initialisation to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
    Left ->init_context int intel_lr_context_deferred_alloc
    (Daniel Vetter)
    Remove unnecessary init flag & ring type test. (Daniel Vetter)
    Improve commit message (Daniel Vetter)
v3: On init/reinit, set the hw next sequence number to the sw next
    sequence number. This is set to 1 at driver load time. This prevents
    the seqno being reset on reinit (Chris Wilson)
v4: Set seqno back to ~0 - 0x1000 at start-of-day, and increment by 0x100
    on reset.
    This makes it obvious which bbs are which after a reset. (David Gordon
    & John Harrison)
    Rebase.
v5: Rebase. Fixed rebase breakage. Put context pinning in separate
    function. Removed code churn. (Thomas Daniel)
v6: Cleanup up issues introduced in v2 & v5 (Thomas Daniel)

Issue: VIZ-4798
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: David Gordon <david.s.gordon@intel.com>
Cc: Thomas Daniel <thomas.daniel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |   1 -
 drivers/gpu/drm/i915/i915_gem.c            |  22 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   2 +-
 drivers/gpu/drm/i915/intel_lrc.c           | 164 ++++++++++++++---------------
 drivers/gpu/drm/i915/intel_lrc.h           |   4 +-
 5 files changed, 99 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18859cd..23dd57d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -890,7 +890,6 @@ struct intel_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	bool rcs_initialized;
 	struct {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5859fc4..e7eb325 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4650,14 +4650,8 @@ int i915_gem_init_rings(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -4743,6 +4737,14 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto out;
 	}
 
+	/*
+	 * Increment the next seqno by 0x100 so we have a visible break
+	 * on re-initialisation
+	 */
+	ret = i915_gem_set_seqno(dev, dev_priv->next_seqno+0x100);
+	if (ret)
+		goto out;
+
 	/* Now it is safe to go back round and do everything else: */
 	for_each_ring(ring, dev_priv, i) {
 		struct drm_i915_gem_request *req;
@@ -4944,6 +4946,14 @@ i915_gem_load(struct drm_device *dev)
 		dev_priv->num_fence_regs =
 				I915_READ(vgtif_reg(avail_rs.fence_num));
 
+	/*
+	 * Set initial sequence number for requests.
+	 * Using this number allows the wraparound to happen early,
+	 * catching any obvious problems.
+	 */
+	dev_priv->next_seqno = ((u32)~0 - 0x1100);
+	dev_priv->last_seqno = ((u32)~0 - 0x1101);
+
 	/* Initialize fence registers to zero */
 	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
 	i915_gem_restore_fences(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a953d49..67ef118 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1009,7 +1009,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 	}
 
 	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
-		int ret = intel_lr_context_deferred_create(ctx, ring);
+		int ret = intel_lr_context_deferred_alloc(ctx, ring);
 		if (ret) {
 			DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
 			return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 639da4d..71892dd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -221,6 +221,9 @@ enum {
 #define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT  0x17
 
 static int intel_lr_context_pin(struct drm_i915_gem_request *rq);
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *default_ctx_obj);
+
 
 /**
  * intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
@@ -978,39 +981,54 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
 	return 0;
 }
 
-static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
+static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
+		struct drm_i915_gem_object *ctx_obj,
+		struct intel_ringbuffer *ringbuf)
 {
-	struct drm_i915_private *dev_priv = rq->i915;
-	struct intel_engine_cs *ring = rq->ring;
-	struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
-	struct intel_ringbuffer *ringbuf = rq->ringbuf;
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret = 0;
 
 	WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
-	if (rq->ctx->engine[ring->id].pin_count++ == 0) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-		if (ret)
-			goto reset_pin_count;
+	ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
+			PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	if (ret)
+		return ret;
 
-		ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
-		if (ret)
-			goto unpin_ctx_obj;
+	ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
+	if (ret)
+		goto unpin_ctx_obj;
 
-		ctx_obj->dirty = true;
+	ctx_obj->dirty = true;
 
-		/* Invalidate GuC TLB. */
-		if (i915.enable_guc_submission)
-			I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-	}
+	/* Invalidate GuC TLB. */
+	if (i915.enable_guc_submission)
+		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
 
 	return ret;
 
 unpin_ctx_obj:
 	i915_gem_object_ggtt_unpin(ctx_obj);
+
+	return ret;
+}
+
+static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
+{
+	int ret = 0;
+	struct intel_engine_cs *ring = rq->ring;
+	struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+	struct intel_ringbuffer *ringbuf = rq->ringbuf;
+
+	if (rq->ctx->engine[ring->id].pin_count++ == 0) {
+		ret = intel_lr_context_do_pin(ring, ctx_obj, ringbuf);
+		if (ret)
+			goto reset_pin_count;
+	}
+	return ret;
+
 reset_pin_count:
 	rq->ctx->engine[ring->id].pin_count = 0;
-
 	return ret;
 }
 
@@ -1420,6 +1438,9 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lrc_setup_hardware_status_page(ring,
+				ring->default_context->engine[ring->id].state);
+
 	I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
 	I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff);
 
@@ -1858,7 +1879,21 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	if (ret)
 		return ret;
 
-	ret = intel_lr_context_deferred_create(ring->default_context, ring);
+	ret = intel_lr_context_deferred_alloc(ring->default_context, ring);
+	if (ret)
+		return ret;
+
+	/* As this is the default context, always pin it */
+	ret = intel_lr_context_do_pin(
+			ring,
+			ring->default_context->engine[ring->id].state,
+			ring->default_context->engine[ring->id].ringbuf);
+	if (ret) {
+		DRM_ERROR(
+			"Failed to pin and map ringbuffer %s: %d\n",
+			ring->name, ret);
+		return ret;
+	}
 
 	return ret;
 }
@@ -2081,14 +2116,8 @@ int intel_logical_rings_init(struct drm_device *dev)
 			goto cleanup_vebox_ring;
 	}
 
-	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-	if (ret)
-		goto cleanup_bsd2_ring;
-
 	return 0;
 
-cleanup_bsd2_ring:
-	intel_logical_ring_cleanup(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_logical_ring_cleanup(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -2358,7 +2387,7 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
 }
 
 /**
- * intel_lr_context_deferred_create() - create the LRC specific bits of a context
+ * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context
  * @ctx: LR context to create.
  * @ring: engine to be used with the context.
  *
@@ -2370,12 +2399,11 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
  *
  * Return: non-zero on error.
  */
-int intel_lr_context_deferred_create(struct intel_context *ctx,
+
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
-	const bool is_global_default_ctx = (ctx == ring->default_context);
 	struct drm_device *dev = ring->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
 	struct intel_ringbuffer *ringbuf;
@@ -2395,82 +2423,50 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
-		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-		if (ret) {
-			DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
-					ret);
-			drm_gem_object_unreference(&ctx_obj->base);
-			return ret;
-		}
-
-		/* Invalidate GuC TLB. */
-		if (i915.enable_guc_submission)
-			I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-	}
-
 	ringbuf = intel_engine_create_ringbuffer(ring, 4 * PAGE_SIZE);
 	if (IS_ERR(ringbuf)) {
 		ret = PTR_ERR(ringbuf);
-		goto error_unpin_ctx;
-	}
-
-	if (is_global_default_ctx) {
-		ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-		if (ret) {
-			DRM_ERROR(
-				  "Failed to pin and map ringbuffer %s: %d\n",
-				  ring->name, ret);
-			goto error_ringbuf;
-		}
+		goto error_deref_obj;
 	}
 
 	ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret);
-		goto error;
+		goto error_ringbuf;
 	}
 
 	ctx->engine[ring->id].ringbuf = ringbuf;
 	ctx->engine[ring->id].state = ctx_obj;
 
-	if (ctx == ring->default_context)
-		lrc_setup_hardware_status_page(ring, ctx_obj);
-	else if (ring->id == RCS && !ctx->rcs_initialized) {
-		if (ring->init_context) {
-			struct drm_i915_gem_request *req;
-
-			ret = i915_gem_request_alloc(ring, ctx, &req);
-			if (ret)
-				return ret;
+	if (ctx != ring->default_context && ring->init_context) {
+		struct drm_i915_gem_request *req;
 
-			ret = ring->init_context(req);
-			if (ret) {
-				DRM_ERROR("ring init context: %d\n", ret);
-				i915_gem_request_cancel(req);
-				ctx->engine[ring->id].ringbuf = NULL;
-				ctx->engine[ring->id].state = NULL;
-				goto error;
-			}
-
-			i915_add_request_no_flush(req);
+		ret = i915_gem_request_alloc(ring,
+			ctx, &req);
+		if (ret) {
+			DRM_ERROR("ring create req: %d\n",
+				ret);
+			i915_gem_request_cancel(req);
+			goto error_ringbuf;
 		}
 
-		ctx->rcs_initialized = true;
+		ret = ring->init_context(req);
+		if (ret) {
+			DRM_ERROR("ring init context: %d\n",
+				ret);
+			i915_gem_request_cancel(req);
+			goto error_ringbuf;
+		}
+		i915_add_request_no_flush(req);
 	}
-
 	return 0;
 
-error:
-	if (is_global_default_ctx)
-		intel_unpin_ringbuffer_obj(ringbuf);
 error_ringbuf:
 	intel_ringbuffer_free(ringbuf);
-error_unpin_ctx:
-	if (is_global_default_ctx)
-		i915_gem_object_ggtt_unpin(ctx_obj);
+error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
+	ctx->engine[ring->id].ringbuf = NULL;
+	ctx->engine[ring->id].state = NULL;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index c0ac4f8..bcd0eef 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -75,8 +75,8 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 #define LRC_STATE_PN	(LRC_PPHWSP_PN + 1)
 
 void intel_lr_context_free(struct intel_context *ctx);
-int intel_lr_context_deferred_create(struct intel_context *ctx,
-				     struct intel_engine_cs *ring);
+int intel_lr_context_deferred_alloc(struct intel_context *ctx,
+				    struct intel_engine_cs *ring);
 void intel_lr_context_unpin(struct drm_i915_gem_request *req);
 void intel_lr_context_reset(struct drm_device *dev,
 			struct intel_context *ctx);
-- 
2.1.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-09-21 10:25 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-18 14:23 [PATCH] drm/i915: Split alloc from init for lrc Nick Hoath
2015-08-18 14:31 ` Chris Wilson
2015-08-18 14:55   ` Nick Hoath
2015-08-18 15:06     ` Chris Wilson
2015-08-26  8:32       ` Daniel Vetter
2015-08-26  8:49         ` Chris Wilson
2015-08-28  8:12 ` shuang.he
  -- strict thread matches above, loose matches on Subject: below --
2015-08-19 12:24 Nick Hoath
2015-08-19 12:37 ` Chris Wilson
2015-08-19 14:40   ` Nick Hoath
2015-08-28 13:25 ` shuang.he
2015-09-04 13:49 Nick Hoath
2015-09-09 13:54 ` Daniel, Thomas
2015-09-10 13:32 Nick Hoath
2015-09-11 11:53 Nick Hoath
2015-09-11 13:09 ` Daniel, Thomas
2015-09-21 10:25 ` Tvrtko Ursulin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox