Re: [PATCH 1/5] drm/i915: Organize Fence registers for future enablement.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 03 Dec 2014, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's be optimistic that for future platforms this will remain the same
> and reorg a bit.
> This reorg in if blocks instead of switch make life easier for future
> platform support addition.
>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c       | 17 ++++++----------
>  drivers/gpu/drm/i915/i915_gpu_error.c | 37 ++++++++++++-----------------------
>  2 files changed, 19 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9d362d3..1b320bd 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3271,17 +3271,12 @@ static void i915_gem_write_fence(struct drm_device *dev, int reg,
>  	     "bogus fence setup with stride: 0x%x, tiling mode: %i\n",
>  	     obj->stride, obj->tiling_mode);
>  
> -	switch (INTEL_INFO(dev)->gen) {
> -	case 9:
> -	case 8:
> -	case 7:
> -	case 6:
> -	case 5:
> -	case 4: i965_write_fence_reg(dev, reg, obj); break;
> -	case 3: i915_write_fence_reg(dev, reg, obj); break;
> -	case 2: i830_write_fence_reg(dev, reg, obj); break;
> -	default: BUG();
> -	}
> +	if (IS_GEN2(dev))
> +		i830_write_fence_reg(dev, reg, obj);
> +	else if (IS_GEN3(dev))
> +		i915_write_fence_reg(dev, reg, obj);
> +	else if (INTEL_INFO(dev)->gen >= 4)
> +		i965_write_fence_reg(dev, reg, obj);
>  
>  	/* And similarly be paranoid that no direct access to this region
>  	 * is reordered to before the fence is installed.
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c4536e1..f117a1d 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -766,32 +766,21 @@ static void i915_gem_record_fences(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int i;
>  
> -	/* Fences */
> -	switch (INTEL_INFO(dev)->gen) {
> -	case 9:
> -	case 8:
> -	case 7:
> -	case 6:
> -		for (i = 0; i < dev_priv->num_fence_regs; i++)
> -			error->fence[i] = I915_READ64(FENCE_REG_SANDYBRIDGE_0 + (i * 8));
> -		break;
> -	case 5:
> -	case 4:
> -		for (i = 0; i < 16; i++)
> -			error->fence[i] = I915_READ64(FENCE_REG_965_0 + (i * 8));
> -		break;
> -	case 3:
> -		if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
> -			for (i = 0; i < 8; i++)
> -				error->fence[i+8] = I915_READ(FENCE_REG_945_8 + (i * 4));
> -	case 2:
> +	if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
> +		for (i = 0; i < 8; i++)
> +			error->fence[i+8] = I915_READ(FENCE_REG_945_8 +
> +						      (i * 4));

Do note the fallthrough from case 3 to case 2 in the original. For these
platforms you'll also need the ones below. Maybe add two loops or loop
to 16 and add an if (i < 8) inside?

BR,
Jani.

> +	else if (IS_GEN3(dev) || IS_GEN2(dev))
>  		for (i = 0; i < 8; i++)
>  			error->fence[i] = I915_READ(FENCE_REG_830_0 + (i * 4));
> -		break;
> -
> -	default:
> -		BUG();
> -	}
> +	else if (IS_GEN5(dev) || IS_GEN4(dev))
> +		for (i = 0; i < 16; i++)
> +			error->fence[i] = I915_READ64(FENCE_REG_965_0 +
> +						      (i * 8));
> +	else if (INTEL_INFO(dev)->gen >= 6)
> +		for (i = 0; i < dev_priv->num_fence_regs; i++)
> +			error->fence[i] = I915_READ64(FENCE_REG_SANDYBRIDGE_0 +
> +						      (i * 8));
>  }
>  
>  
> -- 
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx