[Intel-gfx] [PATCH 2/4] drm/i915/fbc: sanitize i915.enable_fbc during FBC init

[Daniel Vetter]

On Wed, Apr 13, 2016 at 04:01:09PM -0300, Paulo Zanoni wrote:
> The DDX driver changes its behavior depending on the value it reads
> from i915.enable_fbc, so sanitize the value in order to allow it to
> know what's going on. It uses this in order to choose the defaults for
> the TearFree option. Before this patch, it would read -1 and always
> assume that FBC was disabled, so it wouldn't force TearFree.
> 
> v2: Extract intel_sanitize_fbc_option() (Chris).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index fc3c094..c3dffba 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -824,21 +824,14 @@ static bool intel_fbc_can_choose(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>  	struct intel_fbc *fbc = &dev_priv->fbc;
> -	bool enable_by_default = IS_HASWELL(dev_priv) ||
> -				 IS_BROADWELL(dev_priv);
>  
>  	if (intel_vgpu_active(dev_priv->dev)) {
>  		fbc->no_fbc_reason = "VGPU is active";
>  		return false;
>  	}
>  
> -	if (i915.enable_fbc < 0 && !enable_by_default) {
> -		fbc->no_fbc_reason = "disabled per chip default";
> -		return false;
> -	}
> -
>  	if (!i915.enable_fbc) {
> -		fbc->no_fbc_reason = "disabled per module param";
> +		fbc->no_fbc_reason = "disabled per module param or by default";
>  		return false;
>  	}
>  
> @@ -1223,6 +1216,26 @@ void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv)
>  			dev_priv->fbc.visible_pipes_mask |= (1 << crtc->pipe);
>  }
>  
> +/*
> + * The DDX driver changes its behavior depending on the value it reads from
> + * i915.enable_fbc, so sanitize it by translating the default value into either
> + * 0 or 1 in order to allow it to know what's going on.
> + *
> + * Notice that this is done at driver initialization and we still allow user
> + * space to change the value during runtime without sanitizing it again. IGT
> + * relies on being able to change i915.enable_fbc at runtime.
> + */
> +static int intel_sanitize_fbc_option(struct drm_i915_private *dev_priv)
> +{
> +	if (i915.enable_fbc >= 0)
> +		return !!i915.enable_fbc;
> +
> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +		return 1;
> +
> +	return 0;
> +}
> +
>  /**
>   * intel_fbc_init - Initialize FBC
>   * @dev_priv: the i915 device
> @@ -1240,6 +1253,9 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
>  	fbc->active = false;
>  	fbc->work.scheduled = false;
>  
> +	i915.enable_fbc = intel_sanitize_fbc_option(dev_priv);
> +	DRM_DEBUG_KMS("Sanitized enable_fbc value: %d\n", i915.enable_fbc);
> +
>  	if (!HAS_FBC(dev_priv)) {
>  		fbc->no_fbc_reason = "unsupported by this chipset";
>  		return;
> -- 
> 2.8.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch