Re: [PATCH 1/2] drm/i915/fbc: Don't set an illegal fence if unfenced

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On to, 2016-08-18 at 09:21 +0100, Chris Wilson wrote:
> @@ -190,9 +190,11 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
>  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
>  	else
>  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> -	dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg;
>  
> -	I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
> +	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
> +		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg;
> +		I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
> +	}

Here the fence_y_offset is conditional on the fence existing.

> @@ -244,21 +246,24 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
>  		break;
>  	}
> -	dpfc_ctl |= DPFC_CTL_FENCE_EN;
> -	if (IS_GEN5(dev_priv))
> -		dpfc_ctl |= params->fb.fence_reg;
> +
> +	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
> +		dpfc_ctl |= DPFC_CTL_FENCE_EN;
> +		if (IS_GEN5(dev_priv))
> +			dpfc_ctl |= params->fb.fence_reg;
> +		if (IS_GEN6(dev_priv)) {
> +			I915_WRITE(SNB_DPFC_CTL_SA,
> +				   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
> +			I915_WRITE(DPFC_CPU_FENCE_OFFSET,
> +				   params->crtc.fence_y_offset);
> +		}
> +	}
>  
>  	I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset);

Here it is not?

> @@ -305,7 +310,12 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  		break;
>  	}
>  
> -	dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
> +	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
> +		dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
> +		I915_WRITE(SNB_DPFC_CTL_SA,
> +			   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
> +		I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset);
> +	}
>  
>  	if (dev_priv->fbc.false_color)
>  		dpfc_ctl |= FBC_CTL_FALSE_COLOR;
> @@ -324,10 +334,6 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  
>  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
>  
> -	I915_WRITE(SNB_DPFC_CTL_SA,
> -		   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
> -	I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset);

Here it is again moved conditionally. So I suggest you unify that.

This changes the order of the register writes on older platforms so
Tested-bys would be good.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx