drm/i915: Onion unwind for intel_init_ring_common()

Rather than call intel_engine_cleanup() with a partially constructed
engine, unwind the error during intel_init_ring_common().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170403113426.25707-2-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5e7634c00cbdde33d497b0cb78e932cd226ee6ef..c98acc27279af7ac42c9db99d657ffd494a3c95f 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1259,6 +1259,8 @@ static int init_phys_status_page(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
 
+       GEM_BUG_ON(engine->id != RCS);
+
        dev_priv->status_page_dmah =
                drm_pci_alloc(&dev_priv->drm, PAGE_SIZE, PAGE_SIZE);
        if (!dev_priv->status_page_dmah)
@@ -1481,76 +1483,69 @@ static void intel_ring_context_unpin(struct intel_engine_cs *engine,
 static int intel_init_ring_buffer(struct intel_engine_cs *engine)
 {
        struct intel_ring *ring;
-       int ret;
-
-       WARN_ON(engine->buffer);
+       int err;
 
        intel_engine_setup_common(engine);
 
-       ret = intel_engine_init_common(engine);
-       if (ret)
-               goto error;
+       err = intel_engine_init_common(engine);
+       if (err)
+               goto err;
 
-       if (HWS_NEEDS_PHYSICAL(engine->i915)) {
-               WARN_ON(engine->id != RCS);
-               ret = init_phys_status_page(engine);
-               if (ret)
-                       goto error;
-       } else {
-               ret = init_status_page(engine);
-               if (ret)
-                       goto error;
-       }
+       if (HWS_NEEDS_PHYSICAL(engine->i915))
+               err = init_phys_status_page(engine);
+       else
+               err = init_status_page(engine);
+       if (err)
+               goto err;
 
        ring = intel_engine_create_ring(engine, 32 * PAGE_SIZE);
        if (IS_ERR(ring)) {
-               ret = PTR_ERR(ring);
-               goto error;
+               err = PTR_ERR(ring);
+               goto err_hws;
        }
 
        /* Ring wraparound at offset 0 sometimes hangs. No idea why. */
-       ret = intel_ring_pin(ring, engine->i915, I915_GTT_PAGE_SIZE);
-       if (ret) {
-               intel_ring_free(ring);
-               goto error;
-       }
+       err = intel_ring_pin(ring, engine->i915, I915_GTT_PAGE_SIZE);
+       if (err)
+               goto err_ring;
+
+       GEM_BUG_ON(engine->buffer);
        engine->buffer = ring;
 
        return 0;
 
-error:
-       intel_engine_cleanup(engine);
-       return ret;
+err_ring:
+       intel_ring_free(ring);
+err_hws:
+       if (HWS_NEEDS_PHYSICAL(engine->i915))
+               cleanup_phys_status_page(engine);
+       else
+               cleanup_status_page(engine);
+err:
+       intel_engine_cleanup_common(engine);
+       return err;
 }
 
 void intel_engine_cleanup(struct intel_engine_cs *engine)
 {
-       struct drm_i915_private *dev_priv;
-
-       dev_priv = engine->i915;
+       struct drm_i915_private *dev_priv = engine->i915;
 
-       if (engine->buffer) {
-               WARN_ON(INTEL_GEN(dev_priv) > 2 &&
-                       (I915_READ_MODE(engine) & MODE_IDLE) == 0);
+       WARN_ON(INTEL_GEN(dev_priv) > 2 &&
+               (I915_READ_MODE(engine) & MODE_IDLE) == 0);
 
-               intel_ring_unpin(engine->buffer);
-               intel_ring_free(engine->buffer);
-               engine->buffer = NULL;
-       }
+       intel_ring_unpin(engine->buffer);
+       intel_ring_free(engine->buffer);
 
        if (engine->cleanup)
                engine->cleanup(engine);
 
-       if (HWS_NEEDS_PHYSICAL(dev_priv)) {
-               WARN_ON(engine->id != RCS);
+       if (HWS_NEEDS_PHYSICAL(dev_priv))
                cleanup_phys_status_page(engine);
-       } else {
+       else
                cleanup_status_page(engine);
-       }
 
        intel_engine_cleanup_common(engine);
 
-       engine->i915 = NULL;
        dev_priv->engine[engine->id] = NULL;
        kfree(engine);
 }