drm/i915: Try to fix the messy IVB sprite scaling workaround

We now have a very clear method of disabling LP1+ wartermarks,
and we can actually detect if we actually did disable them, or
if they were already disabled. Use that to clean up the
WaCxSRDisabledForSpriteScaling:ivb handling.

I was hoping to apply the workaround in a way that wouldn't
require a blocking wait, but sadly IVB really does appear to
require LP1+ watermarks to be off for an entire frame before
enabling sprite scaling. Simply disabling LP1+ watermarks
during the previous frame is not enough, no matter how early
in the frame we do it :(

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

1 files changed, 45 insertions, 13 deletions

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7e028341b2f5..ecdb64e44e2a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2426,33 +2426,26 @@ static unsigned int ilk_compute_wm_dirty(struct drm_device *dev,
         return dirty;
 }
 
-/*
- * The spec says we shouldn't write when we don't need, because every write
- * causes WMs to be re-evaluated, expending some power.
- */
-static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
-                                struct hsw_wm_values *results)
+static bool _ilk_disable_lp_wm(struct drm_i915_private *dev_priv,
+                               unsigned int dirty)
 {
-        struct drm_device *dev = dev_priv->dev;
         struct hsw_wm_values *previous = &dev_priv->wm.hw;
-        unsigned int dirty;
-        uint32_t val;
-
-        dirty = ilk_compute_wm_dirty(dev_priv->dev, previous, results);
-        if (!dirty)
-                return;
+        bool changed = false;
 
         if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM1_LP_SR_EN) {
                 previous->wm_lp[2] &= ~WM1_LP_SR_EN;
                 I915_WRITE(WM3_LP_ILK, previous->wm_lp[2]);
+                changed = true;
         }
         if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM1_LP_SR_EN) {
                 previous->wm_lp[1] &= ~WM1_LP_SR_EN;
                 I915_WRITE(WM2_LP_ILK, previous->wm_lp[1]);
+                changed = true;
         }
         if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM1_LP_SR_EN) {
                 previous->wm_lp[0] &= ~WM1_LP_SR_EN;
                 I915_WRITE(WM1_LP_ILK, previous->wm_lp[0]);
+                changed = true;
         }
 
         /*
@@ -2460,6 +2453,27 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
          * Doing so could cause underruns.
          */
 
+        return changed;
+}
+
+/*
+ * The spec says we shouldn't write when we don't need, because every write
+ * causes WMs to be re-evaluated, expending some power.
+ */
+static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
+                                struct hsw_wm_values *results)
+{
+        struct drm_device *dev = dev_priv->dev;
+        struct hsw_wm_values *previous = &dev_priv->wm.hw;
+        unsigned int dirty;
+        uint32_t val;
+
+        dirty = ilk_compute_wm_dirty(dev, previous, results);
+        if (!dirty)
+                return;
+
+        _ilk_disable_lp_wm(dev_priv, dirty);
+
         if (dirty & WM_DIRTY_PIPE(PIPE_A))
                 I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
         if (dirty & WM_DIRTY_PIPE(PIPE_B))
@@ -2523,6 +2537,13 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
         dev_priv->wm.hw = *results;
 }
 
+static bool ilk_disable_lp_wm(struct drm_device *dev)
+{
+        struct drm_i915_private *dev_priv = dev->dev_private;
+
+        return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
+}
+
 static void haswell_update_wm(struct drm_crtc *crtc)
 {
         struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2572,6 +2593,7 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
                                      uint32_t sprite_width, int pixel_size,
                                      bool enabled, bool scaled)
 {
+        struct drm_device *dev = plane->dev;
         struct intel_plane *intel_plane = to_intel_plane(plane);
 
         intel_plane->wm.enabled = enabled;
@@ -2579,6 +2601,16 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
         intel_plane->wm.horiz_pixels = sprite_width;
         intel_plane->wm.bytes_per_pixel = pixel_size;
 
+        /*
+         * IVB workaround: must disable low power watermarks for at least
+         * one frame before enabling scaling.  LP watermarks can be re-enabled
+         * when scaling is disabled.
+         *
+         * WaCxSRDisabledForSpriteScaling:ivb
+         */
+        if (IS_IVYBRIDGE(dev) && scaled && ilk_disable_lp_wm(dev))
+                intel_wait_for_vblank(dev, intel_plane->pipe);
+
         haswell_update_wm(crtc);
 }