1. Fix intel_cleanup_ring_buffer() to handle the error cleanup
   case where the ringbuffer has been allocated but map-and-pin
   failed. Unpin it iff it's previously been mapped-and-pinned.

2. Fix the error path in intel_init_ring_buffer(), which already
   called intel_destroy_ringbuffer_obj(), but failed to free the
   actual ringbuffer structure. Calling intel_ringbuffer_free()
   instead does both in one go.

3. With the above change, intel_destroy_ringbuffer_obj() is only
   called in one place (intel_ringbuffer_free()), so flatten it
   into that function.

4. move low-level register accesses from intel_cleanup_ring_buffer()
   (which calls intel_stop_ring_buffer(ring) which calls stop_ring())
   down into stop_ring() itself), which is already doing low-level
   register accesses. Then, intel_cleanup_ring_buffer() no longer
   needs 'dev_priv'.

v3:
   Make intel_unpin_ringbuffer_obj() return early if ringbuffer is
   not mapped, simplifying matters for the caller. [Chris Wilson,
   Daniel Vetter]
   Don't bother to clear a pointer in an object about to be freed.
   [Chris Wilson]

Signed-off-by: Dave Gordon <david.s.gor...@intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 49 ++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6f5b511..db02893 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -551,6 +551,8 @@ static bool stop_ring(struct intel_engine_cs *ring)
                I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING));
        }
 
+       WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
+
        return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0;
 }
 
@@ -2055,6 +2057,9 @@ static int init_phys_status_page(struct intel_engine_cs 
*ring)
 
 void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
 {
+       if (!ringbuf->virtual_start)
+               return;
+
        if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
                vunmap(ringbuf->virtual_start);
        else
@@ -2132,12 +2137,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device 
*dev,
        return 0;
 }
 
-static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
-{
-       drm_gem_object_unreference(&ringbuf->obj->base);
-       ringbuf->obj = NULL;
-}
-
 static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
                                      struct intel_ringbuffer *ringbuf)
 {
@@ -2200,11 +2199,13 @@ struct intel_ringbuffer *
 }
 
 void
-intel_ringbuffer_free(struct intel_ringbuffer *ring)
+intel_ringbuffer_free(struct intel_ringbuffer *ringbuf)
 {
-       intel_destroy_ringbuffer_obj(ring);
-       list_del(&ring->link);
-       kfree(ring);
+       if (ringbuf->obj)
+               drm_gem_object_unreference(&ringbuf->obj->base);
+
+       list_del(&ringbuf->link);
+       kfree(ringbuf);
 }
 
 static int intel_init_ring_buffer(struct drm_device *dev,
@@ -2232,6 +2233,13 @@ static int intel_init_ring_buffer(struct drm_device *dev,
        }
        ring->buffer = ringbuf;
 
+       ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
+       if (ret) {
+               DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n",
+                               ring->name, ret);
+               goto error;
+       }
+
        if (I915_NEED_GFX_HWS(dev)) {
                ret = init_status_page(ring);
                if (ret)
@@ -2243,14 +2251,6 @@ static int intel_init_ring_buffer(struct drm_device *dev,
                        goto error;
        }
 
-       ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
-       if (ret) {
-               DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n",
-                               ring->name, ret);
-               intel_destroy_ringbuffer_obj(ringbuf);
-               goto error;
-       }
-
        ret = i915_cmd_parser_init_ring(ring);
        if (ret)
                goto error;
@@ -2264,19 +2264,16 @@ static int intel_init_ring_buffer(struct drm_device 
*dev,
 
 void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
 {
-       struct drm_i915_private *dev_priv;
+       struct intel_ringbuffer *ringbuf;
 
        if (!intel_ring_initialized(ring))
                return;
 
-       dev_priv = to_i915(ring->dev);
-
-       if (ring->buffer) {
+       ringbuf = ring->buffer;
+       if (ringbuf) {
                intel_stop_ring_buffer(ring);
-               WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & 
MODE_IDLE) == 0);
-
-               intel_unpin_ringbuffer_obj(ring->buffer);
-               intel_ringbuffer_free(ring->buffer);
+               intel_unpin_ringbuffer_obj(ringbuf);
+               intel_ringbuffer_free(ringbuf);
                ring->buffer = NULL;
        }
 
-- 
1.9.1

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

Reply via email to