On 16/12/2015 18:36, Gordon, David S wrote:
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'.
Reviewed-by: Nick Hoath <nicholas.hoath@xxxxxxxxx>
Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++++++++++++++------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index eefce9a..2853754 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -549,6 +549,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;
}
@@ -2057,12 +2059,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)
{
@@ -2125,11 +2121,14 @@ intel_engine_create_ringbuffer(struct intel_engine_cs *engine, int size)
}
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);
+ ringbuf->obj = NULL;
+ }
+ list_del(&ringbuf->link);
+ kfree(ringbuf);
}
static int intel_init_ring_buffer(struct drm_device *dev,
@@ -2157,6 +2156,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)
@@ -2168,14 +2174,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;
@@ -2189,19 +2187,18 @@ error:
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);
+ if (ringbuf->virtual_start)
+ intel_unpin_ringbuffer_obj(ringbuf);
+ intel_ringbuffer_free(ringbuf);
ring->buffer = NULL;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
