[PATCH 05/24] drm/i915: Merge LP1+ watermarks in safer way

ville.syrjala
2014-03-07T16:33:04Z

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On ILK when we disable a particular watermark level, we must
maintain the actual watermark values for that level for some time
(until the next vblank possibly). Otherwise we risk underruns.

In order to achieve that result we must merge the LP1+ watermarks a
bit differently since we must also merge levels that are to be
disabled. We must also make sure we don't overflow the fields in the
watermark registers in case the calculated watermarks come out too
big to fit.

As early as possbile we mark all computed watermark levels as
disabled if they would exceed the register maximums. We make sure
to leave the actual watermarks for such levels zeroed out. The during
merging, we take the maxium values for every level, regardless if
they're disabled or not. That may seem a bit pointless since at the
moment all the watermark levels we merge should have their values
zeroed if the level is already disabled. However soon we will be
dealing with intermediate watermarks that, in addition to the new
watermark values, also contain the previous watermark values, and so
levels that are disabled may no longer be zeroed out.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 81 ++++++++++++++++++++++++++++++++---------
1 file changed, 64 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f061ef1..ba4b23e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1921,6 +1921,16 @@ static void ilk_compute_wm_maximums(const struct drm_device *dev,
max->fbc = ilk_fbc_wm_reg_max(dev);
}

+static void ilk_compute_wm_reg_maximums(struct drm_device *dev,
+ int level,
+ struct ilk_wm_maximums *max)
+{
+ max->pri = ilk_plane_wm_reg_max(dev, level, false);
+ max->spr = ilk_plane_wm_reg_max(dev, level, true);
+ max->cur = ilk_cursor_wm_reg_max(dev, level);
+ max->fbc = ilk_fbc_wm_reg_max(dev);
+}
+
static bool ilk_validate_wm_level(int level,
const struct ilk_wm_maximums *max,
struct intel_wm_level *result)
@@ -2178,9 +2188,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
};
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);
-
pipe_wm->pipe_enabled = params->active;
pipe_wm->sprites_enabled = params->spr.enabled;
pipe_wm->sprites_scaled = params->spr.scaled;
@@ -2193,15 +2200,37 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
if (params->spr.scaled)
max_level = 0;

- for (level = 0; level <= max_level; level++)
- ilk_compute_wm_level(dev_priv, level, params,
- &pipe_wm->wm[level]);
+ ilk_compute_wm_level(dev_priv, 0, params, &pipe_wm->wm[0]);

if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);

+ /* 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 */
- return ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]);
+ if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
+ return false;
+
+ ilk_compute_wm_reg_maximums(dev, 1, &max);
+
+ for (level = 1; level <= max_level; level++) {
+ struct intel_wm_level wm = {};
+
+ ilk_compute_wm_level(dev_priv, level, params, &wm);
+
+ /*
+ * Disable any watermark level that exceeds the
+ * register maximums since such watermarks are
+ * always invalid.
+ */
+ if (!ilk_validate_wm_level(level, &max, &wm))
+ break;
+
+ pipe_wm->wm[level] = wm;
+ }
+
+ return true;
}

/*
@@ -2213,6 +2242,8 @@ static void ilk_merge_wm_level(struct drm_device *dev,
{
const struct intel_crtc *intel_crtc;

+ ret_wm->enable = true;
+
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head) {
const struct intel_pipe_wm *active = &intel_crtc->wm.active;
const struct intel_wm_level *wm = &active->wm[level];
@@ -2220,16 +2251,18 @@ static void ilk_merge_wm_level(struct drm_device *dev,
if (!active->pipe_enabled)
continue;

- if (!wm->enable)
- return;
+ /*
+ * The watermark values may have been used in the past,
+ * so we must maintain them in the registers for some
+ * time even if the level is now disabled.
+ */
+ ret_wm->enable &= wm->enable;

ret_wm->pri_val = max(ret_wm->pri_val, wm->pri_val);
ret_wm->spr_val = max(ret_wm->spr_val, wm->spr_val);
ret_wm->cur_val = max(ret_wm->cur_val, wm->cur_val);
ret_wm->fbc_val = max(ret_wm->fbc_val, wm->fbc_val);
}
-
- ret_wm->enable = true;
}

/*
@@ -2241,6 +2274,7 @@ static void ilk_wm_merge(struct drm_device *dev,
struct intel_pipe_wm *merged)
{
int level, max_level = ilk_wm_max_level(dev);
+ int last_enabled_level = max_level;

/* ILK/SNB/IVB: LP1+ watermarks only w/ single pipe */
if ((INTEL_INFO(dev)->gen <= 6 || IS_IVYBRIDGE(dev)) &&
@@ -2256,15 +2290,19 @@ static void ilk_wm_merge(struct drm_device *dev,

ilk_merge_wm_level(dev, level, wm);

- if (!ilk_validate_wm_level(level, max, wm))
- break;
+ if (level > last_enabled_level)
+ wm->enable = false;
+ else if (!ilk_validate_wm_level(level, max, wm))
+ /* make sure all following levels get disabled */
+ last_enabled_level = level - 1;

/*
* The spec says it is preferred to disable
* FBC WMs instead of disabling a WM level.
*/
if (wm->fbc_val > max->fbc) {
- merged->fbc_wm_enabled = false;
+ if (wm->enable)
+ merged->fbc_wm_enabled = false;
wm->fbc_val = 0;
}
}
@@ -2319,14 +2357,19 @@ static void ilk_compute_wm_results(struct drm_device *dev,
level = ilk_wm_lp_to_level(wm_lp, merged);

r = &merged->wm[level];
- if (!r->enable)
- break;

- results->wm_lp[wm_lp - 1] = WM3_LP_EN |
+ /*
+ * Maintain the watermark values even if the level is
+ * disabled. Doing otherwise could cause underruns.
+ */
+ results->wm_lp[wm_lp - 1] =
(ilk_wm_lp_latency(dev, level) << WM1_LP_LATENCY_SHIFT) |
(r->pri_val << WM1_LP_SR_SHIFT) |
r->cur_val;

+ if (r->enable)
+ results->wm_lp[wm_lp - 1] |= WM1_LP_SR_EN;
+
if (INTEL_INFO(dev)->gen >= 8)
results->wm_lp[wm_lp - 1] |=
r->fbc_val << WM1_LP_FBC_SHIFT_BDW;
@@ -2334,6 +2377,10 @@ static void ilk_compute_wm_results(struct drm_device *dev,
results->wm_lp[wm_lp - 1] |=
r->fbc_val << WM1_LP_FBC_SHIFT;

+ /*
+ * Always set WM1S_LP_EN when spr_val != 0, even if the
+ * level is disabled. Doing otherwise could cause underruns.
+ */
if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
WARN_ON(wm_lp != 1);
results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
--
1.8.3.2