public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 3/5] drm/i915: Unify engine init loop
  2016-07-01 16:47 [PATCH 1/5] " Tvrtko Ursulin
@ 2016-07-01 16:47 ` Tvrtko Ursulin
  0 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-01 16:47 UTC (permalink / raw)
  To: Intel-gfx

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

With the unified common engine setup done, and the execlist engine
initialization loop clearly split into two phases, we can eliminate
the separate legacy engine initialization code.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 --
 drivers/gpu/drm/i915/i915_gem.c         | 51 +--------------------------------
 drivers/gpu/drm/i915/intel_lrc.c        | 25 ++++++++++------
 drivers/gpu/drm/i915/intel_lrc.h        |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++++++++++-------------------
 drivers/gpu/drm/i915/intel_ringbuffer.h | 10 +++----
 6 files changed, 39 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 485ab1148181..c058cb8150be 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2030,7 +2030,6 @@ struct drm_i915_private {
 		int (*execbuf_submit)(struct i915_execbuffer_params *params,
 				      struct drm_i915_gem_execbuffer2 *args,
 				      struct list_head *vmas);
-		int (*init_engines)(struct drm_device *dev);
 		void (*cleanup_engine)(struct intel_engine_cs *engine);
 		void (*stop_engine)(struct intel_engine_cs *engine);
 	} gt;
@@ -3342,7 +3341,6 @@ static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv)
 void i915_gem_reset(struct drm_device *dev);
 bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
 int __must_check i915_gem_init(struct drm_device *dev);
-int i915_gem_init_engines(struct drm_device *dev);
 int __must_check i915_gem_init_hw(struct drm_device *dev);
 void i915_gem_init_swizzling(struct drm_device *dev);
 void i915_gem_cleanup_engines(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1d9878258103..3b5c99833ddd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5029,53 +5029,6 @@ static void init_unused_rings(struct drm_device *dev)
 	}
 }
 
-int i915_gem_init_engines(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret;
-
-	ret = intel_init_render_ring_buffer(dev);
-	if (ret)
-		return ret;
-
-	if (HAS_BSD(dev)) {
-		ret = intel_init_bsd_ring_buffer(dev);
-		if (ret)
-			goto cleanup_render_ring;
-	}
-
-	if (HAS_BLT(dev)) {
-		ret = intel_init_blt_ring_buffer(dev);
-		if (ret)
-			goto cleanup_bsd_ring;
-	}
-
-	if (HAS_VEBOX(dev)) {
-		ret = intel_init_vebox_ring_buffer(dev);
-		if (ret)
-			goto cleanup_blt_ring;
-	}
-
-	if (HAS_BSD2(dev)) {
-		ret = intel_init_bsd2_ring_buffer(dev);
-		if (ret)
-			goto cleanup_vebox_ring;
-	}
-
-	return 0;
-
-cleanup_vebox_ring:
-	intel_cleanup_engine(&dev_priv->engine[VECS]);
-cleanup_blt_ring:
-	intel_cleanup_engine(&dev_priv->engine[BCS]);
-cleanup_bsd_ring:
-	intel_cleanup_engine(&dev_priv->engine[VCS]);
-cleanup_render_ring:
-	intel_cleanup_engine(&dev_priv->engine[RCS]);
-
-	return ret;
-}
-
 int
 i915_gem_init_hw(struct drm_device *dev)
 {
@@ -5157,12 +5110,10 @@ int i915_gem_init(struct drm_device *dev)
 
 	if (!i915.enable_execlists) {
 		dev_priv->gt.execbuf_submit = i915_gem_ringbuffer_submission;
-		dev_priv->gt.init_engines = i915_gem_init_engines;
 		dev_priv->gt.cleanup_engine = intel_cleanup_engine;
 		dev_priv->gt.stop_engine = intel_stop_engine;
 	} else {
 		dev_priv->gt.execbuf_submit = intel_execlists_submission;
-		dev_priv->gt.init_engines = intel_logical_rings_init;
 		dev_priv->gt.cleanup_engine = intel_logical_ring_cleanup;
 		dev_priv->gt.stop_engine = intel_logical_ring_stop;
 	}
@@ -5182,7 +5133,7 @@ int i915_gem_init(struct drm_device *dev)
 	if (ret)
 		goto out_unlock;
 
-	ret = dev_priv->gt.init_engines(dev);
+	ret = intel_engines_init(dev);
 	if (ret)
 		goto out_unlock;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index abb165019af7..bf8f4a67c75b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2153,6 +2153,7 @@ static const struct engine_info {
 	u32 mmio_base;
 	unsigned irq_shift;
 	int (*init)(struct intel_engine_cs *engine);
+	int (*init_ringbuf)(struct intel_engine_cs *engine);
 } intel_engines[] = {
 	[RCS] = {
 		.name = "render ring",
@@ -2161,6 +2162,7 @@ static const struct engine_info {
 		.mmio_base = RENDER_RING_BASE,
 		.irq_shift = GEN8_RCS_IRQ_SHIFT,
 		.init = logical_render_ring_init,
+		.init_ringbuf = intel_init_render_ring_buffer,
 	},
 	[BCS] = {
 		.name = "blitter ring",
@@ -2169,6 +2171,7 @@ static const struct engine_info {
 		.mmio_base = BLT_RING_BASE,
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_blt_ring_buffer,
 	},
 	[VCS] = {
 		.name = "bsd ring",
@@ -2177,6 +2180,7 @@ static const struct engine_info {
 		.mmio_base = GEN6_BSD_RING_BASE,
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_bsd_ring_buffer,
 	},
 	[VCS2] = {
 		.name = "bsd2 ring",
@@ -2185,6 +2189,7 @@ static const struct engine_info {
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_bsd2_ring_buffer,
 	},
 	[VECS] = {
 		.name = "video enhancement ring",
@@ -2193,6 +2198,7 @@ static const struct engine_info {
 		.mmio_base = VEBOX_RING_BASE,
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_vebox_ring_buffer,
 	},
 };
 
@@ -2215,20 +2221,16 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 }
 
 /**
- * intel_logical_rings_init() - allocate, populate and init the Engine Command Streamers
+ * intel_engines_init() - allocate, populate and init the Engine Command Streamers
  * @dev: DRM device.
  *
- * This function inits the engines for an Execlists submission style (the
- * equivalent in the legacy ringbuffer submission world would be
- * i915_gem_init_engines). It does it only for those engines that are present in
- * the hardware.
- *
  * Return: non-zero if the initialization failed.
  */
-int intel_logical_rings_init(struct drm_device *dev)
+int intel_engines_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned int mask = 0;
+	int (*init)(struct intel_engine_cs *engine);
 	unsigned int i;
 	int ret;
 
@@ -2239,10 +2241,15 @@ int intel_logical_rings_init(struct drm_device *dev)
 		if (!HAS_ENGINE(dev_priv, i))
 			continue;
 
-		if (!intel_engines[i].init)
+		if (i915.enable_execlists)
+			init = intel_engines[i].init;
+		else
+			init = intel_engines[i].init_ringbuf;
+
+		if (!init)
 			continue;
 
-		ret = intel_engines[i].init(intel_engine_setup(dev_priv, i));
+		ret = init(intel_engine_setup(dev_priv, i));
 		if (ret)
 			goto cleanup;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 2b8255c19dcc..aa8905c42de7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -67,7 +67,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
 int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
 void intel_logical_ring_stop(struct intel_engine_cs *engine);
 void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
-int intel_logical_rings_init(struct drm_device *dev);
+int intel_engines_init(struct drm_device *dev);
 
 int logical_ring_flush_all_caches(struct drm_i915_gem_request *req);
 /**
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 215424efe05c..a00adc3438f3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -3033,15 +3033,12 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 	intel_ring_init_semaphores(dev_priv, engine);
 }
 
-int intel_init_render_ring_buffer(struct drm_device *dev)
+int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *engine;
+	struct drm_i915_private *dev_priv = engine->i915;
 	struct drm_i915_gem_object *obj;
 	int ret;
 
-	engine = intel_engine_setup(dev_priv, RCS);
-
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	if (INTEL_GEN(dev_priv) >= 8) {
@@ -3080,7 +3077,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
 	/* Workaround batchbuffer to combat CS tlb bug. */
 	if (HAS_BROKEN_CS_TLB(dev_priv)) {
-		obj = i915_gem_object_create(dev, I830_WA_SIZE);
+		obj = i915_gem_object_create(dev_priv->dev, I830_WA_SIZE);
 		if (IS_ERR(obj)) {
 			DRM_ERROR("Failed to allocate batch bo\n");
 			return PTR_ERR(obj);
@@ -3097,7 +3094,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->scratch.gtt_offset = i915_gem_obj_ggtt_offset(obj);
 	}
 
-	ret = intel_init_ring_buffer(dev, engine);
+	ret = intel_init_ring_buffer(dev_priv->dev, engine);
 	if (ret)
 		return ret;
 
@@ -3110,12 +3107,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	return 0;
 }
 
-int intel_init_bsd_ring_buffer(struct drm_device *dev)
+int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, VCS);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -3138,18 +3132,15 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
 	}
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(dev_priv->dev, engine);
 }
 
 /**
  * Initialize the second BSD ring (eg. Broadwell GT3, Skylake GT3)
  */
-int intel_init_bsd2_ring_buffer(struct drm_device *dev)
+int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, VCS2);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -3157,15 +3148,12 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(dev_priv->dev, engine);
 }
 
-int intel_init_blt_ring_buffer(struct drm_device *dev)
+int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, BCS);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -3176,15 +3164,12 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	else
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(dev_priv->dev, engine);
 }
 
-int intel_init_vebox_ring_buffer(struct drm_device *dev)
+int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, VECS);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -3199,7 +3184,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		engine->irq_put = hsw_vebox_put_irq;
 	}
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(dev_priv->dev, engine);
 }
 
 int
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1aeb00cba9e2..a843164ab386 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -459,11 +459,11 @@ int intel_ring_invalidate_all_caches(struct drm_i915_gem_request *req);
 void intel_fini_pipe_control(struct intel_engine_cs *engine);
 int intel_init_pipe_control(struct intel_engine_cs *engine);
 
-int intel_init_render_ring_buffer(struct drm_device *dev);
-int intel_init_bsd_ring_buffer(struct drm_device *dev);
-int intel_init_bsd2_ring_buffer(struct drm_device *dev);
-int intel_init_blt_ring_buffer(struct drm_device *dev);
-int intel_init_vebox_ring_buffer(struct drm_device *dev);
+int intel_init_render_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_blt_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine);
 
 u64 intel_ring_get_active_head(struct intel_engine_cs *engine);
 
-- 
1.9.1

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

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

* [PATCH 1/5] drm/i915: unify first-stage engine struct setup
@ 2016-07-06 10:52 Tvrtko Ursulin
  2016-07-06 10:52 ` [PATCH 2/5] drm/i915: Prepare for engine init unification Tvrtko Ursulin
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 10:52 UTC (permalink / raw)
  To: Intel-gfx

From: Dave Gordon <david.s.gordon@intel.com>

intel_lrc.c has a table of "logical rings" (meaning engines), while
intel_ringbuffer.c has separately open-coded initialisation for each
engine. We can deduplicate this somewhat by using the same first-stage
engine-setup function for both modes.

So here we expose the function that transfers information from the
static table of (all) known engines to the dev_priv->engine array of
engines available on this device (adjusting the names along the way)
and then embed calls to it in both the LRC and the legacy-mode setup.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 40 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 40 +++++++++------------------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  5 +++++
 3 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d47d1a0dbb91..39075a93ef91 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1953,8 +1953,9 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
 }
 
 static inline void
-logical_ring_default_irqs(struct intel_engine_cs *engine, unsigned shift)
+logical_ring_default_irqs(struct intel_engine_cs *engine)
 {
+	unsigned shift = engine->irq_shift;
 	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << shift;
 	engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
 }
@@ -2058,14 +2059,14 @@ static int logical_render_ring_init(struct intel_engine_cs *engine)
 	return ret;
 }
 
-static const struct logical_ring_info {
+static const struct engine_info {
 	const char *name;
 	unsigned exec_id;
 	unsigned guc_id;
 	u32 mmio_base;
 	unsigned irq_shift;
 	int (*init)(struct intel_engine_cs *engine);
-} logical_rings[] = {
+} intel_engines[] = {
 	[RCS] = {
 		.name = "render ring",
 		.exec_id = I915_EXEC_RENDER,
@@ -2108,20 +2109,31 @@ static const struct logical_ring_info {
 	},
 };
 
-static struct intel_engine_cs *
-logical_ring_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id)
+struct intel_engine_cs *
+intel_engine_setup(struct drm_i915_private *dev_priv,
+		   enum intel_engine_id id)
 {
-	const struct logical_ring_info *info = &logical_rings[id];
+	const struct engine_info *info = &intel_engines[id];
 	struct intel_engine_cs *engine = &dev_priv->engine[id];
-	enum forcewake_domains fw_domains;
 
 	engine->id = id;
+	engine->i915 = dev_priv;
 	engine->name = info->name;
 	engine->exec_id = info->exec_id;
-	engine->guc_id = info->guc_id;
+	engine->hw_id = engine->guc_id = info->guc_id;
 	engine->mmio_base = info->mmio_base;
+	engine->irq_shift = info->irq_shift;
 
-	engine->i915 = dev_priv;
+	return engine;
+}
+
+static struct intel_engine_cs *
+logical_ring_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id)
+{
+	struct intel_engine_cs *engine;
+	enum forcewake_domains fw_domains;
+
+	engine = intel_engine_setup(dev_priv, id);
 
 	/* Intentionally left blank. */
 	engine->buffer = NULL;
@@ -2151,7 +2163,7 @@ logical_ring_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id)
 
 	logical_ring_init_platform_invariants(engine);
 	logical_ring_default_vfuncs(engine);
-	logical_ring_default_irqs(engine, info->irq_shift);
+	logical_ring_default_irqs(engine);
 
 	intel_engine_init_hangcheck(engine);
 	i915_gem_batch_pool_init(&dev_priv->drm, &engine->batch_pool);
@@ -2180,14 +2192,14 @@ int intel_logical_rings_init(struct drm_device *dev)
 	WARN_ON(INTEL_INFO(dev_priv)->ring_mask &
 		GENMASK(sizeof(mask) * BITS_PER_BYTE - 1, I915_NUM_ENGINES));
 
-	for (i = 0; i < ARRAY_SIZE(logical_rings); i++) {
+	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
 		if (!HAS_ENGINE(dev_priv, i))
 			continue;
 
-		if (!logical_rings[i].init)
+		if (!intel_engines[i].init)
 			continue;
 
-		ret = logical_rings[i].init(logical_ring_setup(dev_priv, i));
+		ret = intel_engines[i].init(logical_ring_setup(dev_priv, i));
 		if (ret)
 			goto cleanup;
 
@@ -2195,7 +2207,7 @@ int intel_logical_rings_init(struct drm_device *dev)
 	}
 
 	/*
-	 * Catch failures to update logical_rings table when the new engines
+	 * Catch failures to update intel_engines table when the new engines
 	 * are added to the driver by a warning and disabling the forgotten
 	 * engines.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 61e00bf9e87f..c855ff9e28e4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2838,14 +2838,10 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 int intel_init_render_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine = &dev_priv->engine[RCS];
+	struct intel_engine_cs *engine;
 	int ret;
 
-	engine->name = "render ring";
-	engine->id = RCS;
-	engine->exec_id = I915_EXEC_RENDER;
-	engine->hw_id = 0;
-	engine->mmio_base = RENDER_RING_BASE;
+	engine = intel_engine_setup(dev_priv, RCS);
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2900,17 +2896,13 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 int intel_init_bsd_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine = &dev_priv->engine[VCS];
+	struct intel_engine_cs *engine;
 
-	engine->name = "bsd ring";
-	engine->id = VCS;
-	engine->exec_id = I915_EXEC_BSD;
-	engine->hw_id = 1;
+	engine = intel_engine_setup(dev_priv, VCS);
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	if (INTEL_GEN(dev_priv) >= 6) {
-		engine->mmio_base = GEN6_BSD_RING_BASE;
 		/* gen6 bsd needs a special wa for tail updates */
 		if (IS_GEN6(dev_priv))
 			engine->write_tail = gen6_bsd_ring_write_tail;
@@ -2938,13 +2930,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine = &dev_priv->engine[VCS2];
+	struct intel_engine_cs *engine;
 
-	engine->name = "bsd2 ring";
-	engine->id = VCS2;
-	engine->exec_id = I915_EXEC_BSD;
-	engine->hw_id = 4;
-	engine->mmio_base = GEN8_BSD2_RING_BASE;
+	engine = intel_engine_setup(dev_priv, VCS2);
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2958,13 +2946,9 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 int intel_init_blt_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine = &dev_priv->engine[BCS];
+	struct intel_engine_cs *engine;
 
-	engine->name = "blitter ring";
-	engine->id = BCS;
-	engine->exec_id = I915_EXEC_BLT;
-	engine->hw_id = 2;
-	engine->mmio_base = BLT_RING_BASE;
+	engine = intel_engine_setup(dev_priv, BCS);
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2981,13 +2965,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 int intel_init_vebox_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine = &dev_priv->engine[VECS];
+	struct intel_engine_cs *engine;
 
-	engine->name = "video enhancement ring";
-	engine->id = VECS;
-	engine->exec_id = I915_EXEC_VEBOX;
-	engine->hw_id = 3;
-	engine->mmio_base = VEBOX_RING_BASE;
+	engine = intel_engine_setup(dev_priv, VECS);
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 121294c602c3..806b7e7152b3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -147,6 +147,7 @@ struct intel_engine_cs {
 	unsigned int hw_id;
 	unsigned int guc_id; /* XXX same as hw_id? */
 	u32		mmio_base;
+	unsigned int irq_shift;
 	struct intel_ringbuffer *buffer;
 	struct list_head buffers;
 
@@ -359,6 +360,10 @@ struct intel_engine_cs {
 	u32 (*get_cmd_length_mask)(u32 cmd_header);
 };
 
+struct intel_engine_cs *
+intel_engine_setup(struct drm_i915_private *dev_priv,
+		   enum intel_engine_id id);
+
 static inline bool
 intel_engine_initialized(const struct intel_engine_cs *engine)
 {
-- 
1.9.1

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

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

* [PATCH 2/5] drm/i915: Prepare for engine init unification
  2016-07-06 10:52 [PATCH 1/5] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
@ 2016-07-06 10:52 ` Tvrtko Ursulin
  2016-07-06 10:52 ` [PATCH 3/5] drm/i915: Unify engine init loop Tvrtko Ursulin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 10:52 UTC (permalink / raw)
  To: Intel-gfx

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

Move the execlist engine setup to vfuncs so that the engine
init loop is clearly split into the mode agnostic and
specific steps.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 103 ++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 39075a93ef91..fcff30998227 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1978,6 +1978,46 @@ lrc_setup_hws(struct intel_engine_cs *engine,
 	return 0;
 }
 
+static void
+logical_ring_setup(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	enum forcewake_domains fw_domains;
+
+	/* Intentionally left blank. */
+	engine->buffer = NULL;
+
+	fw_domains = intel_uncore_forcewake_for_reg(dev_priv,
+						    RING_ELSP(engine),
+						    FW_REG_WRITE);
+
+	fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+						     RING_CONTEXT_STATUS_PTR(engine),
+						     FW_REG_READ | FW_REG_WRITE);
+
+	fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+						     RING_CONTEXT_STATUS_BUF_BASE(engine),
+						     FW_REG_READ);
+
+	engine->fw_domains = fw_domains;
+
+	INIT_LIST_HEAD(&engine->active_list);
+	INIT_LIST_HEAD(&engine->request_list);
+	INIT_LIST_HEAD(&engine->buffers);
+	INIT_LIST_HEAD(&engine->execlist_queue);
+	spin_lock_init(&engine->execlist_lock);
+
+	tasklet_init(&engine->irq_tasklet,
+		     intel_lrc_irq_handler, (unsigned long)engine);
+
+	logical_ring_init_platform_invariants(engine);
+	logical_ring_default_vfuncs(engine);
+	logical_ring_default_irqs(engine);
+
+	intel_engine_init_hangcheck(engine);
+	i915_gem_batch_pool_init(&dev_priv->drm, &engine->batch_pool);
+}
+
 static int
 logical_ring_init(struct intel_engine_cs *engine)
 {
@@ -2023,6 +2063,8 @@ static int logical_render_ring_init(struct intel_engine_cs *engine)
 	struct drm_i915_private *dev_priv = engine->i915;
 	int ret;
 
+	logical_ring_setup(engine);
+
 	if (HAS_L3_DPF(dev_priv))
 		engine->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
@@ -2059,6 +2101,13 @@ static int logical_render_ring_init(struct intel_engine_cs *engine)
 	return ret;
 }
 
