All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/7] drm/i915: unify first-stage engine struct setup
@ 2016-07-13 15:03 Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 2/7] drm/i915: Prepare for engine init unification Tvrtko Ursulin
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 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 70c699043d0e..1eb6d4608eff 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1978,8 +1978,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;
 }
@@ -2083,14 +2084,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,
@@ -2133,20 +2134,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;
@@ -2176,7 +2188,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);
@@ -2205,14 +2217,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;
 
@@ -2220,7 +2232,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 c8e77c082b21..6db194789f71 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2839,14 +2839,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);
 
@@ -2901,17 +2897,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;
@@ -2939,13 +2931,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);
 
@@ -2959,13 +2947,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);
 
@@ -2982,13 +2966,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 12cb7ed90014..f8eeb50868c3 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;
 
@@ -361,6 +362,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] 15+ messages in thread
* [PATCH 2/7] drm/i915: Prepare for engine init unification
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
@ 2016-07-13 15:03 ` Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 3/7] drm/i915: Unify engine init loop Tvrtko Ursulin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 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>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 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 1eb6d4608eff..604cfbb8aec9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2003,6 +2003,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)
 {
@@ -2048,6 +2088,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;
 
@@ -2084,6 +2126,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;
@@ -2106,7 +2155,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",
@@ -2114,7 +2163,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",
@@ -2122,7 +2171,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",
@@ -2130,7 +2179,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,
 	},
 };
 
@@ -2152,50 +2201,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.
@@ -2224,7 +2229,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] 15+ messages in thread
* [PATCH 3/7] drm/i915: Unify engine init loop
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 2/7] drm/i915: Prepare for engine init unification Tvrtko Ursulin
@ 2016-07-13 15:03 ` Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 4/7] drm/i915: Make more use of the shared engine irq setup Tvrtko Ursulin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 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.

v2: Fix cleanup path for legacy.
v3: Rename constructors. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 --
 drivers/gpu/drm/i915/i915_gem.c         | 51 +--------------------------------
 drivers/gpu/drm/i915/intel_lrc.c        | 45 ++++++++++++++++++-----------
 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, 50 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e76cfe2e2471..65d69e52f9c4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2016,7 +2016,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);
 
@@ -3374,7 +3373,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 adeca0ec4cfb..b788f97cd4cf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5056,53 +5056,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)
 {
@@ -5178,12 +5131,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;
 	}
@@ -5203,7 +5154,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 604cfbb8aec9..2e13a3a26245 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2139,7 +2139,8 @@ static const struct engine_info {
 	unsigned guc_id;
 	u32 mmio_base;
 	unsigned irq_shift;
-	int (*init)(struct intel_engine_cs *engine);
+	int (*init_legacy)(struct intel_engine_cs *engine);
+	int (*init_execlists)(struct intel_engine_cs *engine);
 } intel_engines[] = {
 	[RCS] = {
 		.name = "render ring",
@@ -2147,7 +2148,8 @@ static const struct engine_info {
 		.guc_id = GUC_RENDER_ENGINE,
 		.mmio_base = RENDER_RING_BASE,
 		.irq_shift = GEN8_RCS_IRQ_SHIFT,
-		.init = logical_render_ring_init,
+		.init_execlists = logical_render_ring_init,
+		.init_legacy = intel_init_render_ring_buffer,
 	},
 	[BCS] = {
 		.name = "blitter ring",
@@ -2155,7 +2157,8 @@ static const struct engine_info {
 		.guc_id = GUC_BLITTER_ENGINE,
 		.mmio_base = BLT_RING_BASE,
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
-		.init = logical_xcs_ring_init,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_blt_ring_buffer,
 	},
 	[VCS] = {
 		.name = "bsd ring",
@@ -2163,7 +2166,8 @@ static const struct engine_info {
 		.guc_id = GUC_VIDEO_ENGINE,
 		.mmio_base = GEN6_BSD_RING_BASE,
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
-		.init = logical_xcs_ring_init,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_bsd_ring_buffer,
 	},
 	[VCS2] = {
 		.name = "bsd2 ring",
@@ -2171,7 +2175,8 @@ static const struct engine_info {
 		.guc_id = GUC_VIDEO_ENGINE2,
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
-		.init = logical_xcs_ring_init,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_bsd2_ring_buffer,
 	},
 	[VECS] = {
 		.name = "video enhancement ring",
@@ -2179,7 +2184,8 @@ static const struct engine_info {
 		.guc_id = GUC_VIDEOENHANCE_ENGINE,
 		.mmio_base = VEBOX_RING_BASE,
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
-		.init = logical_xcs_ring_init,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_vebox_ring_buffer,
 	},
 };
 
@@ -2202,20 +2208,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;
 
@@ -2226,10 +2228,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_execlists;
+		else
+			init = intel_engines[i].init_legacy;
+
+		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;
 
@@ -2250,8 +2257,12 @@ int intel_logical_rings_init(struct drm_device *dev)
 	return 0;
 
 cleanup:
-	for (i = 0; i < I915_NUM_ENGINES; i++)
-		intel_logical_ring_cleanup(&dev_priv->engine[i]);
+	for (i = 0; i < I915_NUM_ENGINES; i++) {
+		if (i915.enable_execlists)
+			intel_logical_ring_cleanup(&dev_priv->engine[i]);
+		else
+			intel_cleanup_engine(&dev_priv->engine[i]);
+	}
 
 	return ret;
 }
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 6db194789f71..16ced275c84d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2836,14 +2836,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;
@@ -2877,7 +2874,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;
 
@@ -2894,12 +2891,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);
 
@@ -2922,18 +2916,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);
 
@@ -2941,15 +2932,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);
 
@@ -2960,15 +2948,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);
 
@@ -2983,7 +2968,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 f8eeb50868c3..a25eac17efcd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -484,11 +484,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] 15+ messages in thread
* [PATCH 4/7] drm/i915: Make more use of the shared engine irq setup
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 2/7] drm/i915: Prepare for engine init unification Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 3/7] drm/i915: Unify engine init loop Tvrtko Ursulin
@ 2016-07-13 15:03 ` Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 5/7] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 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 16ced275c84d..62f8c777cef1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2790,6 +2790,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;
@@ -2843,7 +2845,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;
 
@@ -2902,10 +2903,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;
@@ -2929,8 +2927,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);
 }
@@ -2942,10 +2938,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);
@@ -2959,10 +2952,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] 15+ messages in thread
* [PATCH 5/7] drm/i915: Simplify intel_init_ring_buffer prototype
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2016-07-13 15:03 ` [PATCH 4/7] drm/i915: Make more use of the shared engine irq setup Tvrtko Ursulin
@ 2016-07-13 15:03 ` Tvrtko Ursulin
  2016-07-13 15:03 ` [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c Tvrtko Ursulin
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 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>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 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 62f8c777cef1..3a8df752ceae 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2168,21 +2168,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));
 
@@ -2875,7 +2873,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;
 
@@ -2914,7 +2912,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);
 }
 
 /**
@@ -2928,7 +2926,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)
@@ -2941,7 +2939,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)
@@ -2958,7 +2956,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] 15+ messages in thread
* [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2016-07-13 15:03 ` [PATCH 5/7] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
@ 2016-07-13 15:03 ` Tvrtko Ursulin
  2016-07-13 16:04   ` Chris Wilson
  2016-07-13 15:03 ` [PATCH 7/7] drm/i915: Pull out some more common engine init code Tvrtko Ursulin
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 UTC (permalink / raw)
  To: Intel-gfx

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

Common code deserves to be put in a separate file from legacy and
execlists implementation for clarity and ease of maintenance.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/intel_engine_cs.c  | 162 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        | 138 +--------------------------
 drivers/gpu/drm/i915/intel_lrc.h        |   3 +
 drivers/gpu/drm/i915/intel_ringbuffer.h |   4 -
 5 files changed, 168 insertions(+), 140 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_engine_cs.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 684fc1cd08fa..75318ebb8d25 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -40,6 +40,7 @@ i915-y += i915_cmd_parser.o \
 	  i915_gpu_error.o \
 	  i915_trace_points.o \
 	  intel_breadcrumbs.o \
+	  intel_engine_cs.o \
 	  intel_lrc.o \
 	  intel_mocs.o \
 	  intel_ringbuffer.o \
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
new file mode 100644
index 000000000000..80117b499024
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -0,0 +1,162 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+#include "intel_ringbuffer.h"
+#include "intel_lrc.h"
+
+static const struct engine_info {
+	const char *name;
+	unsigned exec_id;
+	unsigned guc_id;
+	u32 mmio_base;
+	unsigned irq_shift;
+	int (*init_legacy)(struct intel_engine_cs *engine);
+	int (*init_execlists)(struct intel_engine_cs *engine);
+} intel_engines[] = {
+	[RCS] = {
+		.name = "render ring",
+		.exec_id = I915_EXEC_RENDER,
+		.guc_id = GUC_RENDER_ENGINE,
+		.mmio_base = RENDER_RING_BASE,
+		.irq_shift = GEN8_RCS_IRQ_SHIFT,
+		.init_execlists = logical_render_ring_init,
+		.init_legacy = intel_init_render_ring_buffer,
+	},
+	[BCS] = {
+		.name = "blitter ring",
+		.exec_id = I915_EXEC_BLT,
+		.guc_id = GUC_BLITTER_ENGINE,
+		.mmio_base = BLT_RING_BASE,
+		.irq_shift = GEN8_BCS_IRQ_SHIFT,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_blt_ring_buffer,
+	},
+	[VCS] = {
+		.name = "bsd ring",
+		.exec_id = I915_EXEC_BSD,
+		.guc_id = GUC_VIDEO_ENGINE,
+		.mmio_base = GEN6_BSD_RING_BASE,
+		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_bsd_ring_buffer,
+	},
+	[VCS2] = {
+		.name = "bsd2 ring",
+		.exec_id = I915_EXEC_BSD,
+		.guc_id = GUC_VIDEO_ENGINE2,
+		.mmio_base = GEN8_BSD2_RING_BASE,
+		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_bsd2_ring_buffer,
+	},
+	[VECS] = {
+		.name = "video enhancement ring",
+		.exec_id = I915_EXEC_VEBOX,
+		.guc_id = GUC_VIDEOENHANCE_ENGINE,
+		.mmio_base = VEBOX_RING_BASE,
+		.irq_shift = GEN8_VECS_IRQ_SHIFT,
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_vebox_ring_buffer,
+	},
+};
+
+static struct intel_engine_cs *
+intel_engine_setup(struct drm_i915_private *dev_priv,
+		   enum intel_engine_id id)
+{
+	const struct engine_info *info = &intel_engines[id];
+	struct intel_engine_cs *engine = &dev_priv->engine[id];
+
+	engine->id = id;
+	engine->i915 = dev_priv;
+	engine->name = info->name;
+	engine->exec_id = info->exec_id;
+	engine->hw_id = engine->guc_id = info->guc_id;
+	engine->mmio_base = info->mmio_base;
+	engine->irq_shift = info->irq_shift;
+
+	return engine;
+}
+
+/**
+ * intel_engines_init() - allocate, populate and init the Engine Command Streamers
+ * @dev: DRM device.
+ *
+ * Return: non-zero if the initialization failed.
+ */
+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;
+
+	WARN_ON(INTEL_INFO(dev_priv)->ring_mask &
+		GENMASK(sizeof(mask) * BITS_PER_BYTE - 1, I915_NUM_ENGINES));
+
+	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
+		if (!HAS_ENGINE(dev_priv, i))
+			continue;
+
+		if (i915.enable_execlists)
+			init = intel_engines[i].init_execlists;
+		else
+			init = intel_engines[i].init_legacy;
+
+		if (!init)
+			continue;
+
+		ret = init(intel_engine_setup(dev_priv, i));
+		if (ret)
+			goto cleanup;
+
+		mask |= ENGINE_MASK(i);
+	}
+
+	/*
+	 * Catch failures to update intel_engines table when the new engines
+	 * are added to the driver by a warning and disabling the forgotten
+	 * engines.
+	 */
+	if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
+		struct intel_device_info *info =
+			(struct intel_device_info *)&dev_priv->info;
+		info->ring_mask = mask;
+	}
+
+	return 0;
+
+cleanup:
+	for (i = 0; i < I915_NUM_ENGINES; i++) {
+		if (i915.enable_execlists)
+			intel_logical_ring_cleanup(&dev_priv->engine[i]);
+		else
+			intel_cleanup_engine(&dev_priv->engine[i]);
+	}
+
+	return ret;
+}
+
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2e13a3a26245..6e88d9af9328 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2083,7 +2083,7 @@ error:
 	return ret;
 }
 
-static int logical_render_ring_init(struct intel_engine_cs *engine)
+int logical_render_ring_init(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 	int ret;
@@ -2126,147 +2126,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)
+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;
-	unsigned guc_id;
-	u32 mmio_base;
-	unsigned irq_shift;
-	int (*init_legacy)(struct intel_engine_cs *engine);
-	int (*init_execlists)(struct intel_engine_cs *engine);
-} intel_engines[] = {
-	[RCS] = {
-		.name = "render ring",
-		.exec_id = I915_EXEC_RENDER,
-		.guc_id = GUC_RENDER_ENGINE,
-		.mmio_base = RENDER_RING_BASE,
-		.irq_shift = GEN8_RCS_IRQ_SHIFT,
-		.init_execlists = logical_render_ring_init,
-		.init_legacy = intel_init_render_ring_buffer,
-	},
-	[BCS] = {
-		.name = "blitter ring",
-		.exec_id = I915_EXEC_BLT,
-		.guc_id = GUC_BLITTER_ENGINE,
-		.mmio_base = BLT_RING_BASE,
-		.irq_shift = GEN8_BCS_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_blt_ring_buffer,
-	},
-	[VCS] = {
-		.name = "bsd ring",
-		.exec_id = I915_EXEC_BSD,
-		.guc_id = GUC_VIDEO_ENGINE,
-		.mmio_base = GEN6_BSD_RING_BASE,
-		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_bsd_ring_buffer,
-	},
-	[VCS2] = {
-		.name = "bsd2 ring",
-		.exec_id = I915_EXEC_BSD,
-		.guc_id = GUC_VIDEO_ENGINE2,
-		.mmio_base = GEN8_BSD2_RING_BASE,
-		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_bsd2_ring_buffer,
-	},
-	[VECS] = {
-		.name = "video enhancement ring",
-		.exec_id = I915_EXEC_VEBOX,
-		.guc_id = GUC_VIDEOENHANCE_ENGINE,
-		.mmio_base = VEBOX_RING_BASE,
-		.irq_shift = GEN8_VECS_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_vebox_ring_buffer,
-	},
-};
-
-struct intel_engine_cs *
-intel_engine_setup(struct drm_i915_private *dev_priv,
-		   enum intel_engine_id id)
-{
-	const struct engine_info *info = &intel_engines[id];
-	struct intel_engine_cs *engine = &dev_priv->engine[id];
-
-	engine->id = id;
-	engine->i915 = dev_priv;
-	engine->name = info->name;
-	engine->exec_id = info->exec_id;
-	engine->hw_id = engine->guc_id = info->guc_id;
-	engine->mmio_base = info->mmio_base;
-	engine->irq_shift = info->irq_shift;
-
-	return engine;
-}
-
-/**
- * intel_engines_init() - allocate, populate and init the Engine Command Streamers
- * @dev: DRM device.
- *
- * Return: non-zero if the initialization failed.
- */
-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;
-
-	WARN_ON(INTEL_INFO(dev_priv)->ring_mask &
-		GENMASK(sizeof(mask) * BITS_PER_BYTE - 1, I915_NUM_ENGINES));
-
-	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
-		if (!HAS_ENGINE(dev_priv, i))
-			continue;
-
-		if (i915.enable_execlists)
-			init = intel_engines[i].init_execlists;
-		else
-			init = intel_engines[i].init_legacy;
-
-		if (!init)
-			continue;
-
-		ret = init(intel_engine_setup(dev_priv, i));
-		if (ret)
-			goto cleanup;
-
-		mask |= ENGINE_MASK(i);
-	}
-
-	/*
-	 * Catch failures to update intel_engines table when the new engines
-	 * are added to the driver by a warning and disabling the forgotten
-	 * engines.
-	 */
-	if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
-		struct intel_device_info *info =
-			(struct intel_device_info *)&dev_priv->info;
-		info->ring_mask = mask;
-	}
-
-	return 0;
-
-cleanup:
-	for (i = 0; i < I915_NUM_ENGINES; i++) {
-		if (i915.enable_execlists)
-			intel_logical_ring_cleanup(&dev_priv->engine[i]);
-		else
-			intel_cleanup_engine(&dev_priv->engine[i]);
-	}
-
-	return ret;
-}
-
 static u32
 make_rpcs(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index aa8905c42de7..938e3eec06cf 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -67,6 +67,9 @@ 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 logical_render_ring_init(struct intel_engine_cs *engine);
+int logical_xcs_ring_init(struct intel_engine_cs *engine);
+
 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.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a25eac17efcd..db7613e8d585 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -362,10 +362,6 @@ 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] 15+ messages in thread
* [PATCH 7/7] drm/i915: Pull out some more common engine init code
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  2016-07-13 15:03 ` [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c Tvrtko Ursulin
@ 2016-07-13 15:03 ` Tvrtko Ursulin
  2016-07-13 15:57 ` ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup Patchwork
  2016-07-13 16:01 ` [PATCH 1/7] " Chris Wilson
  7 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-13 15:03 UTC (permalink / raw)
  To: Intel-gfx

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

Created two common helpers for engine setup and engine init phases
respectively to help with code sharing.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 47 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        | 17 +++---------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++----------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  3 +++
 4 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 80117b499024..e3c9f04ea51e 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -160,3 +160,50 @@ cleanup:
 	return ret;
 }
 
+void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
+{
+	memset(&engine->hangcheck, 0, sizeof(engine->hangcheck));
+}
+
+/**
+ * intel_engines_setup_common - setup engine state not requiring hw access
+ * @engine: Engine to setup.
+ *
+ * Initializes @engine@ structure members shared between legacy and execlists
+ * submission modes which do not require hardware access.
+ *
+ * Typically done early in the submission mode specific engine setup stage.
+ */
+void intel_engine_setup_common(struct intel_engine_cs *engine)
+{
+	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);
+
+	intel_engine_init_hangcheck(engine);
+	i915_gem_batch_pool_init(&engine->i915->drm, &engine->batch_pool);
+}
+
+/**
+ * intel_engines_init_common - initialize cengine state which might require hw access
+ * @engine: Engine to initialize.
+ *
+ * Initializes @engine@ structure members shared between legacy and execlists
+ * submission modes which do require hardware access.
+ *
+ * Typcally done at later stages of submission mode specific engine setup.
+ *
+ * Returns zero on success or an error code on failure.
+ */
+int intel_engine_init_common(struct intel_engine_cs *engine)
+{
+	int ret;
+
+	ret = intel_engine_init_breadcrumbs(engine);
+	if (ret)
+		return ret;
+
+	return i915_cmd_parser_init_ring(engine);
+}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6e88d9af9328..b6af635e3a0f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2009,6 +2009,8 @@ logical_ring_setup(struct intel_engine_cs *engine)
 	struct drm_i915_private *dev_priv = engine->i915;
 	enum forcewake_domains fw_domains;
 
+	intel_engine_setup_common(engine);
+
 	/* Intentionally left blank. */
 	engine->buffer = NULL;
 
@@ -2026,21 +2028,12 @@ logical_ring_setup(struct intel_engine_cs *engine)
 
 	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
@@ -2049,11 +2042,7 @@ logical_ring_init(struct intel_engine_cs *engine)
 	struct i915_gem_context *dctx = engine->i915->kernel_context;
 	int ret;
 
-	ret = intel_engine_init_breadcrumbs(engine);
-	if (ret)
-		goto error;
-
-	ret = i915_cmd_parser_init_ring(engine);
+	ret = intel_engine_init_common(engine);
 	if (ret)
 		goto error;
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3a8df752ceae..94c8ef461721 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -549,11 +549,6 @@ static bool stop_ring(struct intel_engine_cs *engine)
 	return (I915_READ_HEAD(engine) & HEAD_ADDR) == 0;
 }
 
-void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
-{
-	memset(&engine->hangcheck, 0, sizeof(engine->hangcheck));
-}
-
 static int init_ring_common(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
@@ -2176,15 +2171,12 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine)
 
 	WARN_ON(engine->buffer);
 
-	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_priv->drm, &engine->batch_pool);
+	intel_engine_setup_common(engine);
+
 	memset(engine->semaphore.sync_seqno, 0,
 	       sizeof(engine->semaphore.sync_seqno));
 
-	ret = intel_engine_init_breadcrumbs(engine);
+	ret = intel_engine_init_common(engine);
 	if (ret)
 		goto error;
 
@@ -2225,10 +2217,6 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine)
 		goto error;
 	}
 
-	ret = i915_cmd_parser_init_ring(engine);
-	if (ret)
-		goto error;
-
 	return 0;
 
 error:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index db7613e8d585..df7587ab1ba7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -480,6 +480,9 @@ 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);
 
+void intel_engine_setup_common(struct intel_engine_cs *engine);
+int intel_engine_init_common(struct intel_engine_cs *engine);
+
 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);
-- 
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] 15+ messages in thread
* ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (5 preceding siblings ...)
  2016-07-13 15:03 ` [PATCH 7/7] drm/i915: Pull out some more common engine init code Tvrtko Ursulin
@ 2016-07-13 15:57 ` Patchwork
  2016-07-14 10:20   ` Tvrtko Ursulin
  2016-07-13 16:01 ` [PATCH 1/7] " Chris Wilson
  7 siblings, 1 reply; 15+ messages in thread
From: Patchwork @ 2016-07-13 15:57 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

Test prime_self_import:
        Subgroup basic-llseek-size:
                incomplete -> PASS       (ro-byt-n2820)

fi-kbl-qkkr      total:237  pass:174  dwarn:25  dfail:4   fail:7   skip:27 
fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26 
fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40 
ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16 
ro-bdw-i7-5557U  total:237  pass:211  dwarn:1   dfail:0   fail:9   skip:16 
ro-bdw-i7-5600u  total:237  pass:195  dwarn:2   dfail:0   fail:9   skip:31 
ro-bsw-n3050     total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42 
ro-byt-n2820     total:237  pass:178  dwarn:0   dfail:0   fail:7   skip:38 
ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24 
ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24 
ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
ro-skl3-i5-6260u total:237  pass:213  dwarn:3   dfail:0   fail:9   skip:12 
ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1486/

5cd2699 drm-intel-nightly: 2016y-07m-13d-14h-43m-27s UTC integration manifest
f0b1666 drm/i915: Pull out some more common engine init code
be8c237 drm/i915: Move common engine setup into intel_engine_cs.c
a4cb7bc drm/i915: Simplify intel_init_ring_buffer prototype
d3dfae1 drm/i915: Make more use of the shared engine irq setup
ef59023 drm/i915: Unify engine init loop
ac58b8a drm/i915: Prepare for engine init unification
8120dcf 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] 15+ messages in thread
* Re: [PATCH 1/7] drm/i915: unify first-stage engine struct setup
  2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
                   ` (6 preceding siblings ...)
  2016-07-13 15:57 ` ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup Patchwork
@ 2016-07-13 16:01 ` Chris Wilson
  2016-07-14  9:19   ` Tvrtko Ursulin
  7 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2016-07-13 16:01 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Jul 13, 2016 at 04:03:35PM +0100, Tvrtko Ursulin wrote:
> 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>

LGTM, I didn't see anything that would impede applying this series,
so
Reviewed-by: Chris Wilson <chris-wilson.co.uk>

(Goes off to double check...)
-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] 15+ messages in thread
* Re: [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c
  2016-07-13 15:03 ` [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c Tvrtko Ursulin
@ 2016-07-13 16:04   ` Chris Wilson
  2016-07-14  9:25     ` Tvrtko Ursulin
  0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2016-07-13 16:04 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Jul 13, 2016 at 04:03:40PM +0100, Tvrtko Ursulin wrote:
> +	/*
> +	 * Catch failures to update intel_engines table when the new engines
> +	 * are added to the driver by a warning and disabling the forgotten
> +	 * engines.
> +	 */
> +	if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
> +		struct intel_device_info *info =
> +			(struct intel_device_info *)&dev_priv->info;

I snuck in mkwrite_device_info(), so

if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask))
	mkwrite_device_info(dev_priv)->ring_mask = mask;
-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] 15+ messages in thread
* Re: [PATCH 1/7] drm/i915: unify first-stage engine struct setup
  2016-07-13 16:01 ` [PATCH 1/7] " Chris Wilson
@ 2016-07-14  9:19   ` Tvrtko Ursulin
  2016-07-14 10:08     ` Chris Wilson
  0 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-14  9:19 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 13/07/16 17:01, Chris Wilson wrote:
> On Wed, Jul 13, 2016 at 04:03:35PM +0100, Tvrtko Ursulin wrote:
>> 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>
>
> LGTM, I didn't see anything that would impede applying this series,
> so
> Reviewed-by: Chris Wilson <chris-wilson.co.uk>
>
> (Goes off to double check...)

R-b for the series or 1/7 ?

Regards,

Tvrtko

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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c
  2016-07-13 16:04   ` Chris Wilson
@ 2016-07-14  9:25     ` Tvrtko Ursulin
  2016-07-14 10:07       ` Chris Wilson
  0 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-14  9:25 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 13/07/16 17:04, Chris Wilson wrote:
> On Wed, Jul 13, 2016 at 04:03:40PM +0100, Tvrtko Ursulin wrote:
>> +	/*
>> +	 * Catch failures to update intel_engines table when the new engines
>> +	 * are added to the driver by a warning and disabling the forgotten
>> +	 * engines.
>> +	 */
>> +	if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
>> +		struct intel_device_info *info =
>> +			(struct intel_device_info *)&dev_priv->info;
>
> I snuck in mkwrite_device_info(), so
>
> if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask))
> 	mkwrite_device_info(dev_priv)->ring_mask = mask;

This part is just code movement, the block you quote exists before this 
series even!

Follow up patch to this series would be easiest then, or a solitary 
precursor if you insist. Dangers of code movement with edits huh? 
(94b4f3ba483ace6dd4a3f881e19cc18bdbafa6ef)

Regards,

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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c
  2016-07-14  9:25     ` Tvrtko Ursulin
@ 2016-07-14 10:07       ` Chris Wilson
  0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2016-07-14 10:07 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Thu, Jul 14, 2016 at 10:25:39AM +0100, Tvrtko Ursulin wrote:
> 
> On 13/07/16 17:04, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 04:03:40PM +0100, Tvrtko Ursulin wrote:
> >>+	/*
> >>+	 * Catch failures to update intel_engines table when the new engines
> >>+	 * are added to the driver by a warning and disabling the forgotten
> >>+	 * engines.
> >>+	 */
> >>+	if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
> >>+		struct intel_device_info *info =
> >>+			(struct intel_device_info *)&dev_priv->info;
> >
> >I snuck in mkwrite_device_info(), so
> >
> >if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask))
> >	mkwrite_device_info(dev_priv)->ring_mask = mask;
> 
> This part is just code movement, the block you quote exists before
> this series even!
> 
> Follow up patch to this series would be easiest then, or a solitary
> precursor if you insist. Dangers of code movement with edits huh?
> (94b4f3ba483ace6dd4a3f881e19cc18bdbafa6ef)

I was also on a checkpatch witchhunt.
-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] 15+ messages in thread
* Re: [PATCH 1/7] drm/i915: unify first-stage engine struct setup
  2016-07-14  9:19   ` Tvrtko Ursulin
@ 2016-07-14 10:08     ` Chris Wilson
  0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2016-07-14 10:08 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Thu, Jul 14, 2016 at 10:19:16AM +0100, Tvrtko Ursulin wrote:
> 
> On 13/07/16 17:01, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 04:03:35PM +0100, Tvrtko Ursulin wrote:
> >>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>
> >
> >LGTM, I didn't see anything that would impede applying this series,
> >so
> >Reviewed-by: Chris Wilson <chris-wilson.co.uk>
> >
> >(Goes off to double check...)
> 
> R-b for the series or 1/7 ?

Series, just a few more wishlist items for later.
-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] 15+ messages in thread
* Re: ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup
  2016-07-13 15:57 ` ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup Patchwork
@ 2016-07-14 10:20   ` Tvrtko Ursulin
  0 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2016-07-14 10:20 UTC (permalink / raw)
  To: intel-gfx


On 13/07/16 16:57, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/7] drm/i915: unify first-stage engine struct setup
> URL   : https://patchwork.freedesktop.org/series/9821/
> State : success
>
> == Summary ==
>
> Series 9821v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/9821/revisions/1/mbox
>
> Test prime_self_import:
>          Subgroup basic-llseek-size:
>                  incomplete -> PASS       (ro-byt-n2820)
>
> fi-kbl-qkkr      total:237  pass:174  dwarn:25  dfail:4   fail:7   skip:27
> fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26
> fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40
> ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16
> ro-bdw-i7-5557U  total:237  pass:211  dwarn:1   dfail:0   fail:9   skip:16
> ro-bdw-i7-5600u  total:237  pass:195  dwarn:2   dfail:0   fail:9   skip:31
> ro-bsw-n3050     total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42
> ro-byt-n2820     total:237  pass:178  dwarn:0   dfail:0   fail:7   skip:38
> ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24
> ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24
> ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63
> ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58
> ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33
> ro-skl3-i5-6260u total:237  pass:213  dwarn:3   dfail:0   fail:9   skip:12
> ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1486/
>
> 5cd2699 drm-intel-nightly: 2016y-07m-13d-14h-43m-27s UTC integration manifest
> f0b1666 drm/i915: Pull out some more common engine init code
> be8c237 drm/i915: Move common engine setup into intel_engine_cs.c
> a4cb7bc drm/i915: Simplify intel_init_ring_buffer prototype
> d3dfae1 drm/i915: Make more use of the shared engine irq setup
> ef59023 drm/i915: Unify engine init loop
> ac58b8a drm/i915: Prepare for engine init unification
> 8120dcf drm/i915: unify first-stage engine struct setup

Merged to dinq, thanks for the patches and review!

Regards,

Tvrtko

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

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-07-14 10:20 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-13 15:03 [PATCH 1/7] drm/i915: unify first-stage engine struct setup Tvrtko Ursulin
2016-07-13 15:03 ` [PATCH 2/7] drm/i915: Prepare for engine init unification Tvrtko Ursulin
2016-07-13 15:03 ` [PATCH 3/7] drm/i915: Unify engine init loop Tvrtko Ursulin
2016-07-13 15:03 ` [PATCH 4/7] drm/i915: Make more use of the shared engine irq setup Tvrtko Ursulin
2016-07-13 15:03 ` [PATCH 5/7] drm/i915: Simplify intel_init_ring_buffer prototype Tvrtko Ursulin
2016-07-13 15:03 ` [PATCH 6/7] drm/i915: Move common engine setup into intel_engine_cs.c Tvrtko Ursulin
2016-07-13 16:04   ` Chris Wilson
2016-07-14  9:25     ` Tvrtko Ursulin
2016-07-14 10:07       ` Chris Wilson
2016-07-13 15:03 ` [PATCH 7/7] drm/i915: Pull out some more common engine init code Tvrtko Ursulin
2016-07-13 15:57 ` ✓ Ro.CI.BAT: success for series starting with [1/7] drm/i915: unify first-stage engine struct setup Patchwork
2016-07-14 10:20   ` Tvrtko Ursulin
2016-07-13 16:01 ` [PATCH 1/7] " Chris Wilson
2016-07-14  9:19   ` Tvrtko Ursulin
2016-07-14 10:08     ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.