Although we can do a good job of reading out hardware state, the
graphics firmware may have programmed the watermarks in a creative way
that doesn't match how i915 would have chosen to program them. We
shouldn't trust the firmware's watermark programming, but should rather
re-calculate how we think WM's should be programmed and then shove those
values into the hardware.
We can do this pretty easily by creating a dummy top-level state,
running it through the check process to calculate all the values, and
then just programming the watermarks for each CRTC.
v2: Move watermark sanitization after our BIOS fb reconstruction; the
watermark calculations that we do here need to look at pstate->fb,
which isn't setup yet in intel_modeset_setup_hw_state(), even
though we have an enabled & visible plane.
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
---
Jani, based on your logs it looks like the culprit is that we're calculating
watermarks at startup time after we've read out preliminary hardware state (so
we know the primary plane is enabled & visible), but before we reconstruct the
BIOS fb (so pstate->fb is NULL). I think moving the watermark sanitization
later in the process so that we'll have a proper fb setup should hopefully
solve the issue. Can you test this version when you get a chance? I'll also
send a rebased patch #4 since the code movement here means that the previous
version won't apply cleanly.
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_pm.c | 14 +++++----
3 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 20cd6d8..09807c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -629,6 +629,7 @@ struct drm_i915_display_funcs {
struct dpll *best_clock);
int (*compute_pipe_wm)(struct intel_crtc *crtc,
struct drm_atomic_state *state);
+ void (*program_watermarks)(struct intel_crtc_state *cstate);
void (*update_wm)(struct drm_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7b3cfb6..e289311 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14936,6 +14936,54 @@ void intel_modeset_init_hw(struct drm_device *dev)
intel_enable_gt_powersave(dev);
}
+/*
+ * Calculate what we think the watermarks should be for the state we've read
+ * out of the hardware and then immediately program those watermarks so that
+ * we ensure the hardware settings match our internal state.
+ */
+static void sanitize_watermarks(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_atomic_state *state;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *cstate;
+ int ret;
+ int i;
+
+ /* Only supported on platforms that use atomic watermark design */
+ if (!dev_priv->display.program_watermarks)
+ return;
+
+ /*
+ * Calculate what we think WM's should be by creating a dummy state and
+ * running it through the atomic check code.
+ */
+ state = drm_atomic_helper_duplicate_state(dev,
+ dev->mode_config.acquire_ctx);
+ if (WARN_ON(IS_ERR(state)))
+ return;
+
+ ret = intel_atomic_check(dev, state);
+ if (ret) {
+ /*
+ * Just give up and leave watermarks untouched if we get an
+ * error back from 'check'
+ */
+ DRM_DEBUG_KMS("Could not determine valid watermarks for inherited state\n");
+ return;
+ }
+
+ /* Write calculated watermark values back */
+ to_i915(dev)->wm.config = to_intel_atomic_state(state)->wm_config;
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ struct intel_crtc_state *cs = to_intel_crtc_state(cstate);
+
+ dev_priv->display.program_watermarks(cs);
+ }
+
+ drm_atomic_state_free(state);
+}
+
void intel_modeset_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -15059,6 +15107,13 @@ void intel_modeset_init(struct drm_device *dev)
*/
intel_find_initial_plane_obj(crtc, &plane_config);
}
+
+ /*
+ * Make sure hardware watermarks really match the state we read out.
+ * Note that we need to do this after reconstructing the BIOS fb's
+ * since the watermark calculation done here will use pstate->fb.
+ */
+ sanitize_watermarks(dev);
}
static void intel_enable_pipe_a(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 180348b..fbcb072 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3611,15 +3611,19 @@ 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_program_watermarks(struct intel_crtc_state *cstate)
{
- struct drm_device *dev = dev_priv->dev;
+ struct drm_crtc *crtc = cstate->base.crtc;
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct ilk_wm_maximums max;
struct intel_wm_config *config = &dev_priv->wm.config;
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
+ to_intel_crtc(crtc)->wm.active.ilk = cstate->wm.optimal.ilk;
+
ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max);
ilk_wm_merge(dev, config, &max, &lp_wm_1_2);
@@ -3644,7 +3648,6 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
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);
@@ -3662,9 +3665,7 @@ static void ilk_update_wm(struct drm_crtc *crtc)
intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
}
- intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
-
- ilk_program_watermarks(dev_priv);
+ ilk_program_watermarks(cstate);
}
static void skl_pipe_wm_active_state(uint32_t val,
@@ -6972,6 +6973,7 @@ void intel_init_pm(struct drm_device *dev)
dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
dev_priv->display.update_wm = ilk_update_wm;
dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
+ dev_priv->display.program_watermarks = ilk_program_watermarks;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
