* [PATCH] drm/i915: Partial revert of atomic watermark series
@ 2015-10-08 22:28 Matt Roper
2015-10-09 8:41 ` Daniel Vetter
2015-10-09 20:33 ` Zanoni, Paulo R
0 siblings, 2 replies; 5+ messages in thread
From: Matt Roper @ 2015-10-08 22:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Vetter, Daniel
It's been reported that the atomic watermark series triggers some
regressions on SKL, which we haven't been able to track down yet. Let's
temporarily revert these patches while we track down the root cause.
This commit squashes the reverts of:
76305b1 drm/i915: Calculate watermark configuration during atomic check (v2)
a4611e4 drm/i915: Don't set plane visible during HW readout if CRTC is off
a28170f drm/i915: Calculate ILK-style watermarks during atomic check (v3)
de4a9f8 drm/i915: Calculate pipe watermarks into CRTC state (v3)
de165e0 drm/i915: Refactor ilk_update_wm (v3)
Reference: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077190.html
Cc: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Cc: "Vetter, Daniel" <daniel.vetter@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
I just got access to BXT hardware, so I'm hoping that the SKL problems reported
by Paulo will also be reproducible on BXT. However I'm not going to be able to
work on this for a couple days, so it's probably better to do some reverts now
so that we don't leave di-nightly in a broken state in the meantime.
Also note that this is a different regression than the one reported by Jani
(for which we've already reverted a couple patches); his issue happens on the
ILK-style watermark path which isn't relevant for Paulo's issues here.
drivers/gpu/drm/i915/i915_drv.h | 12 --
drivers/gpu/drm/i915/intel_display.c | 60 +--------
drivers/gpu/drm/i915/intel_drv.h | 49 +++-----
drivers/gpu/drm/i915/intel_pm.c | 232 +++++++++++++++++++----------------
4 files changed, 151 insertions(+), 202 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fbf0ae9..4b02be7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -627,8 +627,6 @@ struct drm_i915_display_funcs {
int target, int refclk,
struct dpll *match_clock,
struct dpll *best_clock);
- int (*compute_pipe_wm)(struct intel_crtc *crtc,
- struct drm_atomic_state *state);
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);
@@ -1692,13 +1690,6 @@ struct i915_execbuffer_params {
struct drm_i915_gem_request *request;
};
-/* used in computing the new watermarks state */
-struct intel_wm_config {
- unsigned int num_pipes_active;
- bool sprites_enabled;
- bool sprites_scaled;
-};
-
struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *objects;
@@ -1923,9 +1914,6 @@ struct drm_i915_private {
*/
uint16_t skl_latency[8];
- /* Committed wm config */
- struct intel_wm_config config;
-
/*
* The skl_wm_values structure is a bit too big for stack
* allocation, so we keep the staging struct where we store
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 539c373..d84d5c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11837,12 +11837,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
}
ret = 0;
- if (dev_priv->display.compute_pipe_wm) {
- ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
- if (ret)
- return ret;
- }
-
if (INTEL_INFO(dev)->gen >= 9) {
if (mode_changed)
ret = skl_update_scaler_crtc(pipe_config);
@@ -13048,45 +13042,6 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
return 0;
}
-/*
- * Handle calculation of various watermark data at the end of the atomic check
- * phase. The code here should be run after the per-crtc and per-plane 'check'
- * handlers to ensure that all derived state has been updated.
- */
-static void calc_watermark_data(struct drm_atomic_state *state)
-{
- struct drm_device *dev = state->dev;
- struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
- struct drm_crtc *crtc;
- struct drm_crtc_state *cstate;
- struct drm_plane *plane;
- struct drm_plane_state *pstate;
-
- /*
- * Calculate watermark configuration details now that derived
- * plane/crtc state is all properly updated.
- */
- drm_for_each_crtc(crtc, dev) {
- cstate = drm_atomic_get_existing_crtc_state(state, crtc) ?:
- crtc->state;
-
- if (cstate->active)
- intel_state->wm_config.num_pipes_active++;
- }
- drm_for_each_legacy_plane(plane, dev) {
- pstate = drm_atomic_get_existing_plane_state(state, plane) ?:
- plane->state;
-
- if (!to_intel_plane_state(pstate)->visible)
- continue;
-
- intel_state->wm_config.sprites_enabled = true;
- if (pstate->crtc_w != pstate->src_w >> 16 ||
- pstate->crtc_h != pstate->src_h >> 16)
- intel_state->wm_config.sprites_scaled = true;
- }
-}
-
/**
* intel_atomic_check - validate state object
* @dev: drm device
@@ -13095,7 +13050,6 @@ static void calc_watermark_data(struct drm_atomic_state *state)
static int intel_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
- struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int ret, i;
@@ -13159,15 +13113,10 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
} else
- intel_state->cdclk = to_i915(state->dev)->cdclk_freq;
-
- ret = drm_atomic_helper_check_planes(state->dev, state);
- if (ret)
- return ret;
-
- calc_watermark_data(state);
+ to_intel_atomic_state(state)->cdclk =
+ to_i915(state->dev)->cdclk_freq;
- return 0;
+ return drm_atomic_helper_check_planes(state->dev, state);
}
/**
@@ -13207,7 +13156,6 @@ static int intel_atomic_commit(struct drm_device *dev,
return ret;
drm_atomic_helper_swap_state(dev, state);
- dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -15221,7 +15169,7 @@ static void readout_plane_state(struct intel_crtc *crtc)
struct intel_plane_state *plane_state =
to_intel_plane_state(primary->state);
- plane_state->visible = crtc->active &&
+ plane_state->visible =
primary_get_hw_state(to_intel_plane(primary));
if (plane_state->visible)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e320825..91b6b40 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -250,7 +250,6 @@ struct intel_atomic_state {
unsigned int cdclk;
bool dpll_set;
struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
- struct intel_wm_config wm_config;
};
struct intel_plane_state {
@@ -335,21 +334,6 @@ struct intel_crtc_scaler_state {
/* drm_mode->private_flags */
#define I915_MODE_FLAG_INHERITED 1
-struct intel_pipe_wm {
- struct intel_wm_level wm[5];
- uint32_t linetime;
- bool fbc_wm_enabled;
- bool pipe_enabled;
- bool sprites_enabled;
- bool sprites_scaled;
-};
-
-struct skl_pipe_wm {
- struct skl_wm_level wm[8];
- struct skl_wm_level trans_wm;
- uint32_t linetime;
-};
-
struct intel_crtc_state {
struct drm_crtc_state base;
@@ -487,17 +471,6 @@ struct intel_crtc_state {
/* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
bool disable_lp_wm;
-
- struct {
- /*
- * optimal watermarks, programmed post-vblank when this state
- * is committed
- */
- union {
- struct intel_pipe_wm ilk;
- struct skl_pipe_wm skl;
- } optimal;
- } wm;
};
struct vlv_wm_state {
@@ -509,6 +482,15 @@ struct vlv_wm_state {
bool cxsr;
};
+struct intel_pipe_wm {
+ struct intel_wm_level wm[5];
+ uint32_t linetime;
+ bool fbc_wm_enabled;
+ bool pipe_enabled;
+ bool sprites_enabled;
+ bool sprites_scaled;
+};
+
struct intel_mmio_flip {
struct work_struct work;
struct drm_i915_private *i915;
@@ -516,6 +498,12 @@ struct intel_mmio_flip {
struct intel_crtc *crtc;
};
+struct skl_pipe_wm {
+ struct skl_wm_level wm[8];
+ struct skl_wm_level trans_wm;
+ uint32_t linetime;
+};
+
/*
* Tracking of operations that need to be performed at the beginning/end of an
* atomic commit, outside the atomic section where interrupts are disabled.
@@ -583,10 +571,9 @@ struct intel_crtc {
/* per-pipe watermark state */
struct {
/* watermarks currently being used */
- union {
- struct intel_pipe_wm ilk;
- struct skl_pipe_wm skl;
- } active;
+ struct intel_pipe_wm active;
+ /* SKL wm values currently in use */
+ struct skl_pipe_wm skl_active;
/* allow CxSR on this pipe */
bool cxsr_allowed;
} wm;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 60d120c..4993695 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1772,6 +1772,13 @@ struct ilk_wm_maximums {
uint16_t fbc;
};
+/* used in computing the new watermarks state */
+struct intel_wm_config {
+ unsigned int num_pipes_active;
+ bool sprites_enabled;
+ bool sprites_scaled;
+};
+
/*
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
@@ -2022,11 +2029,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
const struct intel_crtc *intel_crtc,
int level,
struct intel_crtc_state *cstate,
- struct intel_plane_state *pristate,
- struct intel_plane_state *sprstate,
- struct intel_plane_state *curstate,
struct intel_wm_level *result)
{
+ struct intel_plane *intel_plane;
uint16_t pri_latency = dev_priv->wm.pri_latency[level];
uint16_t spr_latency = dev_priv->wm.spr_latency[level];
uint16_t cur_latency = dev_priv->wm.cur_latency[level];
@@ -2038,11 +2043,29 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
cur_latency *= 5;
}
- result->pri_val = ilk_compute_pri_wm(cstate, pristate,
- pri_latency, level);
- result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
- result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
- result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
+ for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) {
+ struct intel_plane_state *pstate =
+ to_intel_plane_state(intel_plane->base.state);
+
+ switch (intel_plane->base.type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ result->pri_val = ilk_compute_pri_wm(cstate, pstate,
+ pri_latency,
+ level);
+ result->fbc_val = ilk_compute_fbc_wm(cstate, pstate,
+ result->pri_val);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ result->spr_val = ilk_compute_spr_wm(cstate, pstate,
+ spr_latency);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ result->cur_val = ilk_compute_cur_wm(cstate, pstate,
+ cur_latency);
+ break;
+ }
+ }
+
result->enable = true;
}
@@ -2301,19 +2324,34 @@ static void skl_setup_wm_latency(struct drm_device *dev)
intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
}
+static void ilk_compute_wm_config(struct drm_device *dev,
+ struct intel_wm_config *config)
+{
+ struct intel_crtc *intel_crtc;
+
+ /* Compute the currently _active_ config */
+ for_each_intel_crtc(dev, intel_crtc) {
+ const struct intel_pipe_wm *wm = &intel_crtc->wm.active;
+
+ if (!wm->pipe_enabled)
+ continue;
+
+ config->sprites_enabled |= wm->sprites_enabled;
+ config->sprites_scaled |= wm->sprites_scaled;
+ config->num_pipes_active++;
+ }
+}
+
/* Compute new watermarks for the pipe */
-static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
- struct drm_atomic_state *state)
+static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
+ struct intel_pipe_wm *pipe_wm)
{
- struct intel_pipe_wm *pipe_wm;
- struct drm_device *dev = intel_crtc->base.dev;
+ struct drm_crtc *crtc = cstate->base.crtc;
+ struct drm_device *dev = crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc_state *cstate = NULL;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane;
- struct drm_plane_state *ps;
- struct intel_plane_state *pristate = NULL;
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 = {
@@ -2321,24 +2359,11 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
};
struct ilk_wm_maximums max;
- cstate = intel_atomic_get_crtc_state(state, intel_crtc);
- if (IS_ERR(cstate))
- return PTR_ERR(cstate);
-
- pipe_wm = &cstate->wm.optimal.ilk;
-
for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- ps = drm_atomic_get_plane_state(state,
- &intel_plane->base);
- if (IS_ERR(ps))
- return PTR_ERR(ps);
-
- if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
- pristate = to_intel_plane_state(ps);
- else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
- sprstate = to_intel_plane_state(ps);
- else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
- curstate = to_intel_plane_state(ps);
+ if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
+ sprstate = to_intel_plane_state(intel_plane->base.state);
+ break;
+ }
}
config.sprites_enabled = sprstate->visible;
@@ -2347,7 +2372,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
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_enabled = sprstate->visible;
pipe_wm->sprites_scaled = config.sprites_scaled;
/* ILK/SNB: LP2+ watermarks only w/o sprites */
@@ -2358,27 +2383,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
if (config.sprites_scaled)
max_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
- pristate, sprstate, curstate, &pipe_wm->wm[0]);
+ ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]);
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- pipe_wm->linetime = hsw_compute_linetime_wm(dev,
- &intel_crtc->base);
+ 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 */
if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
- return -EINVAL;
+ 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, intel_crtc, level, cstate,
- pristate, sprstate, curstate, &wm);
+ ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm);
/*
* Disable any watermark level that exceeds the
@@ -2391,7 +2413,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
pipe_wm->wm[level] = wm;
}
- return 0;
+ return true;
}
/*
@@ -2406,9 +2428,7 @@ static void ilk_merge_wm_level(struct drm_device *dev,
ret_wm->enable = true;
for_each_intel_crtc(dev, intel_crtc) {
- 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_pipe_wm *active = &intel_crtc->wm.active;
const struct intel_wm_level *wm = &active->wm[level];
if (!active->pipe_enabled)
@@ -2556,15 +2576,14 @@ static void ilk_compute_wm_results(struct drm_device *dev,
/* 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 = &cstate->wm.optimal.ilk.wm[0];
+ const struct intel_wm_level *r =
+ &intel_crtc->wm.active.wm[0];
if (WARN_ON(!r->enable))
continue;
- results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime;
+ results->wm_linetime[pipe] = intel_crtc->wm.active.linetime;
results->wm_pipe[pipe] =
(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -2946,12 +2965,11 @@ skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
static void
skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
+ const struct intel_wm_config *config,
struct skl_ddb_allocation *ddb /* out */)
{
struct drm_crtc *crtc = cstate->base.crtc;
struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_wm_config *config = &dev_priv->wm.config;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane;
enum pipe pipe = intel_crtc->pipe;
@@ -3126,6 +3144,15 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
return false;
}
+static void skl_compute_wm_global_parameters(struct drm_device *dev,
+ struct intel_wm_config *config)
+{
+ struct drm_crtc *crtc;
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+ config->num_pipes_active += to_intel_crtc(crtc)->active;
+}
+
static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
struct intel_crtc_state *cstate,
struct intel_plane *intel_plane,
@@ -3530,25 +3557,27 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
}
static bool skl_update_pipe_wm(struct drm_crtc *crtc,
+ struct intel_wm_config *config,
struct skl_ddb_allocation *ddb, /* out */
struct skl_pipe_wm *pipe_wm /* out */)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- skl_allocate_pipe_ddb(cstate, ddb);
+ skl_allocate_pipe_ddb(cstate, config, ddb);
skl_compute_pipe_wm(cstate, ddb, pipe_wm);
- if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
+ if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
return false;
- intel_crtc->wm.active.skl = *pipe_wm;
+ intel_crtc->wm.skl_active = *pipe_wm;
return true;
}
static void skl_update_other_pipe_wm(struct drm_device *dev,
struct drm_crtc *crtc,
+ struct intel_wm_config *config,
struct skl_wm_values *r)
{
struct intel_crtc *intel_crtc;
@@ -3578,7 +3607,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
if (!intel_crtc->active)
continue;
- wm_changed = skl_update_pipe_wm(&intel_crtc->base,
+ wm_changed = skl_update_pipe_wm(&intel_crtc->base, config,
&r->ddb, &pipe_wm);
/*
@@ -3619,8 +3648,8 @@ static void skl_update_wm(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct skl_wm_values *results = &dev_priv->wm.skl_results;
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct skl_pipe_wm *pipe_wm = &cstate->wm.optimal.skl;
+ struct skl_pipe_wm pipe_wm = {};
+ struct intel_wm_config config = {};
/* Clear all dirty flags */
@@ -3628,13 +3657,15 @@ static void skl_update_wm(struct drm_crtc *crtc)
skl_clear_wm(results, intel_crtc->pipe);
- if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
+ skl_compute_wm_global_parameters(dev, &config);
+
+ if (!skl_update_pipe_wm(crtc, &config, &results->ddb, &pipe_wm))
return;
- skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
+ skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
results->dirty[intel_crtc->pipe] = true;
- skl_update_other_pipe_wm(dev, crtc, results);
+ skl_update_other_pipe_wm(dev, crtc, &config, results);
skl_write_wm_values(dev_priv, results);
skl_flush_wm_values(dev_priv, results);
@@ -3642,23 +3673,50 @@ static void skl_update_wm(struct drm_crtc *crtc)
dev_priv->wm.skl_hw = *results;
}
-static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
+static void ilk_update_wm(struct drm_crtc *crtc)
{
- struct drm_device *dev = dev_priv->dev;
- struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct ilk_wm_maximums max;
- struct intel_wm_config *config = &dev_priv->wm.config;
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
+ struct intel_pipe_wm pipe_wm = {};
+ struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
+ struct intel_wm_config config = {};
- ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max);
- ilk_wm_merge(dev, config, &max, &lp_wm_1_2);
+ WARN_ON(cstate->base.active != intel_crtc->active);
+
+ /*
+ * 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(dev);
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ }
+
+ intel_compute_pipe_wm(cstate, &pipe_wm);
+
+ if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
+ return;
+
+ intel_crtc->wm.active = pipe_wm;
+
+ ilk_compute_wm_config(dev, &config);
+
+ ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max);
+ ilk_wm_merge(dev, &config, &max, &lp_wm_1_2);
/* 5/6 split only in single pipe config on IVB+ */
if (INTEL_INFO(dev)->gen >= 7 &&
- config->num_pipes_active == 1 && config->sprites_enabled) {
- ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_5_6, &max);
- ilk_wm_merge(dev, config, &max, &lp_wm_5_6);
+ config.num_pipes_active == 1 && config.sprites_enabled) {
+ ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, &max);
+ ilk_wm_merge(dev, &config, &max, &lp_wm_5_6);
best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6);
} else {
@@ -3673,31 +3731,6 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
ilk_write_wm_values(dev_priv, &results);
}
-static void ilk_update_wm(struct drm_crtc *crtc)
-{
- struct drm_i915_private *dev_priv = to_i915(crtc->dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
-
- WARN_ON(cstate->base.active != intel_crtc->active);
-
- /*
- * 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);
- }
-
- intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
-
- ilk_program_watermarks(dev_priv);
-}
-
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
@@ -3748,8 +3781,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct skl_pipe_wm *active = &cstate->wm.optimal.skl;
+ struct skl_pipe_wm *active = &intel_crtc->wm.skl_active;
enum pipe pipe = intel_crtc->pipe;
int level, i, max_level;
uint32_t temp;
@@ -3793,8 +3825,6 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
temp = hw->plane_trans[pipe][PLANE_CURSOR];
skl_pipe_wm_active_state(temp, active, true, true, i, 0);
-
- intel_crtc->wm.active.skl = *active;
}
void skl_wm_get_hw_state(struct drm_device *dev)
@@ -3814,8 +3844,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct intel_pipe_wm *active = &cstate->wm.optimal.ilk;
+ struct intel_pipe_wm *active = &intel_crtc->wm.active;
enum pipe pipe = intel_crtc->pipe;
static const unsigned int wm0_pipe_reg[] = {
[PIPE_A] = WM0_PIPEA_ILK,
@@ -3854,8 +3883,6 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
for (level = 0; level <= max_level; level++)
active->wm[level].enable = true;
}
-
- intel_crtc->wm.active.ilk = *active;
}
#define _FW_WM(value, plane) \
@@ -7015,7 +7042,6 @@ void intel_init_pm(struct drm_device *dev)
(!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;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Partial revert of atomic watermark series
2015-10-08 22:28 [PATCH] drm/i915: Partial revert of atomic watermark series Matt Roper
@ 2015-10-09 8:41 ` Daniel Vetter
2015-10-09 20:33 ` Zanoni, Paulo R
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-10-09 8:41 UTC (permalink / raw)
To: Matt Roper; +Cc: Vetter, Daniel, intel-gfx
On Thu, Oct 08, 2015 at 03:28:25PM -0700, Matt Roper wrote:
> It's been reported that the atomic watermark series triggers some
> regressions on SKL, which we haven't been able to track down yet. Let's
> temporarily revert these patches while we track down the root cause.
>
> This commit squashes the reverts of:
> 76305b1 drm/i915: Calculate watermark configuration during atomic check (v2)
> a4611e4 drm/i915: Don't set plane visible during HW readout if CRTC is off
> a28170f drm/i915: Calculate ILK-style watermarks during atomic check (v3)
> de4a9f8 drm/i915: Calculate pipe watermarks into CRTC state (v3)
> de165e0 drm/i915: Refactor ilk_update_wm (v3)
>
> Reference: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077190.html
> Cc: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
> Cc: "Vetter, Daniel" <daniel.vetter@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> I just got access to BXT hardware, so I'm hoping that the SKL problems reported
> by Paulo will also be reproducible on BXT. However I'm not going to be able to
> work on this for a couple days, so it's probably better to do some reverts now
> so that we don't leave di-nightly in a broken state in the meantime.
>
> Also note that this is a different regression than the one reported by Jani
> (for which we've already reverted a couple patches); his issue happens on the
> ILK-style watermark path which isn't relevant for Paulo's issues here.
Yeah today's the last day for 4.4 features, I think reverting is the right
move (even if I really want to see atomic watermarks getting in sooner
than later).
Thanks, Daneil
>
> drivers/gpu/drm/i915/i915_drv.h | 12 --
> drivers/gpu/drm/i915/intel_display.c | 60 +--------
> drivers/gpu/drm/i915/intel_drv.h | 49 +++-----
> drivers/gpu/drm/i915/intel_pm.c | 232 +++++++++++++++++++----------------
> 4 files changed, 151 insertions(+), 202 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fbf0ae9..4b02be7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -627,8 +627,6 @@ struct drm_i915_display_funcs {
> int target, int refclk,
> struct dpll *match_clock,
> struct dpll *best_clock);
> - int (*compute_pipe_wm)(struct intel_crtc *crtc,
> - struct drm_atomic_state *state);
> 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);
> @@ -1692,13 +1690,6 @@ struct i915_execbuffer_params {
> struct drm_i915_gem_request *request;
> };
>
> -/* used in computing the new watermarks state */
> -struct intel_wm_config {
> - unsigned int num_pipes_active;
> - bool sprites_enabled;
> - bool sprites_scaled;
> -};
> -
> struct drm_i915_private {
> struct drm_device *dev;
> struct kmem_cache *objects;
> @@ -1923,9 +1914,6 @@ struct drm_i915_private {
> */
> uint16_t skl_latency[8];
>
> - /* Committed wm config */
> - struct intel_wm_config config;
> -
> /*
> * The skl_wm_values structure is a bit too big for stack
> * allocation, so we keep the staging struct where we store
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 539c373..d84d5c0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11837,12 +11837,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> }
>
> ret = 0;
> - if (dev_priv->display.compute_pipe_wm) {
> - ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
> - if (ret)
> - return ret;
> - }
> -
> if (INTEL_INFO(dev)->gen >= 9) {
> if (mode_changed)
> ret = skl_update_scaler_crtc(pipe_config);
> @@ -13048,45 +13042,6 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> return 0;
> }
>
> -/*
> - * Handle calculation of various watermark data at the end of the atomic check
> - * phase. The code here should be run after the per-crtc and per-plane 'check'
> - * handlers to ensure that all derived state has been updated.
> - */
> -static void calc_watermark_data(struct drm_atomic_state *state)
> -{
> - struct drm_device *dev = state->dev;
> - struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *cstate;
> - struct drm_plane *plane;
> - struct drm_plane_state *pstate;
> -
> - /*
> - * Calculate watermark configuration details now that derived
> - * plane/crtc state is all properly updated.
> - */
> - drm_for_each_crtc(crtc, dev) {
> - cstate = drm_atomic_get_existing_crtc_state(state, crtc) ?:
> - crtc->state;
> -
> - if (cstate->active)
> - intel_state->wm_config.num_pipes_active++;
> - }
> - drm_for_each_legacy_plane(plane, dev) {
> - pstate = drm_atomic_get_existing_plane_state(state, plane) ?:
> - plane->state;
> -
> - if (!to_intel_plane_state(pstate)->visible)
> - continue;
> -
> - intel_state->wm_config.sprites_enabled = true;
> - if (pstate->crtc_w != pstate->src_w >> 16 ||
> - pstate->crtc_h != pstate->src_h >> 16)
> - intel_state->wm_config.sprites_scaled = true;
> - }
> -}
> -
> /**
> * intel_atomic_check - validate state object
> * @dev: drm device
> @@ -13095,7 +13050,6 @@ static void calc_watermark_data(struct drm_atomic_state *state)
> static int intel_atomic_check(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> - struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
> int ret, i;
> @@ -13159,15 +13113,10 @@ static int intel_atomic_check(struct drm_device *dev,
> if (ret)
> return ret;
> } else
> - intel_state->cdclk = to_i915(state->dev)->cdclk_freq;
> -
> - ret = drm_atomic_helper_check_planes(state->dev, state);
> - if (ret)
> - return ret;
> -
> - calc_watermark_data(state);
> + to_intel_atomic_state(state)->cdclk =
> + to_i915(state->dev)->cdclk_freq;
>
> - return 0;
> + return drm_atomic_helper_check_planes(state->dev, state);
> }
>
> /**
> @@ -13207,7 +13156,6 @@ static int intel_atomic_commit(struct drm_device *dev,
> return ret;
>
> drm_atomic_helper_swap_state(dev, state);
> - dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
>
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -15221,7 +15169,7 @@ static void readout_plane_state(struct intel_crtc *crtc)
> struct intel_plane_state *plane_state =
> to_intel_plane_state(primary->state);
>
> - plane_state->visible = crtc->active &&
> + plane_state->visible =
> primary_get_hw_state(to_intel_plane(primary));
>
> if (plane_state->visible)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e320825..91b6b40 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -250,7 +250,6 @@ struct intel_atomic_state {
> unsigned int cdclk;
> bool dpll_set;
> struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
> - struct intel_wm_config wm_config;
> };
>
> struct intel_plane_state {
> @@ -335,21 +334,6 @@ struct intel_crtc_scaler_state {
> /* drm_mode->private_flags */
> #define I915_MODE_FLAG_INHERITED 1
>
> -struct intel_pipe_wm {
> - struct intel_wm_level wm[5];
> - uint32_t linetime;
> - bool fbc_wm_enabled;
> - bool pipe_enabled;
> - bool sprites_enabled;
> - bool sprites_scaled;
> -};
> -
> -struct skl_pipe_wm {
> - struct skl_wm_level wm[8];
> - struct skl_wm_level trans_wm;
> - uint32_t linetime;
> -};
> -
> struct intel_crtc_state {
> struct drm_crtc_state base;
>
> @@ -487,17 +471,6 @@ struct intel_crtc_state {
>
> /* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
> bool disable_lp_wm;
> -
> - struct {
> - /*
> - * optimal watermarks, programmed post-vblank when this state
> - * is committed
> - */
> - union {
> - struct intel_pipe_wm ilk;
> - struct skl_pipe_wm skl;
> - } optimal;
> - } wm;
> };
>
> struct vlv_wm_state {
> @@ -509,6 +482,15 @@ struct vlv_wm_state {
> bool cxsr;
> };
>
> +struct intel_pipe_wm {
> + struct intel_wm_level wm[5];
> + uint32_t linetime;
> + bool fbc_wm_enabled;
> + bool pipe_enabled;
> + bool sprites_enabled;
> + bool sprites_scaled;
> +};
> +
> struct intel_mmio_flip {
> struct work_struct work;
> struct drm_i915_private *i915;
> @@ -516,6 +498,12 @@ struct intel_mmio_flip {
> struct intel_crtc *crtc;
> };
>
> +struct skl_pipe_wm {
> + struct skl_wm_level wm[8];
> + struct skl_wm_level trans_wm;
> + uint32_t linetime;
> +};
> +
> /*
> * Tracking of operations that need to be performed at the beginning/end of an
> * atomic commit, outside the atomic section where interrupts are disabled.
> @@ -583,10 +571,9 @@ struct intel_crtc {
> /* per-pipe watermark state */
> struct {
> /* watermarks currently being used */
> - union {
> - struct intel_pipe_wm ilk;
> - struct skl_pipe_wm skl;
> - } active;
> + struct intel_pipe_wm active;
> + /* SKL wm values currently in use */
> + struct skl_pipe_wm skl_active;
> /* allow CxSR on this pipe */
> bool cxsr_allowed;
> } wm;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 60d120c..4993695 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1772,6 +1772,13 @@ struct ilk_wm_maximums {
> uint16_t fbc;
> };
>
> +/* used in computing the new watermarks state */
> +struct intel_wm_config {
> + unsigned int num_pipes_active;
> + bool sprites_enabled;
> + bool sprites_scaled;
> +};
> +
> /*
> * For both WM_PIPE and WM_LP.
> * mem_value must be in 0.1us units.
> @@ -2022,11 +2029,9 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
> const struct intel_crtc *intel_crtc,
> int level,
> struct intel_crtc_state *cstate,
> - struct intel_plane_state *pristate,
> - struct intel_plane_state *sprstate,
> - struct intel_plane_state *curstate,
> struct intel_wm_level *result)
> {
> + struct intel_plane *intel_plane;
> uint16_t pri_latency = dev_priv->wm.pri_latency[level];
> uint16_t spr_latency = dev_priv->wm.spr_latency[level];
> uint16_t cur_latency = dev_priv->wm.cur_latency[level];
> @@ -2038,11 +2043,29 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
> cur_latency *= 5;
> }
>
> - result->pri_val = ilk_compute_pri_wm(cstate, pristate,
> - pri_latency, level);
> - result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
> - result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
> - result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
> + for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) {
> + struct intel_plane_state *pstate =
> + to_intel_plane_state(intel_plane->base.state);
> +
> + switch (intel_plane->base.type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + result->pri_val = ilk_compute_pri_wm(cstate, pstate,
> + pri_latency,
> + level);
> + result->fbc_val = ilk_compute_fbc_wm(cstate, pstate,
> + result->pri_val);
> + break;
> + case DRM_PLANE_TYPE_OVERLAY:
> + result->spr_val = ilk_compute_spr_wm(cstate, pstate,
> + spr_latency);
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + result->cur_val = ilk_compute_cur_wm(cstate, pstate,
> + cur_latency);
> + break;
> + }
> + }
> +
> result->enable = true;
> }
>
> @@ -2301,19 +2324,34 @@ static void skl_setup_wm_latency(struct drm_device *dev)
> intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
> }
>
> +static void ilk_compute_wm_config(struct drm_device *dev,
> + struct intel_wm_config *config)
> +{
> + struct intel_crtc *intel_crtc;
> +
> + /* Compute the currently _active_ config */
> + for_each_intel_crtc(dev, intel_crtc) {
> + const struct intel_pipe_wm *wm = &intel_crtc->wm.active;
> +
> + if (!wm->pipe_enabled)
> + continue;
> +
> + config->sprites_enabled |= wm->sprites_enabled;
> + config->sprites_scaled |= wm->sprites_scaled;
> + config->num_pipes_active++;
> + }
> +}
> +
> /* Compute new watermarks for the pipe */
> -static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> - struct drm_atomic_state *state)
> +static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> + struct intel_pipe_wm *pipe_wm)
> {
> - struct intel_pipe_wm *pipe_wm;
> - struct drm_device *dev = intel_crtc->base.dev;
> + struct drm_crtc *crtc = cstate->base.crtc;
> + struct drm_device *dev = crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc_state *cstate = NULL;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane;
> - struct drm_plane_state *ps;
> - struct intel_plane_state *pristate = NULL;
> 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 = {
> @@ -2321,24 +2359,11 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> };
> struct ilk_wm_maximums max;
>
> - cstate = intel_atomic_get_crtc_state(state, intel_crtc);
> - if (IS_ERR(cstate))
> - return PTR_ERR(cstate);
> -
> - pipe_wm = &cstate->wm.optimal.ilk;
> -
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - ps = drm_atomic_get_plane_state(state,
> - &intel_plane->base);
> - if (IS_ERR(ps))
> - return PTR_ERR(ps);
> -
> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> - pristate = to_intel_plane_state(ps);
> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
> - sprstate = to_intel_plane_state(ps);
> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
> - curstate = to_intel_plane_state(ps);
> + if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
> + sprstate = to_intel_plane_state(intel_plane->base.state);
> + break;
> + }
> }
>
> config.sprites_enabled = sprstate->visible;
> @@ -2347,7 +2372,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> 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_enabled = sprstate->visible;
> pipe_wm->sprites_scaled = config.sprites_scaled;
>
> /* ILK/SNB: LP2+ watermarks only w/o sprites */
> @@ -2358,27 +2383,24 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> if (config.sprites_scaled)
> max_level = 0;
>
> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
> - pristate, sprstate, curstate, &pipe_wm->wm[0]);
> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]);
>
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> - pipe_wm->linetime = hsw_compute_linetime_wm(dev,
> - &intel_crtc->base);
> + 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 */
> if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
> - return -EINVAL;
> + 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, intel_crtc, level, cstate,
> - pristate, sprstate, curstate, &wm);
> + ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm);
>
> /*
> * Disable any watermark level that exceeds the
> @@ -2391,7 +2413,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> pipe_wm->wm[level] = wm;
> }
>
> - return 0;
> + return true;
> }
>
> /*
> @@ -2406,9 +2428,7 @@ static void ilk_merge_wm_level(struct drm_device *dev,
> ret_wm->enable = true;
>
> for_each_intel_crtc(dev, intel_crtc) {
> - 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_pipe_wm *active = &intel_crtc->wm.active;
> const struct intel_wm_level *wm = &active->wm[level];
>
> if (!active->pipe_enabled)
> @@ -2556,15 +2576,14 @@ static void ilk_compute_wm_results(struct drm_device *dev,
>
> /* 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 = &cstate->wm.optimal.ilk.wm[0];
> + const struct intel_wm_level *r =
> + &intel_crtc->wm.active.wm[0];
>
> if (WARN_ON(!r->enable))
> continue;
>
> - results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime;
> + results->wm_linetime[pipe] = intel_crtc->wm.active.linetime;
>
> results->wm_pipe[pipe] =
> (r->pri_val << WM0_PIPE_PLANE_SHIFT) |
> @@ -2946,12 +2965,11 @@ skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
>
> static void
> skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> + const struct intel_wm_config *config,
> struct skl_ddb_allocation *ddb /* out */)
> {
> struct drm_crtc *crtc = cstate->base.crtc;
> struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_wm_config *config = &dev_priv->wm.config;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane;
> enum pipe pipe = intel_crtc->pipe;
> @@ -3126,6 +3144,15 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
> return false;
> }
>
> +static void skl_compute_wm_global_parameters(struct drm_device *dev,
> + struct intel_wm_config *config)
> +{
> + struct drm_crtc *crtc;
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> + config->num_pipes_active += to_intel_crtc(crtc)->active;
> +}
> +
> static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> struct intel_crtc_state *cstate,
> struct intel_plane *intel_plane,
> @@ -3530,25 +3557,27 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
> }
>
> static bool skl_update_pipe_wm(struct drm_crtc *crtc,
> + struct intel_wm_config *config,
> struct skl_ddb_allocation *ddb, /* out */
> struct skl_pipe_wm *pipe_wm /* out */)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
>
> - skl_allocate_pipe_ddb(cstate, ddb);
> + skl_allocate_pipe_ddb(cstate, config, ddb);
> skl_compute_pipe_wm(cstate, ddb, pipe_wm);
>
> - if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
> + if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
> return false;
>
> - intel_crtc->wm.active.skl = *pipe_wm;
> + intel_crtc->wm.skl_active = *pipe_wm;
>
> return true;
> }
>
> static void skl_update_other_pipe_wm(struct drm_device *dev,
> struct drm_crtc *crtc,
> + struct intel_wm_config *config,
> struct skl_wm_values *r)
> {
> struct intel_crtc *intel_crtc;
> @@ -3578,7 +3607,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
> if (!intel_crtc->active)
> continue;
>
> - wm_changed = skl_update_pipe_wm(&intel_crtc->base,
> + wm_changed = skl_update_pipe_wm(&intel_crtc->base, config,
> &r->ddb, &pipe_wm);
>
> /*
> @@ -3619,8 +3648,8 @@ static void skl_update_wm(struct drm_crtc *crtc)
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct skl_wm_values *results = &dev_priv->wm.skl_results;
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct skl_pipe_wm *pipe_wm = &cstate->wm.optimal.skl;
> + struct skl_pipe_wm pipe_wm = {};
> + struct intel_wm_config config = {};
>
>
> /* Clear all dirty flags */
> @@ -3628,13 +3657,15 @@ static void skl_update_wm(struct drm_crtc *crtc)
>
> skl_clear_wm(results, intel_crtc->pipe);
>
> - if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
> + skl_compute_wm_global_parameters(dev, &config);
> +
> + if (!skl_update_pipe_wm(crtc, &config, &results->ddb, &pipe_wm))
> return;
>
> - skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
> + skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
> results->dirty[intel_crtc->pipe] = true;
>
> - skl_update_other_pipe_wm(dev, crtc, results);
> + skl_update_other_pipe_wm(dev, crtc, &config, results);
> skl_write_wm_values(dev_priv, results);
> skl_flush_wm_values(dev_priv, results);
>
> @@ -3642,23 +3673,50 @@ static void skl_update_wm(struct drm_crtc *crtc)
> dev_priv->wm.skl_hw = *results;
> }
>
> -static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
> +static void ilk_update_wm(struct drm_crtc *crtc)
> {
> - struct drm_device *dev = dev_priv->dev;
> - struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct ilk_wm_maximums max;
> - struct intel_wm_config *config = &dev_priv->wm.config;
> struct ilk_wm_values results = {};
> enum intel_ddb_partitioning partitioning;
> + struct intel_pipe_wm pipe_wm = {};
> + struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
> + struct intel_wm_config config = {};
>
> - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max);
> - ilk_wm_merge(dev, config, &max, &lp_wm_1_2);
> + WARN_ON(cstate->base.active != intel_crtc->active);
> +
> + /*
> + * 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(dev);
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + }
> +
> + intel_compute_pipe_wm(cstate, &pipe_wm);
> +
> + if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
> + return;
> +
> + intel_crtc->wm.active = pipe_wm;
> +
> + ilk_compute_wm_config(dev, &config);
> +
> + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max);
> + ilk_wm_merge(dev, &config, &max, &lp_wm_1_2);
>
> /* 5/6 split only in single pipe config on IVB+ */
> if (INTEL_INFO(dev)->gen >= 7 &&
> - config->num_pipes_active == 1 && config->sprites_enabled) {
> - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_5_6, &max);
> - ilk_wm_merge(dev, config, &max, &lp_wm_5_6);
> + config.num_pipes_active == 1 && config.sprites_enabled) {
> + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, &max);
> + ilk_wm_merge(dev, &config, &max, &lp_wm_5_6);
>
> best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6);
> } else {
> @@ -3673,31 +3731,6 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
> ilk_write_wm_values(dev_priv, &results);
> }
>
> -static void ilk_update_wm(struct drm_crtc *crtc)
> -{
> - struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> -
> - WARN_ON(cstate->base.active != intel_crtc->active);
> -
> - /*
> - * 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);
> - }
> -
> - intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
> -
> - ilk_program_watermarks(dev_priv);
> -}
> -
> static void skl_pipe_wm_active_state(uint32_t val,
> struct skl_pipe_wm *active,
> bool is_transwm,
> @@ -3748,8 +3781,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct skl_pipe_wm *active = &cstate->wm.optimal.skl;
> + struct skl_pipe_wm *active = &intel_crtc->wm.skl_active;
> enum pipe pipe = intel_crtc->pipe;
> int level, i, max_level;
> uint32_t temp;
> @@ -3793,8 +3825,6 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
>
> temp = hw->plane_trans[pipe][PLANE_CURSOR];
> skl_pipe_wm_active_state(temp, active, true, true, i, 0);
> -
> - intel_crtc->wm.active.skl = *active;
> }
>
> void skl_wm_get_hw_state(struct drm_device *dev)
> @@ -3814,8 +3844,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct ilk_wm_values *hw = &dev_priv->wm.hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct intel_pipe_wm *active = &cstate->wm.optimal.ilk;
> + struct intel_pipe_wm *active = &intel_crtc->wm.active;
> enum pipe pipe = intel_crtc->pipe;
> static const unsigned int wm0_pipe_reg[] = {
> [PIPE_A] = WM0_PIPEA_ILK,
> @@ -3854,8 +3883,6 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> for (level = 0; level <= max_level; level++)
> active->wm[level].enable = true;
> }
> -
> - intel_crtc->wm.active.ilk = *active;
> }
>
> #define _FW_WM(value, plane) \
> @@ -7015,7 +7042,6 @@ void intel_init_pm(struct drm_device *dev)
> (!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;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane latency. "
> "Disable CxSR\n");
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Partial revert of atomic watermark series
2015-10-08 22:28 [PATCH] drm/i915: Partial revert of atomic watermark series Matt Roper
2015-10-09 8:41 ` Daniel Vetter
@ 2015-10-09 20:33 ` Zanoni, Paulo R
2015-10-09 21:22 ` [PATCH] drm/i915: revert a few more watermark commits Paulo Zanoni
1 sibling, 1 reply; 5+ messages in thread
From: Zanoni, Paulo R @ 2015-10-09 20:33 UTC (permalink / raw)
To: Roper, Matthew D, intel-gfx@lists.freedesktop.org; +Cc: Vetter, Daniel
Em Qui, 2015-10-08 às 15:28 -0700, Matt Roper escreveu:
> It's been reported that the atomic watermark series triggers some
> regressions on SKL, which we haven't been able to track down yet.
> Let's
> temporarily revert these patches while we track down the root cause.
>
> This commit squashes the reverts of:
> 76305b1 drm/i915: Calculate watermark configuration during atomic
> check (v2)
> a4611e4 drm/i915: Don't set plane visible during HW readout if CRTC
> is off
> a28170f drm/i915: Calculate ILK-style watermarks during atomic
> check (v3)
> de4a9f8 drm/i915: Calculate pipe watermarks into CRTC state (v3)
> de165e0 drm/i915: Refactor ilk_update_wm (v3)
I see this patch is already applied. Unfortunately it doesn't solve the
problem for me. I was carrying a revert of these patches and also of:
47c99438b52d12df50e182583634a4cfede3c920
drm/i915: Drop intel_update_sprite_watermarks
But today I did a git rebase and just nightly + revert of 47c994 is not
solving the problem anymore... I suspect that something else was merged
in the meantime that broke the tree even more, and a normal git bisect
won't help anymore since I'll have to apply tons of additional patches
on top of each bisect step... I'll try to discover what else is causing
the problem.
Anyway, thanks for the help!
>
> Reference: http://lists.freedesktop.org/archives/intel-gfx/2015-Octob
> er/077190.html
> Cc: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
> Cc: "Vetter, Daniel" <daniel.vetter@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> I just got access to BXT hardware, so I'm hoping that the SKL
> problems reported
> by Paulo will also be reproducible on BXT. However I'm not going to
> be able to
> work on this for a couple days, so it's probably better to do some
> reverts now
> so that we don't leave di-nightly in a broken state in the meantime.
>
> Also note that this is a different regression than the one reported
> by Jani
> (for which we've already reverted a couple patches); his issue
> happens on the
> ILK-style watermark path which isn't relevant for Paulo's issues
> here.
>
> drivers/gpu/drm/i915/i915_drv.h | 12 --
> drivers/gpu/drm/i915/intel_display.c | 60 +--------
> drivers/gpu/drm/i915/intel_drv.h | 49 +++-----
> drivers/gpu/drm/i915/intel_pm.c | 232 +++++++++++++++++++------
> ----------
> 4 files changed, 151 insertions(+), 202 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index fbf0ae9..4b02be7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -627,8 +627,6 @@ struct drm_i915_display_funcs {
> int target, int refclk,
> struct dpll *match_clock,
> struct dpll *best_clock);
> - int (*compute_pipe_wm)(struct intel_crtc *crtc,
> - struct drm_atomic_state *state);
> 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);
> @@ -1692,13 +1690,6 @@ struct i915_execbuffer_params {
> struct drm_i915_gem_request *request;
> };
>
> -/* used in computing the new watermarks state */
> -struct intel_wm_config {
> - unsigned int num_pipes_active;
> - bool sprites_enabled;
> - bool sprites_scaled;
> -};
> -
> struct drm_i915_private {
> struct drm_device *dev;
> struct kmem_cache *objects;
> @@ -1923,9 +1914,6 @@ struct drm_i915_private {
> */
> uint16_t skl_latency[8];
>
> - /* Committed wm config */
> - struct intel_wm_config config;
> -
> /*
> * The skl_wm_values structure is a bit too big for
> stack
> * allocation, so we keep the staging struct where
> we store
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 539c373..d84d5c0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11837,12 +11837,6 @@ static int intel_crtc_atomic_check(struct
> drm_crtc *crtc,
> }
>
> ret = 0;
> - if (dev_priv->display.compute_pipe_wm) {
> - ret = dev_priv->display.compute_pipe_wm(intel_crtc,
> state);
> - if (ret)
> - return ret;
> - }
> -
> if (INTEL_INFO(dev)->gen >= 9) {
> if (mode_changed)
> ret = skl_update_scaler_crtc(pipe_config);
> @@ -13048,45 +13042,6 @@ static int intel_modeset_checks(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -/*
> - * Handle calculation of various watermark data at the end of the
> atomic check
> - * phase. The code here should be run after the per-crtc and per
> -plane 'check'
> - * handlers to ensure that all derived state has been updated.
> - */
> -static void calc_watermark_data(struct drm_atomic_state *state)
> -{
> - struct drm_device *dev = state->dev;
> - struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *cstate;
> - struct drm_plane *plane;
> - struct drm_plane_state *pstate;
> -
> - /*
> - * Calculate watermark configuration details now that
> derived
> - * plane/crtc state is all properly updated.
> - */
> - drm_for_each_crtc(crtc, dev) {
> - cstate = drm_atomic_get_existing_crtc_state(state,
> crtc) ?:
> - crtc->state;
> -
> - if (cstate->active)
> - intel_state->wm_config.num_pipes_active++;
> - }
> - drm_for_each_legacy_plane(plane, dev) {
> - pstate = drm_atomic_get_existing_plane_state(state,
> plane) ?:
> - plane->state;
> -
> - if (!to_intel_plane_state(pstate)->visible)
> - continue;
> -
> - intel_state->wm_config.sprites_enabled = true;
> - if (pstate->crtc_w != pstate->src_w >> 16 ||
> - pstate->crtc_h != pstate->src_h >> 16)
> - intel_state->wm_config.sprites_scaled =
> true;
> - }
> -}
> -
> /**
> * intel_atomic_check - validate state object
> * @dev: drm device
> @@ -13095,7 +13050,6 @@ static void calc_watermark_data(struct
> drm_atomic_state *state)
> static int intel_atomic_check(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> - struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
> int ret, i;
> @@ -13159,15 +13113,10 @@ static int intel_atomic_check(struct
> drm_device *dev,
> if (ret)
> return ret;
> } else
> - intel_state->cdclk = to_i915(state->dev)
> ->cdclk_freq;
> -
> - ret = drm_atomic_helper_check_planes(state->dev, state);
> - if (ret)
> - return ret;
> -
> - calc_watermark_data(state);
> + to_intel_atomic_state(state)->cdclk =
> + to_i915(state->dev)->cdclk_freq;
>
> - return 0;
> + return drm_atomic_helper_check_planes(state->dev, state);
> }
>
> /**
> @@ -13207,7 +13156,6 @@ static int intel_atomic_commit(struct
> drm_device *dev,
> return ret;
>
> drm_atomic_helper_swap_state(dev, state);
> - dev_priv->wm.config = to_intel_atomic_state(state)
> ->wm_config;
>
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -15221,7 +15169,7 @@ static void readout_plane_state(struct
> intel_crtc *crtc)
> struct intel_plane_state *plane_state =
> to_intel_plane_state(primary->state);
>
> - plane_state->visible = crtc->active &&
> + plane_state->visible =
> primary_get_hw_state(to_intel_plane(primary));
>
> if (plane_state->visible)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e320825..91b6b40 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -250,7 +250,6 @@ struct intel_atomic_state {
> unsigned int cdclk;
> bool dpll_set;
> struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
> - struct intel_wm_config wm_config;
> };
>
> struct intel_plane_state {
> @@ -335,21 +334,6 @@ struct intel_crtc_scaler_state {
> /* drm_mode->private_flags */
> #define I915_MODE_FLAG_INHERITED 1
>
> -struct intel_pipe_wm {
> - struct intel_wm_level wm[5];
> - uint32_t linetime;
> - bool fbc_wm_enabled;
> - bool pipe_enabled;
> - bool sprites_enabled;
> - bool sprites_scaled;
> -};
> -
> -struct skl_pipe_wm {
> - struct skl_wm_level wm[8];
> - struct skl_wm_level trans_wm;
> - uint32_t linetime;
> -};
> -
> struct intel_crtc_state {
> struct drm_crtc_state base;
>
> @@ -487,17 +471,6 @@ struct intel_crtc_state {
>
> /* IVB sprite scaling w/a
> (WaCxSRDisabledForSpriteScaling:ivb) */
> bool disable_lp_wm;
> -
> - struct {
> - /*
> - * optimal watermarks, programmed post-vblank when
> this state
> - * is committed
> - */
> - union {
> - struct intel_pipe_wm ilk;
> - struct skl_pipe_wm skl;
> - } optimal;
> - } wm;
> };
>
> struct vlv_wm_state {
> @@ -509,6 +482,15 @@ struct vlv_wm_state {
> bool cxsr;
> };
>
> +struct intel_pipe_wm {
> + struct intel_wm_level wm[5];
> + uint32_t linetime;
> + bool fbc_wm_enabled;
> + bool pipe_enabled;
> + bool sprites_enabled;
> + bool sprites_scaled;
> +};
> +
> struct intel_mmio_flip {
> struct work_struct work;
> struct drm_i915_private *i915;
> @@ -516,6 +498,12 @@ struct intel_mmio_flip {
> struct intel_crtc *crtc;
> };
>
> +struct skl_pipe_wm {
> + struct skl_wm_level wm[8];
> + struct skl_wm_level trans_wm;
> + uint32_t linetime;
> +};
> +
> /*
> * Tracking of operations that need to be performed at the
> beginning/end of an
> * atomic commit, outside the atomic section where interrupts are
> disabled.
> @@ -583,10 +571,9 @@ struct intel_crtc {
> /* per-pipe watermark state */
> struct {
> /* watermarks currently being used */
> - union {
> - struct intel_pipe_wm ilk;
> - struct skl_pipe_wm skl;
> - } active;
> + struct intel_pipe_wm active;
> + /* SKL wm values currently in use */
> + struct skl_pipe_wm skl_active;
> /* allow CxSR on this pipe */
> bool cxsr_allowed;
> } wm;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 60d120c..4993695 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1772,6 +1772,13 @@ struct ilk_wm_maximums {
> uint16_t fbc;
> };
>
> +/* used in computing the new watermarks state */
> +struct intel_wm_config {
> + unsigned int num_pipes_active;
> + bool sprites_enabled;
> + bool sprites_scaled;
> +};
> +
> /*
> * For both WM_PIPE and WM_LP.
> * mem_value must be in 0.1us units.
> @@ -2022,11 +2029,9 @@ static void ilk_compute_wm_level(const struct
> drm_i915_private *dev_priv,
> const struct intel_crtc
> *intel_crtc,
> int level,
> struct intel_crtc_state *cstate,
> - struct intel_plane_state *pristate,
> - struct intel_plane_state *sprstate,
> - struct intel_plane_state *curstate,
> struct intel_wm_level *result)
> {
> + struct intel_plane *intel_plane;
> uint16_t pri_latency = dev_priv->wm.pri_latency[level];
> uint16_t spr_latency = dev_priv->wm.spr_latency[level];
> uint16_t cur_latency = dev_priv->wm.cur_latency[level];
> @@ -2038,11 +2043,29 @@ static void ilk_compute_wm_level(const struct
> drm_i915_private *dev_priv,
> cur_latency *= 5;
> }
>
> - result->pri_val = ilk_compute_pri_wm(cstate, pristate,
> - pri_latency, level);
> - result->spr_val = ilk_compute_spr_wm(cstate, sprstate,
> spr_latency);
> - result->cur_val = ilk_compute_cur_wm(cstate, curstate,
> cur_latency);
> - result->fbc_val = ilk_compute_fbc_wm(cstate, pristate,
> result->pri_val);
> + for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc,
> intel_plane) {
> + struct intel_plane_state *pstate =
> + to_intel_plane_state(intel_plane
> ->base.state);
> +
> + switch (intel_plane->base.type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + result->pri_val = ilk_compute_pri_wm(cstate,
> pstate,
> +
> pri_latency,
> + level);
> + result->fbc_val = ilk_compute_fbc_wm(cstate,
> pstate,
> + result
> ->pri_val);
> + break;
> + case DRM_PLANE_TYPE_OVERLAY:
> + result->spr_val = ilk_compute_spr_wm(cstate,
> pstate,
> +
> spr_latency);
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + result->cur_val = ilk_compute_cur_wm(cstate,
> pstate,
> +
> cur_latency);
> + break;
> + }
> + }
> +
> result->enable = true;
> }
>
> @@ -2301,19 +2324,34 @@ static void skl_setup_wm_latency(struct
> drm_device *dev)
> intel_print_wm_latency(dev, "Gen9 Plane", dev_priv
> ->wm.skl_latency);
> }
>
> +static void ilk_compute_wm_config(struct drm_device *dev,
> + struct intel_wm_config *config)
> +{
> + struct intel_crtc *intel_crtc;
> +
> + /* Compute the currently _active_ config */
> + for_each_intel_crtc(dev, intel_crtc) {
> + const struct intel_pipe_wm *wm = &intel_crtc
> ->wm.active;
> +
> + if (!wm->pipe_enabled)
> + continue;
> +
> + config->sprites_enabled |= wm->sprites_enabled;
> + config->sprites_scaled |= wm->sprites_scaled;
> + config->num_pipes_active++;
> + }
> +}
> +
> /* Compute new watermarks for the pipe */
> -static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> - struct drm_atomic_state *state)
> +static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> + struct intel_pipe_wm *pipe_wm)
> {
> - struct intel_pipe_wm *pipe_wm;
> - struct drm_device *dev = intel_crtc->base.dev;
> + struct drm_crtc *crtc = cstate->base.crtc;
> + struct drm_device *dev = crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc_state *cstate = NULL;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane;
> - struct drm_plane_state *ps;
> - struct intel_plane_state *pristate = NULL;
> 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 = {
> @@ -2321,24 +2359,11 @@ static int ilk_compute_pipe_wm(struct
> intel_crtc *intel_crtc,
> };
> struct ilk_wm_maximums max;
>
> - cstate = intel_atomic_get_crtc_state(state, intel_crtc);
> - if (IS_ERR(cstate))
> - return PTR_ERR(cstate);
> -
> - pipe_wm = &cstate->wm.optimal.ilk;
> -
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - ps = drm_atomic_get_plane_state(state,
> - &intel_plane->base);
> - if (IS_ERR(ps))
> - return PTR_ERR(ps);
> -
> - if (intel_plane->base.type ==
> DRM_PLANE_TYPE_PRIMARY)
> - pristate = to_intel_plane_state(ps);
> - else if (intel_plane->base.type ==
> DRM_PLANE_TYPE_OVERLAY)
> - sprstate = to_intel_plane_state(ps);
> - else if (intel_plane->base.type ==
> DRM_PLANE_TYPE_CURSOR)
> - curstate = to_intel_plane_state(ps);
> + if (intel_plane->base.type ==
> DRM_PLANE_TYPE_OVERLAY) {
> + sprstate = to_intel_plane_state(intel_plane
> ->base.state);
> + break;
> + }
> }
>
> config.sprites_enabled = sprstate->visible;
> @@ -2347,7 +2372,7 @@ static int ilk_compute_pipe_wm(struct
> intel_crtc *intel_crtc,
> 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_enabled = sprstate->visible;
> pipe_wm->sprites_scaled = config.sprites_scaled;
>
> /* ILK/SNB: LP2+ watermarks only w/o sprites */
> @@ -2358,27 +2383,24 @@ static int ilk_compute_pipe_wm(struct
> intel_crtc *intel_crtc,
> if (config.sprites_scaled)
> max_level = 0;
>
> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
> - pristate, sprstate, curstate, &pipe_wm
> ->wm[0]);
> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
> &pipe_wm->wm[0]);
>
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> - pipe_wm->linetime = hsw_compute_linetime_wm(dev,
> -
> &intel_crtc->base);
> + 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 */
> if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
> - return -EINVAL;
> + 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, intel_crtc, level,
> cstate,
> - pristate, sprstate, curstate,
> &wm);
> + ilk_compute_wm_level(dev_priv, intel_crtc, level,
> cstate, &wm);
>
> /*
> * Disable any watermark level that exceeds the
> @@ -2391,7 +2413,7 @@ static int ilk_compute_pipe_wm(struct
> intel_crtc *intel_crtc,
> pipe_wm->wm[level] = wm;
> }
>
> - return 0;
> + return true;
> }
>
> /*
> @@ -2406,9 +2428,7 @@ static void ilk_merge_wm_level(struct
> drm_device *dev,
> ret_wm->enable = true;
>
> for_each_intel_crtc(dev, intel_crtc) {
> - 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_pipe_wm *active = &intel_crtc
> ->wm.active;
> const struct intel_wm_level *wm = &active
> ->wm[level];
>
> if (!active->pipe_enabled)
> @@ -2556,15 +2576,14 @@ static void ilk_compute_wm_results(struct
> drm_device *dev,
>
> /* 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 = &cstate
> ->wm.optimal.ilk.wm[0];
> + const struct intel_wm_level *r =
> + &intel_crtc->wm.active.wm[0];
>
> if (WARN_ON(!r->enable))
> continue;
>
> - results->wm_linetime[pipe] = cstate
> ->wm.optimal.ilk.linetime;
> + results->wm_linetime[pipe] = intel_crtc
> ->wm.active.linetime;
>
> results->wm_pipe[pipe] =
> (r->pri_val << WM0_PIPE_PLANE_SHIFT) |
> @@ -2946,12 +2965,11 @@ skl_get_total_relative_data_rate(const struct
> intel_crtc_state *cstate)
>
> static void
> skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> + const struct intel_wm_config *config,
> struct skl_ddb_allocation *ddb /* out */)
> {
> struct drm_crtc *crtc = cstate->base.crtc;
> struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_wm_config *config = &dev_priv->wm.config;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane;
> enum pipe pipe = intel_crtc->pipe;
> @@ -3126,6 +3144,15 @@ static bool skl_ddb_allocation_changed(const
> struct skl_ddb_allocation *new_ddb,
> return false;
> }
>
> +static void skl_compute_wm_global_parameters(struct drm_device *dev,
> + struct intel_wm_config
> *config)
> +{
> + struct drm_crtc *crtc;
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> + config->num_pipes_active += to_intel_crtc(crtc)
> ->active;
> +}
> +
> static bool skl_compute_plane_wm(const struct drm_i915_private
> *dev_priv,
> struct intel_crtc_state *cstate,
> struct intel_plane *intel_plane,
> @@ -3530,25 +3557,27 @@ static void skl_flush_wm_values(struct
> drm_i915_private *dev_priv,
> }
>
> static bool skl_update_pipe_wm(struct drm_crtc *crtc,
> + struct intel_wm_config *config,
> struct skl_ddb_allocation *ddb, /*
> out */
> struct skl_pipe_wm *pipe_wm /* out
> */)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc
> ->state);
>
> - skl_allocate_pipe_ddb(cstate, ddb);
> + skl_allocate_pipe_ddb(cstate, config, ddb);
> skl_compute_pipe_wm(cstate, ddb, pipe_wm);
>
> - if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm,
> sizeof(*pipe_wm)))
> + if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm,
> sizeof(*pipe_wm)))
> return false;
>
> - intel_crtc->wm.active.skl = *pipe_wm;
> + intel_crtc->wm.skl_active = *pipe_wm;
>
> return true;
> }
>
> static void skl_update_other_pipe_wm(struct drm_device *dev,
> struct drm_crtc *crtc,
> + struct intel_wm_config *config,
> struct skl_wm_values *r)
> {
> struct intel_crtc *intel_crtc;
> @@ -3578,7 +3607,7 @@ static void skl_update_other_pipe_wm(struct
> drm_device *dev,
> if (!intel_crtc->active)
> continue;
>
> - wm_changed = skl_update_pipe_wm(&intel_crtc->base,
> + wm_changed = skl_update_pipe_wm(&intel_crtc->base,
> config,
> &r->ddb, &pipe_wm);
>
> /*
> @@ -3619,8 +3648,8 @@ static void skl_update_wm(struct drm_crtc
> *crtc)
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct skl_wm_values *results = &dev_priv->wm.skl_results;
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc
> ->state);
> - struct skl_pipe_wm *pipe_wm = &cstate->wm.optimal.skl;
> + struct skl_pipe_wm pipe_wm = {};
> + struct intel_wm_config config = {};
>
>
> /* Clear all dirty flags */
> @@ -3628,13 +3657,15 @@ static void skl_update_wm(struct drm_crtc
> *crtc)
>
> skl_clear_wm(results, intel_crtc->pipe);
>
> - if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
> + skl_compute_wm_global_parameters(dev, &config);
> +
> + if (!skl_update_pipe_wm(crtc, &config, &results->ddb,
> &pipe_wm))
> return;
>
> - skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
> + skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
> results->dirty[intel_crtc->pipe] = true;
>
> - skl_update_other_pipe_wm(dev, crtc, results);
> + skl_update_other_pipe_wm(dev, crtc, &config, results);
> skl_write_wm_values(dev_priv, results);
> skl_flush_wm_values(dev_priv, results);
>
> @@ -3642,23 +3673,50 @@ static void skl_update_wm(struct drm_crtc
> *crtc)
> dev_priv->wm.skl_hw = *results;
> }
>
> -static void ilk_program_watermarks(struct drm_i915_private
> *dev_priv)
> +static void ilk_update_wm(struct drm_crtc *crtc)
> {
> - struct drm_device *dev = dev_priv->dev;
> - struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {},
> *best_lp_wm;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc
> ->state);
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct ilk_wm_maximums max;
> - struct intel_wm_config *config = &dev_priv->wm.config;
> struct ilk_wm_values results = {};
> enum intel_ddb_partitioning partitioning;
> + struct intel_pipe_wm pipe_wm = {};
> + struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {},
> *best_lp_wm;
> + struct intel_wm_config config = {};
>
> - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2,
> &max);
> - ilk_wm_merge(dev, config, &max, &lp_wm_1_2);
> + WARN_ON(cstate->base.active != intel_crtc->active);
> +
> + /*
> + * 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(dev);
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + }
> +
> + intel_compute_pipe_wm(cstate, &pipe_wm);
> +
> + if (!memcmp(&intel_crtc->wm.active, &pipe_wm,
> sizeof(pipe_wm)))
> + return;
> +
> + intel_crtc->wm.active = pipe_wm;
> +
> + ilk_compute_wm_config(dev, &config);
> +
> + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2,
> &max);
> + ilk_wm_merge(dev, &config, &max, &lp_wm_1_2);
>
> /* 5/6 split only in single pipe config on IVB+ */
> if (INTEL_INFO(dev)->gen >= 7 &&
> - config->num_pipes_active == 1 && config
> ->sprites_enabled) {
> - ilk_compute_wm_maximums(dev, 1, config,
> INTEL_DDB_PART_5_6, &max);
> - ilk_wm_merge(dev, config, &max, &lp_wm_5_6);
> + config.num_pipes_active == 1 && config.sprites_enabled)
> {
> + ilk_compute_wm_maximums(dev, 1, &config,
> INTEL_DDB_PART_5_6, &max);
> + ilk_wm_merge(dev, &config, &max, &lp_wm_5_6);
>
> best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2,
> &lp_wm_5_6);
> } else {
> @@ -3673,31 +3731,6 @@ static void ilk_program_watermarks(struct
> drm_i915_private *dev_priv)
> ilk_write_wm_values(dev_priv, &results);
> }
>
> -static void ilk_update_wm(struct drm_crtc *crtc)
> -{
> - struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc
> ->state);
> -
> - WARN_ON(cstate->base.active != intel_crtc->active);
> -
> - /*
> - * 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);
> - }
> -
> - intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
> -
> - ilk_program_watermarks(dev_priv);
> -}
> -
> static void skl_pipe_wm_active_state(uint32_t val,
> struct skl_pipe_wm *active,
> bool is_transwm,
> @@ -3748,8 +3781,7 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc
> ->state);
> - struct skl_pipe_wm *active = &cstate->wm.optimal.skl;
> + struct skl_pipe_wm *active = &intel_crtc->wm.skl_active;
> enum pipe pipe = intel_crtc->pipe;
> int level, i, max_level;
> uint32_t temp;
> @@ -3793,8 +3825,6 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
>
> temp = hw->plane_trans[pipe][PLANE_CURSOR];
> skl_pipe_wm_active_state(temp, active, true, true, i, 0);
> -
> - intel_crtc->wm.active.skl = *active;
> }
>
> void skl_wm_get_hw_state(struct drm_device *dev)
> @@ -3814,8 +3844,7 @@ static void ilk_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct ilk_wm_values *hw = &dev_priv->wm.hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc
> ->state);
> - struct intel_pipe_wm *active = &cstate->wm.optimal.ilk;
> + struct intel_pipe_wm *active = &intel_crtc->wm.active;
> enum pipe pipe = intel_crtc->pipe;
> static const unsigned int wm0_pipe_reg[] = {
> [PIPE_A] = WM0_PIPEA_ILK,
> @@ -3854,8 +3883,6 @@ static void ilk_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
> for (level = 0; level <= max_level; level++)
> active->wm[level].enable = true;
> }
> -
> - intel_crtc->wm.active.ilk = *active;
> }
>
> #define _FW_WM(value, plane) \
> @@ -7015,7 +7042,6 @@ void intel_init_pm(struct drm_device *dev)
> (!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;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane
> latency. "
> "Disable CxSR\n");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] drm/i915: revert a few more watermark commits
2015-10-09 20:33 ` Zanoni, Paulo R
@ 2015-10-09 21:22 ` Paulo Zanoni
2015-10-13 12:00 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Paulo Zanoni @ 2015-10-09 21:22 UTC (permalink / raw)
To: intel-gfx
This is a squash of the following commits:
Revert "drm/i915: Drop intel_update_sprite_watermarks"
This reverts commit 47c99438b52d12df50e182583634a4cfede3c920.
Revert "drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check"
This reverts commit 7809e5ae35b9d8d0710f0874b2e3f10be144e38b.
Revert "drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v3)"
This reverts commit 3a05f5e2e78eab7ffe816abb59b6769e331a1957.
With these reverts, SKL finally stops failing every single FBC test
with FIFO underrun error messages. After some brief testing, it also
seems that this commit prevents the machine from completely freezing
when we run igt/kms_fbc_crc (see fd.o #92355).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92355
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 +
drivers/gpu/drm/i915/intel_atomic.c | 1 -
drivers/gpu/drm/i915/intel_display.c | 44 +---
drivers/gpu/drm/i915/intel_drv.h | 9 +-
drivers/gpu/drm/i915/intel_pm.c | 419 +++++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_sprite.c | 15 ++
6 files changed, 293 insertions(+), 199 deletions(-)
The fact that I needed more reverts after rebasing suggests that maybe the
problem is happening due to some sort of timing problem? Maybe we're missing
some lock?
The machine freezes while running igt/kms_fbc_crc/blt are also curious, but I
didn't spend too much time investigating them.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf14096..22c8b0a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -628,6 +628,10 @@ struct drm_i915_display_funcs {
struct dpll *match_clock,
struct dpll *best_clock);
void (*update_wm)(struct drm_crtc *crtc);
+ void (*update_sprite_wm)(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width, uint32_t sprite_height,
+ int pixel_size, bool enable, bool scaled);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
/* Returns the active state of the crtc, and if the crtc is active,
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 05b1203..f1975f2 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -94,7 +94,6 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
crtc_state->update_pipe = false;
- crtc_state->disable_lp_wm = false;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cddb0c6..66a37d8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4804,6 +4804,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_plane *plane;
if (atomic->wait_vblank)
intel_wait_for_vblank(dev, crtc->pipe);
@@ -4822,6 +4823,10 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
if (atomic->post_enable_primary)
intel_post_enable_primary(&crtc->base);
+ drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
+ intel_update_sprite_watermarks(plane, &crtc->base,
+ 0, 0, 0, false, false);
+
memset(atomic, 0, sizeof(*atomic));
}
@@ -11577,30 +11582,16 @@ retry:
static bool intel_wm_need_update(struct drm_plane *plane,
struct drm_plane_state *state)
{
- struct intel_plane_state *new = to_intel_plane_state(state);
- struct intel_plane_state *cur = to_intel_plane_state(plane->state);
-
- /* Update watermarks on tiling or size changes. */
+ /* Update watermarks on tiling changes. */
if (!plane->state->fb || !state->fb ||
plane->state->fb->modifier[0] != state->fb->modifier[0] ||
- plane->state->rotation != state->rotation ||
- drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
- drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
- drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
- drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
+ plane->state->rotation != state->rotation)
return true;
- return false;
-}
-
-static bool needs_scaling(struct intel_plane_state *state)
-{
- int src_w = drm_rect_width(&state->src) >> 16;
- int src_h = drm_rect_height(&state->src) >> 16;
- int dst_w = drm_rect_width(&state->dst);
- int dst_h = drm_rect_height(&state->dst);
+ if (plane->state->crtc_w != state->crtc_w)
+ return true;
- return (src_w != dst_w || src_h != dst_h);
+ return false;
}
int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
@@ -11618,6 +11609,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
bool mode_changed = needs_modeset(crtc_state);
bool was_crtc_enabled = crtc->state->active;
bool is_crtc_enabled = crtc_state->active;
+
bool turn_off, turn_on, visible, was_visible;
struct drm_framebuffer *fb = plane_state->fb;
@@ -11735,23 +11727,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
case DRM_PLANE_TYPE_CURSOR:
break;
case DRM_PLANE_TYPE_OVERLAY:
- /*
- * WaCxSRDisabledForSpriteScaling:ivb
- *
- * cstate->update_wm was already set above, so this flag will
- * take effect when we commit and program watermarks.
- */
- if (IS_IVYBRIDGE(dev) &&
- needs_scaling(to_intel_plane_state(plane_state)) &&
- !needs_scaling(old_plane_state)) {
- to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
- } else if (turn_off && !mode_changed) {
+ if (turn_off && !mode_changed) {
intel_crtc->atomic.wait_vblank = true;
intel_crtc->atomic.update_sprite_watermarks |=
1 << i;
}
-
- break;
}
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 91b6b40..0598932 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -468,9 +468,6 @@ struct intel_crtc_state {
/* w/a for waiting 2 vblanks during crtc enable */
enum pipe hsw_workaround_pipe;
-
- /* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
- bool disable_lp_wm;
};
struct vlv_wm_state {
@@ -1399,6 +1396,12 @@ void intel_init_clock_gating(struct drm_device *dev);
void intel_suspend_hw(struct drm_device *dev);
int ilk_wm_max_level(const struct drm_device *dev);
void intel_update_watermarks(struct drm_crtc *crtc);
+void intel_update_sprite_watermarks(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
+ bool enabled, bool scaled);
void intel_init_pm(struct drm_device *dev);
void intel_pm_setup(struct drm_device *dev);
void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d031d74..d13551f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1765,6 +1765,13 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
}
+struct skl_pipe_wm_parameters {
+ bool active;
+ uint32_t pipe_htotal;
+ uint32_t pixel_rate; /* in KHz */
+ struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
+};
+
struct ilk_wm_maximums {
uint16_t pri;
uint16_t spr;
@@ -2805,40 +2812,18 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
#define SKL_DDB_SIZE 896 /* in blocks */
#define BXT_DDB_SIZE 512
-/*
- * Return the index of a plane in the SKL DDB and wm result arrays. Primary
- * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
- * other universal planes are in indices 1..n. Note that this may leave unused
- * indices between the top "sprite" plane and the cursor.
- */
-static int
-skl_wm_plane_id(const struct intel_plane *plane)
-{
- switch (plane->base.type) {
- case DRM_PLANE_TYPE_PRIMARY:
- return 0;
- case DRM_PLANE_TYPE_CURSOR:
- return PLANE_CURSOR;
- case DRM_PLANE_TYPE_OVERLAY:
- return plane->plane + 1;
- default:
- MISSING_CASE(plane->base.type);
- return plane->plane;
- }
-}
-
static void
skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
- const struct intel_crtc_state *cstate,
+ struct drm_crtc *for_crtc,
const struct intel_wm_config *config,
+ const struct skl_pipe_wm_parameters *params,
struct skl_ddb_entry *alloc /* out */)
{
- struct drm_crtc *for_crtc = cstate->base.crtc;
struct drm_crtc *crtc;
unsigned int pipe_size, ddb_size;
int nth_active_pipe;
- if (!cstate->base.active) {
+ if (!params->active) {
alloc->start = 0;
alloc->end = 0;
return;
@@ -2904,29 +2889,19 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
}
static unsigned int
-skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
- const struct drm_plane_state *pstate,
- int y)
+skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- struct drm_framebuffer *fb = pstate->fb;
/* for planar format */
- if (fb->pixel_format == DRM_FORMAT_NV12) {
+ if (p->y_bytes_per_pixel) {
if (y) /* y-plane data rate */
- return intel_crtc->config->pipe_src_w *
- intel_crtc->config->pipe_src_h *
- drm_format_plane_cpp(fb->pixel_format, 0);
+ return p->horiz_pixels * p->vert_pixels * p->y_bytes_per_pixel;
else /* uv-plane data rate */
- return (intel_crtc->config->pipe_src_w/2) *
- (intel_crtc->config->pipe_src_h/2) *
- drm_format_plane_cpp(fb->pixel_format, 1);
+ return (p->horiz_pixels/2) * (p->vert_pixels/2) * p->bytes_per_pixel;
}
/* for packed formats */
- return intel_crtc->config->pipe_src_w *
- intel_crtc->config->pipe_src_h *
- drm_format_plane_cpp(fb->pixel_format, 0);
+ return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
}
/*
@@ -2935,51 +2910,46 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
* 3 * 4096 * 8192 * 4 < 2^32
*/
static unsigned int
-skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
+skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
+ const struct skl_pipe_wm_parameters *params)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- struct drm_device *dev = intel_crtc->base.dev;
- const struct intel_plane *intel_plane;
unsigned int total_data_rate = 0;
+ int plane;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- const struct drm_plane_state *pstate = intel_plane->base.state;
+ for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
+ const struct intel_plane_wm_parameters *p;
- if (pstate->fb == NULL)
+ p = ¶ms->plane[plane];
+ if (!p->enabled)
continue;
- /* packed/uv */
- total_data_rate += skl_plane_relative_data_rate(cstate,
- pstate,
- 0);
-
- if (pstate->fb->pixel_format == DRM_FORMAT_NV12)
- /* y-plane */
- total_data_rate += skl_plane_relative_data_rate(cstate,
- pstate,
- 1);
+ total_data_rate += skl_plane_relative_data_rate(p, 0); /* packed/uv */
+ if (p->y_bytes_per_pixel) {
+ total_data_rate += skl_plane_relative_data_rate(p, 1); /* y-plane */
+ }
}
return total_data_rate;
}
static void
-skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
+skl_allocate_pipe_ddb(struct drm_crtc *crtc,
const struct intel_wm_config *config,
+ const struct skl_pipe_wm_parameters *params,
struct skl_ddb_allocation *ddb /* out */)
{
- struct drm_crtc *crtc = cstate->base.crtc;
struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_plane *intel_plane;
enum pipe pipe = intel_crtc->pipe;
struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
uint16_t alloc_size, start, cursor_blocks;
uint16_t minimum[I915_MAX_PLANES];
uint16_t y_minimum[I915_MAX_PLANES];
unsigned int total_data_rate;
+ int plane;
- skl_ddb_get_pipe_allocation_limits(dev, cstate, config, alloc);
+ skl_ddb_get_pipe_allocation_limits(dev, crtc, config, params, alloc);
alloc_size = skl_ddb_entry_size(alloc);
if (alloc_size == 0) {
memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
@@ -2996,20 +2966,17 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
alloc->end -= cursor_blocks;
/* 1. Allocate the mininum required blocks for each active plane */
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- struct drm_plane *plane = &intel_plane->base;
- struct drm_framebuffer *fb = plane->fb;
- int id = skl_wm_plane_id(intel_plane);
+ for_each_plane(dev_priv, pipe, plane) {
+ const struct intel_plane_wm_parameters *p;
- if (fb == NULL)
- continue;
- if (plane->type == DRM_PLANE_TYPE_CURSOR)
+ p = ¶ms->plane[plane];
+ if (!p->enabled)
continue;
- minimum[id] = 8;
- alloc_size -= minimum[id];
- y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
- alloc_size -= y_minimum[id];
+ minimum[plane] = 8;
+ alloc_size -= minimum[plane];
+ y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
+ alloc_size -= y_minimum[plane];
}
/*
@@ -3018,50 +2985,45 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
*
* FIXME: we may not allocate every single block here.
*/
- total_data_rate = skl_get_total_relative_data_rate(cstate);
+ total_data_rate = skl_get_total_relative_data_rate(intel_crtc, params);
start = alloc->start;
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- struct drm_plane *plane = &intel_plane->base;
- struct drm_plane_state *pstate = intel_plane->base.state;
+ for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
+ const struct intel_plane_wm_parameters *p;
unsigned int data_rate, y_data_rate;
uint16_t plane_blocks, y_plane_blocks = 0;
- int id = skl_wm_plane_id(intel_plane);
- if (pstate->fb == NULL)
- continue;
- if (plane->type == DRM_PLANE_TYPE_CURSOR)
+ p = ¶ms->plane[plane];
+ if (!p->enabled)
continue;
- data_rate = skl_plane_relative_data_rate(cstate, pstate, 0);
+ data_rate = skl_plane_relative_data_rate(p, 0);
/*
* allocation for (packed formats) or (uv-plane part of planar format):
* promote the expression to 64 bits to avoid overflowing, the
* result is < available as data_rate / total_data_rate < 1
*/
- plane_blocks = minimum[id];
+ plane_blocks = minimum[plane];
plane_blocks += div_u64((uint64_t)alloc_size * data_rate,
total_data_rate);
- ddb->plane[pipe][id].start = start;
- ddb->plane[pipe][id].end = start + plane_blocks;
+ ddb->plane[pipe][plane].start = start;
+ ddb->plane[pipe][plane].end = start + plane_blocks;
start += plane_blocks;
/*
* allocation for y_plane part of planar format:
*/
- if (pstate->fb->pixel_format == DRM_FORMAT_NV12) {
- y_data_rate = skl_plane_relative_data_rate(cstate,
- pstate,
- 1);
- y_plane_blocks = y_minimum[id];
+ if (p->y_bytes_per_pixel) {
+ y_data_rate = skl_plane_relative_data_rate(p, 1);
+ y_plane_blocks = y_minimum[plane];
y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
total_data_rate);
- ddb->y_plane[pipe][id].start = start;
- ddb->y_plane[pipe][id].end = start + y_plane_blocks;
+ ddb->y_plane[pipe][plane].start = start;
+ ddb->y_plane[pipe][plane].end = start + y_plane_blocks;
start += y_plane_blocks;
}
@@ -3148,21 +3110,87 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
struct intel_wm_config *config)
{
struct drm_crtc *crtc;
+ struct drm_plane *plane;
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
config->num_pipes_active += to_intel_crtc(crtc)->active;
+
+ /* FIXME: I don't think we need those two global parameters on SKL */
+ list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+
+ config->sprites_enabled |= intel_plane->wm.enabled;
+ config->sprites_scaled |= intel_plane->wm.scaled;
+ }
+}
+
+static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
+ struct skl_pipe_wm_parameters *p)
+{
+ struct drm_device *dev = crtc->dev;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum pipe pipe = intel_crtc->pipe;
+ struct drm_plane *plane;
+ struct drm_framebuffer *fb;
+ int i = 1; /* Index for sprite planes start */
+
+ p->active = intel_crtc->active;
+ if (p->active) {
+ p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
+ p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
+
+ fb = crtc->primary->state->fb;
+ /* For planar: Bpp is for uv plane, y_Bpp is for y plane */
+ if (fb) {
+ p->plane[0].enabled = true;
+ p->plane[0].bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
+ drm_format_plane_cpp(fb->pixel_format, 1) :
+ drm_format_plane_cpp(fb->pixel_format, 0);
+ p->plane[0].y_bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
+ drm_format_plane_cpp(fb->pixel_format, 0) : 0;
+ p->plane[0].tiling = fb->modifier[0];
+ } else {
+ p->plane[0].enabled = false;
+ p->plane[0].bytes_per_pixel = 0;
+ p->plane[0].y_bytes_per_pixel = 0;
+ p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
+ }
+ p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
+ p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
+ p->plane[0].rotation = crtc->primary->state->rotation;
+
+ fb = crtc->cursor->state->fb;
+ p->plane[PLANE_CURSOR].y_bytes_per_pixel = 0;
+ if (fb) {
+ p->plane[PLANE_CURSOR].enabled = true;
+ p->plane[PLANE_CURSOR].bytes_per_pixel = fb->bits_per_pixel / 8;
+ p->plane[PLANE_CURSOR].horiz_pixels = crtc->cursor->state->crtc_w;
+ p->plane[PLANE_CURSOR].vert_pixels = crtc->cursor->state->crtc_h;
+ } else {
+ p->plane[PLANE_CURSOR].enabled = false;
+ p->plane[PLANE_CURSOR].bytes_per_pixel = 0;
+ p->plane[PLANE_CURSOR].horiz_pixels = 64;
+ p->plane[PLANE_CURSOR].vert_pixels = 64;
+ }
+ }
+
+ list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+
+ if (intel_plane->pipe == pipe &&
+ plane->type == DRM_PLANE_TYPE_OVERLAY)
+ p->plane[i++] = intel_plane->wm;
+ }
}
static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
- struct intel_crtc_state *cstate,
- struct intel_plane *intel_plane,
+ struct skl_pipe_wm_parameters *p,
+ struct intel_plane_wm_parameters *p_params,
uint16_t ddb_allocation,
int level,
uint16_t *out_blocks, /* out */
uint8_t *out_lines /* out */)
{
- struct drm_plane *plane = &intel_plane->base;
- struct drm_framebuffer *fb = plane->state->fb;
uint32_t latency = dev_priv->wm.skl_latency[level];
uint32_t method1, method2;
uint32_t plane_bytes_per_line, plane_blocks_per_line;
@@ -3170,35 +3198,31 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
uint32_t selected_result;
uint8_t bytes_per_pixel;
- if (latency == 0 || !cstate->base.active || !fb)
+ if (latency == 0 || !p->active || !p_params->enabled)
return false;
- bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
- drm_format_plane_cpp(DRM_FORMAT_NV12, 0) :
- drm_format_plane_cpp(DRM_FORMAT_NV12, 1);
- method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
+ bytes_per_pixel = p_params->y_bytes_per_pixel ?
+ p_params->y_bytes_per_pixel :
+ p_params->bytes_per_pixel;
+ method1 = skl_wm_method1(p->pixel_rate,
bytes_per_pixel,
latency);
- method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
- cstate->base.adjusted_mode.crtc_htotal,
- cstate->pipe_src_w,
+ method2 = skl_wm_method2(p->pixel_rate,
+ p->pipe_htotal,
+ p_params->horiz_pixels,
bytes_per_pixel,
- fb->modifier[0],
+ p_params->tiling,
latency);
- plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
+ plane_bytes_per_line = p_params->horiz_pixels * bytes_per_pixel;
plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
- if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
- fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
+ if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
+ p_params->tiling == I915_FORMAT_MOD_Yf_TILED) {
uint32_t min_scanlines = 4;
uint32_t y_tile_minimum;
- if (intel_rotation_90_or_270(plane->state->rotation)) {
- int bpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
- drm_format_plane_cpp(fb->pixel_format, 1) :
- drm_format_plane_cpp(fb->pixel_format, 0);
-
- switch (bpp) {
+ if (intel_rotation_90_or_270(p_params->rotation)) {
+ switch (p_params->bytes_per_pixel) {
case 1:
min_scanlines = 16;
break;
@@ -3222,8 +3246,8 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
res_lines = DIV_ROUND_UP(selected_result, plane_blocks_per_line);
if (level >= 1 && level <= 7) {
- if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
- fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)
+ if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
+ p_params->tiling == I915_FORMAT_MOD_Yf_TILED)
res_lines += 4;
else
res_blocks++;
@@ -3240,80 +3264,84 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb,
- struct intel_crtc_state *cstate,
+ struct skl_pipe_wm_parameters *p,
+ enum pipe pipe,
int level,
+ int num_planes,
struct skl_wm_level *result)
{
- struct drm_device *dev = dev_priv->dev;
- struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- struct intel_plane *intel_plane;
uint16_t ddb_blocks;
- enum pipe pipe = intel_crtc->pipe;
-
- for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- int i = skl_wm_plane_id(intel_plane);
+ int i;
+ for (i = 0; i < num_planes; i++) {
ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
result->plane_en[i] = skl_compute_plane_wm(dev_priv,
- cstate,
- intel_plane,
+ p, &p->plane[i],
ddb_blocks,
level,
&result->plane_res_b[i],
&result->plane_res_l[i]);
}
+
+ ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][PLANE_CURSOR]);
+ result->plane_en[PLANE_CURSOR] = skl_compute_plane_wm(dev_priv, p,
+ &p->plane[PLANE_CURSOR],
+ ddb_blocks, level,
+ &result->plane_res_b[PLANE_CURSOR],
+ &result->plane_res_l[PLANE_CURSOR]);
}
static uint32_t
-skl_compute_linetime_wm(struct intel_crtc_state *cstate)
+skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
{
- if (!cstate->base.active)
+ if (!to_intel_crtc(crtc)->active)
return 0;
- if (WARN_ON(skl_pipe_pixel_rate(cstate) == 0))
+ if (WARN_ON(p->pixel_rate == 0))
return 0;
- return DIV_ROUND_UP(8 * cstate->base.adjusted_mode.crtc_htotal * 1000,
- skl_pipe_pixel_rate(cstate));
+ return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
}
-static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
+static void skl_compute_transition_wm(struct drm_crtc *crtc,
+ struct skl_pipe_wm_parameters *params,
struct skl_wm_level *trans_wm /* out */)
{
- struct drm_crtc *crtc = cstate->base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_plane *intel_plane;
+ int i;
- if (!cstate->base.active)
+ if (!params->active)
return;
/* Until we know more, just disable transition WMs */
- for_each_intel_plane_on_crtc(crtc->dev, intel_crtc, intel_plane) {
- int i = skl_wm_plane_id(intel_plane);
-
+ for (i = 0; i < intel_num_planes(intel_crtc); i++)
trans_wm->plane_en[i] = false;
- }
+ trans_wm->plane_en[PLANE_CURSOR] = false;
}
-static void skl_compute_pipe_wm(struct intel_crtc_state *cstate,
+static void skl_compute_pipe_wm(struct drm_crtc *crtc,
struct skl_ddb_allocation *ddb,
+ struct skl_pipe_wm_parameters *params,
struct skl_pipe_wm *pipe_wm)
{
- struct drm_device *dev = cstate->base.crtc->dev;
+ struct drm_device *dev = crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int level, max_level = ilk_wm_max_level(dev);
for (level = 0; level <= max_level; level++) {
- skl_compute_wm_level(dev_priv, ddb, cstate,
- level, &pipe_wm->wm[level]);
+ skl_compute_wm_level(dev_priv, ddb, params, intel_crtc->pipe,
+ level, intel_num_planes(intel_crtc),
+ &pipe_wm->wm[level]);
}
- pipe_wm->linetime = skl_compute_linetime_wm(cstate);
+ pipe_wm->linetime = skl_compute_linetime_wm(crtc, params);
- skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
+ skl_compute_transition_wm(crtc, params, &pipe_wm->trans_wm);
}
static void skl_compute_wm_results(struct drm_device *dev,
+ struct skl_pipe_wm_parameters *p,
struct skl_pipe_wm *p_wm,
struct skl_wm_values *r,
struct intel_crtc *intel_crtc)
@@ -3557,15 +3585,16 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
}
static bool skl_update_pipe_wm(struct drm_crtc *crtc,
+ struct skl_pipe_wm_parameters *params,
struct intel_wm_config *config,
struct skl_ddb_allocation *ddb, /* out */
struct skl_pipe_wm *pipe_wm /* out */)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- skl_allocate_pipe_ddb(cstate, config, ddb);
- skl_compute_pipe_wm(cstate, ddb, pipe_wm);
+ skl_compute_wm_pipe_parameters(crtc, params);
+ skl_allocate_pipe_ddb(crtc, config, params, ddb);
+ skl_compute_pipe_wm(crtc, ddb, params, pipe_wm);
if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
return false;
@@ -3598,6 +3627,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
*/
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head) {
+ struct skl_pipe_wm_parameters params = {};
struct skl_pipe_wm pipe_wm = {};
bool wm_changed;
@@ -3607,7 +3637,8 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
if (!intel_crtc->active)
continue;
- wm_changed = skl_update_pipe_wm(&intel_crtc->base, config,
+ wm_changed = skl_update_pipe_wm(&intel_crtc->base,
+ ¶ms, config,
&r->ddb, &pipe_wm);
/*
@@ -3617,7 +3648,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
*/
WARN_ON(!wm_changed);
- skl_compute_wm_results(dev, &pipe_wm, r, intel_crtc);
+ skl_compute_wm_results(dev, ¶ms, &pipe_wm, r, intel_crtc);
r->dirty[intel_crtc->pipe] = true;
}
}
@@ -3647,6 +3678,7 @@ static void skl_update_wm(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct skl_pipe_wm_parameters params = {};
struct skl_wm_values *results = &dev_priv->wm.skl_results;
struct skl_pipe_wm pipe_wm = {};
struct intel_wm_config config = {};
@@ -3659,10 +3691,11 @@ static void skl_update_wm(struct drm_crtc *crtc)
skl_compute_wm_global_parameters(dev, &config);
- if (!skl_update_pipe_wm(crtc, &config, &results->ddb, &pipe_wm))
+ if (!skl_update_pipe_wm(crtc, ¶ms, &config,
+ &results->ddb, &pipe_wm))
return;
- skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
+ skl_compute_wm_results(dev, ¶ms, &pipe_wm, results, intel_crtc);
results->dirty[intel_crtc->pipe] = true;
skl_update_other_pipe_wm(dev, crtc, &config, results);
@@ -3673,6 +3706,39 @@ static void skl_update_wm(struct drm_crtc *crtc)
dev_priv->wm.skl_hw = *results;
}
+static void
+skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
+ uint32_t sprite_width, uint32_t sprite_height,
+ int pixel_size, bool enabled, bool scaled)
+{
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_framebuffer *fb = plane->state->fb;
+
+ intel_plane->wm.enabled = enabled;
+ intel_plane->wm.scaled = scaled;
+ intel_plane->wm.horiz_pixels = sprite_width;
+ intel_plane->wm.vert_pixels = sprite_height;
+ intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
+
+ /* For planar: Bpp is for UV plane, y_Bpp is for Y plane */
+ intel_plane->wm.bytes_per_pixel =
+ (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
+ drm_format_plane_cpp(plane->state->fb->pixel_format, 1) : pixel_size;
+ intel_plane->wm.y_bytes_per_pixel =
+ (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
+ drm_format_plane_cpp(plane->state->fb->pixel_format, 0) : 0;
+
+ /*
+ * Framebuffer can be NULL on plane disable, but it does not
+ * matter for watermarks if we assume no tiling in that case.
+ */
+ if (fb)
+ intel_plane->wm.tiling = fb->modifier[0];
+ intel_plane->wm.rotation = plane->state->rotation;
+
+ skl_update_wm(crtc);
+}
+
static void ilk_update_wm(struct drm_crtc *crtc)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -3688,18 +3754,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
WARN_ON(cstate->base.active != intel_crtc->active);
- /*
- * 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(dev);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- }
-
intel_compute_pipe_wm(cstate, &pipe_wm);
if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
@@ -3731,6 +3785,28 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, &results);
}
+static void
+ilk_update_sprite_wm(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width, uint32_t sprite_height,
+ int pixel_size, bool enabled, bool scaled)
+{
+ struct drm_device *dev = plane->dev;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+
+ /*
+ * 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);
+
+ ilk_update_wm(crtc);
+}
+
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
@@ -4108,6 +4184,21 @@ void intel_update_watermarks(struct drm_crtc *crtc)
dev_priv->display.update_wm(crtc);
}
+void intel_update_sprite_watermarks(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
+ bool enabled, bool scaled)
+{
+ struct drm_i915_private *dev_priv = plane->dev->dev_private;
+
+ if (dev_priv->display.update_sprite_wm)
+ dev_priv->display.update_sprite_wm(plane, crtc,
+ sprite_width, sprite_height,
+ pixel_size, enabled, scaled);
+}
+
/**
* Lock protecting IPS related data structures
*/
@@ -7022,6 +7113,7 @@ void intel_init_pm(struct drm_device *dev)
dev_priv->display.init_clock_gating =
skl_init_clock_gating;
dev_priv->display.update_wm = skl_update_wm;
+ dev_priv->display.update_sprite_wm = skl_update_sprite_wm;
} else if (HAS_PCH_SPLIT(dev)) {
ilk_setup_wm_latency(dev);
@@ -7030,6 +7122,7 @@ void intel_init_pm(struct drm_device *dev)
(!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.update_sprite_wm = ilk_update_sprite_wm;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index dd2d568..b229c67 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -192,6 +192,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
u32 plane_ctl, stride_div, stride;
+ int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
const struct drm_intel_sprite_colorkey *key =
&to_intel_plane_state(drm_plane->state)->ckey;
unsigned long surf_addr;
@@ -210,6 +211,10 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
rotation = drm_plane->state->rotation;
plane_ctl |= skl_plane_ctl_rotation(rotation);
+ intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h,
+ pixel_size, true,
+ src_w != crtc_w || src_h != crtc_h);
+
stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
fb->pixel_format);
@@ -291,6 +296,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
I915_WRITE(PLANE_SURF(pipe, plane), 0);
POSTING_READ(PLANE_SURF(pipe, plane));
+
+ intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
}
static void
@@ -533,6 +540,10 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
sprctl |= SPRITE_PIPE_CSC_ENABLE;
+ intel_update_sprite_watermarks(plane, crtc, src_w, src_h, pixel_size,
+ true,
+ src_w != crtc_w || src_h != crtc_h);
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -666,6 +677,10 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
if (IS_GEN6(dev))
dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
+ intel_update_sprite_watermarks(plane, crtc, src_w, src_h,
+ pixel_size, true,
+ src_w != crtc_w || src_h != crtc_h);
+
/* Sizes are 0 based */
src_w--;
src_h--;
--
2.5.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: revert a few more watermark commits
2015-10-09 21:22 ` [PATCH] drm/i915: revert a few more watermark commits Paulo Zanoni
@ 2015-10-13 12:00 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-10-13 12:00 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Fri, Oct 09, 2015 at 06:22:43PM -0300, Paulo Zanoni wrote:
> This is a squash of the following commits:
>
> Revert "drm/i915: Drop intel_update_sprite_watermarks"
> This reverts commit 47c99438b52d12df50e182583634a4cfede3c920.
>
> Revert "drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check"
> This reverts commit 7809e5ae35b9d8d0710f0874b2e3f10be144e38b.
>
> Revert "drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v3)"
> This reverts commit 3a05f5e2e78eab7ffe816abb59b6769e331a1957.
>
> With these reverts, SKL finally stops failing every single FBC test
> with FIFO underrun error messages. After some brief testing, it also
> seems that this commit prevents the machine from completely freezing
> when we run igt/kms_fbc_crc (see fd.o #92355).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92355
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 +
> drivers/gpu/drm/i915/intel_atomic.c | 1 -
> drivers/gpu/drm/i915/intel_display.c | 44 +---
> drivers/gpu/drm/i915/intel_drv.h | 9 +-
> drivers/gpu/drm/i915/intel_pm.c | 419 +++++++++++++++++++++--------------
> drivers/gpu/drm/i915/intel_sprite.c | 15 ++
> 6 files changed, 293 insertions(+), 199 deletions(-)
>
>
> The fact that I needed more reverts after rebasing suggests that maybe the
> problem is happening due to some sort of timing problem? Maybe we're missing
> some lock?
>
> The machine freezes while running igt/kms_fbc_crc/blt are also curious, but I
> didn't spend too much time investigating them.
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bf14096..22c8b0a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -628,6 +628,10 @@ struct drm_i915_display_funcs {
> struct dpll *match_clock,
> struct dpll *best_clock);
> void (*update_wm)(struct drm_crtc *crtc);
> + void (*update_sprite_wm)(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + uint32_t sprite_width, uint32_t sprite_height,
> + int pixel_size, bool enable, bool scaled);
> int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
> /* Returns the active state of the crtc, and if the crtc is active,
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 05b1203..f1975f2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -94,7 +94,6 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> __drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
>
> crtc_state->update_pipe = false;
> - crtc_state->disable_lp_wm = false;
>
> return &crtc_state->base;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cddb0c6..66a37d8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4804,6 +4804,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_plane *plane;
>
> if (atomic->wait_vblank)
> intel_wait_for_vblank(dev, crtc->pipe);
> @@ -4822,6 +4823,10 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> if (atomic->post_enable_primary)
> intel_post_enable_primary(&crtc->base);
>
> + drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
> + intel_update_sprite_watermarks(plane, &crtc->base,
> + 0, 0, 0, false, false);
> +
> memset(atomic, 0, sizeof(*atomic));
> }
>
> @@ -11577,30 +11582,16 @@ retry:
> static bool intel_wm_need_update(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> - struct intel_plane_state *new = to_intel_plane_state(state);
> - struct intel_plane_state *cur = to_intel_plane_state(plane->state);
> -
> - /* Update watermarks on tiling or size changes. */
> + /* Update watermarks on tiling changes. */
> if (!plane->state->fb || !state->fb ||
> plane->state->fb->modifier[0] != state->fb->modifier[0] ||
> - plane->state->rotation != state->rotation ||
> - drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
> - drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
> - drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
> - drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
> + plane->state->rotation != state->rotation)
> return true;
>
> - return false;
> -}
> -
> -static bool needs_scaling(struct intel_plane_state *state)
> -{
> - int src_w = drm_rect_width(&state->src) >> 16;
> - int src_h = drm_rect_height(&state->src) >> 16;
> - int dst_w = drm_rect_width(&state->dst);
> - int dst_h = drm_rect_height(&state->dst);
> + if (plane->state->crtc_w != state->crtc_w)
> + return true;
>
> - return (src_w != dst_w || src_h != dst_h);
> + return false;
> }
>
> int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> @@ -11618,6 +11609,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> bool mode_changed = needs_modeset(crtc_state);
> bool was_crtc_enabled = crtc->state->active;
> bool is_crtc_enabled = crtc_state->active;
> +
> bool turn_off, turn_on, visible, was_visible;
> struct drm_framebuffer *fb = plane_state->fb;
>
> @@ -11735,23 +11727,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> case DRM_PLANE_TYPE_CURSOR:
> break;
> case DRM_PLANE_TYPE_OVERLAY:
> - /*
> - * WaCxSRDisabledForSpriteScaling:ivb
> - *
> - * cstate->update_wm was already set above, so this flag will
> - * take effect when we commit and program watermarks.
> - */
> - if (IS_IVYBRIDGE(dev) &&
> - needs_scaling(to_intel_plane_state(plane_state)) &&
> - !needs_scaling(old_plane_state)) {
> - to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
> - } else if (turn_off && !mode_changed) {
> + if (turn_off && !mode_changed) {
> intel_crtc->atomic.wait_vblank = true;
> intel_crtc->atomic.update_sprite_watermarks |=
> 1 << i;
> }
> -
> - break;
> }
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 91b6b40..0598932 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -468,9 +468,6 @@ struct intel_crtc_state {
>
> /* w/a for waiting 2 vblanks during crtc enable */
> enum pipe hsw_workaround_pipe;
> -
> - /* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
> - bool disable_lp_wm;
> };
>
> struct vlv_wm_state {
> @@ -1399,6 +1396,12 @@ void intel_init_clock_gating(struct drm_device *dev);
> void intel_suspend_hw(struct drm_device *dev);
> int ilk_wm_max_level(const struct drm_device *dev);
> void intel_update_watermarks(struct drm_crtc *crtc);
> +void intel_update_sprite_watermarks(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + uint32_t sprite_width,
> + uint32_t sprite_height,
> + int pixel_size,
> + bool enabled, bool scaled);
> void intel_init_pm(struct drm_device *dev);
> void intel_pm_setup(struct drm_device *dev);
> void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d031d74..d13551f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1765,6 +1765,13 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
> return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
> }
>
> +struct skl_pipe_wm_parameters {
> + bool active;
> + uint32_t pipe_htotal;
> + uint32_t pixel_rate; /* in KHz */
> + struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
> +};
> +
> struct ilk_wm_maximums {
> uint16_t pri;
> uint16_t spr;
> @@ -2805,40 +2812,18 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
> #define SKL_DDB_SIZE 896 /* in blocks */
> #define BXT_DDB_SIZE 512
>
> -/*
> - * Return the index of a plane in the SKL DDB and wm result arrays. Primary
> - * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
> - * other universal planes are in indices 1..n. Note that this may leave unused
> - * indices between the top "sprite" plane and the cursor.
> - */
> -static int
> -skl_wm_plane_id(const struct intel_plane *plane)
> -{
> - switch (plane->base.type) {
> - case DRM_PLANE_TYPE_PRIMARY:
> - return 0;
> - case DRM_PLANE_TYPE_CURSOR:
> - return PLANE_CURSOR;
> - case DRM_PLANE_TYPE_OVERLAY:
> - return plane->plane + 1;
> - default:
> - MISSING_CASE(plane->base.type);
> - return plane->plane;
> - }
> -}
> -
> static void
> skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> - const struct intel_crtc_state *cstate,
> + struct drm_crtc *for_crtc,
> const struct intel_wm_config *config,
> + const struct skl_pipe_wm_parameters *params,
> struct skl_ddb_entry *alloc /* out */)
> {
> - struct drm_crtc *for_crtc = cstate->base.crtc;
> struct drm_crtc *crtc;
> unsigned int pipe_size, ddb_size;
> int nth_active_pipe;
>
> - if (!cstate->base.active) {
> + if (!params->active) {
> alloc->start = 0;
> alloc->end = 0;
> return;
> @@ -2904,29 +2889,19 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> }
>
> static unsigned int
> -skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> - const struct drm_plane_state *pstate,
> - int y)
> +skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
> {
> - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> - struct drm_framebuffer *fb = pstate->fb;
>
> /* for planar format */
> - if (fb->pixel_format == DRM_FORMAT_NV12) {
> + if (p->y_bytes_per_pixel) {
> if (y) /* y-plane data rate */
> - return intel_crtc->config->pipe_src_w *
> - intel_crtc->config->pipe_src_h *
> - drm_format_plane_cpp(fb->pixel_format, 0);
> + return p->horiz_pixels * p->vert_pixels * p->y_bytes_per_pixel;
> else /* uv-plane data rate */
> - return (intel_crtc->config->pipe_src_w/2) *
> - (intel_crtc->config->pipe_src_h/2) *
> - drm_format_plane_cpp(fb->pixel_format, 1);
> + return (p->horiz_pixels/2) * (p->vert_pixels/2) * p->bytes_per_pixel;
> }
>
> /* for packed formats */
> - return intel_crtc->config->pipe_src_w *
> - intel_crtc->config->pipe_src_h *
> - drm_format_plane_cpp(fb->pixel_format, 0);
> + return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
> }
>
> /*
> @@ -2935,51 +2910,46 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> * 3 * 4096 * 8192 * 4 < 2^32
> */
> static unsigned int
> -skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
> +skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
> + const struct skl_pipe_wm_parameters *params)
> {
> - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> - struct drm_device *dev = intel_crtc->base.dev;
> - const struct intel_plane *intel_plane;
> unsigned int total_data_rate = 0;
> + int plane;
>
> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - const struct drm_plane_state *pstate = intel_plane->base.state;
> + for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
> + const struct intel_plane_wm_parameters *p;
>
> - if (pstate->fb == NULL)
> + p = ¶ms->plane[plane];
> + if (!p->enabled)
> continue;
>
> - /* packed/uv */
> - total_data_rate += skl_plane_relative_data_rate(cstate,
> - pstate,
> - 0);
> -
> - if (pstate->fb->pixel_format == DRM_FORMAT_NV12)
> - /* y-plane */
> - total_data_rate += skl_plane_relative_data_rate(cstate,
> - pstate,
> - 1);
> + total_data_rate += skl_plane_relative_data_rate(p, 0); /* packed/uv */
> + if (p->y_bytes_per_pixel) {
> + total_data_rate += skl_plane_relative_data_rate(p, 1); /* y-plane */
> + }
> }
>
> return total_data_rate;
> }
>
> static void
> -skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> +skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> const struct intel_wm_config *config,
> + const struct skl_pipe_wm_parameters *params,
> struct skl_ddb_allocation *ddb /* out */)
> {
> - struct drm_crtc *crtc = cstate->base.crtc;
> struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_plane *intel_plane;
> enum pipe pipe = intel_crtc->pipe;
> struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
> uint16_t alloc_size, start, cursor_blocks;
> uint16_t minimum[I915_MAX_PLANES];
> uint16_t y_minimum[I915_MAX_PLANES];
> unsigned int total_data_rate;
> + int plane;
>
> - skl_ddb_get_pipe_allocation_limits(dev, cstate, config, alloc);
> + skl_ddb_get_pipe_allocation_limits(dev, crtc, config, params, alloc);
> alloc_size = skl_ddb_entry_size(alloc);
> if (alloc_size == 0) {
> memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> @@ -2996,20 +2966,17 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> alloc->end -= cursor_blocks;
>
> /* 1. Allocate the mininum required blocks for each active plane */
> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - struct drm_plane *plane = &intel_plane->base;
> - struct drm_framebuffer *fb = plane->fb;
> - int id = skl_wm_plane_id(intel_plane);
> + for_each_plane(dev_priv, pipe, plane) {
> + const struct intel_plane_wm_parameters *p;
>
> - if (fb == NULL)
> - continue;
> - if (plane->type == DRM_PLANE_TYPE_CURSOR)
> + p = ¶ms->plane[plane];
> + if (!p->enabled)
> continue;
>
> - minimum[id] = 8;
> - alloc_size -= minimum[id];
> - y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
> - alloc_size -= y_minimum[id];
> + minimum[plane] = 8;
> + alloc_size -= minimum[plane];
> + y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
> + alloc_size -= y_minimum[plane];
> }
>
> /*
> @@ -3018,50 +2985,45 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> *
> * FIXME: we may not allocate every single block here.
> */
> - total_data_rate = skl_get_total_relative_data_rate(cstate);
> + total_data_rate = skl_get_total_relative_data_rate(intel_crtc, params);
>
> start = alloc->start;
> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - struct drm_plane *plane = &intel_plane->base;
> - struct drm_plane_state *pstate = intel_plane->base.state;
> + for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
> + const struct intel_plane_wm_parameters *p;
> unsigned int data_rate, y_data_rate;
> uint16_t plane_blocks, y_plane_blocks = 0;
> - int id = skl_wm_plane_id(intel_plane);
>
> - if (pstate->fb == NULL)
> - continue;
> - if (plane->type == DRM_PLANE_TYPE_CURSOR)
> + p = ¶ms->plane[plane];
> + if (!p->enabled)
> continue;
>
> - data_rate = skl_plane_relative_data_rate(cstate, pstate, 0);
> + data_rate = skl_plane_relative_data_rate(p, 0);
>
> /*
> * allocation for (packed formats) or (uv-plane part of planar format):
> * promote the expression to 64 bits to avoid overflowing, the
> * result is < available as data_rate / total_data_rate < 1
> */
> - plane_blocks = minimum[id];
> + plane_blocks = minimum[plane];
> plane_blocks += div_u64((uint64_t)alloc_size * data_rate,
> total_data_rate);
>
> - ddb->plane[pipe][id].start = start;
> - ddb->plane[pipe][id].end = start + plane_blocks;
> + ddb->plane[pipe][plane].start = start;
> + ddb->plane[pipe][plane].end = start + plane_blocks;
>
> start += plane_blocks;
>
> /*
> * allocation for y_plane part of planar format:
> */
> - if (pstate->fb->pixel_format == DRM_FORMAT_NV12) {
> - y_data_rate = skl_plane_relative_data_rate(cstate,
> - pstate,
> - 1);
> - y_plane_blocks = y_minimum[id];
> + if (p->y_bytes_per_pixel) {
> + y_data_rate = skl_plane_relative_data_rate(p, 1);
> + y_plane_blocks = y_minimum[plane];
> y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
> total_data_rate);
>
> - ddb->y_plane[pipe][id].start = start;
> - ddb->y_plane[pipe][id].end = start + y_plane_blocks;
> + ddb->y_plane[pipe][plane].start = start;
> + ddb->y_plane[pipe][plane].end = start + y_plane_blocks;
>
> start += y_plane_blocks;
> }
> @@ -3148,21 +3110,87 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
> struct intel_wm_config *config)
> {
> struct drm_crtc *crtc;
> + struct drm_plane *plane;
>
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> config->num_pipes_active += to_intel_crtc(crtc)->active;
> +
> + /* FIXME: I don't think we need those two global parameters on SKL */
> + list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> +
> + config->sprites_enabled |= intel_plane->wm.enabled;
> + config->sprites_scaled |= intel_plane->wm.scaled;
> + }
> +}
> +
> +static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> + struct skl_pipe_wm_parameters *p)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum pipe pipe = intel_crtc->pipe;
> + struct drm_plane *plane;
> + struct drm_framebuffer *fb;
> + int i = 1; /* Index for sprite planes start */
> +
> + p->active = intel_crtc->active;
> + if (p->active) {
> + p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> + p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
> +
> + fb = crtc->primary->state->fb;
> + /* For planar: Bpp is for uv plane, y_Bpp is for y plane */
> + if (fb) {
> + p->plane[0].enabled = true;
> + p->plane[0].bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
> + drm_format_plane_cpp(fb->pixel_format, 1) :
> + drm_format_plane_cpp(fb->pixel_format, 0);
> + p->plane[0].y_bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
> + drm_format_plane_cpp(fb->pixel_format, 0) : 0;
> + p->plane[0].tiling = fb->modifier[0];
> + } else {
> + p->plane[0].enabled = false;
> + p->plane[0].bytes_per_pixel = 0;
> + p->plane[0].y_bytes_per_pixel = 0;
> + p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
> + }
> + p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
> + p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
> + p->plane[0].rotation = crtc->primary->state->rotation;
> +
> + fb = crtc->cursor->state->fb;
> + p->plane[PLANE_CURSOR].y_bytes_per_pixel = 0;
> + if (fb) {
> + p->plane[PLANE_CURSOR].enabled = true;
> + p->plane[PLANE_CURSOR].bytes_per_pixel = fb->bits_per_pixel / 8;
> + p->plane[PLANE_CURSOR].horiz_pixels = crtc->cursor->state->crtc_w;
> + p->plane[PLANE_CURSOR].vert_pixels = crtc->cursor->state->crtc_h;
> + } else {
> + p->plane[PLANE_CURSOR].enabled = false;
> + p->plane[PLANE_CURSOR].bytes_per_pixel = 0;
> + p->plane[PLANE_CURSOR].horiz_pixels = 64;
> + p->plane[PLANE_CURSOR].vert_pixels = 64;
> + }
> + }
> +
> + list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> +
> + if (intel_plane->pipe == pipe &&
> + plane->type == DRM_PLANE_TYPE_OVERLAY)
> + p->plane[i++] = intel_plane->wm;
> + }
> }
>
> static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> - struct intel_crtc_state *cstate,
> - struct intel_plane *intel_plane,
> + struct skl_pipe_wm_parameters *p,
> + struct intel_plane_wm_parameters *p_params,
> uint16_t ddb_allocation,
> int level,
> uint16_t *out_blocks, /* out */
> uint8_t *out_lines /* out */)
> {
> - struct drm_plane *plane = &intel_plane->base;
> - struct drm_framebuffer *fb = plane->state->fb;
> uint32_t latency = dev_priv->wm.skl_latency[level];
> uint32_t method1, method2;
> uint32_t plane_bytes_per_line, plane_blocks_per_line;
> @@ -3170,35 +3198,31 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> uint32_t selected_result;
> uint8_t bytes_per_pixel;
>
> - if (latency == 0 || !cstate->base.active || !fb)
> + if (latency == 0 || !p->active || !p_params->enabled)
> return false;
>
> - bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
> - drm_format_plane_cpp(DRM_FORMAT_NV12, 0) :
> - drm_format_plane_cpp(DRM_FORMAT_NV12, 1);
> - method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
> + bytes_per_pixel = p_params->y_bytes_per_pixel ?
> + p_params->y_bytes_per_pixel :
> + p_params->bytes_per_pixel;
> + method1 = skl_wm_method1(p->pixel_rate,
> bytes_per_pixel,
> latency);
> - method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
> - cstate->base.adjusted_mode.crtc_htotal,
> - cstate->pipe_src_w,
> + method2 = skl_wm_method2(p->pixel_rate,
> + p->pipe_htotal,
> + p_params->horiz_pixels,
> bytes_per_pixel,
> - fb->modifier[0],
> + p_params->tiling,
> latency);
>
> - plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
> + plane_bytes_per_line = p_params->horiz_pixels * bytes_per_pixel;
> plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
>
> - if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> - fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> + if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
> + p_params->tiling == I915_FORMAT_MOD_Yf_TILED) {
> uint32_t min_scanlines = 4;
> uint32_t y_tile_minimum;
> - if (intel_rotation_90_or_270(plane->state->rotation)) {
> - int bpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
> - drm_format_plane_cpp(fb->pixel_format, 1) :
> - drm_format_plane_cpp(fb->pixel_format, 0);
> -
> - switch (bpp) {
> + if (intel_rotation_90_or_270(p_params->rotation)) {
> + switch (p_params->bytes_per_pixel) {
> case 1:
> min_scanlines = 16;
> break;
> @@ -3222,8 +3246,8 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> res_lines = DIV_ROUND_UP(selected_result, plane_blocks_per_line);
>
> if (level >= 1 && level <= 7) {
> - if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> - fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)
> + if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
> + p_params->tiling == I915_FORMAT_MOD_Yf_TILED)
> res_lines += 4;
> else
> res_blocks++;
> @@ -3240,80 +3264,84 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>
> static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> struct skl_ddb_allocation *ddb,
> - struct intel_crtc_state *cstate,
> + struct skl_pipe_wm_parameters *p,
> + enum pipe pipe,
> int level,
> + int num_planes,
> struct skl_wm_level *result)
> {
> - struct drm_device *dev = dev_priv->dev;
> - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> - struct intel_plane *intel_plane;
> uint16_t ddb_blocks;
> - enum pipe pipe = intel_crtc->pipe;
> -
> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - int i = skl_wm_plane_id(intel_plane);
> + int i;
>
> + for (i = 0; i < num_planes; i++) {
> ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
>
> result->plane_en[i] = skl_compute_plane_wm(dev_priv,
> - cstate,
> - intel_plane,
> + p, &p->plane[i],
> ddb_blocks,
> level,
> &result->plane_res_b[i],
> &result->plane_res_l[i]);
> }
> +
> + ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][PLANE_CURSOR]);
> + result->plane_en[PLANE_CURSOR] = skl_compute_plane_wm(dev_priv, p,
> + &p->plane[PLANE_CURSOR],
> + ddb_blocks, level,
> + &result->plane_res_b[PLANE_CURSOR],
> + &result->plane_res_l[PLANE_CURSOR]);
> }
>
> static uint32_t
> -skl_compute_linetime_wm(struct intel_crtc_state *cstate)
> +skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
> {
> - if (!cstate->base.active)
> + if (!to_intel_crtc(crtc)->active)
> return 0;
>
> - if (WARN_ON(skl_pipe_pixel_rate(cstate) == 0))
> + if (WARN_ON(p->pixel_rate == 0))
> return 0;
>
> - return DIV_ROUND_UP(8 * cstate->base.adjusted_mode.crtc_htotal * 1000,
> - skl_pipe_pixel_rate(cstate));
> + return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> }
>
> -static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
> +static void skl_compute_transition_wm(struct drm_crtc *crtc,
> + struct skl_pipe_wm_parameters *params,
> struct skl_wm_level *trans_wm /* out */)
> {
> - struct drm_crtc *crtc = cstate->base.crtc;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_plane *intel_plane;
> + int i;
>
> - if (!cstate->base.active)
> + if (!params->active)
> return;
>
> /* Until we know more, just disable transition WMs */
> - for_each_intel_plane_on_crtc(crtc->dev, intel_crtc, intel_plane) {
> - int i = skl_wm_plane_id(intel_plane);
> -
> + for (i = 0; i < intel_num_planes(intel_crtc); i++)
> trans_wm->plane_en[i] = false;
> - }
> + trans_wm->plane_en[PLANE_CURSOR] = false;
> }
>
> -static void skl_compute_pipe_wm(struct intel_crtc_state *cstate,
> +static void skl_compute_pipe_wm(struct drm_crtc *crtc,
> struct skl_ddb_allocation *ddb,
> + struct skl_pipe_wm_parameters *params,
> struct skl_pipe_wm *pipe_wm)
> {
> - struct drm_device *dev = cstate->base.crtc->dev;
> + struct drm_device *dev = crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int level, max_level = ilk_wm_max_level(dev);
>
> for (level = 0; level <= max_level; level++) {
> - skl_compute_wm_level(dev_priv, ddb, cstate,
> - level, &pipe_wm->wm[level]);
> + skl_compute_wm_level(dev_priv, ddb, params, intel_crtc->pipe,
> + level, intel_num_planes(intel_crtc),
> + &pipe_wm->wm[level]);
> }
> - pipe_wm->linetime = skl_compute_linetime_wm(cstate);
> + pipe_wm->linetime = skl_compute_linetime_wm(crtc, params);
>
> - skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
> + skl_compute_transition_wm(crtc, params, &pipe_wm->trans_wm);
> }
>
> static void skl_compute_wm_results(struct drm_device *dev,
> + struct skl_pipe_wm_parameters *p,
> struct skl_pipe_wm *p_wm,
> struct skl_wm_values *r,
> struct intel_crtc *intel_crtc)
> @@ -3557,15 +3585,16 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
> }
>
> static bool skl_update_pipe_wm(struct drm_crtc *crtc,
> + struct skl_pipe_wm_parameters *params,
> struct intel_wm_config *config,
> struct skl_ddb_allocation *ddb, /* out */
> struct skl_pipe_wm *pipe_wm /* out */)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
>
> - skl_allocate_pipe_ddb(cstate, config, ddb);
> - skl_compute_pipe_wm(cstate, ddb, pipe_wm);
> + skl_compute_wm_pipe_parameters(crtc, params);
> + skl_allocate_pipe_ddb(crtc, config, params, ddb);
> + skl_compute_pipe_wm(crtc, ddb, params, pipe_wm);
>
> if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
> return false;
> @@ -3598,6 +3627,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
> */
> list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
> base.head) {
> + struct skl_pipe_wm_parameters params = {};
> struct skl_pipe_wm pipe_wm = {};
> bool wm_changed;
>
> @@ -3607,7 +3637,8 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
> if (!intel_crtc->active)
> continue;
>
> - wm_changed = skl_update_pipe_wm(&intel_crtc->base, config,
> + wm_changed = skl_update_pipe_wm(&intel_crtc->base,
> + ¶ms, config,
> &r->ddb, &pipe_wm);
>
> /*
> @@ -3617,7 +3648,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
> */
> WARN_ON(!wm_changed);
>
> - skl_compute_wm_results(dev, &pipe_wm, r, intel_crtc);
> + skl_compute_wm_results(dev, ¶ms, &pipe_wm, r, intel_crtc);
> r->dirty[intel_crtc->pipe] = true;
> }
> }
> @@ -3647,6 +3678,7 @@ static void skl_update_wm(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct skl_pipe_wm_parameters params = {};
> struct skl_wm_values *results = &dev_priv->wm.skl_results;
> struct skl_pipe_wm pipe_wm = {};
> struct intel_wm_config config = {};
> @@ -3659,10 +3691,11 @@ static void skl_update_wm(struct drm_crtc *crtc)
>
> skl_compute_wm_global_parameters(dev, &config);
>
> - if (!skl_update_pipe_wm(crtc, &config, &results->ddb, &pipe_wm))
> + if (!skl_update_pipe_wm(crtc, ¶ms, &config,
> + &results->ddb, &pipe_wm))
> return;
>
> - skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
> + skl_compute_wm_results(dev, ¶ms, &pipe_wm, results, intel_crtc);
> results->dirty[intel_crtc->pipe] = true;
>
> skl_update_other_pipe_wm(dev, crtc, &config, results);
> @@ -3673,6 +3706,39 @@ static void skl_update_wm(struct drm_crtc *crtc)
> dev_priv->wm.skl_hw = *results;
> }
>
> +static void
> +skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
> + uint32_t sprite_width, uint32_t sprite_height,
> + int pixel_size, bool enabled, bool scaled)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct drm_framebuffer *fb = plane->state->fb;
> +
> + intel_plane->wm.enabled = enabled;
> + intel_plane->wm.scaled = scaled;
> + intel_plane->wm.horiz_pixels = sprite_width;
> + intel_plane->wm.vert_pixels = sprite_height;
> + intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
> +
> + /* For planar: Bpp is for UV plane, y_Bpp is for Y plane */
> + intel_plane->wm.bytes_per_pixel =
> + (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
> + drm_format_plane_cpp(plane->state->fb->pixel_format, 1) : pixel_size;
> + intel_plane->wm.y_bytes_per_pixel =
> + (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
> + drm_format_plane_cpp(plane->state->fb->pixel_format, 0) : 0;
> +
> + /*
> + * Framebuffer can be NULL on plane disable, but it does not
> + * matter for watermarks if we assume no tiling in that case.
> + */
> + if (fb)
> + intel_plane->wm.tiling = fb->modifier[0];
> + intel_plane->wm.rotation = plane->state->rotation;
> +
> + skl_update_wm(crtc);
> +}
> +
> static void ilk_update_wm(struct drm_crtc *crtc)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -3688,18 +3754,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
>
> WARN_ON(cstate->base.active != intel_crtc->active);
>
> - /*
> - * 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(dev);
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> - }
> -
> intel_compute_pipe_wm(cstate, &pipe_wm);
>
> if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
> @@ -3731,6 +3785,28 @@ static void ilk_update_wm(struct drm_crtc *crtc)
> ilk_write_wm_values(dev_priv, &results);
> }
>
> +static void
> +ilk_update_sprite_wm(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + uint32_t sprite_width, uint32_t sprite_height,
> + int pixel_size, bool enabled, bool scaled)
> +{
> + struct drm_device *dev = plane->dev;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> +
> + /*
> + * 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);
> +
> + ilk_update_wm(crtc);
> +}
> +
> static void skl_pipe_wm_active_state(uint32_t val,
> struct skl_pipe_wm *active,
> bool is_transwm,
> @@ -4108,6 +4184,21 @@ void intel_update_watermarks(struct drm_crtc *crtc)
> dev_priv->display.update_wm(crtc);
> }
>
> +void intel_update_sprite_watermarks(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + uint32_t sprite_width,
> + uint32_t sprite_height,
> + int pixel_size,
> + bool enabled, bool scaled)
> +{
> + struct drm_i915_private *dev_priv = plane->dev->dev_private;
> +
> + if (dev_priv->display.update_sprite_wm)
> + dev_priv->display.update_sprite_wm(plane, crtc,
> + sprite_width, sprite_height,
> + pixel_size, enabled, scaled);
> +}
> +
> /**
> * Lock protecting IPS related data structures
> */
> @@ -7022,6 +7113,7 @@ void intel_init_pm(struct drm_device *dev)
> dev_priv->display.init_clock_gating =
> skl_init_clock_gating;
> dev_priv->display.update_wm = skl_update_wm;
> + dev_priv->display.update_sprite_wm = skl_update_sprite_wm;
> } else if (HAS_PCH_SPLIT(dev)) {
> ilk_setup_wm_latency(dev);
>
> @@ -7030,6 +7122,7 @@ void intel_init_pm(struct drm_device *dev)
> (!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.update_sprite_wm = ilk_update_sprite_wm;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane latency. "
> "Disable CxSR\n");
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index dd2d568..b229c67 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -192,6 +192,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
> u32 plane_ctl, stride_div, stride;
> + int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key =
> &to_intel_plane_state(drm_plane->state)->ckey;
> unsigned long surf_addr;
> @@ -210,6 +211,10 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> rotation = drm_plane->state->rotation;
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> + intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h,
> + pixel_size, true,
> + src_w != crtc_w || src_h != crtc_h);
> +
> stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> fb->pixel_format);
>
> @@ -291,6 +296,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>
> I915_WRITE(PLANE_SURF(pipe, plane), 0);
> POSTING_READ(PLANE_SURF(pipe, plane));
> +
> + intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
> }
>
> static void
> @@ -533,6 +540,10 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> sprctl |= SPRITE_PIPE_CSC_ENABLE;
>
> + intel_update_sprite_watermarks(plane, crtc, src_w, src_h, pixel_size,
> + true,
> + src_w != crtc_w || src_h != crtc_h);
> +
> /* Sizes are 0 based */
> src_w--;
> src_h--;
> @@ -666,6 +677,10 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> if (IS_GEN6(dev))
> dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
>
> + intel_update_sprite_watermarks(plane, crtc, src_w, src_h,
> + pixel_size, true,
> + src_w != crtc_w || src_h != crtc_h);
> +
> /* Sizes are 0 based */
> src_w--;
> src_h--;
> --
> 2.5.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-10-13 11:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08 22:28 [PATCH] drm/i915: Partial revert of atomic watermark series Matt Roper
2015-10-09 8:41 ` Daniel Vetter
2015-10-09 20:33 ` Zanoni, Paulo R
2015-10-09 21:22 ` [PATCH] drm/i915: revert a few more watermark commits Paulo Zanoni
2015-10-13 12:00 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox