diff mbox

[CI,1/2] drm/i915: Split intel_engine allocation and initialisation

Message ID 20170124110135.6418-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Jan. 24, 2017, 11:01 a.m. UTC 
In order to reset the GPU early on in the module load sequence, we need
to allocate the basic engine structs (to populate the mmio offsets etc).
Currently, the engine initialisation allocates both the base struct and
also allocate auxiliary objects, which depend upon state setup quite
late in the load sequence. We split off the allocation callback for
later and allow ourselves to allocate the engine structs themselves
early.

v2: Different paint for the unwind following error.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c        | 19 +++++++-
 drivers/gpu/drm/i915/i915_drv.h        |  3 ++
 drivers/gpu/drm/i915/intel_engine_cs.c | 80 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_lrc.h       |  2 -
 4 files changed, 81 insertions(+), 23 deletions(-)

Comments

Chris Wilson Jan. 24, 2017, 12:31 p.m. UTC | #1 
On Tue, Jan 24, 2017 at 12:24:10PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [CI,1/2] drm/i915: Split intel_engine allocation and initialisation
> URL   : https://patchwork.freedesktop.org/series/18475/
> State : success
> 
> == Summary ==
> 
> Series 18475v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/18475/revisions/1/mbox/
> 
> 
> fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
> fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
> fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
> fi-bxt-t5700     total:79   pass:66   dwarn:0   dfail:0   fail:0   skip:12 
> fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
> fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
> fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
> fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
> fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
> fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
> fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
> fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
> fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
> fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
> fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
> fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
> fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 
> 
> 64fc20ef2f4bf8a6b563a812485fc6ac86637fcd drm-tip: 2017y-01m-24d-10h-02m-05s UTC integration manifest
> 689a956 drm/i915: Reset the gpu on takeover
> 1b6a05b drm/i915: Split intel_engine allocation and initialisation

Now to find those hang on init/kexec/resume bugs and politely ask them
to test after a few of years of ignoring them...
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ca168b22ee26..f8d1ffeccc69 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -755,6 +755,15 @@  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 	return -ENOMEM;
 }
 
+static void i915_engines_cleanup(struct drm_i915_private *i915)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	for_each_engine(engine, i915, id)
+		kfree(engine);
+}
+
 static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
 {
 	destroy_workqueue(dev_priv->hotplug.dp_wq);
@@ -817,12 +826,15 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	mutex_init(&dev_priv->pps_mutex);
 
 	intel_uc_init_early(dev_priv);
-
 	i915_memcpy_init_early(dev_priv);
 
+	ret = intel_engines_init_early(dev_priv);
+	if (ret)
+		return ret;
+
 	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
-		return ret;
+		goto err_engines;
 
 	ret = intel_gvt_init(dev_priv);
 	if (ret < 0)
@@ -857,6 +869,8 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	intel_gvt_cleanup(dev_priv);
 err_workqueues:
 	i915_workqueues_cleanup(dev_priv);
+err_engines:
+	i915_engines_cleanup(dev_priv);
 	return ret;
 }
 
@@ -869,6 +883,7 @@  static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 	i915_perf_fini(dev_priv);
 	i915_gem_load_cleanup(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
+	i915_engines_cleanup(dev_priv);
 }
 
 static int i915_mmio_setup(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 244628065f94..55bd2d2f8a0d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2941,6 +2941,9 @@  extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
 extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
 
+int intel_engines_init_early(struct drm_i915_private *dev_priv);
+int intel_engines_init(struct drm_i915_private *dev_priv);
+
 /* intel_hotplug.c */
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			   u32 pin_mask, u32 long_mask);
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 371acf109e34..69a6416d1223 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -110,21 +110,20 @@  intel_engine_setup(struct drm_i915_private *dev_priv,
 }
 
 /**
- * intel_engines_init() - allocate, populate and init the Engine Command Streamers
+ * intel_engines_init_early() - allocate the Engine Command Streamers
  * @dev_priv: i915 device private
  *
  * Return: non-zero if the initialization failed.
  */
-int intel_engines_init(struct drm_i915_private *dev_priv)
+int intel_engines_init_early(struct drm_i915_private *dev_priv)
 {
 	struct intel_device_info *device_info = mkwrite_device_info(dev_priv);
 	unsigned int ring_mask = INTEL_INFO(dev_priv)->ring_mask;
 	unsigned int mask = 0;
-	int (*init)(struct intel_engine_cs *engine);
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	unsigned int i;
-	int ret;
+	int err;
 
 	WARN_ON(ring_mask == 0);
 	WARN_ON(ring_mask &
@@ -134,23 +133,65 @@  int intel_engines_init(struct drm_i915_private *dev_priv)
 		if (!HAS_ENGINE(dev_priv, i))
 			continue;
 
+		err = intel_engine_setup(dev_priv, i);
+		if (err)
+			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 != ring_mask))
+		device_info->ring_mask = mask;
+
+	device_info->num_rings = hweight32(mask);
+
+	return 0;
+
+cleanup:
+	for_each_engine(engine, dev_priv, id)
+		kfree(engine);
+	return err;
+}
+
+/**
+ * intel_engines_init() - allocate, populate and init the Engine Command Streamers
+ * @dev_priv: i915 device private
+ *
+ * Return: non-zero if the initialization failed.
+ */
+int intel_engines_init(struct drm_i915_private *dev_priv)
+{
+	struct intel_device_info *device_info = mkwrite_device_info(dev_priv);
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id, err_id;
+	unsigned int mask = 0;
+	int err = 0;
+
+	for_each_engine(engine, dev_priv, id) {
+		int (*init)(struct intel_engine_cs *engine);
+
 		if (i915.enable_execlists)
-			init = intel_engines[i].init_execlists;
+			init = intel_engines[id].init_execlists;
 		else
-			init = intel_engines[i].init_legacy;
-
-		if (!init)
+			init = intel_engines[id].init_legacy;
+		if (!init) {
+			kfree(engine);
+			dev_priv->engine[id] = NULL;
 			continue;
+		}
 
-		ret = intel_engine_setup(dev_priv, i);
-		if (ret)
-			goto cleanup;
-
-		ret = init(dev_priv->engine[i]);
-		if (ret)
+		err = init(engine);
+		if (err) {
+			err_id = id;
 			goto cleanup;
+		}
 
-		mask |= ENGINE_MASK(i);
+		mask |= ENGINE_MASK(id);
 	}
 
 	/*
@@ -158,7 +199,7 @@  int intel_engines_init(struct drm_i915_private *dev_priv)
 	 * are added to the driver by a warning and disabling the forgotten
 	 * engines.
 	 */
-	if (WARN_ON(mask != ring_mask))
+	if (WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask))
 		device_info->ring_mask = mask;
 
 	device_info->num_rings = hweight32(mask);
@@ -167,13 +208,14 @@  int intel_engines_init(struct drm_i915_private *dev_priv)
 
 cleanup:
 	for_each_engine(engine, dev_priv, id) {
-		if (i915.enable_execlists)
+		if (id >= err_id)
+			kfree(engine);
+		else if (i915.enable_execlists)
 			intel_logical_ring_cleanup(engine);
 		else
 			intel_engine_cleanup(engine);
 	}
-
-	return ret;
+	return err;
 }
 
 void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno)
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 0c852c024227..c8009c7bfbdd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -68,8 +68,6 @@  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_i915_private *dev_priv);
-
 /* Logical Ring Contexts */
 
 /* One extra page is added before LRC for GuC as shared data */