[Intel-gfx] [PATCH 2/2] drm/i915: add SW tracking to FBC enabling

[Paulo Zanoni]

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

Currently, calling intel_fbc_enabled() will trigger a register read.
And we call it a lot of times, even when FBC is disabled, so saving a
few cycles would be a good thing.

Another reason for this patch is because we currently call
intel_fbc_enabled() while the HW is runtime suspended, so the read
makes no sense and triggers a WARN. This happens even if FBC is
disabled by default. Of course one could argue that we just shouldn't
be calling intel_fbc_enabled() while the driver is runtime suspended,
and I agree that's a good argument, but I still think that the reason
explained in the first paragraph already justifies the patch.
This problem can easily be reproduced with many subtests of
igt/pm_rpm, and it is a regression introduced by:

    commit c5ad011d7d256ecbe173324029e992817194d2b0
    Author: Rodrigo Vivi <rodrigo.vivi at intel.com>
    Date:   Mon Aug 4 03:51:38 2014 -0700
        drm/i915: FBC flush nuke for BDW

Testcase: igt/pm_rpm/cursor (and others)
Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++++++-----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5fce16c..999bd57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -662,6 +662,10 @@ struct i915_fbc {
 
 	bool false_color;
 
+	/* Tracks whether the HW is actually enabled, not whether the feature is
+	 * possible. */
+	bool enabled;
+
 	struct intel_fbc_work {
 		struct delayed_work work;
 		struct drm_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2ca9fdb..6b41620 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -71,6 +71,8 @@ static void i8xx_disable_fbc(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 fbc_ctl;
 
+	dev_priv->fbc.enabled = false;
+
 	/* Disable compression */
 	fbc_ctl = I915_READ(FBC_CONTROL);
 	if ((fbc_ctl & FBC_CTL_EN) == 0)
@@ -99,6 +101,8 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 	int i;
 	u32 fbc_ctl;
 
+	dev_priv->fbc.enabled = true;
+
 	cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
 	if (fb->pitches[0] < cfb_pitch)
 		cfb_pitch = fb->pitches[0];
@@ -153,6 +157,8 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	u32 dpfc_ctl;
 
+	dev_priv->fbc.enabled = true;
+
 	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN;
 	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
@@ -173,6 +179,8 @@ static void g4x_disable_fbc(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpfc_ctl;
 
+	dev_priv->fbc.enabled = false;
+
 	/* Disable compression */
 	dpfc_ctl = I915_READ(DPFC_CONTROL);
 	if (dpfc_ctl & DPFC_CTL_EN) {
@@ -224,6 +232,8 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	u32 dpfc_ctl;
 
+	dev_priv->fbc.enabled = true;
+
 	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane);
 	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
 		dev_priv->fbc.threshold++;
@@ -264,6 +274,8 @@ static void ironlake_disable_fbc(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpfc_ctl;
 
+	dev_priv->fbc.enabled = false;
+
 	/* Disable compression */
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
 	if (dpfc_ctl & DPFC_CTL_EN) {
@@ -290,6 +302,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	u32 dpfc_ctl;
 
+	dev_priv->fbc.enabled = true;
+
 	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
 	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
 		dev_priv->fbc.threshold++;
@@ -339,16 +353,7 @@ bool intel_fbc_enabled(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* If it wasn't never enabled by kernel parameter or platform default
-	 * we can avoid reading registers so many times in vain
-	 */
-	if (!i915.enable_fbc)
-		return false;
-
-	if (!dev_priv->display.fbc_enabled)
-		return false;
-
-	return dev_priv->display.fbc_enabled(dev);
+	return dev_priv->fbc.enabled;
 }
 
 void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
@@ -7360,8 +7365,10 @@ void intel_fini_runtime_pm(struct drm_i915_private *dev_priv)
 
 static void intel_init_fbc(struct drm_i915_private *dev_priv)
 {
-	if (!HAS_FBC(dev_priv))
+	if (!HAS_FBC(dev_priv)) {
+		dev_priv->fbc.enabled = false;
 		return;
+	}
 
 	if (INTEL_INFO(dev_priv)->gen >= 7) {
 		dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
@@ -7383,6 +7390,8 @@ static void intel_init_fbc(struct drm_i915_private *dev_priv)
 		/* This value was pulled out of someone's hat */
 		I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
 	}
+
+	dev_priv->fbc.enabled = dev_priv->display.fbc_enabled(dev_priv->dev);
 }
 
 /* Set up chip specific power management-related functions */
-- 
2.1.0