[Intel-gfx] [PATCH] drm/i915: extract fence stealing code

[Chris Wilson]

On Fri, 15 Jan 2010 15:39:01 +0100, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> 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.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |  104 +++++++++++++++++++++-----------------
>  1 files changed, 57 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7112457..bca064a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2373,6 +2373,55 @@ 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 *old_obj_priv = NULL;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_gem_object *old_obj = NULL;
> +	int i, avail, ret;

I'm going to be incredibly picky here now... The old_ prefix is no longer
useful.

> +
> +	/* 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;
> +
> +		old_obj_priv = reg->obj->driver_private;
> +		if (!old_obj_priv->pin_count)
> +		    avail++;
> +	}
> +
> +	if (avail == 0)
> +		return -ENOSPC;
> +
> +	/* None unused, try to steal one or wait for a user to finish */
> +	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;
> +
> +		/* found one! */
> +		i = old_obj_priv->fence_reg;
> +		break;
> +	}

I'd still feel safer with a paranoid check here since we are using both an
array of registers and a lru list.

  obj = NULL; /* instead of the NULL initializer? */
  list_for_each_entry(obj_priv, &dev_priv->mm.fence_list, fence_list) {
    if (obj_priv->obj->pin_count == 0) {
      obj = obj_priv->obj;
      break;
    }
  }
  if (obj == NULL) {
    WARN_ONCE()
    return -ENOSPC;
  } 

 ref (obj);
 i = obj_priv->fence_reg;
 put_fence (obj);
 unref (obj);

 return i;
-ickle

-- 
Chris Wilson, Intel Open Source Technology Centre