[PATCH 0/6] drm/i915/skl: Finally fix watermarks

[PATCH 0/6] drm/i915/skl: Finally fix watermarks

[Lyude]

To Sebastian Reichel:
	If this e-mail has the bizarre email address formatting issue you
	noticed in the last patch series I sent please let me know. I haven't
	gotten a chance to properly look at the e-mail you forwarded to me to
	see what's causing the problem, but I double checked the Cc: line for
	this e-mail manually before sending it out so hopefully I should be
	good for now…

Anyway, onto the actual patch series:

Unfortunately as a few of you are aware, Skylake is still very prone to pipe
underruns. Most of this comes from not doing things atomically enough (e.g.
needing to ensure that we update watermarks with other plane attributes, not
forcefully flushing pipes until we need to, etc.). Now that I've finally got a
grasp on how double buffered registers, arming registers, etc. works on skl,
I've written up patches that fix all of the pipe underruns on Skylake I was
able to reproduce.

Of course, one of the prerequisites for this patch series to actually fix all
of the pipe underruns is the patch I previously submitted that added support
for Skylake's SAGV.

Originally this patch series left behind the issue of running into pipe
underruns when we disabled pipes, however I've now managed to fix that behavior
as well. As such I've renamed the patch series appropriately instead of just
incrementing the version.

Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>

Lyude (5):
  drm/i915/skl: Update plane watermarks atomically during plane updates
  drm/i915/skl: Actually reuse wm values when pipes don't change
  drm/i915/skl: Always wait for pipes to update after a flush
  drm/i915/skl: Only flush pipes when we change the ddb allocation
  drm/i915/skl: Fix extra whitespace in skl_flush_wm_values()

Matt Roper (1):
  drm/i915/gen9: Only copy WM results for changed pipes to skl_hw

 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/intel_display.c |   5 ++
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_pm.c      | 133 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_sprite.c  |   2 +
 5 files changed, 120 insertions(+), 23 deletions(-)

-- 
2.7.4

[PATCH 1/6] drm/i915/skl: Update plane watermarks atomically during plane updates

[Lyude]

Thanks to Ville for suggesting this as a potential solution to pipe
underruns on Skylake.

On Skylake all of the registers for configuring planes, including the
registers for configuring their watermarks, are double buffered. New
values written to them won't take effect until said registers are
"armed", which is done by writing to the PLANE_SURF (or in the case of
cursor planes, the CURBASE register) register.

With this in mind, up until now we've been updating watermarks on skl
like this:

  non-modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - intel_pre_plane_update:
        - intel_update_watermarks()
     - {vblank happens; new watermarks + old plane values => underrun }
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
        - end vblank evasion
  }

  or

  modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - crtc_enable:
        - intel_update_watermarks()
     - {vblank happens; new watermarks + old plane values => underrun }
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
        - end vblank evasion
  }

Now we update watermarks atomically like this:

  non-modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - intel_pre_plane_update:
        - intel_update_watermarks() (wm values aren't written yet)
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
        - write new wm values
        - end vblank evasion
  }

  modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - crtc_enable:
        - intel_update_watermarks() (actual wm values aren't written
          yet)
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
	- write new wm values
        - end vblank evasion
  }

Which is more of a step in the right direction to fixing all of the
underrun issues we're currently seeing with skl

Changes since original patch series:
- Remove mutex_lock/mutex_unlock since they don't do anything and we're
  not touching global state
- Move skl_write_cursor_wm/skl_write_plane_wm functions into intel_pm.c, make
  externally visible
- Add skl_write_plane_wm calls to skl_update_plane
- Fix conditional for for loop in skl_write_plane_wm (level < max_level
  should be level <= max_level)
- Make diagram in commit more accurate to what's actually happening
- Add Fixes:

Fixes: 2d41c0b59afc ("drm/i915/skl: SKL Watermark Computation")
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com> <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  5 ++++
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_pm.c      | 48 +++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_sprite.c  |  2 ++
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 78beb7e..c0d2074 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3031,6 +3031,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_x = x_offset;
 	intel_crtc->adjusted_y = y_offset;
 
+	skl_write_plane_wm(intel_crtc, 0);
+
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
 	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
@@ -10242,6 +10244,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	int pipe = intel_crtc->pipe;
 	uint32_t cntl = 0;
 
+	if (IS_SKYLAKE(dev_priv))
+		skl_write_cursor_wm(intel_crtc);
+
 	if (plane_state && plane_state->visible) {
 		cntl = MCURSOR_GAMMA_ENABLE;
 		switch (plane_state->base.crtc_w) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e74d851..f1f54d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1709,6 +1709,8 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
 void skl_wm_get_hw_state(struct drm_device *dev);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */);
+void skl_write_cursor_wm(struct intel_crtc *intel_crtc);
+void skl_write_plane_wm(struct intel_crtc *intel_crtc, int plane);
 uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
 bool ilk_disable_lp_wm(struct drm_device *dev);
 int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 64d628c..fa86bea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3680,6 +3680,39 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
 		I915_WRITE(reg, 0);
 }
 
+void skl_write_plane_wm(struct intel_crtc *intel_crtc,
+			int plane)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	int level, max_level = ilk_wm_max_level(dev);
+	enum pipe pipe = intel_crtc->pipe;
+
+	for (level = 0; level <= max_level; level++) {
+		I915_WRITE(PLANE_WM(pipe, plane, level),
+			   wm->plane[pipe][plane][level]);
+	}
+	I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]);
+}
+
+void skl_write_cursor_wm(struct intel_crtc *intel_crtc)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	int level, max_level = ilk_wm_max_level(dev);
+	enum pipe pipe = intel_crtc->pipe;
+
+	for (level = 0; level <= max_level; level++) {
+		I915_WRITE(CUR_WM(pipe, level),
+			   wm->plane[pipe][PLANE_CURSOR][level]);
+	}
+	I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]);
+}
+
 static void skl_write_wm_values(struct drm_i915_private *dev_priv,
 				const struct skl_wm_values *new)
 {
@@ -3687,7 +3720,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
 	struct intel_crtc *crtc;
 
 	for_each_intel_crtc(dev, crtc) {
-		int i, level, max_level = ilk_wm_max_level(dev);
+		int i;
 		enum pipe pipe = crtc->pipe;
 
 		if ((new->dirty_pipes & drm_crtc_mask(&crtc->base)) == 0)
@@ -3697,19 +3730,6 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
 
 		I915_WRITE(PIPE_WM_LINETIME(pipe), new->wm_linetime[pipe]);
 
-		for (level = 0; level <= max_level; level++) {
-			for (i = 0; i < intel_num_planes(crtc); i++)
-				I915_WRITE(PLANE_WM(pipe, i, level),
-					   new->plane[pipe][i][level]);
-			I915_WRITE(CUR_WM(pipe, level),
-				   new->plane[pipe][PLANE_CURSOR][level]);
-		}
-		for (i = 0; i < intel_num_planes(crtc); i++)
-			I915_WRITE(PLANE_WM_TRANS(pipe, i),
-				   new->plane_trans[pipe][i]);
-		I915_WRITE(CUR_WM_TRANS(pipe),
-			   new->plane_trans[pipe][PLANE_CURSOR]);
-
 		for (i = 0; i < intel_num_planes(crtc); i++) {
 			skl_ddb_entry_write(dev_priv,
 					    PLANE_BUF_CFG(pipe, i),
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0de935a..50026f1 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -238,6 +238,8 @@ skl_update_plane(struct drm_plane *drm_plane,
 	crtc_w--;
 	crtc_h--;
 
+	skl_write_plane_wm(to_intel_crtc(crtc_state->base.crtc), plane);
+
 	if (key->flags) {
 		I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
 		I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
-- 
2.7.4

[PATCH 2/6] drm/i915/gen9: Only copy WM results for changed pipes to skl_hw

[Lyude]

From: Matt Roper <matthew.d.roper@intel.com>

When we write watermark values to the hardware, those values are stored
in dev_priv->wm.skl_hw.  However with recent watermark changes, the
results structure we're copying from only contains valid watermark and
DDB values for the pipes that are actually changing; the values for
other pipes remain 0.  Thus a blind copy of the entire skl_wm_values
structure will clobber the values for unchanged pipes...we need to be
more selective and only copy over the values for the changing pipes.

This mistake was hidden until recently due to another bug that caused us
to erroneously re-calculate watermarks for all active pipes rather than
changing pipes.  Only when that bug was fixed was the impact of this bug
discovered (e.g., modesets failing with "Requested display configuration
exceeds system watermark limitations" messages and leaving watermarks
non-functional, even ones initiated by intel_fbdev_restore_mode).

Changes since v1:
 - Add a function for copying a pipe's wm values
   (skl_copy_wm_for_pipe()) so we can reuse this later

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: 734fa01f3a17 ("drm/i915/gen9: Calculate watermarks during atomic 'check' (v2)")
Fixes: 9b6130227495 ("drm/i915/gen9: Re-allocate DDB only for changed pipes")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fa86bea..b7d4af1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3966,6 +3966,24 @@ skl_compute_ddb(struct drm_atomic_state *state)
 	return 0;
 }
 
+static void
+skl_copy_wm_for_pipe(struct skl_wm_values *dst,
+		     struct skl_wm_values *src,
+		     enum pipe pipe)
+{
+	dst->wm_linetime[pipe] = src->wm_linetime[pipe];
+	memcpy(dst->plane[pipe], src->plane[pipe],
+	       sizeof(dst->plane[pipe]));
+	memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
+	       sizeof(dst->plane_trans[pipe]));
+
+	dst->ddb.pipe[pipe] = src->ddb.pipe[pipe];
+	memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
+	       sizeof(dst->ddb.y_plane[pipe]));
+	memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
+	       sizeof(dst->ddb.plane[pipe]));
+}
+
 static int
 skl_compute_wm(struct drm_atomic_state *state)
 {
@@ -4038,8 +4056,10 @@ static void skl_update_wm(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct skl_wm_values *results = &dev_priv->wm.skl_results;
+	struct skl_wm_values *hw_vals = &dev_priv->wm.skl_hw;
 	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
+	int pipe;
 
 	if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)
 		return;
@@ -4051,8 +4071,12 @@ static void skl_update_wm(struct drm_crtc *crtc)
 	skl_write_wm_values(dev_priv, results);
 	skl_flush_wm_values(dev_priv, results);
 
-	/* store the new configuration */
-	dev_priv->wm.skl_hw = *results;
+	/*
+	 * Store the new configuration (but only for the pipes that have
+	 * changed; the other values weren't recomputed).
+	 */
+	for_each_pipe_masked(dev_priv, pipe, results->dirty_pipes)
+		skl_copy_wm_for_pipe(hw_vals, results, pipe);
 
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
-- 
2.7.4

[PATCH 3/6] drm/i915/skl: Actually reuse wm values when pipes don't change

[Lyude]

Up until now we've actually been making the mistake of leaving the
watermark results for each pipe completely blank in skl_compute_wm()
when they haven't changed, fix this.

Fixes: 734fa01f3a17 ("drm/i915/gen9: Calculate watermarks during atomic 'check' (v2)")
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com> <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b7d4af1..788db86 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3987,10 +3987,13 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
 static int
 skl_compute_wm(struct drm_atomic_state *state)
 {
+	struct drm_device *dev = state->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *cstate;
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct skl_wm_values *results = &intel_state->wm_results;
+	struct skl_wm_values *hw_wm = &dev_priv->wm.skl_hw;
 	struct skl_pipe_wm *pipe_wm;
 	bool changed = false;
 	int ret, i;
@@ -4039,12 +4042,14 @@ skl_compute_wm(struct drm_atomic_state *state)
 		if (changed)
 			results->dirty_pipes |= drm_crtc_mask(crtc);
 
-		if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)
+		if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0) {
 			/* This pipe's WM's did not change */
+			skl_copy_wm_for_pipe(results, hw_wm, intel_crtc->pipe);
 			continue;
+		}
 
 		intel_cstate->update_wm_pre = true;
-		skl_compute_wm_results(crtc->dev, pipe_wm, results, intel_crtc);
+		skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
 	}
 
 	return 0;
-- 
2.7.4

[PATCH 4/6] drm/i915/skl: Always wait for pipes to update after a flush

[Lyude]

As we've learned, all watermark updates on Skylake have to be strictly
atomic or things fail. While the bspec doesn't mandate that we need to
wait for pipes to finish after the third iteration of flushes, not doing
so gives us the opportunity to break this atomicity later. This example
assumes that we're lucky enough not to be interrupted by the scheduler
at any point during this:

 - Start with pipe A and pipe B enabled
 - Enable pipe C
 - Flush pipe A in pass 1, wait until update finishes
 - Flush pipe B in pass 3, continue without waiting for next vblank
 - Start another wm update
 - We enter the next vblank for pipe B before we finish writing all the
   vm values
 - *Underrun*

As such, we always need to wait for each pipe we flush to update so as
to never break this atomicity.

Fixes: 0e8fb7ba7ca5 ("drm/i915/skl: Flush the WM configuration")
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com> <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 788db86..2e31df4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3859,8 +3859,11 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 	/*
 	 * Third pass: flush the pipes that got more space allocated.
 	 *
-	 * We don't need to actively wait for the update here, next vblank
-	 * will just get more DDB space with the correct WM values.
+	 * While the hardware doesn't require to wait for the next vblank here,
+	 * continuing before the pipe finishes updating could result in us
+	 * trying to update the wm values again before the pipe finishes
+	 * updating, which results in the hardware using intermediate wm values
+	 * and subsequently underrunning pipes.
 	 */
 	for_each_intel_crtc(dev, crtc) {
 		if (!crtc->active)
@@ -3876,6 +3879,16 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 			continue;
 
 		skl_wm_flush_pipe(dev_priv, pipe, 3);
+
+		/*
+		 * The only time we can get away with not waiting for an update
+		 * is when we just enabled the pipe, e.g. when it doesn't have
+		 * vblanks enabled anyway.
+		 */
+		if (drm_crtc_vblank_get(&crtc->base) == 0) {
+			intel_wait_for_vblank(dev, pipe);
+			drm_crtc_vblank_put(&crtc->base);
+		}
 	}
 }
 
-- 
2.7.4

[PATCH 5/6] drm/i915/skl: Only flush pipes when we change the ddb allocation

[Lyude]

Manual pipe flushes are only necessary in order to make sure that we prevent
pipes with changed ddb allocations from overlapping from one another at
any point in time. Additionally, forcing us to wait for the next vblank
every time we have to update the watermark values because the cursor was
moving between screens will introduce a rather noticable lag for users.

Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com> <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c97724d..9e1e045 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1597,6 +1597,7 @@ struct skl_ddb_allocation {
 
 struct skl_wm_values {
 	unsigned dirty_pipes;
+	bool ddb_changed;
 	struct skl_ddb_allocation ddb;
 	uint32_t wm_linetime[I915_MAX_PIPES];
 	uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2e31df4..4178bdd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3809,6 +3809,12 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 	new_ddb = &new_values->ddb;
 	cur_ddb = &dev_priv->wm.skl_hw.ddb;
 
+	/* We only ever need to flush when the ddb allocations change */
+	if (!new_values->ddb_changed)
+		return;
+
+	new_values->ddb_changed = false;
+
 	/*
 	 * First pass: flush the pipes with the new allocation contained into
 	 * the old space.
@@ -3926,6 +3932,22 @@ pipes_modified(struct drm_atomic_state *state)
 	return ret;
 }
 
+static bool
+skl_pipe_ddb_changed(struct skl_ddb_allocation *old,
+		     struct skl_ddb_allocation *new,
+		     enum pipe pipe)
+{
+	if (memcmp(&old->pipe[pipe], &new->pipe[pipe],
+		   sizeof(old->pipe[pipe])) != 0 ||
+	    memcmp(&old->plane[pipe], &new->plane[pipe],
+		   sizeof(old->plane[pipe])) != 0 ||
+	    memcmp(&old->y_plane[pipe], &new->y_plane[pipe],
+		   sizeof(old->y_plane[pipe])) != 0)
+		return true;
+
+	return false;
+}
+
 static int
 skl_compute_ddb(struct drm_atomic_state *state)
 {
@@ -3933,7 +3955,8 @@ skl_compute_ddb(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct intel_crtc *intel_crtc;
-	struct skl_ddb_allocation *ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb;
 	uint32_t realloc_pipes = pipes_modified(state);
 	int ret;
 
@@ -3971,9 +3994,13 @@ skl_compute_ddb(struct drm_atomic_state *state)
 		if (IS_ERR(cstate))
 			return PTR_ERR(cstate);
 
-		ret = skl_allocate_pipe_ddb(cstate, ddb);
+		ret = skl_allocate_pipe_ddb(cstate, new_ddb);
 		if (ret)
 			return ret;
+
+		if (!intel_state->wm_results.ddb_changed &&
+		    skl_pipe_ddb_changed(old_ddb, new_ddb, intel_crtc->pipe))
+			intel_state->wm_results.ddb_changed = true;
 	}
 
 	return 0;
-- 
2.7.4

[PATCH 6/6] drm/i915/skl: Fix extra whitespace in skl_flush_wm_values()

[Lyude]

Similar to how a vehicle will travel faster if you paint flames on it,
cleaning up this extra whitespace is guaranteed to provide additional
stability while updating watermark values.

Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4178bdd..a08a638 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3837,7 +3837,6 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 		reallocated[pipe] = true;
 	}
 
-
 	/*
 	 * Second pass: flush the pipes that are having their allocation
 	 * reduced, but overlapping with a previous allocation.
-- 
2.7.4