Revert "drm/i915: Add two-stage ILK-style watermark programming (v10)"
This reverts commit 396e33ae204f52abebec9e578f44c749305500f4.
This commit was triggering some FIFO underrun warnings on ILK-IVB
platforms (but surprisingly not on HSW/BDW that share more or less the
same codepaths). These underruns were caught by the continuous
integration (CI) system and could be reproduced consistently when
running the basic acceptance tests (BAT) on the affected platforms.
Note that this revert will cause a visible regression for some
end-users; the "flicker when mouse moves between monitors in X" issue
that was reported before this patch was merged will now return. However
regressions that are visible to CI have higher priority since they
prevent proper testing of future patches on those platforms. Hopefully
we'll be able to figure out the cause of the underruns quickly and
remerge an improved version of this patch to fix the regression.
Cc: Daniel Vetter <daniel@ffwll.ch>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93640
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1453232584-8543-1-git-send-email-matthew.d.roper@intel.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a0f5659..d70d96f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -896,7 +896,6 @@
mutex_init(&dev_priv->sb_lock);
mutex_init(&dev_priv->modeset_restore_lock);
mutex_init(&dev_priv->av_mutex);
- mutex_init(&dev_priv->wm.wm_mutex);
intel_pm_setup(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index af30148..d3b98c2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -628,11 +628,7 @@
struct dpll *best_clock);
int (*compute_pipe_wm)(struct intel_crtc *crtc,
struct drm_atomic_state *state);
- int (*compute_intermediate_wm)(struct drm_device *dev,
- struct intel_crtc *intel_crtc,
- struct intel_crtc_state *newstate);
- void (*initial_watermarks)(struct intel_crtc_state *cstate);
- void (*optimize_watermarks)(struct intel_crtc_state *cstate);
+ void (*program_watermarks)(struct intel_crtc_state *cstate);
void (*update_wm)(struct drm_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
@@ -1938,13 +1934,6 @@
};
uint8_t max_level;
-
- /*
- * Should be held around atomic WM register writing; also
- * protects * intel_crtc->wm.active and
- * cstate->wm.need_postvbl_update.
- */
- struct mutex wm_mutex;
} wm;
struct i915_runtime_pm pm;
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 9682d94..4625f8a 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -97,7 +97,6 @@
crtc_state->disable_lp_wm = false;
crtc_state->disable_cxsr = false;
crtc_state->wm_changed = false;
- crtc_state->wm.need_postvbl_update = false;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a851cb7..3260fc6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4833,42 +4833,7 @@
intel_set_memory_cxsr(dev_priv, false);
}
- /*
- * 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 (pipe_config->disable_lp_wm) {
- ilk_disable_lp_wm(dev);
- intel_wait_for_vblank(dev, crtc->pipe);
- }
-
- /*
- * If we're doing a modeset, we're done. No need to do any pre-vblank
- * watermark programming here.
- */
- if (needs_modeset(&pipe_config->base))
- return;
-
- /*
- * For platforms that support atomic watermarks, program the
- * 'intermediate' watermarks immediately. On pre-gen9 platforms, these
- * will be the intermediate values that are safe for both pre- and
- * post- vblank; when vblank happens, the 'active' values will be set
- * to the final 'target' values and we'll do this again to get the
- * optimal watermarks. For gen9+ platforms, the values we program here
- * will be the final target values which will get automatically latched
- * at vblank time; no further programming will be necessary.
- *
- * If a platform hasn't been transitioned to atomic watermarks yet,
- * we'll continue to update watermarks the old way, if flags tell
- * us to.
- */
- if (dev_priv->display.initial_watermarks != NULL)
- dev_priv->display.initial_watermarks(pipe_config);
- else if (pipe_config->wm_changed)
+ if (!needs_modeset(&pipe_config->base) && pipe_config->wm_changed)
intel_update_watermarks(&crtc->base);
}
@@ -11914,11 +11879,6 @@
pipe_config->wm_changed = true;
}
- /* Pre-gen9 platforms need two-step watermark updates */
- if (pipe_config->wm_changed && INTEL_INFO(dev)->gen < 9 &&
- dev_priv->display.optimize_watermarks)
- to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
-
if (visible || was_visible)
intel_crtc->atomic.fb_bits |=
to_intel_plane(plane)->frontbuffer_bit;
@@ -12075,29 +12035,8 @@
ret = 0;
if (dev_priv->display.compute_pipe_wm) {
ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
- if (ret) {
- DRM_DEBUG_KMS("Target pipe watermarks are invalid\n");
+ if (ret)
return ret;
- }
- }
-
- if (dev_priv->display.compute_intermediate_wm &&
- !to_intel_atomic_state(state)->skip_intermediate_wm) {
- if (WARN_ON(!dev_priv->display.compute_pipe_wm))
- return 0;
-
- /*
- * Calculate 'intermediate' watermarks that satisfy both the
- * old state and the new state. We can program these
- * immediately.
- */
- ret = dev_priv->display.compute_intermediate_wm(crtc->dev,
- intel_crtc,
- pipe_config);
- if (ret) {
- DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n");
- return ret;
- }
}
if (INTEL_INFO(dev)->gen >= 9) {
@@ -13562,7 +13501,6 @@
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc_state *crtc_state;
struct drm_crtc *crtc;
- struct intel_crtc_state *intel_cstate;
int ret = 0, i;
bool hw_check = intel_state->modeset;
@@ -13662,20 +13600,6 @@
drm_atomic_helper_wait_for_vblanks(dev, state);
- /*
- * Now that the vblank has passed, we can go ahead and program the
- * optimal watermarks on platforms that need two-step watermark
- * programming.
- *
- * TODO: Move this (and other cleanup) to an async worker eventually.
- */
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- intel_cstate = to_intel_crtc_state(crtc->state);
-
- if (dev_priv->display.optimize_watermarks)
- dev_priv->display.optimize_watermarks(intel_cstate);
- }
-
mutex_lock(&dev->struct_mutex);
drm_atomic_helper_cleanup_planes(dev, state);
mutex_unlock(&dev->struct_mutex);
@@ -15342,7 +15266,7 @@
int i;
/* Only supported on platforms that use atomic watermark design */
- if (!dev_priv->display.optimize_watermarks)
+ if (!dev_priv->display.program_watermarks)
return;
/*
@@ -15363,13 +15287,6 @@
if (WARN_ON(IS_ERR(state)))
goto fail;
- /*
- * Hardware readout is the only time we don't want to calculate
- * intermediate watermarks (since we don't trust the current
- * watermarks).
- */
- to_intel_atomic_state(state)->skip_intermediate_wm = true;
-
ret = intel_atomic_check(dev, state);
if (ret) {
/*
@@ -15392,8 +15309,7 @@
for_each_crtc_in_state(state, crtc, cstate, i) {
struct intel_crtc_state *cs = to_intel_crtc_state(cstate);
- cs->wm.need_postvbl_update = true;
- dev_priv->display.optimize_watermarks(cs);
+ dev_priv->display.program_watermarks(cs);
}
drm_atomic_state_free(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 059b46e..15917e3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -260,12 +260,6 @@
struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
struct intel_wm_config wm_config;
-
- /*
- * Current watermarks can't be trusted during hardware readout, so
- * don't bother calculating intermediate watermarks.
- */
- bool skip_intermediate_wm;
};
struct intel_plane_state {
@@ -513,29 +507,13 @@
struct {
/*
- * Optimal watermarks, programmed post-vblank when this state
- * is committed.
+ * optimal watermarks, programmed post-vblank when this state
+ * is committed
*/
union {
struct intel_pipe_wm ilk;
struct skl_pipe_wm skl;
} optimal;
-
- /*
- * Intermediate watermarks; these can be programmed immediately
- * since they satisfy both the current configuration we're
- * switching away from and the new configuration we're switching
- * to.
- */
- struct intel_pipe_wm intermediate;
-
- /*
- * Platforms with two-step watermark programming will need to
- * update watermark programming post-vblank to switch from the
- * safe intermediate watermarks to the optimal final
- * watermarks.
- */
- bool need_postvbl_update;
} wm;
};
@@ -622,7 +600,6 @@
struct intel_pipe_wm ilk;
struct skl_pipe_wm skl;
} active;
-
/* allow CxSR on this pipe */
bool cxsr_allowed;
} wm;
@@ -1583,7 +1560,6 @@
void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb /* out */);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
-bool ilk_disable_lp_wm(struct drm_device *dev);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 18055555..20bf854 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2275,29 +2275,6 @@
intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
}
-static bool ilk_validate_pipe_wm(struct drm_device *dev,
- struct intel_pipe_wm *pipe_wm)
-{
- /* LP0 watermark maximums depend on this pipe alone */
- const struct intel_wm_config config = {
- .num_pipes_active = 1,
- .sprites_enabled = pipe_wm->sprites_enabled,
- .sprites_scaled = pipe_wm->sprites_scaled,
- };
- struct ilk_wm_maximums max;
-
- /* LP0 watermarks always use 1/2 DDB partitioning */
- ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
-
- /* At least LP0 must be valid */
- if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) {
- DRM_DEBUG_KMS("LP0 watermark invalid\n");
- return false;
- }
-
- return true;
-}
-
/* Compute new watermarks for the pipe */
static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
struct drm_atomic_state *state)
@@ -2312,6 +2289,10 @@
struct intel_plane_state *sprstate = NULL;
struct intel_plane_state *curstate = NULL;
int level, max_level = ilk_wm_max_level(dev);
+ /* LP0 watermark maximums depend on this pipe alone */
+ struct intel_wm_config config = {
+ .num_pipes_active = 1,
+ };
struct ilk_wm_maximums max;
cstate = intel_atomic_get_crtc_state(state, intel_crtc);
@@ -2335,18 +2316,21 @@
curstate = to_intel_plane_state(ps);
}
- pipe_wm->pipe_enabled = cstate->base.active;
- pipe_wm->sprites_enabled = sprstate->visible;
- pipe_wm->sprites_scaled = sprstate->visible &&
+ config.sprites_enabled = sprstate->visible;
+ config.sprites_scaled = sprstate->visible &&
(drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 ||
drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16);
+ pipe_wm->pipe_enabled = cstate->base.active;
+ pipe_wm->sprites_enabled = config.sprites_enabled;
+ pipe_wm->sprites_scaled = config.sprites_scaled;
+
/* ILK/SNB: LP2+ watermarks only w/o sprites */
if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible)
max_level = 1;
/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
- if (pipe_wm->sprites_scaled)
+ if (config.sprites_scaled)
max_level = 0;
ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
@@ -2355,8 +2339,12 @@
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm->linetime = hsw_compute_linetime_wm(dev, cstate);
- if (!ilk_validate_pipe_wm(dev, pipe_wm))
- return false;
+ /* LP0 watermarks always use 1/2 DDB partitioning */
+ ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
+
+ /* At least LP0 must be valid */
+ if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
+ return -EINVAL;
ilk_compute_wm_reg_maximums(dev, 1, &max);
@@ -2381,59 +2369,6 @@
}
/*
- * Build a set of 'intermediate' watermark values that satisfy both the old
- * state and the new state. These can be programmed to the hardware
- * immediately.
- */
-static int ilk_compute_intermediate_wm(struct drm_device *dev,
- struct intel_crtc *intel_crtc,
- struct intel_crtc_state *newstate)
-{
- struct intel_pipe_wm *a = &newstate->wm.intermediate;
- struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk;
- int level, max_level = ilk_wm_max_level(dev);
-
- /*
- * Start with the final, target watermarks, then combine with the
- * currently active watermarks to get values that are safe both before
- * and after the vblank.
- */
- *a = newstate->wm.optimal.ilk;
- a->pipe_enabled |= b->pipe_enabled;
- a->sprites_enabled |= b->sprites_enabled;
- a->sprites_scaled |= b->sprites_scaled;
-
- for (level = 0; level <= max_level; level++) {
- struct intel_wm_level *a_wm = &a->wm[level];
- const struct intel_wm_level *b_wm = &b->wm[level];
-
- a_wm->enable &= b_wm->enable;
- a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val);
- a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val);
- a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val);
- a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val);
- }
-
- /*
- * We need to make sure that these merged watermark values are
- * actually a valid configuration themselves. If they're not,
- * there's no safe way to transition from the old state to
- * the new state, so we need to fail the atomic transaction.
- */
- if (!ilk_validate_pipe_wm(dev, a))
- return -EINVAL;
-
- /*
- * If our intermediate WM are identical to the final WM, then we can
- * omit the post-vblank programming; only update if it's different.
- */
- if (memcmp(a, &newstate->wm.optimal.ilk, sizeof(*a)) != 0)
- newstate->wm.need_postvbl_update = false;
-
- return 0;
-}
-
-/*
* Merge the watermarks from all active pipes for a specific level.
*/
static void ilk_merge_wm_level(struct drm_device *dev,
@@ -2445,7 +2380,9 @@
ret_wm->enable = true;
for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_pipe_wm *active = &intel_crtc->wm.active.ilk;
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
+ const struct intel_pipe_wm *active = &cstate->wm.optimal.ilk;
const struct intel_wm_level *wm = &active->wm[level];
if (!active->pipe_enabled)
@@ -2593,14 +2530,15 @@
/* LP0 register values */
for_each_intel_crtc(dev, intel_crtc) {
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
enum pipe pipe = intel_crtc->pipe;
- const struct intel_wm_level *r =
- &intel_crtc->wm.active.ilk.wm[0];
+ const struct intel_wm_level *r = &cstate->wm.optimal.ilk.wm[0];
if (WARN_ON(!r->enable))
continue;
- results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime;
+ results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime;
results->wm_pipe[pipe] =
(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -2807,7 +2745,7 @@
dev_priv->wm.hw = *results;
}
-bool ilk_disable_lp_wm(struct drm_device *dev)
+static bool ilk_disable_lp_wm(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3700,9 +3638,11 @@
}
}
-static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
+static void ilk_program_watermarks(struct intel_crtc_state *cstate)
{
- struct drm_device *dev = dev_priv->dev;
+ struct drm_crtc *crtc = cstate->base.crtc;
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct ilk_wm_maximums max;
struct intel_wm_config config = {};
@@ -3733,28 +3673,28 @@
ilk_write_wm_values(dev_priv, &results);
}
-static void ilk_initial_watermarks(struct intel_crtc_state *cstate)
+static void ilk_update_wm(struct drm_crtc *crtc)
{
- struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- mutex_lock(&dev_priv->wm.wm_mutex);
- intel_crtc->wm.active.ilk = cstate->wm.intermediate;
- ilk_program_watermarks(dev_priv);
- mutex_unlock(&dev_priv->wm.wm_mutex);
-}
+ WARN_ON(cstate->base.active != intel_crtc->active);
-static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
-{
- struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
-
- mutex_lock(&dev_priv->wm.wm_mutex);
- if (cstate->wm.need_postvbl_update) {
- intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
- ilk_program_watermarks(dev_priv);
+ /*
+ * 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 (cstate->disable_lp_wm) {
+ ilk_disable_lp_wm(crtc->dev);
+ intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
}
- mutex_unlock(&dev_priv->wm.wm_mutex);
+
+ intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
+
+ ilk_program_watermarks(cstate);
}
static void skl_pipe_wm_active_state(uint32_t val,
@@ -7081,13 +7021,9 @@
dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) ||
(!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
+ dev_priv->display.update_wm = ilk_update_wm;
dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
- dev_priv->display.compute_intermediate_wm =
- ilk_compute_intermediate_wm;
- dev_priv->display.initial_watermarks =
- ilk_initial_watermarks;
- dev_priv->display.optimize_watermarks =
- ilk_optimize_watermarks;
+ dev_priv->display.program_watermarks = ilk_program_watermarks;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");