drm/i915: extract fence stealing code

The spaghetti logic in there tripped up my brain's code parser for a
few secs. Prevent this from happening again by extracting the fence
stealing code into a seperate functions. IMHO this slightly clears up
the code flow.

v2: Beautified according to ickle's comments.
v3: ickle forgot to flush his comment queue ... Now there's also a
we-are-paranoid BUG_ON in there.
v4: I've forgotten to switch on my brain when doing v3. Now the BUG_ON
actually checks something useful.
v5: Clean up a stale comment as noted by Eric Anholt.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eric Anholt <eric@anholt.net>

1 files changed, 60 insertions, 47 deletions

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e6b85cdbc60f..ce1c0262bc63 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2382,6 +2382,58 @@ static void i830_write_fence_reg(struct drm_i915_fence_reg *reg)
         I915_WRITE(FENCE_REG_830_0 + (regnum * 4), val);
 }
 
+static int i915_find_fence_reg(struct drm_device *dev)
+{
+        struct drm_i915_fence_reg *reg = NULL;
+        struct drm_i915_gem_object *obj_priv = NULL;
+        struct drm_i915_private *dev_priv = dev->dev_private;
+        struct drm_gem_object *obj = NULL;
+        int i, avail, ret;
+
+        /* First try to find a free reg */
+        avail = 0;
+        for (i = dev_priv->fence_reg_start; i < dev_priv->num_fence_regs; i++) {
+                reg = &dev_priv->fence_regs[i];
+                if (!reg->obj)
+                        return i;
+
+                obj_priv = reg->obj->driver_private;
+                if (!obj_priv->pin_count)
+                    avail++;
+        }
+
+        if (avail == 0)
+                return -ENOSPC;
+
+        /* None available, try to steal one or wait for a user to finish */
+        i = I915_FENCE_REG_NONE;
+        list_for_each_entry(obj_priv, &dev_priv->mm.fence_list,
+                            fence_list) {
+                obj = obj_priv->obj;
+
+                if (obj_priv->pin_count)
+                        continue;
+
+                /* found one! */
+                i = obj_priv->fence_reg;
+                break;
+        }
+
+        BUG_ON(i == I915_FENCE_REG_NONE);
+
+        /* We only have a reference on obj from the active list. put_fence_reg
+         * might drop that one, causing a use-after-free in it. So hold a
+         * private reference to obj like the other callers of put_fence_reg
+         * (set_tiling ioctl) do. */
+        drm_gem_object_reference(obj);
+        ret = i915_gem_object_put_fence_reg(obj);
+        drm_gem_object_unreference(obj);
+        if (ret != 0)
+                return ret;
+
+        return i;
+}
+
 /**
  * i915_gem_object_get_fence_reg - set up a fence reg for an object
  * @obj: object to map through a fence reg
@@ -2402,8 +2454,7 @@ i915_gem_object_get_fence_reg(struct drm_gem_object *obj)
         struct drm_i915_private *dev_priv = dev->dev_private;
         struct drm_i915_gem_object *obj_priv = obj->driver_private;
         struct drm_i915_fence_reg *reg = NULL;
-        struct drm_i915_gem_object *old_obj_priv = NULL;
-        int i, ret, avail;
+        int ret;
 
         /* Just update our place in the LRU if our fence is getting used. */
         if (obj_priv->fence_reg != I915_FENCE_REG_NONE) {
@@ -2431,51 +2482,12 @@ i915_gem_object_get_fence_reg(struct drm_gem_object *obj)
                 break;
         }
 
-        /* First try to find a free reg */
-        avail = 0;
-        for (i = dev_priv->fence_reg_start; i < dev_priv->num_fence_regs; i++) {
-                reg = &dev_priv->fence_regs[i];
-                if (!reg->obj)
-                        break;
-
-                old_obj_priv = reg->obj->driver_private;
-                if (!old_obj_priv->pin_count)
-                    avail++;
-        }
-
-        /* None available, try to steal one or wait for a user to finish */
-        if (i == dev_priv->num_fence_regs) {
-                struct drm_gem_object *old_obj = NULL;
-
-                if (avail == 0)
-                        return -ENOSPC;
-
-                list_for_each_entry(old_obj_priv, &dev_priv->mm.fence_list,
-                                    fence_list) {
-                        old_obj = old_obj_priv->obj;
-
-                        if (old_obj_priv->pin_count)
-                                continue;
-
-                        /* Take a reference, as otherwise the wait_rendering
-                         * below may cause the object to get freed out from
-                         * under us.
-                         */
-                        drm_gem_object_reference(old_obj);
-
-                        break;
-                }
-
-                i = old_obj_priv->fence_reg;
-                reg = &dev_priv->fence_regs[i];
-
-                ret = i915_gem_object_put_fence_reg(old_obj);
-                drm_gem_object_unreference(old_obj);
-                if (ret != 0) 
-                        return ret;
-        }
+        ret = i915_find_fence_reg(dev);
+        if (ret < 0)
+                return ret;
 
-        obj_priv->fence_reg = i;
+        obj_priv->fence_reg = ret;
+        reg = &dev_priv->fence_regs[obj_priv->fence_reg];
         list_add_tail(&obj_priv->fence_list, &dev_priv->mm.fence_list);
 
         reg->obj = obj;
@@ -2489,7 +2501,8 @@ i915_gem_object_get_fence_reg(struct drm_gem_object *obj)
         else
                 i830_write_fence_reg(reg);
 
-        trace_i915_gem_object_get_fence(obj, i, obj_priv->tiling_mode);
+        trace_i915_gem_object_get_fence(obj, obj_priv->fence_reg,
+                        obj_priv->tiling_mode);
 
         return 0;
 }