+static int logical_xcs_ring_init(struct intel_engine_cs *engine)
+{
+	logical_ring_setup(engine);
+
+	return logical_ring_init(engine);
+}
+
 static const struct engine_info {
 	const char *name;
 	unsigned exec_id;
@@ -2081,7 +2130,7 @@ static const struct engine_info {
 		.guc_id = GUC_BLITTER_ENGINE,
 		.mmio_base = BLT_RING_BASE,
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
-		.init = logical_ring_init,
+		.init = logical_xcs_ring_init,
 	},
 	[VCS] = {
 		.name = "bsd ring",
@@ -2089,7 +2138,7 @@ static const struct engine_info {
 		.guc_id = GUC_VIDEO_ENGINE,
 		.mmio_base = GEN6_BSD_RING_BASE,
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
-		.init = logical_ring_init,
+		.init = logical_xcs_ring_init,
 	},
 	[VCS2] = {
 		.name = "bsd2 ring",
@@ -2097,7 +2146,7 @@ static const struct engine_info {
 		.guc_id = GUC_VIDEO_ENGINE2,
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
-		.init = logical_ring_init,
+		.init = logical_xcs_ring_init,
 	},
 	[VECS] = {
 		.name = "video enhancement ring",
@@ -2105,7 +2154,7 @@ static const struct engine_info {
 		.guc_id = GUC_VIDEOENHANCE_ENGINE,
 		.mmio_base = VEBOX_RING_BASE,
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
-		.init = logical_ring_init,
+		.init = logical_xcs_ring_init,
 	},
 };
 
@@ -2127,50 +2176,6 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	return engine;
 }
 
-static struct intel_engine_cs *
-logical_ring_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id)
-{
-	struct intel_engine_cs *engine;
-	enum forcewake_domains fw_domains;
-
-	engine = intel_engine_setup(dev_priv, id);
-
-	/* Intentionally left blank. */
-	engine->buffer = NULL;
-
-	fw_domains = intel_uncore_forcewake_for_reg(dev_priv,
-						    RING_ELSP(engine),
-						    FW_REG_WRITE);
-
-	fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
-						     RING_CONTEXT_STATUS_PTR(engine),
-						     FW_REG_READ | FW_REG_WRITE);
-
-	fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
-						     RING_CONTEXT_STATUS_BUF_BASE(engine),
-						     FW_REG_READ);
-
-	engine->fw_domains = fw_domains;
-
-	INIT_LIST_HEAD(&engine->active_list);
-	INIT_LIST_HEAD(&engine->request_list);
-	INIT_LIST_HEAD(&engine->buffers);
-	INIT_LIST_HEAD(&engine->execlist_queue);
-	spin_lock_init(&engine->execlist_lock);
-
-	tasklet_init(&engine->irq_tasklet,
-		     intel_lrc_irq_handler, (unsigned long)engine);
-
-	logical_ring_init_platform_invariants(engine);
-	logical_ring_default_vfuncs(engine);
-	logical_ring_default_irqs(engine);
-
-	intel_engine_init_hangcheck(engine);
-	i915_gem_batch_pool_init(&dev_priv->drm, &engine->batch_pool);
-
-	return engine;
-}
-
 /**
  * intel_logical_rings_init() - allocate, populate and init the Engine Command Streamers
  * @dev: DRM device.
@@ -2199,7 +2204,7 @@ int intel_logical_rings_init(struct drm_device *dev)
 		if (!intel_engines[i].init)
 			continue;
 
-		ret = intel_engines[i].init(logical_ring_setup(dev_priv, i));
+		ret = intel_engines[i].init(intel_engine_setup(dev_priv, i));
 		if (ret)
 			goto cleanup;
 
-- 
1.9.1

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

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

* [PATCH 3/5] drm/i915: Unify engine init loop
  2016-07-06 10:52 [PATCH 1/5] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
  2016-07-06 10:52 ` [PATCH 2/5] drm/i915: Prepare for engine init unification Tvrtko Ursulin
@ 2016-07-06 10:52 ` Tvrtko Ursulin
  2016-07-06 11:05   ` Chris Wilson
  2016-07-06 10:52 ` [PATCH 4/5] drm/i915: Make more use of the shared engine irq setup Tvrtko Ursulin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 10:52 UTC (permalink / raw)
  To: Intel-gfx

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

With the unified common engine setup done, and the execlist engine
initialization loop clearly split into two phases, we can eliminate
the separate legacy engine initialization code.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 --
 drivers/gpu/drm/i915/i915_gem.c         | 51 +--------------------------------
 drivers/gpu/drm/i915/intel_lrc.c        | 25 ++++++++++------
 drivers/gpu/drm/i915/intel_lrc.h        |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 45 ++++++++++-------------------
 drivers/gpu/drm/i915/intel_ringbuffer.h | 10 +++----
 6 files changed, 38 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c269e0ad4057..5eae10b90b39 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2015,7 +2015,6 @@ struct drm_i915_private {
 		int (*execbuf_submit)(struct i915_execbuffer_params *params,
 				      struct drm_i915_gem_execbuffer2 *args,
 				      struct list_head *vmas);
-		int (*init_engines)(struct drm_device *dev);
 		void (*cleanup_engine)(struct intel_engine_cs *engine);
 		void (*stop_engine)(struct intel_engine_cs *engine);
 
@@ -3362,7 +3361,6 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error)
 void i915_gem_reset(struct drm_device *dev);
 bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
 int __must_check i915_gem_init(struct drm_device *dev);
-int i915_gem_init_engines(struct drm_device *dev);
 int __must_check i915_gem_init_hw(struct drm_device *dev);
 void i915_gem_init_swizzling(struct drm_device *dev);
 void i915_gem_cleanup_engines(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8f50919ba9b4..a37af3a25ff3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5063,53 +5063,6 @@ static void init_unused_rings(struct drm_device *dev)
 	}
 }
 
-int i915_gem_init_engines(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
-
-	ret = intel_init_render_ring_buffer(dev);
-	if (ret)
-		return ret;
-
-	if (HAS_BSD(dev)) {
-		ret = intel_init_bsd_ring_buffer(dev);
-		if (ret)
-			goto cleanup_render_ring;
-	}
-
-	if (HAS_BLT(dev)) {
-		ret = intel_init_blt_ring_buffer(dev);
-		if (ret)
-			goto cleanup_bsd_ring;
-	}
-
-	if (HAS_VEBOX(dev)) {
-		ret = intel_init_vebox_ring_buffer(dev);
-		if (ret)
-			goto cleanup_blt_ring;
-	}
-
-	if (HAS_BSD2(dev)) {
-		ret = intel_init_bsd2_ring_buffer(dev);
-		if (ret)
-			goto cleanup_vebox_ring;
-	}
-
-	return 0;
-
-cleanup_vebox_ring:
-	intel_cleanup_engine(&dev_priv->engine[VECS]);
-cleanup_blt_ring:
-	intel_cleanup_engine(&dev_priv->engine[BCS]);
-cleanup_bsd_ring:
-	intel_cleanup_engine(&dev_priv->engine[VCS]);
-cleanup_render_ring:
-	intel_cleanup_engine(&dev_priv->engine[RCS]);
-
-	return ret;
-}
-
 int
 i915_gem_init_hw(struct drm_device *dev)
 {
@@ -5185,12 +5138,10 @@ int i915_gem_init(struct drm_device *dev)
 
 	if (!i915.enable_execlists) {
 		dev_priv->gt.execbuf_submit = i915_gem_ringbuffer_submission;
-		dev_priv->gt.init_engines = i915_gem_init_engines;
 		dev_priv->gt.cleanup_engine = intel_cleanup_engine;
 		dev_priv->gt.stop_engine = intel_stop_engine;
 	} else {
 		dev_priv->gt.execbuf_submit = intel_execlists_submission;
-		dev_priv->gt.init_engines = intel_logical_rings_init;
 		dev_priv->gt.cleanup_engine = intel_logical_ring_cleanup;
 		dev_priv->gt.stop_engine = intel_logical_ring_stop;
 	}
@@ -5210,7 +5161,7 @@ int i915_gem_init(struct drm_device *dev)
 	if (ret)
 		goto out_unlock;
 
-	ret = dev_priv->gt.init_engines(dev);
+	ret = intel_engines_init(dev);
 	if (ret)
 		goto out_unlock;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fcff30998227..5bebcb16e488 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2115,6 +2115,7 @@ static const struct engine_info {
 	u32 mmio_base;
 	unsigned irq_shift;
 	int (*init)(struct intel_engine_cs *engine);
+	int (*init_ringbuf)(struct intel_engine_cs *engine);
 } intel_engines[] = {
 	[RCS] = {
 		.name = "render ring",
@@ -2123,6 +2124,7 @@ static const struct engine_info {
 		.mmio_base = RENDER_RING_BASE,
 		.irq_shift = GEN8_RCS_IRQ_SHIFT,
 		.init = logical_render_ring_init,
+		.init_ringbuf = intel_init_render_ring_buffer,
 	},
 	[BCS] = {
 		.name = "blitter ring",
@@ -2131,6 +2133,7 @@ static const struct engine_info {
 		.mmio_base = BLT_RING_BASE,
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_blt_ring_buffer,
 	},
 	[VCS] = {
 		.name = "bsd ring",
@@ -2139,6 +2142,7 @@ static const struct engine_info {
 		.mmio_base = GEN6_BSD_RING_BASE,
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_bsd_ring_buffer,
 	},
 	[VCS2] = {
 		.name = "bsd2 ring",
@@ -2147,6 +2151,7 @@ static const struct engine_info {
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_bsd2_ring_buffer,
 	},
 	[VECS] = {
 		.name = "video enhancement ring",
@@ -2155,6 +2160,7 @@ static const struct engine_info {
 		.mmio_base = VEBOX_RING_BASE,
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
 		.init = logical_xcs_ring_init,
+		.init_ringbuf = intel_init_vebox_ring_buffer,
 	},
 };
 
@@ -2177,20 +2183,16 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 }
 
 /**
- * intel_logical_rings_init() - allocate, populate and init the Engine Command Streamers
+ * intel_engines_init() - allocate, populate and init the Engine Command Streamers
  * @dev: DRM device.
  *
- * This function inits the engines for an Execlists submission style (the
- * equivalent in the legacy ringbuffer submission world would be
- * i915_gem_init_engines). It does it only for those engines that are present in
- * the hardware.
- *
  * Return: non-zero if the initialization failed.
  */
-int intel_logical_rings_init(struct drm_device *dev)
+int intel_engines_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	unsigned int mask = 0;
+	int (*init)(struct intel_engine_cs *engine);
 	unsigned int i;
 	int ret;
 
@@ -2201,10 +2203,15 @@ int intel_logical_rings_init(struct drm_device *dev)
 		if (!HAS_ENGINE(dev_priv, i))
 			continue;
 
-		if (!intel_engines[i].init)
+		if (i915.enable_execlists)
+			init = intel_engines[i].init;
+		else
+			init = intel_engines[i].init_ringbuf;
+
+		if (!init)
 			continue;
 
-		ret = intel_engines[i].init(intel_engine_setup(dev_priv, i));
+		ret = init(intel_engine_setup(dev_priv, i));
 		if (ret)
 			goto cleanup;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 2b8255c19dcc..aa8905c42de7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -67,7 +67,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
 int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
 void intel_logical_ring_stop(struct intel_engine_cs *engine);
 void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
-int intel_logical_rings_init(struct drm_device *dev);
+int intel_engines_init(struct drm_device *dev);
 
 int logical_ring_flush_all_caches(struct drm_i915_gem_request *req);
 /**
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c855ff9e28e4..91ca268565e6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2835,14 +2835,11 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 	intel_ring_init_semaphores(dev_priv, engine);
 }
 
-int intel_init_render_ring_buffer(struct drm_device *dev)
+int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine;
+	struct drm_i915_private *dev_priv = engine->i915;
 	int ret;
 
-	engine = intel_engine_setup(dev_priv, RCS);
-
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
@@ -2876,7 +2873,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	engine->init_hw = init_render_ring;
 	engine->cleanup = render_ring_cleanup;
 
-	ret = intel_init_ring_buffer(dev, engine);
+	ret = intel_init_ring_buffer(&dev_priv->drm, engine);
 	if (ret)
 		return ret;
 
@@ -2893,12 +2890,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	return 0;
 }
 
-int intel_init_bsd_ring_buffer(struct drm_device *dev)
+int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, VCS);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2921,18 +2915,15 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
 	}
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(&dev_priv->drm, engine);
 }
 
 /**
  * Initialize the second BSD ring (eg. Broadwell GT3, Skylake GT3)
  */
-int intel_init_bsd2_ring_buffer(struct drm_device *dev)
+int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, VCS2);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2940,15 +2931,12 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(&dev_priv->drm, engine);
 }
 
-int intel_init_blt_ring_buffer(struct drm_device *dev)
+int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, BCS);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2959,15 +2947,12 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	else
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(&dev_priv->drm, engine);
 }
 
-int intel_init_vebox_ring_buffer(struct drm_device *dev)
+int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_engine_cs *engine;
-
-	engine = intel_engine_setup(dev_priv, VECS);
+	struct drm_i915_private *dev_priv = engine->i915;
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
@@ -2982,7 +2967,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		engine->irq_disable = hsw_vebox_irq_disable;
 	}
 
-	return intel_init_ring_buffer(dev, engine);
+	return intel_init_ring_buffer(&dev_priv->drm, engine);
 }
 
 int
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 806b7e7152b3..09ea5148ac2d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -482,11 +482,11 @@ int intel_ring_invalidate_all_caches(struct drm_i915_gem_request *req);
 int intel_init_pipe_control(struct intel_engine_cs *engine, int size);
 void intel_fini_pipe_control(struct intel_engine_cs *engine);
 
-int intel_init_render_ring_buffer(struct drm_device *dev);
-int intel_init_bsd_ring_buffer(struct drm_device *dev);
-int intel_init_bsd2_ring_buffer(struct drm_device *dev);
-int intel_init_blt_ring_buffer(struct drm_device *dev);
-int intel_init_vebox_ring_buffer(struct drm_device *dev);
+int intel_init_render_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_blt_ring_buffer(struct intel_engine_cs *engine);
+int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine);
 
 u64 intel_ring_get_active_head(struct intel_engine_cs *engine);
 static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine)
-- 
1.9.1

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

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

* [PATCH 4/5] drm/i915: Make more use of the shared engine irq setup
  2016-07-06 10:52 [PATCH 1/5] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
  2016-07-06 10:52 ` [PATCH 2/5] drm/i915: Prepare for engine init unification Tvrtko Ursulin
  2016-07-06 10:52 ` [PATCH 3/5] drm/i915: Unify engine init loop Tvrtko Ursulin
@ 2016-07-06 10:52 ` Tvrtko Ursulin
  2016-07-06 10:52 ` [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
  2016-07-06 11:23 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: unify first-stage engine struct setup Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 10:52 UTC (permalink / raw)
  To: Intel-gfx

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

Use more of the shared engine setup data for legacy engine
initialization. This time to simplify the irq initialization
code.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 91ca268565e6..6b24c1642d84 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2789,6 +2789,8 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 static void intel_ring_init_irq(struct drm_i915_private *dev_priv,
 				struct intel_engine_cs *engine)
 {
+	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << engine->irq_shift;
+
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable = gen8_irq_enable;
 		engine->irq_disable = gen8_irq_disable;
@@ -2842,7 +2844,6 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 
 	intel_ring_default_vfuncs(dev_priv, engine);
 
-	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 	if (HAS_L3_DPF(dev_priv))
 		engine->irq_keep_mask = GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
@@ -2901,10 +2902,7 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
 		if (IS_GEN6(dev_priv))
 			engine->write_tail = gen6_bsd_ring_write_tail;
 		engine->flush = gen6_bsd_ring_flush;
-		if (INTEL_GEN(dev_priv) >= 8)
-			engine->irq_enable_mask =
-				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
-		else
+		if (INTEL_GEN(dev_priv) < 8)
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
 	} else {
 		engine->mmio_base = BSD_RING_BASE;
@@ -2928,8 +2926,6 @@ int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_bsd_ring_flush;
-	engine->irq_enable_mask =
-			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
 
 	return intel_init_ring_buffer(&dev_priv->drm, engine);
 }
@@ -2941,10 +2937,7 @@ int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	if (INTEL_GEN(dev_priv) >= 8)
-		engine->irq_enable_mask =
-			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-	else
+	if (INTEL_GEN(dev_priv) < 8)
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
 
 	return intel_init_ring_buffer(&dev_priv->drm, engine);
@@ -2958,10 +2951,7 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 
 	engine->flush = gen6_ring_flush;
 
-	if (INTEL_GEN(dev_priv) >= 8) {
-		engine->irq_enable_mask =
-			GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
-	} else {
+	if (INTEL_GEN(dev_priv) < 8) {
 		engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
 		engine->irq_enable = hsw_vebox_irq_enable;
 		engine->irq_disable = hsw_vebox_irq_disable;
-- 
1.9.1

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

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

* [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype
  2016-07-06 10:52 [PATCH 1/5] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2016-07-06 10:52 ` [PATCH 4/5] drm/i915: Make more use of the shared engine irq setup Tvrtko Ursulin
@ 2016-07-06 10:52 ` Tvrtko Ursulin
  2016-07-06 10:58   ` Chris Wilson
  2016-07-06 11:23 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: unify first-stage engine struct setup Patchwork
  4 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 10:52 UTC (permalink / raw)
  To: Intel-gfx

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

Engine contains dev_priv so need to pass it in.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6b24c1642d84..26fe312005d1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2167,21 +2167,19 @@ static void intel_ring_context_unpin(struct i915_gem_context *ctx,
 	i915_gem_context_unreference(ctx);
 }
 
-static int intel_init_ring_buffer(struct drm_device *dev,
-				  struct intel_engine_cs *engine)
+static int intel_init_ring_buffer(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = engine->i915;
 	struct intel_ringbuffer *ringbuf;
 	int ret;
 
 	WARN_ON(engine->buffer);
 
-	engine->i915 = dev_priv;
 	INIT_LIST_HEAD(&engine->active_list);
 	INIT_LIST_HEAD(&engine->request_list);
 	INIT_LIST_HEAD(&engine->execlist_queue);
 	INIT_LIST_HEAD(&engine->buffers);
-	i915_gem_batch_pool_init(dev, &engine->batch_pool);
+	i915_gem_batch_pool_init(&dev_priv->drm, &engine->batch_pool);
 	memset(engine->semaphore.sync_seqno, 0,
 	       sizeof(engine->semaphore.sync_seqno));
 
@@ -2874,7 +2872,7 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 	engine->init_hw = init_render_ring;
 	engine->cleanup = render_ring_cleanup;
 
-	ret = intel_init_ring_buffer(&dev_priv->drm, engine);
+	ret = intel_init_ring_buffer(engine);
 	if (ret)
 		return ret;
 
@@ -2913,7 +2911,7 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
 	}
 
-	return intel_init_ring_buffer(&dev_priv->drm, engine);
+	return intel_init_ring_buffer(engine);
 }
 
 /**
@@ -2927,7 +2925,7 @@ int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine)
 
 	engine->flush = gen6_bsd_ring_flush;
 
-	return intel_init_ring_buffer(&dev_priv->drm, engine);
+	return intel_init_ring_buffer(engine);
 }
 
 int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
@@ -2940,7 +2938,7 @@ int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
 	if (INTEL_GEN(dev_priv) < 8)
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
 
-	return intel_init_ring_buffer(&dev_priv->drm, engine);
+	return intel_init_ring_buffer(engine);
 }
 
 int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
@@ -2957,7 +2955,7 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 		engine->irq_disable = hsw_vebox_irq_disable;
 	}
 
-	return intel_init_ring_buffer(&dev_priv->drm, engine);
+	return intel_init_ring_buffer(engine);
 }
 
 int
-- 
1.9.1

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

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

* Re: [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype
  2016-07-06 10:52 ` [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
@ 2016-07-06 10:58   ` Chris Wilson
  2016-07-06 12:38     ` Tvrtko Ursulin
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-07-06 10:58 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Jul 06, 2016 at 11:52:11AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Engine contains dev_priv so need to pass it in.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 6b24c1642d84..26fe312005d1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2167,21 +2167,19 @@ static void intel_ring_context_unpin(struct i915_gem_context *ctx,
>  	i915_gem_context_unreference(ctx);
>  }
>  
> -static int intel_init_ring_buffer(struct drm_device *dev,
> -				  struct intel_engine_cs *engine)
> +static int intel_init_ring_buffer(struct intel_engine_cs *engine)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = engine->i915;
>  	struct intel_ringbuffer *ringbuf;
>  	int ret;
>  
>  	WARN_ON(engine->buffer);
>  
> -	engine->i915 = dev_priv;
>  	INIT_LIST_HEAD(&engine->active_list);
>  	INIT_LIST_HEAD(&engine->request_list);
>  	INIT_LIST_HEAD(&engine->execlist_queue);
>  	INIT_LIST_HEAD(&engine->buffers);

Patch 6/5, Move all the common init code to the dispatch?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/5] drm/i915: Unify engine init loop
  2016-07-06 10:52 ` [PATCH 3/5] drm/i915: Unify engine init loop Tvrtko Ursulin
@ 2016-07-06 11:05   ` Chris Wilson
  2016-07-06 12:42     ` Tvrtko Ursulin
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-07-06 11:05 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Jul 06, 2016 at 11:52:09AM +0100, Tvrtko Ursulin wrote:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index fcff30998227..5bebcb16e488 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2115,6 +2115,7 @@ static const struct engine_info {
>  	u32 mmio_base;
>  	unsigned irq_shift;
>  	int (*init)(struct intel_engine_cs *engine);
> +	int (*init_ringbuf)(struct intel_engine_cs *engine);

Blatant favouritisim!

The consensus (all of 2, perhas 3, people) name was to call them

	intel_legacy_submission.c
	intel_execlists_submission.c
	intel_guc_submission.c

from that we would then have

	int (*init_legacy_sumission)(struct intel_engine_cs *engine);
	int (*init_execlists_sumission)(struct intel_engine_cs *engine);

init_ringbuf() means to me to initialise the ringbuffer structure
associated both with execlists and legacy.

What I tried a couple of years ago was doing all the immediate setup
inside intel_ringbuffer.c and then dispatching to intel_execlists.c for
it to override as needed.

> @@ -2201,10 +2203,15 @@ int intel_logical_rings_init(struct drm_device *dev)
>  		if (!HAS_ENGINE(dev_priv, i))
>  			continue;
>  
> -		if (!intel_engines[i].init)
> +		if (i915.enable_execlists)
> +			init = intel_engines[i].init;
> +		else
> +			init = intel_engines[i].init_ringbuf;

I think I prefer your patch, but I'd like to some all the common setup in
one place.  intel_engine_cs.c ?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: unify first-stage engine struct setup
  2016-07-06 10:52 [PATCH 1/5] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2016-07-06 10:52 ` [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
@ 2016-07-06 11:23 ` Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2016-07-06 11:23 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915: unify first-stage engine struct setup
URL   : https://patchwork.freedesktop.org/series/9551/
State : failure

== Summary ==

Series 9551v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/9551/revisions/1/mbox

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> INCOMPLETE (fi-skl-i7-6700k)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-bdw-i5-5250u)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                skip       -> DMESG-WARN (ro-bdw-i7-5557U)

fi-kbl-qkkr      total:235  pass:164  dwarn:28  dfail:1   fail:2   skip:40 
fi-skl-i5-6260u  total:235  pass:205  dwarn:0   dfail:2   fail:2   skip:26 
fi-skl-i7-6700k  total:106  pass:81   dwarn:0   dfail:1   fail:0   skip:23 
fi-snb-i7-2600   total:235  pass:179  dwarn:0   dfail:0   fail:2   skip:54 
ro-bdw-i5-5250u  total:229  pass:203  dwarn:1   dfail:1   fail:1   skip:23 
ro-bdw-i7-5557U  total:229  pass:204  dwarn:2   dfail:1   fail:0   skip:22 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-bsw-n3050     total:229  pass:177  dwarn:0   dfail:1   fail:2   skip:49 
ro-byt-n2820     total:229  pass:181  dwarn:0   dfail:1   fail:2   skip:45 
ro-hsw-i3-4010u  total:229  pass:197  dwarn:0   dfail:1   fail:0   skip:31 
ro-hsw-i7-4770r  total:229  pass:197  dwarn:0   dfail:1   fail:0   skip:31 
ro-ilk-i7-620lm  total:229  pass:157  dwarn:0   dfail:1   fail:1   skip:70 
ro-ilk1-i5-650   total:224  pass:157  dwarn:0   dfail:1   fail:1   skip:65 
ro-ivb-i7-3770   total:229  pass:188  dwarn:0   dfail:1   fail:0   skip:40 
ro-skl3-i5-6260u total:229  pass:208  dwarn:1   dfail:1   fail:0   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1432/

2a6e307 drm-intel-nightly: 2016y-07m-05d-11h-50m-09s UTC integration manifest
c23ce21 drm/i915: Simplify intel_init_ring_buffer prototype
1b214cc drm/i915: Make more use of the shared engine irq setup
ad357bb drm/i915: Unify engine init loop
4c4de2a drm/i915: Prepare for engine init unification
d84461b drm/i915: unify first-stage engine struct setup

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

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

* Re: [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype
  2016-07-06 10:58   ` Chris Wilson
@ 2016-07-06 12:38     ` Tvrtko Ursulin
  2016-07-06 12:47       ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 12:38 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 06/07/16 11:58, Chris Wilson wrote:
> On Wed, Jul 06, 2016 at 11:52:11AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Engine contains dev_priv so need to pass it in.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++----------
>>   1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index 6b24c1642d84..26fe312005d1 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -2167,21 +2167,19 @@ static void intel_ring_context_unpin(struct i915_gem_context *ctx,
>>   	i915_gem_context_unreference(ctx);
>>   }
>>
>> -static int intel_init_ring_buffer(struct drm_device *dev,
>> -				  struct intel_engine_cs *engine)
>> +static int intel_init_ring_buffer(struct intel_engine_cs *engine)
>>   {
>> -	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct drm_i915_private *dev_priv = engine->i915;
>>   	struct intel_ringbuffer *ringbuf;
>>   	int ret;
>>
>>   	WARN_ON(engine->buffer);
>>
>> -	engine->i915 = dev_priv;
>>   	INIT_LIST_HEAD(&engine->active_list);
>>   	INIT_LIST_HEAD(&engine->request_list);
>>   	INIT_LIST_HEAD(&engine->execlist_queue);
>>   	INIT_LIST_HEAD(&engine->buffers);
>
> Patch 6/5, Move all the common init code to the dispatch?

What do you mean by dispatch?

But yes, that could be the next step.

Regards,

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

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

* Re: [PATCH 3/5] drm/i915: Unify engine init loop
  2016-07-06 11:05   ` Chris Wilson
@ 2016-07-06 12:42     ` Tvrtko Ursulin
  0 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-07-06 12:42 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 06/07/16 12:05, Chris Wilson wrote:
> On Wed, Jul 06, 2016 at 11:52:09AM +0100, Tvrtko Ursulin wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index fcff30998227..5bebcb16e488 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -2115,6 +2115,7 @@ static const struct engine_info {
>>   	u32 mmio_base;
>>   	unsigned irq_shift;
>>   	int (*init)(struct intel_engine_cs *engine);
>> +	int (*init_ringbuf)(struct intel_engine_cs *engine);
>
> Blatant favouritisim!
>
> The consensus (all of 2, perhas 3, people) name was to call them
>
> 	intel_legacy_submission.c
> 	intel_execlists_submission.c
> 	intel_guc_submission.c
>
> from that we would then have
>
> 	int (*init_legacy_sumission)(struct intel_engine_cs *engine);
> 	int (*init_execlists_sumission)(struct intel_engine_cs *engine);
>
> init_ringbuf() means to me to initialise the ringbuffer structure
> associated both with execlists and legacy.

Agreed on the naming.

> What I tried a couple of years ago was doing all the immediate setup
> inside intel_ringbuffer.c and then dispatching to intel_execlists.c for
> it to override as needed.
>
>> @@ -2201,10 +2203,15 @@ int intel_logical_rings_init(struct drm_device *dev)
>>   		if (!HAS_ENGINE(dev_priv, i))
>>   			continue;
>>
>> -		if (!intel_engines[i].init)
>> +		if (i915.enable_execlists)
>> +			init = intel_engines[i].init;
>> +		else
>> +			init = intel_engines[i].init_ringbuf;
>
> I think I prefer your patch, but I'd like to some all the common setup in
> one place.  intel_engine_cs.c ?

Yes I was thinking that it needs a new home now. intel_engine_cs sounds OK.

Regards,

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

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

* Re: [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype
  2016-07-06 12:38     ` Tvrtko Ursulin
@ 2016-07-06 12:47       ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-07-06 12:47 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Jul 06, 2016 at 01:38:49PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/07/16 11:58, Chris Wilson wrote:
> >On Wed, Jul 06, 2016 at 11:52:11AM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>Engine contains dev_priv so need to pass it in.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++----------
> >>  1 file changed, 8 insertions(+), 10 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>index 6b24c1642d84..26fe312005d1 100644
> >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>@@ -2167,21 +2167,19 @@ static void intel_ring_context_unpin(struct i915_gem_context *ctx,
> >>  	i915_gem_context_unreference(ctx);
> >>  }
> >>
> >>-static int intel_init_ring_buffer(struct drm_device *dev,
> >>-				  struct intel_engine_cs *engine)
> >>+static int intel_init_ring_buffer(struct intel_engine_cs *engine)
> >>  {
> >>-	struct drm_i915_private *dev_priv = to_i915(dev);
> >>+	struct drm_i915_private *dev_priv = engine->i915;
> >>  	struct intel_ringbuffer *ringbuf;
> >>  	int ret;
> >>
> >>  	WARN_ON(engine->buffer);
> >>
> >>-	engine->i915 = dev_priv;
> >>  	INIT_LIST_HEAD(&engine->active_list);
> >>  	INIT_LIST_HEAD(&engine->request_list);
> >>  	INIT_LIST_HEAD(&engine->execlist_queue);
> >>  	INIT_LIST_HEAD(&engine->buffers);
> >
> >Patch 6/5, Move all the common init code to the dispatch?
> 
> What do you mean by dispatch?

The if-else-chain in the caller. These can all be done there since they
are common to all implementations.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-07-06 12:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-06 10:52 [PATCH 1/5] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
2016-07-06 10:52 ` [PATCH 2/5] drm/i915: Prepare for engine init unification Tvrtko Ursulin
2016-07-06 10:52 ` [PATCH 3/5] drm/i915: Unify engine init loop Tvrtko Ursulin
2016-07-06 11:05   ` Chris Wilson
2016-07-06 12:42     ` Tvrtko Ursulin
2016-07-06 10:52 ` [PATCH 4/5] drm/i915: Make more use of the shared engine irq setup Tvrtko Ursulin
2016-07-06 10:52 ` [PATCH 5/5] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
2016-07-06 10:58   ` Chris Wilson
2016-07-06 12:38     ` Tvrtko Ursulin
2016-07-06 12:47       ` Chris Wilson
2016-07-06 11:23 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: unify first-stage engine struct setup Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2016-07-01 16:47 [PATCH 1/5] " Tvrtko Ursulin
2016-07-01 16:47 ` [PATCH 3/5] drm/i915: Unify engine init loop Tvrtko Ursulin

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