[2/4] drm/i915/fbc: sanitize i915.enable_fbc during FBC init

Submitted by Paulo Zanoni on April 13, 2016, 7:01 p.m.

Details

Message ID 1460574069-14005-1-git-send-email-paulo.r.zanoni@intel.com
State New
Headers show
Series "Enable FBC on SKL" ( rev: 9 8 7 6 5 4 3 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Paulo Zanoni April 13, 2016, 7:01 p.m.
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@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

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;

Comments

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@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx