[PATCH 0/4] FBC trivial patches

[PATCH 0/4] FBC trivial patches

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Let's try to get the easy stuff merged while the rest is not ready
yet.

This is a nice opportunity for you to easily increase your patch
review count!

Paulo Zanoni (4):
  drm/i915: print FBC compression status on debugfs
  drm/i915: add FBC_ROTATION to enum no_fbc_reason
  drm/i915: unify no_fbc_reason message printing
  drm/i915: don't set the FBC plane select bits on HSW+

 drivers/gpu/drm/i915/i915_debugfs.c | 51 +++++---------------------
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_reg.h     |  3 ++
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_fbc.c    | 73 +++++++++++++++++++++++++------------
 5 files changed, 64 insertions(+), 65 deletions(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/4] drm/i915: print FBC compression status on debugfs

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We already had a few bugs in the past where FBC was compressing
nothing when it was enabled, which makes the feature quite useless.
Add this information to debugfs so the test suites can check for
regressions in this piece of the code.

Our igt/tests/kms_frontbuffer_tracking already has support for this
message.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 5 +++++
 drivers/gpu/drm/i915/i915_reg.h     | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 92cf273..7358f6d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1638,6 +1638,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		seq_putc(m, '\n');
 	}
 
+	if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))
+		seq_printf(m, "Compressing: %s\n",
+			   yesno(I915_READ(FBC_STATUS2) &
+				 FBC_COMPRESSION_MASK));
+
 	intel_runtime_pm_put(dev_priv);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 40a3a64..0c0b12a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1951,6 +1951,9 @@ enum skl_disp_power_wells {
 #define FBC_FENCE_OFF		0x03218 /* BSpec typo has 321Bh */
 #define FBC_TAG			0x03300
 
+#define FBC_STATUS2		0x43214
+#define  FBC_COMPRESSION_MASK	0x7ff
+
 #define FBC_LL_SIZE		(1536)
 
 /* Framebuffer compression for GM45+ */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: add FBC_ROTATION to enum no_fbc_reason

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because we're currently using FBC_UNSUPPORTED_MODE for two different
cases.

This commit will also allow us to write the next one without hiding
information from the user.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
 drivers/gpu/drm/i915/i915_drv.h     | 1 +
 drivers/gpu/drm/i915/intel_fbc.c    | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7358f6d..405022b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1632,6 +1632,9 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		case FBC_CHIP_DEFAULT:
 			seq_puts(m, "disabled per chip default");
 			break;
+		case FBC_ROTATION:
+			seq_puts(m, "rotation not supported");
+			break;
 		default:
 			seq_puts(m, "unknown reason");
 		}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 611fbd8..f9ff452 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -926,6 +926,7 @@ struct i915_fbc {
 		FBC_MULTIPLE_PIPES, /* more than one pipe active */
 		FBC_MODULE_PARAM,
 		FBC_CHIP_DEFAULT, /* disabled by default on this chip */
+		FBC_ROTATION, /* rotation is not supported */
 	} no_fbc_reason;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 6abb834..43704a4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -587,7 +587,7 @@ void intel_fbc_update(struct drm_device *dev)
 	}
 	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
 	    crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
-		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+		if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
 			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
 		goto out_disable;
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: unify no_fbc_reason message printing

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This commit has two main advantages: simplify intel_fbc_update()
and deduplicate the strings.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 49 +++------------------------
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_fbc.c    | 66 +++++++++++++++++++++++++------------
 3 files changed, 50 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 405022b..eaa567c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1594,52 +1594,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 
 	intel_runtime_pm_get(dev_priv);
 
-	if (intel_fbc_enabled(dev)) {
+	if (intel_fbc_enabled(dev))
 		seq_puts(m, "FBC enabled\n");
-	} else {
-		seq_puts(m, "FBC disabled: ");
-		switch (dev_priv->fbc.no_fbc_reason) {
-		case FBC_OK:
-			seq_puts(m, "FBC actived, but currently disabled in hardware");
-			break;
-		case FBC_UNSUPPORTED:
-			seq_puts(m, "unsupported by this chipset");
-			break;
-		case FBC_NO_OUTPUT:
-			seq_puts(m, "no outputs");
-			break;
-		case FBC_STOLEN_TOO_SMALL:
-			seq_puts(m, "not enough stolen memory");
-			break;
-		case FBC_UNSUPPORTED_MODE:
-			seq_puts(m, "mode not supported");
-			break;
-		case FBC_MODE_TOO_LARGE:
-			seq_puts(m, "mode too large");
-			break;
-		case FBC_BAD_PLANE:
-			seq_puts(m, "FBC unsupported on plane");
-			break;
-		case FBC_NOT_TILED:
-			seq_puts(m, "scanout buffer not tiled");
-			break;
-		case FBC_MULTIPLE_PIPES:
-			seq_puts(m, "multiple pipes are enabled");
-			break;
-		case FBC_MODULE_PARAM:
-			seq_puts(m, "disabled per module param (default off)");
-			break;
-		case FBC_CHIP_DEFAULT:
-			seq_puts(m, "disabled per chip default");
-			break;
-		case FBC_ROTATION:
-			seq_puts(m, "rotation not supported");
-			break;
-		default:
-			seq_puts(m, "unknown reason");
-		}
-		seq_putc(m, '\n');
-	}
+	else
+		seq_printf(m, "FBC disabled: %s\n",
+			  intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason));
 
 	if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))
 		seq_printf(m, "Compressing: %s\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b28029a..77f24e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,6 +1255,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
 			  enum fb_op_origin origin);
 void intel_fbc_flush(struct drm_i915_private *dev_priv,
 		     unsigned int frontbuffer_bits);
+const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
 
 /* intel_hdmi.c */
 void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 43704a4..9b300bd 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -432,6 +432,39 @@ void intel_fbc_disable(struct drm_device *dev)
 	dev_priv->fbc.crtc = NULL;
 }
 
+const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
+{
+	switch (reason) {
+	case FBC_OK:
+		return "FBC enabled but currently disabled in hardware";
+	case FBC_UNSUPPORTED:
+		return "unsupported by this chipset";
+	case FBC_NO_OUTPUT:
+		return "no output";
+	case FBC_STOLEN_TOO_SMALL:
+		return "not enough stolen memory";
+	case FBC_UNSUPPORTED_MODE:
+		return "mode incompatible with compression";
+	case FBC_MODE_TOO_LARGE:
+		return "mode too large for compression";
+	case FBC_BAD_PLANE:
+		return "FBC unsupported on plane";
+	case FBC_NOT_TILED:
+		return "framebuffer not tiled or fenced";
+	case FBC_MULTIPLE_PIPES:
+		return "more than one pipe active";
+	case FBC_MODULE_PARAM:
+		return "disabled per module param";
+	case FBC_CHIP_DEFAULT:
+		return "disabled per chip default";
+	case FBC_ROTATION:
+		return "rotation unsupported";
+	default:
+		MISSING_CASE(reason);
+		return "unknown reason";
+	}
+}
+
 static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
 			      enum no_fbc_reason reason)
 {
@@ -439,6 +472,8 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
 		return false;
 
 	dev_priv->fbc.no_fbc_reason = reason;
+	DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason));
+
 	return true;
 }
 
@@ -459,8 +494,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 		if (intel_crtc_active(tmp_crtc) &&
 		    to_intel_plane_state(tmp_crtc->primary->state)->visible) {
 			if (one_pipe_only && crtc) {
-				if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
-					DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
+				set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES);
 				return NULL;
 			}
 			crtc = tmp_crtc;
@@ -471,8 +505,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 	}
 
 	if (!crtc || crtc->primary->fb == NULL) {
-		if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
-			DRM_DEBUG_KMS("no output, disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT);
 		return NULL;
 	}
 
@@ -516,14 +549,12 @@ void intel_fbc_update(struct drm_device *dev)
 		i915.enable_fbc = 0;
 
 	if (i915.enable_fbc < 0) {
-		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
-			DRM_DEBUG_KMS("disabled per chip default\n");
+		set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT);
 		goto out_disable;
 	}
 
 	if (!i915.enable_fbc) {
-		if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
-			DRM_DEBUG_KMS("fbc disabled per module param\n");
+		set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM);
 		goto out_disable;
 	}
 
@@ -547,9 +578,7 @@ void intel_fbc_update(struct drm_device *dev)
 
 	if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
 	    (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
-		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
-			DRM_DEBUG_KMS("mode incompatible with compression, "
-				      "disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE);
 		goto out_disable;
 	}
 
@@ -565,14 +594,12 @@ void intel_fbc_update(struct drm_device *dev)
 	}
 	if (intel_crtc->config->pipe_src_w > max_width ||
 	    intel_crtc->config->pipe_src_h > max_height) {
-		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
-			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE);
 		goto out_disable;
 	}
 	if ((INTEL_INFO(dev)->gen < 4 || HAS_DDI(dev)) &&
 	    intel_crtc->plane != PLANE_A) {
-		if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
-			DRM_DEBUG_KMS("plane not A, disabling compression\n");
+		set_no_fbc_reason(dev_priv, FBC_BAD_PLANE);
 		goto out_disable;
 	}
 
@@ -581,14 +608,12 @@ void intel_fbc_update(struct drm_device *dev)
 	 */
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
-		if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
-			DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
+		set_no_fbc_reason(dev_priv, FBC_NOT_TILED);
 		goto out_disable;
 	}
 	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
 	    crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
-		if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
-			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_ROTATION);
 		goto out_disable;
 	}
 
@@ -598,8 +623,7 @@ void intel_fbc_update(struct drm_device *dev)
 
 	if (i915_gem_stolen_setup_compression(dev, obj->base.size,
 					      drm_format_plane_cpp(fb->pixel_format, 0))) {
-		if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
-			DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
+		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
 		goto out_disable;
 	}
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This commit is just to make the intentions explicit: on HSW+ these
bits are MBZ, but since we only support plane A and the macro
evaluates to zero when plane A is the parameter, we're not fixing any
bug.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 9b300bd..8b980e5 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -258,11 +258,14 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
 	struct drm_framebuffer *fb = crtc->primary->fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	u32 dpfc_ctl;
+	u32 dpfc_ctl = 0;
 
 	dev_priv->fbc.enabled = true;
 
-	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
+
+	if (IS_IVYBRIDGE(dev))
+		dpfc_ctl |= IVB_DPFC_CTL_PLANE(intel_crtc->plane);
+
 	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
 		dev_priv->fbc.threshold++;
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+

[Chris Wilson]

On Thu, Jun 11, 2015 at 04:02:27PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This commit is just to make the intentions explicit: on HSW+ these
> bits are MBZ, but since we only support plane A and the macro
> evaluates to zero when plane A is the parameter, we're not fixing any
> bug.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 9b300bd..8b980e5 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -258,11 +258,14 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
>  	struct drm_framebuffer *fb = crtc->primary->fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	u32 dpfc_ctl;
> +	u32 dpfc_ctl = 0;
>  
>  	dev_priv->fbc.enabled = true;
>  
> -	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
> +
An extra line of whitespace, just because.

Minor bikeshed would be to dpfc_ctl = 0 here, so that the construction
of dpfc_ctl is in a single logical block (admittedly in this case you
have have to read back a few lines to find the initializer).

> +	if (IS_IVYBRIDGE(dev))
> +		dpfc_ctl |= IVB_DPFC_CTL_PLANE(intel_crtc->plane);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: unify no_fbc_reason message printing

[Chris Wilson]

On Thu, Jun 11, 2015 at 04:02:26PM -0300, Paulo Zanoni wrote:
> @@ -439,6 +472,8 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
>  		return false;
>  
>  	dev_priv->fbc.no_fbc_reason = reason;
> +	DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason));
> +
>  	return true;

The bool return is now unused.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] FBC trivial patches

[Chris Wilson]

On Thu, Jun 11, 2015 at 04:02:23PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Let's try to get the easy stuff merged while the rest is not ready
> yet.
> 
> This is a nice opportunity for you to easily increase your patch
> review count!

Sigh. I was until you said this. A couple of minor comments on 3 and 4.
Otherwise ltgm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: print FBC compression status on debugfs

[Ville Syrjälä]

On Thu, Jun 11, 2015 at 04:02:24PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We already had a few bugs in the past where FBC was compressing
> nothing when it was enabled, which makes the feature quite useless.
> Add this information to debugfs so the test suites can check for
> regressions in this piece of the code.
> 
> Our igt/tests/kms_frontbuffer_tracking already has support for this
> message.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 5 +++++
>  drivers/gpu/drm/i915/i915_reg.h     | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 92cf273..7358f6d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1638,6 +1638,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
>  		seq_putc(m, '\n');
>  	}
>  
> +	if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))

We already have HAS_FBC check in this function so the VLV check is
pointless.

> +		seq_printf(m, "Compressing: %s\n",
> +			   yesno(I915_READ(FBC_STATUS2) &
> +				 FBC_COMPRESSION_MASK));
> +
>  	intel_runtime_pm_put(dev_priv);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 40a3a64..0c0b12a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1951,6 +1951,9 @@ enum skl_disp_power_wells {
>  #define FBC_FENCE_OFF		0x03218 /* BSpec typo has 321Bh */
>  #define FBC_TAG			0x03300
>  
> +#define FBC_STATUS2		0x43214
> +#define  FBC_COMPRESSION_MASK	0x7ff
> +
>  #define FBC_LL_SIZE		(1536)
>  
>  /* Framebuffer compression for GM45+ */
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: unify no_fbc_reason message printing

[Damien Lespiau]

On Thu, Jun 11, 2015 at 04:02:26PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This commit has two main advantages: simplify intel_fbc_update()
> and deduplicate the strings.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I had some things around that topic as well in feb. May be of interest:

http://lists.freedesktop.org/archives/intel-gfx/2015-February/060850.html

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 49 +++------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c    | 66 +++++++++++++++++++++++++------------
>  3 files changed, 50 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 405022b..eaa567c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1594,52 +1594,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> -	if (intel_fbc_enabled(dev)) {
> +	if (intel_fbc_enabled(dev))
>  		seq_puts(m, "FBC enabled\n");
> -	} else {
> -		seq_puts(m, "FBC disabled: ");
> -		switch (dev_priv->fbc.no_fbc_reason) {
> -		case FBC_OK:
> -			seq_puts(m, "FBC actived, but currently disabled in hardware");
> -			break;
> -		case FBC_UNSUPPORTED:
> -			seq_puts(m, "unsupported by this chipset");
> -			break;
> -		case FBC_NO_OUTPUT:
> -			seq_puts(m, "no outputs");
> -			break;
> -		case FBC_STOLEN_TOO_SMALL:
> -			seq_puts(m, "not enough stolen memory");
> -			break;
> -		case FBC_UNSUPPORTED_MODE:
> -			seq_puts(m, "mode not supported");
> -			break;
> -		case FBC_MODE_TOO_LARGE:
> -			seq_puts(m, "mode too large");
> -			break;
> -		case FBC_BAD_PLANE:
> -			seq_puts(m, "FBC unsupported on plane");
> -			break;
> -		case FBC_NOT_TILED:
> -			seq_puts(m, "scanout buffer not tiled");
> -			break;
> -		case FBC_MULTIPLE_PIPES:
> -			seq_puts(m, "multiple pipes are enabled");
> -			break;
> -		case FBC_MODULE_PARAM:
> -			seq_puts(m, "disabled per module param (default off)");
> -			break;
> -		case FBC_CHIP_DEFAULT:
> -			seq_puts(m, "disabled per chip default");
> -			break;
> -		case FBC_ROTATION:
> -			seq_puts(m, "rotation not supported");
> -			break;
> -		default:
> -			seq_puts(m, "unknown reason");
> -		}
> -		seq_putc(m, '\n');
> -	}
> +	else
> +		seq_printf(m, "FBC disabled: %s\n",
> +			  intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason));
>  
>  	if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))
>  		seq_printf(m, "Compressing: %s\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b28029a..77f24e0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1255,6 +1255,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>  			  enum fb_op_origin origin);
>  void intel_fbc_flush(struct drm_i915_private *dev_priv,
>  		     unsigned int frontbuffer_bits);
> +const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>  
>  /* intel_hdmi.c */
>  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 43704a4..9b300bd 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -432,6 +432,39 @@ void intel_fbc_disable(struct drm_device *dev)
>  	dev_priv->fbc.crtc = NULL;
>  }
>  
> +const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
> +{
> +	switch (reason) {
> +	case FBC_OK:
> +		return "FBC enabled but currently disabled in hardware";
> +	case FBC_UNSUPPORTED:
> +		return "unsupported by this chipset";
> +	case FBC_NO_OUTPUT:
> +		return "no output";
> +	case FBC_STOLEN_TOO_SMALL:
> +		return "not enough stolen memory";
> +	case FBC_UNSUPPORTED_MODE:
> +		return "mode incompatible with compression";
> +	case FBC_MODE_TOO_LARGE:
> +		return "mode too large for compression";
> +	case FBC_BAD_PLANE:
> +		return "FBC unsupported on plane";
> +	case FBC_NOT_TILED:
> +		return "framebuffer not tiled or fenced";
> +	case FBC_MULTIPLE_PIPES:
> +		return "more than one pipe active";
> +	case FBC_MODULE_PARAM:
> +		return "disabled per module param";
> +	case FBC_CHIP_DEFAULT:
> +		return "disabled per chip default";
> +	case FBC_ROTATION:
> +		return "rotation unsupported";
> +	default:
> +		MISSING_CASE(reason);
> +		return "unknown reason";
> +	}
> +}
> +
>  static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
>  			      enum no_fbc_reason reason)
>  {
> @@ -439,6 +472,8 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
>  		return false;
>  
>  	dev_priv->fbc.no_fbc_reason = reason;
> +	DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason));
> +
>  	return true;
>  }
>  
> @@ -459,8 +494,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  		if (intel_crtc_active(tmp_crtc) &&
>  		    to_intel_plane_state(tmp_crtc->primary->state)->visible) {
>  			if (one_pipe_only && crtc) {
> -				if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
> -					DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
> +				set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES);
>  				return NULL;
>  			}
>  			crtc = tmp_crtc;
> @@ -471,8 +505,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  	}
>  
>  	if (!crtc || crtc->primary->fb == NULL) {
> -		if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
> -			DRM_DEBUG_KMS("no output, disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT);
>  		return NULL;
>  	}
>  
> @@ -516,14 +549,12 @@ void intel_fbc_update(struct drm_device *dev)
>  		i915.enable_fbc = 0;
>  
>  	if (i915.enable_fbc < 0) {
> -		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
> -			DRM_DEBUG_KMS("disabled per chip default\n");
> +		set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT);
>  		goto out_disable;
>  	}
>  
>  	if (!i915.enable_fbc) {
> -		if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
> -			DRM_DEBUG_KMS("fbc disabled per module param\n");
> +		set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM);
>  		goto out_disable;
>  	}
>  
> @@ -547,9 +578,7 @@ void intel_fbc_update(struct drm_device *dev)
>  
>  	if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
>  	    (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
> -		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
> -			DRM_DEBUG_KMS("mode incompatible with compression, "
> -				      "disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE);
>  		goto out_disable;
>  	}
>  
> @@ -565,14 +594,12 @@ void intel_fbc_update(struct drm_device *dev)
>  	}
>  	if (intel_crtc->config->pipe_src_w > max_width ||
>  	    intel_crtc->config->pipe_src_h > max_height) {
> -		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
> -			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE);
>  		goto out_disable;
>  	}
>  	if ((INTEL_INFO(dev)->gen < 4 || HAS_DDI(dev)) &&
>  	    intel_crtc->plane != PLANE_A) {
> -		if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
> -			DRM_DEBUG_KMS("plane not A, disabling compression\n");
> +		set_no_fbc_reason(dev_priv, FBC_BAD_PLANE);
>  		goto out_disable;
>  	}
>  
> @@ -581,14 +608,12 @@ void intel_fbc_update(struct drm_device *dev)
>  	 */
>  	if (obj->tiling_mode != I915_TILING_X ||
>  	    obj->fence_reg == I915_FENCE_REG_NONE) {
> -		if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
> -			DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
> +		set_no_fbc_reason(dev_priv, FBC_NOT_TILED);
>  		goto out_disable;
>  	}
>  	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>  	    crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
> -		if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
> -			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_ROTATION);
>  		goto out_disable;
>  	}
>  
> @@ -598,8 +623,7 @@ void intel_fbc_update(struct drm_device *dev)
>  
>  	if (i915_gem_stolen_setup_compression(dev, obj->base.size,
>  					      drm_format_plane_cpp(fb->pixel_format, 0))) {
> -		if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
> -			DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
> +		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
>  		goto out_disable;
>  	}
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 0/4] FBC trivial patches, V2

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Thanks for the V1 reviews and comments!

Paulo Zanoni (4):
  drm/i915: print FBC compression status on debugfs
  drm/i915: add FBC_ROTATION to enum no_fbc_reason
  drm/i915: unify no_fbc_reason message printing
  drm/i915: don't set the FBC plane select bits on HSW+

 drivers/gpu/drm/i915/i915_debugfs.c | 51 +++++--------------------
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_reg.h     |  3 ++
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_fbc.c    | 75 ++++++++++++++++++++++++-------------
 5 files changed, 64 insertions(+), 67 deletions(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/4] drm/i915: print FBC compression status on debugfs

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We already had a few bugs in the past where FBC was compressing
nothing when it was enabled, which makes the feature quite useless.
Add this information to debugfs so the test suites can check for
regressions in this piece of the code.

Our igt/tests/kms_frontbuffer_tracking already has support for this
message.

v2: - Remove pointless VLV check (Ville).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 5 +++++
 drivers/gpu/drm/i915/i915_reg.h     | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 92cf273..b3f42ec 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1638,6 +1638,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		seq_putc(m, '\n');
 	}
 
+	if (INTEL_INFO(dev_priv)->gen >= 7)
+		seq_printf(m, "Compressing: %s\n",
+			   yesno(I915_READ(FBC_STATUS2) &
+				 FBC_COMPRESSION_MASK));
+
 	intel_runtime_pm_put(dev_priv);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 40a3a64..0c0b12a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1951,6 +1951,9 @@ enum skl_disp_power_wells {
 #define FBC_FENCE_OFF		0x03218 /* BSpec typo has 321Bh */
 #define FBC_TAG			0x03300
 
+#define FBC_STATUS2		0x43214
+#define  FBC_COMPRESSION_MASK	0x7ff
+
 #define FBC_LL_SIZE		(1536)
 
 /* Framebuffer compression for GM45+ */
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: add FBC_ROTATION to enum no_fbc_reason

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because we're currently using FBC_UNSUPPORTED_MODE for two different
cases.

This commit will also allow us to write the next one without hiding
information from the user.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
 drivers/gpu/drm/i915/i915_drv.h     | 1 +
 drivers/gpu/drm/i915/intel_fbc.c    | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b3f42ec..f039b18 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1632,6 +1632,9 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		case FBC_CHIP_DEFAULT:
 			seq_puts(m, "disabled per chip default");
 			break;
+		case FBC_ROTATION:
+			seq_puts(m, "rotation not supported");
+			break;
 		default:
 			seq_puts(m, "unknown reason");
 		}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 611fbd8..f9ff452 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -926,6 +926,7 @@ struct i915_fbc {
 		FBC_MULTIPLE_PIPES, /* more than one pipe active */
 		FBC_MODULE_PARAM,
 		FBC_CHIP_DEFAULT, /* disabled by default on this chip */
+		FBC_ROTATION, /* rotation is not supported */
 	} no_fbc_reason;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 6abb834..43704a4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -587,7 +587,7 @@ void intel_fbc_update(struct drm_device *dev)
 	}
 	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
 	    crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
-		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+		if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
 			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
 		goto out_disable;
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: unify no_fbc_reason message printing

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This commit has two main advantages: simplify intel_fbc_update()
and deduplicate the strings.

v2:
 - Rebase due to changes on P1.
 - set_no_fbc_reason() can now return void (Chris).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 49 +++-----------------------
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_fbc.c    | 70 ++++++++++++++++++++++++-------------
 3 files changed, 51 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f039b18..ee48b95 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1594,52 +1594,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 
 	intel_runtime_pm_get(dev_priv);
 
-	if (intel_fbc_enabled(dev)) {
+	if (intel_fbc_enabled(dev))
 		seq_puts(m, "FBC enabled\n");
-	} else {
-		seq_puts(m, "FBC disabled: ");
-		switch (dev_priv->fbc.no_fbc_reason) {
-		case FBC_OK:
-			seq_puts(m, "FBC actived, but currently disabled in hardware");
-			break;
-		case FBC_UNSUPPORTED:
-			seq_puts(m, "unsupported by this chipset");
-			break;
-		case FBC_NO_OUTPUT:
-			seq_puts(m, "no outputs");
-			break;
-		case FBC_STOLEN_TOO_SMALL:
-			seq_puts(m, "not enough stolen memory");
-			break;
-		case FBC_UNSUPPORTED_MODE:
-			seq_puts(m, "mode not supported");
-			break;
-		case FBC_MODE_TOO_LARGE:
-			seq_puts(m, "mode too large");
-			break;
-		case FBC_BAD_PLANE:
-			seq_puts(m, "FBC unsupported on plane");
-			break;
-		case FBC_NOT_TILED:
-			seq_puts(m, "scanout buffer not tiled");
-			break;
-		case FBC_MULTIPLE_PIPES:
-			seq_puts(m, "multiple pipes are enabled");
-			break;
-		case FBC_MODULE_PARAM:
-			seq_puts(m, "disabled per module param (default off)");
-			break;
-		case FBC_CHIP_DEFAULT:
-			seq_puts(m, "disabled per chip default");
-			break;
-		case FBC_ROTATION:
-			seq_puts(m, "rotation not supported");
-			break;
-		default:
-			seq_puts(m, "unknown reason");
-		}
-		seq_putc(m, '\n');
-	}
+	else
+		seq_printf(m, "FBC disabled: %s\n",
+			  intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason));
 
 	if (INTEL_INFO(dev_priv)->gen >= 7)
 		seq_printf(m, "Compressing: %s\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b28029a..77f24e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,6 +1255,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
 			  enum fb_op_origin origin);
 void intel_fbc_flush(struct drm_i915_private *dev_priv,
 		     unsigned int frontbuffer_bits);
+const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
 
 /* intel_hdmi.c */
 void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 43704a4..1ff288c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -432,14 +432,47 @@ void intel_fbc_disable(struct drm_device *dev)
 	dev_priv->fbc.crtc = NULL;
 }
 
-static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
+const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
+{
+	switch (reason) {
+	case FBC_OK:
+		return "FBC enabled but currently disabled in hardware";
+	case FBC_UNSUPPORTED:
+		return "unsupported by this chipset";
+	case FBC_NO_OUTPUT:
+		return "no output";
+	case FBC_STOLEN_TOO_SMALL:
+		return "not enough stolen memory";
+	case FBC_UNSUPPORTED_MODE:
+		return "mode incompatible with compression";
+	case FBC_MODE_TOO_LARGE:
+		return "mode too large for compression";
+	case FBC_BAD_PLANE:
+		return "FBC unsupported on plane";
+	case FBC_NOT_TILED:
+		return "framebuffer not tiled or fenced";
+	case FBC_MULTIPLE_PIPES:
+		return "more than one pipe active";
+	case FBC_MODULE_PARAM:
+		return "disabled per module param";
+	case FBC_CHIP_DEFAULT:
+		return "disabled per chip default";
+	case FBC_ROTATION:
+		return "rotation unsupported";
+	default:
+		MISSING_CASE(reason);
+		return "unknown reason";
+	}
+}
+
+static void set_no_fbc_reason(struct drm_i915_private *dev_priv,
 			      enum no_fbc_reason reason)
 {
 	if (dev_priv->fbc.no_fbc_reason == reason)
-		return false;
+		return;
 
 	dev_priv->fbc.no_fbc_reason = reason;
-	return true;
+	DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason));
 }
 
 static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
@@ -459,8 +492,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 		if (intel_crtc_active(tmp_crtc) &&
 		    to_intel_plane_state(tmp_crtc->primary->state)->visible) {
 			if (one_pipe_only && crtc) {
-				if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
-					DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
+				set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES);
 				return NULL;
 			}
 			crtc = tmp_crtc;
@@ -471,8 +503,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 	}
 
 	if (!crtc || crtc->primary->fb == NULL) {
-		if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
-			DRM_DEBUG_KMS("no output, disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT);
 		return NULL;
 	}
 
@@ -516,14 +547,12 @@ void intel_fbc_update(struct drm_device *dev)
 		i915.enable_fbc = 0;
 
 	if (i915.enable_fbc < 0) {
-		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
-			DRM_DEBUG_KMS("disabled per chip default\n");
+		set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT);
 		goto out_disable;
 	}
 
 	if (!i915.enable_fbc) {
-		if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
-			DRM_DEBUG_KMS("fbc disabled per module param\n");
+		set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM);
 		goto out_disable;
 	}
 
@@ -547,9 +576,7 @@ void intel_fbc_update(struct drm_device *dev)
 
 	if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
 	    (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
-		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
-			DRM_DEBUG_KMS("mode incompatible with compression, "
-				      "disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE);
 		goto out_disable;
 	}
 
@@ -565,14 +592,12 @@ void intel_fbc_update(struct drm_device *dev)
 	}
 	if (intel_crtc->config->pipe_src_w > max_width ||
 	    intel_crtc->config->pipe_src_h > max_height) {
-		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
-			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE);
 		goto out_disable;
 	}
 	if ((INTEL_INFO(dev)->gen < 4 || HAS_DDI(dev)) &&
 	    intel_crtc->plane != PLANE_A) {
-		if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
-			DRM_DEBUG_KMS("plane not A, disabling compression\n");
+		set_no_fbc_reason(dev_priv, FBC_BAD_PLANE);
 		goto out_disable;
 	}
 
@@ -581,14 +606,12 @@ void intel_fbc_update(struct drm_device *dev)
 	 */
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
-		if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
-			DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
+		set_no_fbc_reason(dev_priv, FBC_NOT_TILED);
 		goto out_disable;
 	}
 	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
 	    crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
-		if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
-			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
+		set_no_fbc_reason(dev_priv, FBC_ROTATION);
 		goto out_disable;
 	}
 
@@ -598,8 +621,7 @@ void intel_fbc_update(struct drm_device *dev)
 
 	if (i915_gem_stolen_setup_compression(dev, obj->base.size,
 					      drm_format_plane_cpp(fb->pixel_format, 0))) {
-		if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
-			DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
+		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
 		goto out_disable;
 	}
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This commit is just to make the intentions explicit: on HSW+ these
bits are MBZ, but since we only support plane A and the macro
evaluates to zero when plane A is the parameter, we're not fixing any
bug.

v2:
 - Remove useless extra blank like (Chris).
 - Init dpfc_ctl in another place (Chris).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1ff288c..50ed333 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -262,7 +262,10 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
 
 	dev_priv->fbc.enabled = true;
 
-	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
+	dpfc_ctl = 0;
+	if (IS_IVYBRIDGE(dev))
+		dpfc_ctl |= IVB_DPFC_CTL_PLANE(intel_crtc->plane);
+
 	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
 		dev_priv->fbc.threshold++;
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] FBC trivial patches, V2

[Chris Wilson]

On Fri, Jun 12, 2015 at 02:36:17PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Thanks for the V1 reviews and comments!
> 
> Paulo Zanoni (4):
>   drm/i915: print FBC compression status on debugfs
>   drm/i915: add FBC_ROTATION to enum no_fbc_reason
>   drm/i915: unify no_fbc_reason message printing
>   drm/i915: don't set the FBC plane select bits on HSW+
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6573
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                                  312/312              312/312
IVB                                  343/343              343/343
BYT                                  287/287              287/287
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] FBC trivial patches, V2

[Daniel Vetter]

On Fri, Jun 12, 2015 at 06:42:55PM +0100, Chris Wilson wrote:
> On Fri, Jun 12, 2015 at 02:36:17PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > Thanks for the V1 reviews and comments!
> > 
> > Paulo Zanoni (4):
> >   drm/i915: print FBC compression status on debugfs
> >   drm/i915: add FBC_ROTATION to enum no_fbc_reason
> >   drm/i915: unify no_fbc_reason message printing
> >   drm/i915: don't set the FBC plane select bits on HSW+
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

All merged to dinq, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx