Re: [Intel-gfx] [PATCH 2/2] drm/i915: Onion unwind for intel_init_ring_common()

[Joonas Lahtinen]

On ma, 2017-04-03 at 12:34 +0100, Chris Wilson wrote:
> Rather than call intel_engine_cleanup() with a partially constructed
> engine, unwind the error during intel_init_ring_common().
> 
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: Onion unwind for intel_init_ring_common()

[Chris Wilson]

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

Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 77 +++--
 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5e7634c00cbd..c98acc27279a 100644
--- 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(_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);
 }
-- 
2.11.0

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