From: Ville Syrjälä <[email protected]>

Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.

What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.

This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...

REVERT THE REVERT, just do the ilk_init_lp_watermarks() motion.
---
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 40 ++++++++++++++++--------------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..bd198bd3e10d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3676,6 +3676,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 
                intel_pps_unlock_regs_wa(dev_priv);
                intel_modeset_init_hw(dev);
+               intel_init_clock_gating(dev_priv);
 
                spin_lock_irq(&dev_priv->irq_lock);
                if (dev_priv->display.hpd_irq_setup)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07118c0b69d3..352a6739ed70 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
        mutex_unlock(&dev_priv->wm.wm_mutex);
 }
 
+/*
+ * FIXME should probably kill this and improve
+ * the real watermark readout/sanitation instead
+ */
+static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
+{
+       I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+       I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+       I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+       /*
+        * Don't touch WM1S_LP_EN here.
+        * Doing so could cause underruns.
+        */
+}
+
 void ilk_wm_get_hw_state(struct drm_device *dev)
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        struct ilk_wm_values *hw = &dev_priv->wm.hw;
        struct drm_crtc *crtc;
 
+       ilk_init_lp_watermarks(dev_priv);
+
        for_each_crtc(dev, crtc)
                ilk_pipe_wm_get_hw_state(crtc);
 
@@ -8213,18 +8231,6 @@ static void g4x_disable_trickle_feed(struct 
drm_i915_private *dev_priv)
        }
 }
 
-static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
-{
-       I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
-       I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
-       I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
-
-       /*
-        * Don't touch WM1S_LP_EN here.
-        * Doing so could cause underruns.
-        */
-}
-
 static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
 {
        uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
@@ -8258,8 +8264,6 @@ static void ilk_init_clock_gating(struct drm_i915_private 
*dev_priv)
                   (I915_READ(DISP_ARB_CTL) |
                    DISP_FBC_WM_DIS));
 
-       ilk_init_lp_watermarks(dev_priv);
-
        /*
         * Based on the document from hardware guys the following bits
         * should be set unconditionally in order to enable FBC.
@@ -8372,8 +8376,6 @@ static void gen6_init_clock_gating(struct 
drm_i915_private *dev_priv)
        I915_WRITE(GEN6_GT_MODE,
                   _MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
 
-       ilk_init_lp_watermarks(dev_priv);
-
        I915_WRITE(CACHE_MODE_0,
                   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
 
@@ -8600,8 +8602,6 @@ static void bdw_init_clock_gating(struct drm_i915_private 
*dev_priv)
                                                 I915_GTT_PAGE_SIZE_2M);
        enum pipe pipe;
 
-       ilk_init_lp_watermarks(dev_priv);
-
        /* WaSwitchSolVfFArbitrationPriority:bdw */
        I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
 
@@ -8652,8 +8652,6 @@ static void bdw_init_clock_gating(struct drm_i915_private 
*dev_priv)
 
 static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
 {
-       ilk_init_lp_watermarks(dev_priv);
-
        /* L3 caching of data atomics doesn't work -- disable it. */
        I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
        I915_WRITE(HSW_ROW_CHICKEN3,
@@ -8708,8 +8706,6 @@ static void ivb_init_clock_gating(struct drm_i915_private 
*dev_priv)
 {
        uint32_t snpcr;
 
-       ilk_init_lp_watermarks(dev_priv);
-
        I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
 
        /* WaDisableEarlyCull:ivb */
-- 
2.15.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to