From 664f87c5bfcc7798bd5b16e14792f1e9ba2956ea Mon Sep 17 00:00:00 2001

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

Date: Thu, 12 May 2016 15:11:40 -0700

Subject: [PATCH 15/17] drm/i915/gen9: Calculate watermarks during atomic

 'check' (v2)

Moving watermark calculation into the check phase will allow us to to

reject display configurations for which there are no valid watermark

values before we start trying to program the hardware (although those

tests will come in a subsequent patch).

Another advantage of moving this calculation to the check phase is that

we can calculate the watermarks in a single shot as part of the atomic

transaction.  The watermark interfaces we inherited from our legacy

modesetting days are a bit broken in the atomic design because they use

per-crtc entry points but actually re-calculate and re-program something

that is really more of a global state.  That worked okay in the legacy

modesetting world because operations only ever updated a single CRTC at

a time.  However in the atomic world, a transaction can involve multiple

CRTC's, which means we wind up computing and programming the watermarks

NxN times (where N is the number of CRTC's involved).  With this patch

we eliminate the redundant re-calculation of watermark data for atomic

states (which was the cause of the WARN_ON(!wm_changed) problems that

have plagued us for a while).

We still need to work on the 'commit' side of watermark handling so that

we aren't doing redundant NxN programming of watermarks, but that's

content for future patches.

v2:

 - Bail out of skl_write_wm_values() if the CRTC isn't active.  Now that

   we set dirty_pipes to ~0 if the active pipes change (because

   we need to deal with DDB changes), we can now wind up here for

   disabled pipes, whereas we couldn't before.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89055

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92181

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Tested-by: Daniel Stone <daniels@collabora.com>

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Link: http://patchwork.freedesktop.org/patch/msgid/1463091100-13747-1-git-send-email-matthew.d.roper@intel.com

---

 drivers/gpu/drm/i915/intel_display.c |   2 +-

 drivers/gpu/drm/i915/intel_drv.h     |   2 +-

 drivers/gpu/drm/i915/intel_pm.c      | 140 +++++++++++++----------------------

 3 files changed, 54 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c

index 2190bac..a75daac 100644

--- a/drivers/gpu/drm/i915/intel_display.c

+++ b/drivers/gpu/drm/i915/intel_display.c

@@ -13627,7 +13627,7 @@ static int intel_atomic_commit(struct drm_device *dev,

 	drm_atomic_helper_swap_state(dev, state);

 	dev_priv->wm.config = intel_state->wm_config;

 	dev_priv->wm.distrust_bios_wm = false;

-	dev_priv->wm.skl_results.ddb = intel_state->ddb;

+	dev_priv->wm.skl_results = intel_state->wm_results;

 	intel_shared_dpll_commit(state);

 	if (intel_state->modeset) {

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h

index 2218290..ab0be7a 100644

--- a/drivers/gpu/drm/i915/intel_drv.h

+++ b/drivers/gpu/drm/i915/intel_drv.h

@@ -314,7 +314,7 @@ struct intel_atomic_state {

 	bool skip_intermediate_wm;

 	/* Gen9+ only */

-	struct skl_ddb_allocation ddb;

+	struct skl_wm_values wm_results;

 };

 struct intel_plane_state {

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c

index 342aa66..b072417 100644

--- a/drivers/gpu/drm/i915/intel_pm.c

+++ b/drivers/gpu/drm/i915/intel_pm.c

@@ -3221,23 +3221,6 @@ static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,

 	return ret;

 }

-static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,

-				       const struct intel_crtc *intel_crtc)

-{

-	struct drm_device *dev = intel_crtc->base.dev;

-	struct drm_i915_private *dev_priv = dev->dev_private;

-	const struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;

-

-	/*

-	 * If ddb allocation of pipes changed, it may require recalculation of

-	 * watermarks

-	 */

-	if (memcmp(new_ddb->pipe, cur_ddb->pipe, sizeof(new_ddb->pipe)))

-		return true;

-

-	return false;

-}

-

 static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,

 				struct intel_crtc_state *cstate,

 				struct intel_plane_state *intel_pstate,

@@ -3533,6 +3516,8 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,

 		if ((new->dirty_pipes & drm_crtc_mask(&crtc->base)) == 0)

 			continue;

+		if (!crtc->active)

+			continue;

 		I915_WRITE(PIPE_WM_LINETIME(pipe), new->wm_linetime[pipe]);

@@ -3716,66 +3701,9 @@ static int skl_update_pipe_wm(struct drm_crtc_state *cstate,

 	else

 		*changed = true;

-	intel_crtc->wm.active.skl = *pipe_wm;

-

 	return 0;

 }

-static void skl_update_other_pipe_wm(struct drm_device *dev,

-				     struct drm_crtc *crtc,

-				     struct skl_wm_values *r)

-{

-	struct intel_crtc *intel_crtc;

-	struct intel_crtc *this_crtc = to_intel_crtc(crtc);

-

-	/*

-	 * If the WM update hasn't changed the allocation for this_crtc (the

-	 * crtc we are currently computing the new WM values for), other

-	 * enabled crtcs will keep the same allocation and we don't need to

-	 * recompute anything for them.

-	 */

-	if (!skl_ddb_allocation_changed(&r->ddb, this_crtc))

-		return;

-

-	/*

-	 * Otherwise, because of this_crtc being freshly enabled/disabled, the

-	 * other active pipes need new DDB allocation and WM values.

-	 */

-	for_each_intel_crtc(dev, intel_crtc) {

-		struct skl_pipe_wm pipe_wm = {};

-		bool wm_changed;

-

-		if (this_crtc->pipe == intel_crtc->pipe)

-			continue;

-

-		if (!intel_crtc->active)

-			continue;

-

-		skl_update_pipe_wm(intel_crtc->base.state,

-				   &r->ddb, &pipe_wm, &wm_changed);

-

-		/*

-		 * If we end up re-computing the other pipe WM values, it's

-		 * because it was really needed, so we expect the WM values to

-		 * be different.

-		 */

-		WARN_ON(!wm_changed);

-

-		skl_compute_wm_results(dev, &pipe_wm, r, intel_crtc);

-		r->dirty_pipes |= drm_crtc_mask(&intel_crtc->base);

-	}

-}

-

-static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)

-{

-	watermarks->wm_linetime[pipe] = 0;

-	memset(watermarks->plane[pipe], 0,

-	       sizeof(uint32_t) * 8 * I915_MAX_PLANES);

-	memset(watermarks->plane_trans[pipe],

-	       0, sizeof(uint32_t) * I915_MAX_PLANES);

-	watermarks->plane_trans[pipe][PLANE_CURSOR] = 0;

-}

-

 static int

 skl_compute_ddb(struct drm_atomic_state *state)

 {

@@ -3783,6 +3711,7 @@ 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;

 	unsigned realloc_pipes = dev_priv->active_crtcs;

 	int ret;

@@ -3808,8 +3737,10 @@ skl_compute_ddb(struct drm_atomic_state *state)

 	 * any other display updates race with this transaction, so we need

 	 * to grab the lock on *all* CRTC's.

 	 */

-	if (intel_state->active_pipe_changes)

+	if (intel_state->active_pipe_changes) {

 		realloc_pipes = ~0;

+		intel_state->wm_results.dirty_pipes = ~0;

+	}

 	for_each_intel_crtc_mask(dev, intel_crtc, realloc_pipes) {

 		struct intel_crtc_state *cstate;

@@ -3818,7 +3749,7 @@ skl_compute_ddb(struct drm_atomic_state *state)

 		if (IS_ERR(cstate))

 			return PTR_ERR(cstate);

-		ret = skl_allocate_pipe_ddb(cstate, &intel_state->ddb);

+		ret = skl_allocate_pipe_ddb(cstate, ddb);

 		if (ret)

 			return ret;

 	}

@@ -3831,8 +3762,11 @@ skl_compute_wm(struct drm_atomic_state *state)

 {

 	struct drm_crtc *crtc;

 	struct drm_crtc_state *cstate;

-	int ret, i;

+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);

+	struct skl_wm_values *results = &intel_state->wm_results;

+	struct skl_pipe_wm *pipe_wm;

 	bool changed = false;

+	int ret, i;

 	/*

 	 * If this transaction isn't actually touching any CRTC's, don't

@@ -3847,10 +3781,45 @@ skl_compute_wm(struct drm_atomic_state *state)

 	if (!changed)

 		return 0;

+	/* Clear all dirty flags */

+	results->dirty_pipes = 0;

+

 	ret = skl_compute_ddb(state);

 	if (ret)

 		return ret;

+	/*

+	 * Calculate WM's for all pipes that are part of this transaction.

+	 * Note that the DDB allocation above may have added more CRTC's that

+	 * weren't otherwise being modified (and set bits in dirty_pipes) if

+	 * pipe allocations had to change.

+	 *

+	 * FIXME:  Now that we're doing this in the atomic check phase, we

+	 * should allow skl_update_pipe_wm() to return failure in cases where

+	 * no suitable watermark values can be found.

+	 */

+	for_each_crtc_in_state(state, crtc, cstate, i) {

+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

+		struct intel_crtc_state *intel_cstate =

+			to_intel_crtc_state(cstate);

+

+		pipe_wm = &intel_cstate->wm.skl.optimal;

+		ret = skl_update_pipe_wm(cstate, &results->ddb, pipe_wm,

+					 &changed);

+		if (ret)

+			return ret;

+

+		if (changed)

+			results->dirty_pipes |= drm_crtc_mask(crtc);

+

+		if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)

+			/* This pipe's WM's did not change */

+			continue;

+

+		intel_cstate->update_wm_pre = true;

+		skl_compute_wm_results(crtc->dev, pipe_wm, results, intel_crtc);

+	}

+

 	return 0;

 }

@@ -3862,26 +3831,21 @@ static void skl_update_wm(struct drm_crtc *crtc)

 	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.skl.optimal;

-	bool wm_changed;

-

-	/* Clear all dirty flags */

-	results->dirty_pipes = 0;

-	skl_clear_wm(results, intel_crtc->pipe);

-

-	skl_update_pipe_wm(crtc->state, &results->ddb, pipe_wm, &wm_changed);

-	if (!wm_changed)

+	if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)

 		return;

-	skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);

-	results->dirty_pipes |= drm_crtc_mask(&intel_crtc->base);

+	intel_crtc->wm.active.skl = *pipe_wm;

+

+	mutex_lock(&dev_priv->wm.wm_mutex);

-	skl_update_other_pipe_wm(dev, crtc, results);

 	skl_write_wm_values(dev_priv, results);

 	skl_flush_wm_values(dev_priv, results);

 	/* store the new configuration */

 	dev_priv->wm.skl_hw = *results;

+

+	mutex_unlock(&dev_priv->wm.wm_mutex);

 }

 static void ilk_compute_wm_config(struct drm_device *dev,

-- 

2.7.4