All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area
@ 2019-10-11 20:09 Ville Syrjala
  2019-10-11 20:09 ` [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes() Ville Syrjala
                   ` (10 more replies)
  0 siblings, 11 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

Made the mistake of looking at some skl+ wm/ddb stuff,
and then proceeded to throw out a bunch of useless things.

I stopped when I saw struct skl_ddb_allocation and decided
that maybe that starts to cut a bit too close to the dbuf
slice stuff.

Ville Syrjälä (8):
  drm/i915: Nuke the useless changed param from
    skl_ddb_add_affected_pipes()
  drm/i915: Nuke 'realloc_pipes'
  drm/i915: Make dirty_pipes refer to pipes
  drm/i915: Don't set undefined bits in dirty_pipes
  drm/i915: Streamline skl_commit_modeset_enables()
  drm/i915: Polish WM_LINETIME register stuff
  drm/i915: Move linetime wms into the crtc state
  drm/i915: Nuke skl wm.dirty_pipes bitmask

 drivers/gpu/drm/i915/display/intel_display.c  | 138 ++++++++---
 .../drm/i915/display/intel_display_types.h    |   6 +-
 drivers/gpu/drm/i915/i915_drv.h               |   2 -
 drivers/gpu/drm/i915/i915_reg.h               |  14 +-
 drivers/gpu/drm/i915/intel_pm.c               | 228 +++---------------
 5 files changed, 150 insertions(+), 238 deletions(-)

-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes()
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-10-16 14:25   ` Lisovskiy, Stanislav
  2019-10-11 20:09 ` [PATCH 2/8] drm/i915: Nuke 'realloc_pipes' Ville Syrjala
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

changed==true just means we have some crtcs in the state. All the
stuff following this only operates on crtcs in the state anyway so
there is no point in having this bool.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b306e2338f5a..49568270a89d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5424,35 +5424,14 @@ skl_print_wm_changes(struct intel_atomic_state *state)
 }
 
 static int
-skl_ddb_add_affected_pipes(struct intel_atomic_state *state, bool *changed)
+skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 {
 	struct drm_device *dev = state->base.dev;
 	const struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
 	u32 realloc_pipes = pipes_modified(state);
-	int ret, i;
-
-	/*
-	 * When we distrust bios wm we always need to recompute to set the
-	 * expected DDB allocations for each CRTC.
-	 */
-	if (dev_priv->wm.distrust_bios_wm)
-		(*changed) = true;
-
-	/*
-	 * If this transaction isn't actually touching any CRTC's, don't
-	 * bother with watermark calculation.  Note that if we pass this
-	 * test, we're guaranteed to hold at least one CRTC state mutex,
-	 * which means we can safely use values like dev_priv->active_pipes
-	 * since any racing commits that want to update them would need to
-	 * hold _all_ CRTC state mutexes.
-	 */
-	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
-		(*changed) = true;
-
-	if (!*changed)
-		return 0;
+	int ret;
 
 	/*
 	 * If this is our first atomic update following hardware readout,
@@ -5576,14 +5555,13 @@ skl_compute_wm(struct intel_atomic_state *state)
 	struct intel_crtc_state *new_crtc_state;
 	struct intel_crtc_state *old_crtc_state;
 	struct skl_ddb_values *results = &state->wm_results;
-	bool changed = false;
 	int ret, i;
 
 	/* Clear all dirty flags */
 	results->dirty_pipes = 0;
 
-	ret = skl_ddb_add_affected_pipes(state, &changed);
-	if (ret || !changed)
+	ret = skl_ddb_add_affected_pipes(state);
+	if (ret)
 		return ret;
 
 	/*
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/8] drm/i915: Nuke 'realloc_pipes'
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
  2019-10-11 20:09 ` [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes() Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-10-16 14:27   ` Lisovskiy, Stanislav
  2019-10-11 20:09 ` [PATCH 3/8] drm/i915: Make dirty_pipes refer to pipes Ville Syrjala
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

The 'realloc_pipes' bitmask is pointless. It is either:
a) the set of pipes which are already part of the state,
   in which case adding them again is entirely redundant
b) the set of all pipes which we then add to the state

Also the fact that 'realloc_pipes' uses the crtc indexes is
going to bite is at some point so best get rid of it quick.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 49568270a89d..3536c2e975e7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5235,19 +5235,6 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
 	return false;
 }
 
-static u32
-pipes_modified(struct intel_atomic_state *state)
-{
-	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state;
-	u32 i, ret = 0;
-
-	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
-		ret |= drm_crtc_mask(&crtc->base);
-
-	return ret;
-}
-
 static int
 skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state,
 			    struct intel_crtc_state *new_crtc_state)
@@ -5423,14 +5410,26 @@ skl_print_wm_changes(struct intel_atomic_state *state)
 	}
 }
 
+static int intel_add_all_pipes(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc *crtc;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_crtc_state *crtc_state;
+
+		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	return 0;
+}
+
 static int
 skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 {
-	struct drm_device *dev = state->base.dev;
-	const struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state;
-	u32 realloc_pipes = pipes_modified(state);
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	int ret;
 
 	/*
@@ -5440,7 +5439,7 @@ skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 	 * ensure a full DDB recompute.
 	 */
 	if (dev_priv->wm.distrust_bios_wm) {
-		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+		ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
 				       state->base.acquire_ctx);
 		if (ret)
 			return ret;
@@ -5471,18 +5470,11 @@ skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 	 * to grab the lock on *all* CRTC's.
 	 */
 	if (state->active_pipe_changes || state->modeset) {
-		realloc_pipes = ~0;
 		state->wm_results.dirty_pipes = ~0;
-	}
 
-	/*
-	 * We're not recomputing for the pipes not included in the commit, so
-	 * make sure we start with the current state.
-	 */
-	for_each_intel_crtc_mask(dev, crtc, realloc_pipes) {
-		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
-		if (IS_ERR(crtc_state))
-			return PTR_ERR(crtc_state);
+		ret = intel_add_all_pipes(state);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/8] drm/i915: Make dirty_pipes refer to pipes
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
  2019-10-11 20:09 ` [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes() Ville Syrjala
  2019-10-11 20:09 ` [PATCH 2/8] drm/i915: Nuke 'realloc_pipes' Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-10-16 14:28   ` Lisovskiy, Stanislav
  2019-10-11 20:09 ` [PATCH 4/8] drm/i915: Don't set undefined bits in dirty_pipes Ville Syrjala
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

Despite the its name dirty_pipes refers to crtc indexes. Let's
change its behaviout to match the name.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  9 +++------
 drivers/gpu/drm/i915/intel_pm.c              | 13 ++++++-------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a146ec02a0c1..b9b51bd0c956 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13883,7 +13883,6 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
 	unsigned int updated = 0;
 	bool progress;
-	enum pipe pipe;
 	int i;
 	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
 	u8 required_slices = state->wm_results.ddb.enabled_slices;
@@ -13908,12 +13907,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 		progress = false;
 
 		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+			enum pipe pipe = crtc->pipe;
 			bool vbl_wait = false;
-			unsigned int cmask = drm_crtc_mask(&crtc->base);
-
-			pipe = crtc->pipe;
 
-			if (updated & cmask || !new_crtc_state->base.active)
+			if (updated & BIT(crtc->pipe) || !new_crtc_state->base.active)
 				continue;
 
 			if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
@@ -13921,7 +13918,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 							INTEL_NUM_PIPES(dev_priv), i))
 				continue;
 
-			updated |= cmask;
+			updated |= BIT(pipe);
 			entries[i] = new_crtc_state->wm.skl.ddb;
 
 			/*
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3536c2e975e7..2b71d52a4ede 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5575,7 +5575,7 @@ skl_compute_wm(struct intel_atomic_state *state)
 		if (!skl_pipe_wm_equals(crtc,
 					&old_crtc_state->wm.skl.optimal,
 					&new_crtc_state->wm.skl.optimal))
-			results->dirty_pipes |= drm_crtc_mask(&crtc->base);
+			results->dirty_pipes |= BIT(crtc->pipe);
 	}
 
 	ret = skl_compute_ddb(state);
@@ -5595,7 +5595,7 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 	struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
 	enum pipe pipe = crtc->pipe;
 
-	if (!(state->wm_results.dirty_pipes & drm_crtc_mask(&crtc->base)))
+	if ((state->wm_results.dirty_pipes & BIT(crtc->pipe)) == 0)
 		return;
 
 	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
@@ -5604,12 +5604,11 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 static void skl_initial_wm(struct intel_atomic_state *state,
 			   struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct skl_ddb_values *results = &state->wm_results;
 
-	if ((results->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) == 0)
+	if ((results->dirty_pipes & BIT(crtc->pipe)) == 0)
 		return;
 
 	mutex_lock(&dev_priv->wm.wm_mutex);
@@ -5758,7 +5757,7 @@ void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
 		skl_pipe_wm_get_hw_state(crtc, &crtc_state->wm.skl.optimal);
 
 		if (crtc->active)
-			hw->dirty_pipes |= drm_crtc_mask(&crtc->base);
+			hw->dirty_pipes |= BIT(crtc->pipe);
 	}
 
 	if (dev_priv->active_pipes) {
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/8] drm/i915: Don't set undefined bits in dirty_pipes
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-10-11 20:09 ` [PATCH 3/8] drm/i915: Make dirty_pipes refer to pipes Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-11-29  9:24     ` [Intel-gfx] " Lisovskiy, Stanislav
  2019-10-11 20:09 ` [PATCH 5/8] drm/i915: Streamline skl_commit_modeset_enables() Ville Syrjala
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

skl_commit_modeset_enables() straight up compares dirty_pipes
with a bitmask of already committed pipes. If we set bits in
dirty_pipes for non-existent pipes that comparison will never
work right. So let's limit ourselves to bits that exist.

And we'll do the same for the active_pipes_changed bitmask.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2b71d52a4ede..f21eb9250d23 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5444,7 +5444,7 @@ skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 		if (ret)
 			return ret;
 
-		state->active_pipe_changes = ~0;
+		state->active_pipe_changes = INTEL_INFO(dev_priv)->pipe_mask;
 
 		/*
 		 * We usually only initialize state->active_pipes if we
@@ -5470,7 +5470,7 @@ skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 	 * to grab the lock on *all* CRTC's.
 	 */
 	if (state->active_pipe_changes || state->modeset) {
-		state->wm_results.dirty_pipes = ~0;
+		state->wm_results.dirty_pipes = INTEL_INFO(dev_priv)->pipe_mask;
 
 		ret = intel_add_all_pipes(state);
 		if (ret)
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/8] drm/i915: Streamline skl_commit_modeset_enables()
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (3 preceding siblings ...)
  2019-10-11 20:09 ` [PATCH 4/8] drm/i915: Don't set undefined bits in dirty_pipes Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-12-03 13:47   ` [Intel-gfx] " Lisovskiy, Stanislav
  2019-10-11 20:09 ` [PATCH 6/8] drm/i915: Polish WM_LINETIME register stuff Ville Syrjala
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

skl_commit_modeset_enables() is a bit of mess. Let's streamline
it by simply tracking which pipes still need to be updated.
As a bonus we get rid of the state->wm_results.dirty_pipes usage.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b9b51bd0c956..9eab67bbf61d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13881,17 +13881,19 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
-	unsigned int updated = 0;
-	bool progress;
-	int i;
 	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
 	u8 required_slices = state->wm_results.ddb.enabled_slices;
 	struct skl_ddb_entry entries[I915_MAX_PIPES] = {};
+	u8 dirty_pipes = 0;
+	int i;
 
-	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i)
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		/* ignore allocations for crtc's that have been turned off. */
-		if (new_crtc_state->base.active)
+		if (new_crtc_state->base.active) {
 			entries[i] = old_crtc_state->wm.skl.ddb;
+			dirty_pipes |= BIT(crtc->pipe);
+		}
+	}
 
 	/* If 2nd DBuf slice required, enable it here */
 	if (INTEL_GEN(dev_priv) >= 11 && required_slices > hw_enabled_slices)
@@ -13903,14 +13905,12 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 	 * never overlap with eachother inbetween CRTC updates. Otherwise we'll
 	 * cause pipe underruns and other bad stuff.
 	 */
-	do {
-		progress = false;
-
-		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+	while (dirty_pipes) {
+		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+						    new_crtc_state, i) {
 			enum pipe pipe = crtc->pipe;
-			bool vbl_wait = false;
 
-			if (updated & BIT(crtc->pipe) || !new_crtc_state->base.active)
+			if ((dirty_pipes & BIT(pipe)) == 0)
 				continue;
 
 			if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
@@ -13918,8 +13918,11 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 							INTEL_NUM_PIPES(dev_priv), i))
 				continue;
 
-			updated |= BIT(pipe);
 			entries[i] = new_crtc_state->wm.skl.ddb;
+			dirty_pipes &= ~BIT(pipe);
+
+			intel_update_crtc(crtc, state, old_crtc_state,
+					  new_crtc_state);
 
 			/*
 			 * If this is an already active pipe, it's DDB changed,
@@ -13930,18 +13933,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 			if (!skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
 						 &old_crtc_state->wm.skl.ddb) &&
 			    !new_crtc_state->base.active_changed &&
-			    state->wm_results.dirty_pipes != updated)
-				vbl_wait = true;
-
-			intel_update_crtc(crtc, state, old_crtc_state,
-					  new_crtc_state);
-
-			if (vbl_wait)
+			    dirty_pipes)
 				intel_wait_for_vblank(dev_priv, pipe);
-
-			progress = true;
 		}
-	} while (progress);
+	}
 
 	/* If 2nd DBuf slice is no more required disable it */
 	if (INTEL_GEN(dev_priv) >= 11 && required_slices < hw_enabled_slices)
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/8] drm/i915: Polish WM_LINETIME register stuff
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (4 preceding siblings ...)
  2019-10-11 20:09 ` [PATCH 5/8] drm/i915: Streamline skl_commit_modeset_enables() Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-10-14 14:56   ` [PATCH v2 " Ville Syrjala
  2019-10-11 20:09 ` [PATCH 7/8] drm/i915: Move linetime wms into the crtc state Ville Syrjala
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

Let's store the normal and IPS linetime watermarks individually,
and while at it we'll pimp the register definitions as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  3 +-
 drivers/gpu/drm/i915/i915_reg.h               | 14 ++--
 drivers/gpu/drm/i915/intel_pm.c               | 72 ++++++++++---------
 3 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..841952332c7e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -631,7 +631,8 @@ struct intel_crtc_scaler_state {
 
 struct intel_pipe_wm {
 	struct intel_wm_level wm[5];
-	u32 linetime;
+	u16 linetime;
+	u16 ips_linetime;
 	bool fbc_wm_enabled;
 	bool pipe_enabled;
 	bool sprites_enabled;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0fb9030b89f1..e1db85768084 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10394,13 +10394,13 @@ enum skl_power_gate {
 #define  D_COMP_COMP_DISABLE		(1 << 0)
 
 /* Pipe WM_LINETIME - watermark line time */
-#define _PIPE_WM_LINETIME_A		0x45270
-#define _PIPE_WM_LINETIME_B		0x45274
-#define PIPE_WM_LINETIME(pipe) _MMIO_PIPE(pipe, _PIPE_WM_LINETIME_A, _PIPE_WM_LINETIME_B)
-#define   PIPE_WM_LINETIME_MASK			(0x1ff)
-#define   PIPE_WM_LINETIME_TIME(x)		((x))
-#define   PIPE_WM_LINETIME_IPS_LINETIME_MASK	(0x1ff << 16)
-#define   PIPE_WM_LINETIME_IPS_LINETIME(x)	((x) << 16)
+#define _WM_LINETIME_A		0x45270
+#define _WM_LINETIME_B		0x45274
+#define WM_LINETIME(pipe) _MMIO_PIPE(pipe, _WM_LINETIME_A, _WM_LINETIME_B)
+#define  HSW_LINETIME_MASK	REG_GENMASK(8, 0)
+#define  HSW_LINETIME(x)	REG_FIELD_PREP(HSW_LINETIME_MASK, (x))
+#define  HSW_IPS_LINETIME_MASK	REG_GENMASK(24, 16)
+#define  HSW_IPS_LINETIME(x)	REG_FIELD_PREP(HSW_IPS_LINETIME_MASK, (x))
 
 /* SFUSE_STRAP */
 #define SFUSE_STRAP			_MMIO(0xc2014)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f21eb9250d23..f6893b771078 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2764,31 +2764,31 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
 }
 
 static u32
-hsw_compute_linetime_wm(const struct intel_crtc_state *crtc_state)
+hsw_linetime_wm(const struct intel_crtc_state *crtc_state)
 {
-	const struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(crtc_state->base.state);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->base.adjusted_mode;
-	u32 linetime, ips_linetime;
 
 	if (!crtc_state->base.active)
 		return 0;
-	if (WARN_ON(adjusted_mode->crtc_clock == 0))
-		return 0;
-	if (WARN_ON(intel_state->cdclk.logical.cdclk == 0))
-		return 0;
 
-	/* The WM are computed with base on how long it takes to fill a single
-	 * row at the given clock rate, multiplied by 8.
-	 * */
-	linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-				     adjusted_mode->crtc_clock);
-	ips_linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-					 intel_state->cdclk.logical.cdclk);
+	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
+				 adjusted_mode->crtc_clock);
+}
+
+static u32
+hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state)
+{
+	const struct intel_atomic_state *state =
+		to_intel_atomic_state(crtc_state->base.state);
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
 
-	return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
-	       PIPE_WM_LINETIME_TIME(linetime);
+	if (!crtc_state->base.active)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
+				 state->cdclk.logical.cdclk);
 }
 
 static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
@@ -3128,8 +3128,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	ilk_compute_wm_level(dev_priv, intel_crtc, 0, crtc_state,
 			     pristate, sprstate, curstate, &pipe_wm->wm[0]);
 
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		pipe_wm->linetime = hsw_compute_linetime_wm(crtc_state);
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		pipe_wm->linetime = hsw_linetime_wm(crtc_state);
+		pipe_wm->ips_linetime = hsw_ips_linetime_wm(crtc_state);
+	}
 
 	if (!ilk_validate_pipe_wm(dev_priv, pipe_wm))
 		return -EINVAL;
@@ -3329,7 +3331,7 @@ static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 				   enum intel_ddb_partitioning partitioning,
 				   struct ilk_wm_values *results)
 {
-	struct intel_crtc *intel_crtc;
+	struct intel_crtc *crtc;
 	int level, wm_lp;
 
 	results->enable_fbc_wm = merged->fbc_wm_enabled;
@@ -3374,15 +3376,17 @@ static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 	}
 
 	/* LP0 register values */
-	for_each_intel_crtc(&dev_priv->drm, intel_crtc) {
-		enum pipe pipe = intel_crtc->pipe;
-		const struct intel_wm_level *r =
-			&intel_crtc->wm.active.ilk.wm[0];
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		enum pipe pipe = crtc->pipe;
+		const struct intel_pipe_wm *pipe_wm = &crtc->wm.active.ilk;
+		const struct intel_wm_level *r = &pipe_wm->wm[0];
 
 		if (WARN_ON(!r->enable))
 			continue;
 
-		results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime;
+		results->wm_linetime[pipe] =
+			HSW_LINETIME(pipe_wm->linetime) |
+			HSW_IPS_LINETIME(pipe_wm->ips_linetime);
 
 		results->wm_pipe[pipe] =
 			(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -3535,11 +3539,11 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
-		I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results->wm_linetime[0]);
+		I915_WRITE(WM_LINETIME(PIPE_A), results->wm_linetime[0]);
 	if (dirty & WM_DIRTY_LINETIME(PIPE_B))
-		I915_WRITE(PIPE_WM_LINETIME(PIPE_B), results->wm_linetime[1]);
+		I915_WRITE(WM_LINETIME(PIPE_B), results->wm_linetime[1]);
 	if (dirty & WM_DIRTY_LINETIME(PIPE_C))
-		I915_WRITE(PIPE_WM_LINETIME(PIPE_C), results->wm_linetime[2]);
+		I915_WRITE(WM_LINETIME(PIPE_C), results->wm_linetime[2]);
 
 	if (dirty & WM_DIRTY_DDB) {
 		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -5598,7 +5602,7 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 	if ((state->wm_results.dirty_pipes & BIT(crtc->pipe)) == 0)
 		return;
 
-	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
+	I915_WRITE(WM_LINETIME(pipe), HSW_LINETIME(pipe_wm->linetime));
 }
 
 static void skl_initial_wm(struct intel_atomic_state *state,
@@ -5740,7 +5744,8 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 	if (!crtc->active)
 		return;
 
-	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
+	val = I915_READ(WM_LINETIME(pipe));
+	out->linetime = REG_FIELD_GET(HSW_LINETIME_MASK, val);
 }
 
 void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
@@ -5782,7 +5787,7 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 
 	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
+		hw->wm_linetime[pipe] = I915_READ(WM_LINETIME(pipe));
 
 	memset(active, 0, sizeof(*active));
 
@@ -5801,7 +5806,10 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 		active->wm[0].pri_val = (tmp & WM0_PIPE_PLANE_MASK) >> WM0_PIPE_PLANE_SHIFT;
 		active->wm[0].spr_val = (tmp & WM0_PIPE_SPRITE_MASK) >> WM0_PIPE_SPRITE_SHIFT;
 		active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
-		active->linetime = hw->wm_linetime[pipe];
+		active->linetime = REG_FIELD_GET(HSW_LINETIME_MASK,
+						 hw->wm_linetime[pipe]);
+		active->ips_linetime = REG_FIELD_GET(HSW_IPS_LINETIME_MASK,
+						     hw->wm_linetime[pipe]);
 	} else {
 		int level, max_level = ilk_wm_max_level(dev_priv);
 
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/8] drm/i915: Move linetime wms into the crtc state
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (5 preceding siblings ...)
  2019-10-11 20:09 ` [PATCH 6/8] drm/i915: Polish WM_LINETIME register stuff Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-10-11 20:09 ` [PATCH 8/8] drm/i915: Nuke skl wm.dirty_pipes bitmask Ville Syrjala
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

The linetime watermarks really have very little in common with the
plane watermarks. It looks to be cleaner to simply track them in
the crtc_state and program them from the normal modeset/fastset
paths.

The only dark cloud comes from the fact that the register is
still supposedly single buffered. So in theory it might still
need some form of two stage programming. Note that even though
HSW/BDWhave two stage programming we never computed any special
intermediate values for the linetime watermarks, and on SKL+
we don't even have the two stage stuff plugged in since everything
else is double buffered. So let's assume it's all fine and
continue doing what we've been doing.

Actually on HSW/BDW the value should not even change without
a full modeset since it doesn't account for pfit downscaling.
Thus only fastboot might be affected. But on SKL+ the pfit
scaling factor is take into consideration so the value may
change during any fastset.

As a bonus we'll plug this thing into the state
checker/dump now.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  94 +++++++++++++-
 .../drm/i915/display/intel_display_types.h    |   7 +-
 drivers/gpu/drm/i915/i915_drv.h               |   1 -
 drivers/gpu/drm/i915/intel_pm.c               | 116 +-----------------
 4 files changed, 97 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 9eab67bbf61d..3eb6f337dff0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6397,6 +6397,16 @@ static void icl_pipe_mbus_enable(struct intel_crtc *crtc)
 	I915_WRITE(PIPE_MBUS_DBOX_CTL(pipe), val);
 }
 
+static void hsw_set_linetime_wm(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	I915_WRITE(WM_LINETIME(crtc->pipe),
+		   HSW_LINETIME(crtc_state->linetime) |
+		   HSW_IPS_LINETIME(crtc_state->linetime));
+}
+
 static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 				struct intel_atomic_state *state)
 {
@@ -6465,6 +6475,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (INTEL_GEN(dev_priv) < 9)
 		intel_disable_primary_plane(pipe_config);
 
+	hsw_set_linetime_wm(pipe_config);
+
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_set_pipe_chicken(intel_crtc);
 
@@ -10401,6 +10413,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	enum intel_display_power_domain power_domain;
 	u64 power_domain_mask;
 	bool active;
+	u32 tmp;
 
 	intel_crtc_init_scalers(crtc, pipe_config);
 
@@ -10464,7 +10477,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->csc_mode = I915_READ(PIPE_CSC_MODE(crtc->pipe));
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		u32 tmp = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
+		tmp = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
 
 		if (tmp & SKL_BOTTOM_COLOR_GAMMA_ENABLE)
 			pipe_config->gamma_enable = true;
@@ -10477,6 +10490,12 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 
 	intel_color_get_config(pipe_config);
 
+	tmp = I915_READ(WM_LINETIME(crtc->pipe));
+	pipe_config->linetime = REG_FIELD_GET(HSW_LINETIME_MASK, tmp);
+	if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		pipe_config->ips_linetime =
+			REG_FIELD_GET(HSW_IPS_LINETIME_MASK, tmp);
+
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	WARN_ON(power_domain_mask & BIT_ULL(power_domain));
 
@@ -11784,6 +11803,53 @@ static bool c8_planes_changed(const struct intel_crtc_state *new_crtc_state)
 	return !old_crtc_state->c8_planes != !new_crtc_state->c8_planes;
 }
 
+static u16 hsw_linetime_wm(const struct intel_crtc_state *crtc_state)
+{
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
+
+	if (!crtc_state->base.enable)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
+				 adjusted_mode->crtc_clock);
+}
+
+static u16 hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state)
+{
+	const struct intel_atomic_state *state =
+		to_intel_atomic_state(crtc_state->base.state);
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
+
+	if (!crtc_state->base.enable)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
+				 state->cdclk.logical.cdclk);
+}
+
+static u16 skl_linetime_wm(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
+	u16 linetime_wm;
+
+	if (!crtc_state->base.enable)
+		return 0;
+
+	linetime_wm = DIV_ROUND_UP(adjusted_mode->crtc_htotal * 1000 * 8,
+				   crtc_state->pixel_rate);
+
+	/* Display WA #1135: BXT:ALL GLK:ALL */
+	if (IS_GEN9_LP(dev_priv) && dev_priv->ipc_enabled)
+		linetime_wm /= 2;
+
+	return linetime_wm;
+}
+
 static int intel_crtc_atomic_check(struct drm_crtc *_crtc,
 				   struct drm_crtc_state *_crtc_state)
 {
@@ -11861,6 +11927,13 @@ static int intel_crtc_atomic_check(struct drm_crtc *_crtc,
 	if (HAS_IPS(dev_priv))
 		crtc_state->ips_enabled = hsw_compute_ips_config(crtc_state);
 
+	if (INTEL_GEN(dev_priv) >= 9) {
+		crtc_state->linetime = skl_linetime_wm(crtc_state);
+	} else if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
+		crtc_state->linetime = hsw_linetime_wm(crtc_state);
+		crtc_state->ips_linetime = hsw_ips_linetime_wm(crtc_state);
+	}
+
 	return ret;
 }
 
@@ -12154,6 +12227,9 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
 		      pipe_config->pipe_src_w, pipe_config->pipe_src_h,
 		      pipe_config->pixel_rate);
 
+	DRM_DEBUG_KMS("linetime: %d, ips linetime: %d\n",
+		      pipe_config->linetime, pipe_config->ips_linetime);
+
 	if (INTEL_GEN(dev_priv) >= 9)
 		DRM_DEBUG_KMS("num_scalers: %d, scaler_users: 0x%x, scaler_id: %d\n",
 			      crtc->num_scalers,
@@ -12836,10 +12912,12 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		PIPE_CONF_CHECK_BOOL(gamma_enable);
 		PIPE_CONF_CHECK_BOOL(csc_enable);
 
+		PIPE_CONF_CHECK_I(linetime);
+		PIPE_CONF_CHECK_I(ips_linetime);
+
 		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
 		if (bp_gamma)
 			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode, base.gamma_lut, bp_gamma);
-
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -13711,6 +13789,18 @@ static void intel_pipe_fastset(const struct intel_crtc_state *old_crtc_state,
 			ironlake_pfit_disable(old_crtc_state);
 	}
 
+	/*
+	 * The register is supposedly single buffered so perhaps
+	 * not 100% correct to do this here. But SKL+ calculate
+	 * this based on the adjust pixel rate so pfit changes do
+	 * affect it and so it must be updated for fastsets.
+	 * HSW/BDW only really need this here for fastboot, after
+	 * that the value should not change without a full modeset.
+	 */
+	if (INTEL_GEN(dev_priv) >= 9 ||
+	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		hsw_set_linetime_wm(new_crtc_state);
+
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_set_pipe_chicken(crtc);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 841952332c7e..9b606e507778 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -631,8 +631,6 @@ struct intel_crtc_scaler_state {
 
 struct intel_pipe_wm {
 	struct intel_wm_level wm[5];
-	u16 linetime;
-	u16 ips_linetime;
 	bool fbc_wm_enabled;
 	bool pipe_enabled;
 	bool sprites_enabled;
@@ -648,7 +646,6 @@ struct skl_plane_wm {
 
 struct skl_pipe_wm {
 	struct skl_plane_wm planes[I915_MAX_PLANES];
-	u32 linetime;
 };
 
 enum vlv_wm_level {
@@ -990,6 +987,10 @@ struct intel_crtc_state {
 	} dsc_params;
 	struct drm_dsc_config dp_dsc_cfg;
 
+	/* HSW+ linetime watermarks */
+	u16 linetime;
+	u16 ips_linetime;
+
 	/* Forward Error correction State */
 	bool fec_enable;
 };
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c46b339064c0..eba9055c4405 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -842,7 +842,6 @@ struct ilk_wm_values {
 	u32 wm_pipe[3];
 	u32 wm_lp[3];
 	u32 wm_lp_spr[3];
-	u32 wm_linetime[3];
 	bool enable_fbc_wm;
 	enum intel_ddb_partitioning partitioning;
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f6893b771078..390d1f8d45ba 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2763,34 +2763,6 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
 	result->enable = true;
 }
 
-static u32
-hsw_linetime_wm(const struct intel_crtc_state *crtc_state)
-{
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->base.adjusted_mode;
-
-	if (!crtc_state->base.active)
-		return 0;
-
-	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-				 adjusted_mode->crtc_clock);
-}
-
-static u32
-hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state)
-{
-	const struct intel_atomic_state *state =
-		to_intel_atomic_state(crtc_state->base.state);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->base.adjusted_mode;
-
-	if (!crtc_state->base.active)
-		return 0;
-
-	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-				 state->cdclk.logical.cdclk);
-}
-
 static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 				  u16 wm[8])
 {
@@ -3128,11 +3100,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	ilk_compute_wm_level(dev_priv, intel_crtc, 0, crtc_state,
 			     pristate, sprstate, curstate, &pipe_wm->wm[0]);
 
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		pipe_wm->linetime = hsw_linetime_wm(crtc_state);
-		pipe_wm->ips_linetime = hsw_ips_linetime_wm(crtc_state);
-	}
-
 	if (!ilk_validate_pipe_wm(dev_priv, pipe_wm))
 		return -EINVAL;
 
@@ -3384,10 +3351,6 @@ static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 		if (WARN_ON(!r->enable))
 			continue;
 
-		results->wm_linetime[pipe] =
-			HSW_LINETIME(pipe_wm->linetime) |
-			HSW_IPS_LINETIME(pipe_wm->ips_linetime);
-
 		results->wm_pipe[pipe] =
 			(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
 			(r->spr_val << WM0_PIPE_SPRITE_SHIFT) |
@@ -3426,7 +3389,6 @@ ilk_find_best_result(struct drm_i915_private *dev_priv,
 
 /* dirty bits used to track which watermarks need changes */
 #define WM_DIRTY_PIPE(pipe) (1 << (pipe))
-#define WM_DIRTY_LINETIME(pipe) (1 << (8 + (pipe)))
 #define WM_DIRTY_LP(wm_lp) (1 << (15 + (wm_lp)))
 #define WM_DIRTY_LP_ALL (WM_DIRTY_LP(1) | WM_DIRTY_LP(2) | WM_DIRTY_LP(3))
 #define WM_DIRTY_FBC (1 << 24)
@@ -3441,12 +3403,6 @@ static unsigned int ilk_compute_wm_dirty(struct drm_i915_private *dev_priv,
 	int wm_lp;
 
 	for_each_pipe(dev_priv, pipe) {
-		if (old->wm_linetime[pipe] != new->wm_linetime[pipe]) {
-			dirty |= WM_DIRTY_LINETIME(pipe);
-			/* Must disable LP1+ watermarks too */
-			dirty |= WM_DIRTY_LP_ALL;
-		}
-
 		if (old->wm_pipe[pipe] != new->wm_pipe[pipe]) {
 			dirty |= WM_DIRTY_PIPE(pipe);
 			/* Must disable LP1+ watermarks too */
@@ -3538,13 +3494,6 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 	if (dirty & WM_DIRTY_PIPE(PIPE_C))
 		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
 
-	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
-		I915_WRITE(WM_LINETIME(PIPE_A), results->wm_linetime[0]);
-	if (dirty & WM_DIRTY_LINETIME(PIPE_B))
-		I915_WRITE(WM_LINETIME(PIPE_B), results->wm_linetime[1]);
-	if (dirty & WM_DIRTY_LINETIME(PIPE_C))
-		I915_WRITE(WM_LINETIME(PIPE_C), results->wm_linetime[2]);
-
 	if (dirty & WM_DIRTY_DDB) {
 		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 			val = I915_READ(WM_MISC);
@@ -4884,24 +4833,6 @@ skl_compute_wm_levels(const struct intel_crtc_state *crtc_state,
 	}
 }
 
-static u32
-skl_compute_linetime_wm(const struct intel_crtc_state *crtc_state)
-{
-	struct drm_atomic_state *state = crtc_state->base.state;
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	uint_fixed_16_16_t linetime_us;
-	u32 linetime_wm;
-
-	linetime_us = intel_get_linetime_us(crtc_state);
-	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
-
-	/* Display WA #1135: BXT:ALL GLK:ALL */
-	if (IS_GEN9_LP(dev_priv) && dev_priv->ipc_enabled)
-		linetime_wm /= 2;
-
-	return linetime_wm;
-}
-
 static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
 				      const struct skl_wm_params *wp,
 				      struct skl_plane_wm *wm)
@@ -5089,8 +5020,6 @@ static int skl_build_pipe_wm(struct intel_crtc_state *crtc_state)
 			return ret;
 	}
 
-	pipe_wm->linetime = skl_compute_linetime_wm(crtc_state);
-
 	return 0;
 }
 
@@ -5215,7 +5144,7 @@ static bool skl_pipe_wm_equals(struct intel_crtc *crtc,
 			return false;
 	}
 
-	return wm1->linetime == wm2->linetime;
+	return true;
 }
 
 static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
@@ -5591,38 +5520,6 @@ skl_compute_wm(struct intel_atomic_state *state)
 	return 0;
 }
 
-static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
-				      struct intel_crtc_state *crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
-	enum pipe pipe = crtc->pipe;
-
-	if ((state->wm_results.dirty_pipes & BIT(crtc->pipe)) == 0)
-		return;
-
-	I915_WRITE(WM_LINETIME(pipe), HSW_LINETIME(pipe_wm->linetime));
-}
-
-static void skl_initial_wm(struct intel_atomic_state *state,
-			   struct intel_crtc_state *crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct skl_ddb_values *results = &state->wm_results;
-
-	if ((results->dirty_pipes & BIT(crtc->pipe)) == 0)
-		return;
-
-	mutex_lock(&dev_priv->wm.wm_mutex);
-
-	if (crtc_state->base.active_changed)
-		skl_atomic_update_crtc_wm(state, crtc_state);
-
-	mutex_unlock(&dev_priv->wm.wm_mutex);
-}
-
 static void ilk_compute_wm_config(struct drm_i915_private *dev_priv,
 				  struct intel_wm_config *config)
 {
@@ -5743,9 +5640,6 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 
 	if (!crtc->active)
 		return;
-
-	val = I915_READ(WM_LINETIME(pipe));
-	out->linetime = REG_FIELD_GET(HSW_LINETIME_MASK, val);
 }
 
 void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
@@ -5786,8 +5680,6 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 	};
 
 	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hw->wm_linetime[pipe] = I915_READ(WM_LINETIME(pipe));
 
 	memset(active, 0, sizeof(*active));
 
@@ -5806,10 +5698,6 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 		active->wm[0].pri_val = (tmp & WM0_PIPE_PLANE_MASK) >> WM0_PIPE_PLANE_SHIFT;
 		active->wm[0].spr_val = (tmp & WM0_PIPE_SPRITE_MASK) >> WM0_PIPE_SPRITE_SHIFT;
 		active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
-		active->linetime = REG_FIELD_GET(HSW_LINETIME_MASK,
-						 hw->wm_linetime[pipe]);
-		active->ips_linetime = REG_FIELD_GET(HSW_IPS_LINETIME_MASK,
-						     hw->wm_linetime[pipe]);
 	} else {
 		int level, max_level = ilk_wm_max_level(dev_priv);
 
@@ -8993,8 +8881,6 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
 	/* For FIFO watermark updates */
 	if (INTEL_GEN(dev_priv) >= 9) {
 		skl_setup_wm_latency(dev_priv);
-		dev_priv->display.initial_watermarks = skl_initial_wm;
-		dev_priv->display.atomic_update_watermarks = skl_atomic_update_crtc_wm;
 		dev_priv->display.compute_global_watermarks = skl_compute_wm;
 	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		ilk_setup_wm_latency(dev_priv);
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 8/8] drm/i915: Nuke skl wm.dirty_pipes bitmask
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (6 preceding siblings ...)
  2019-10-11 20:09 ` [PATCH 7/8] drm/i915: Move linetime wms into the crtc state Ville Syrjala
@ 2019-10-11 20:09 ` Ville Syrjala
  2019-11-29  9:18     ` [Intel-gfx] " Lisovskiy, Stanislav
  2019-10-11 21:35 ` ✗ Fi.CI.BUILD: failure for drm/i915: Some cleanup near the SKL wm/ddb area Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-11 20:09 UTC (permalink / raw)
  To: intel-gfx

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

The dirty_pipes bitmask is now unused. Get rid of it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 -
 drivers/gpu/drm/i915/intel_pm.c | 35 +--------------------------------
 2 files changed, 1 insertion(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eba9055c4405..c16ba85e2ec8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -901,7 +901,6 @@ struct skl_ddb_allocation {
 };
 
 struct skl_ddb_values {
-	unsigned dirty_pipes;
 	struct skl_ddb_allocation ddb;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 390d1f8d45ba..ccbb732cff44 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5130,23 +5130,6 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
 	return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm);
 }
 
-static bool skl_pipe_wm_equals(struct intel_crtc *crtc,
-			       const struct skl_pipe_wm *wm1,
-			       const struct skl_pipe_wm *wm2)
-{
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum plane_id plane_id;
-
-	for_each_plane_id_on_crtc(crtc, plane_id) {
-		if (!skl_plane_wm_equals(dev_priv,
-					 &wm1->planes[plane_id],
-					 &wm2->planes[plane_id]))
-			return false;
-	}
-
-	return true;
-}
-
 static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
 					   const struct skl_ddb_entry *b)
 {
@@ -5403,8 +5386,6 @@ skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 	 * to grab the lock on *all* CRTC's.
 	 */
 	if (state->active_pipe_changes || state->modeset) {
-		state->wm_results.dirty_pipes = INTEL_INFO(dev_priv)->pipe_mask;
-
 		ret = intel_add_all_pipes(state);
 		if (ret)
 			return ret;
@@ -5479,12 +5460,8 @@ skl_compute_wm(struct intel_atomic_state *state)
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *new_crtc_state;
 	struct intel_crtc_state *old_crtc_state;
-	struct skl_ddb_values *results = &state->wm_results;
 	int ret, i;
 
-	/* Clear all dirty flags */
-	results->dirty_pipes = 0;
-
 	ret = skl_ddb_add_affected_pipes(state);
 	if (ret)
 		return ret;
@@ -5492,8 +5469,7 @@ skl_compute_wm(struct intel_atomic_state *state)
 	/*
 	 * Calculate WM's for all pipes that are part of this transaction.
 	 * Note that skl_ddb_add_affected_pipes may have added more CRTC's that
-	 * weren't otherwise being modified (and set bits in dirty_pipes) if
-	 * pipe allocations had to change.
+	 * weren't otherwise being modified if pipe allocations had to change.
 	 */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
@@ -5504,11 +5480,6 @@ skl_compute_wm(struct intel_atomic_state *state)
 		ret = skl_wm_add_affected_planes(state, crtc);
 		if (ret)
 			return ret;
-
-		if (!skl_pipe_wm_equals(crtc,
-					&old_crtc_state->wm.skl.optimal,
-					&new_crtc_state->wm.skl.optimal))
-			results->dirty_pipes |= BIT(crtc->pipe);
 	}
 
 	ret = skl_compute_ddb(state);
@@ -5644,7 +5615,6 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 
 void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
 {
-	struct skl_ddb_values *hw = &dev_priv->wm.skl_hw;
 	struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
@@ -5654,9 +5624,6 @@ void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
 		crtc_state = to_intel_crtc_state(crtc->base.state);
 
 		skl_pipe_wm_get_hw_state(crtc, &crtc_state->wm.skl.optimal);
-
-		if (crtc->active)
-			hw->dirty_pipes |= BIT(crtc->pipe);
 	}
 
 	if (dev_priv->active_pipes) {
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* ✗ Fi.CI.BUILD: failure for drm/i915: Some cleanup near the SKL wm/ddb area
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (7 preceding siblings ...)
  2019-10-11 20:09 ` [PATCH 8/8] drm/i915: Nuke skl wm.dirty_pipes bitmask Ville Syrjala
@ 2019-10-11 21:35 ` Patchwork
  2019-10-14 18:19 ` ✓ Fi.CI.BAT: success for drm/i915: Some cleanup near the SKL wm/ddb area (rev2) Patchwork
  2019-10-15  3:21 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-11 21:35 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Some cleanup near the SKL wm/ddb area
URL   : https://patchwork.freedesktop.org/series/67930/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/gvt/handlers.o
drivers/gpu/drm/i915/gvt/handlers.c: In function ‘init_generic_mmio_info’:
drivers/gpu/drm/i915/gvt/handlers.c:2400:9: error: implicit declaration of function ‘PIPE_WM_LINETIME’; did you mean ‘WM_LINETIME’? [-Werror=implicit-function-declaration]
  MMIO_D(PIPE_WM_LINETIME(PIPE_A), D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1811:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~
drivers/gpu/drm/i915/gvt/handlers.c:2400:2: note: in expansion of macro ‘MMIO_D’
  MMIO_D(PIPE_WM_LINETIME(PIPE_A), D_ALL);
  ^~~~~~
drivers/gpu/drm/i915/gvt/handlers.c:2400:9: error: incompatible type for argument 1 of ‘i915_mmio_reg_offset’
  MMIO_D(PIPE_WM_LINETIME(PIPE_A), D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1811:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~
drivers/gpu/drm/i915/gvt/handlers.c:2400:2: note: in expansion of macro ‘MMIO_D’
  MMIO_D(PIPE_WM_LINETIME(PIPE_A), D_ALL);
  ^~~~~~
In file included from ./drivers/gpu/drm/i915/i915_drv.h:63:0,
                 from drivers/gpu/drm/i915/gvt/handlers.c:39:
./drivers/gpu/drm/i915/i915_reg.h:189:19: note: expected ‘i915_reg_t {aka struct <anonymous>}’ but argument is of type ‘int’
 static inline u32 i915_mmio_reg_offset(i915_reg_t reg)
                   ^~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gvt/handlers.c:2401:9: error: incompatible type for argument 1 of ‘i915_mmio_reg_offset’
  MMIO_D(PIPE_WM_LINETIME(PIPE_B), D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1811:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~
drivers/gpu/drm/i915/gvt/handlers.c:2401:2: note: in expansion of macro ‘MMIO_D’
  MMIO_D(PIPE_WM_LINETIME(PIPE_B), D_ALL);
  ^~~~~~
In file included from ./drivers/gpu/drm/i915/i915_drv.h:63:0,
                 from drivers/gpu/drm/i915/gvt/handlers.c:39:
./drivers/gpu/drm/i915/i915_reg.h:189:19: note: expected ‘i915_reg_t {aka struct <anonymous>}’ but argument is of type ‘int’
 static inline u32 i915_mmio_reg_offset(i915_reg_t reg)
                   ^~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gvt/handlers.c:2402:9: error: incompatible type for argument 1 of ‘i915_mmio_reg_offset’
  MMIO_D(PIPE_WM_LINETIME(PIPE_C), D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1811:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~
drivers/gpu/drm/i915/gvt/handlers.c:2402:2: note: in expansion of macro ‘MMIO_D’
  MMIO_D(PIPE_WM_LINETIME(PIPE_C), D_ALL);
  ^~~~~~
In file included from ./drivers/gpu/drm/i915/i915_drv.h:63:0,
                 from drivers/gpu/drm/i915/gvt/handlers.c:39:
./drivers/gpu/drm/i915/i915_reg.h:189:19: note: expected ‘i915_reg_t {aka struct <anonymous>}’ but argument is of type ‘int’
 static inline u32 i915_mmio_reg_offset(i915_reg_t reg)
                   ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/i915/gvt/handlers.o' failed
make[4]: *** [drivers/gpu/drm/i915/gvt/handlers.o] Error 1
scripts/Makefile.build:509: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:509: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:509: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1650: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 6/8] drm/i915: Polish WM_LINETIME register stuff
  2019-10-11 20:09 ` [PATCH 6/8] drm/i915: Polish WM_LINETIME register stuff Ville Syrjala
@ 2019-10-14 14:56   ` Ville Syrjala
  2019-12-03 14:02     ` [Intel-gfx] " Lisovskiy, Stanislav
  0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjala @ 2019-10-14 14:56 UTC (permalink / raw)
  To: intel-gfx

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

Let's store the normal and IPS linetime watermarks individually,
and while at it we'll pimp the register definitions as well.

v2: Deal with gvt

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  3 +-
 drivers/gpu/drm/i915/gvt/handlers.c           |  6 +-
 drivers/gpu/drm/i915/i915_reg.h               | 14 ++--
 drivers/gpu/drm/i915/intel_pm.c               | 72 ++++++++++---------
 4 files changed, 52 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..841952332c7e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -631,7 +631,8 @@ struct intel_crtc_scaler_state {
 
 struct intel_pipe_wm {
 	struct intel_wm_level wm[5];
-	u32 linetime;
+	u16 linetime;
+	u16 ips_linetime;
 	bool fbc_wm_enabled;
 	bool pipe_enabled;
 	bool sprites_enabled;
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index 45a9124e53b6..f2ad477ede6b 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -2397,9 +2397,9 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
 	MMIO_F(_MMIO(0x7144c), 0xc, 0, 0, 0, D_PRE_SKL, NULL, NULL);
 	MMIO_F(_MMIO(0x7244c), 0xc, 0, 0, 0, D_PRE_SKL, NULL, NULL);
 
-	MMIO_D(PIPE_WM_LINETIME(PIPE_A), D_ALL);
-	MMIO_D(PIPE_WM_LINETIME(PIPE_B), D_ALL);
-	MMIO_D(PIPE_WM_LINETIME(PIPE_C), D_ALL);
+	MMIO_D(WM_LINETIME(PIPE_A), D_ALL);
+	MMIO_D(WM_LINETIME(PIPE_B), D_ALL);
+	MMIO_D(WM_LINETIME(PIPE_C), D_ALL);
 	MMIO_D(SPLL_CTL, D_ALL);
 	MMIO_D(_MMIO(_WRPLL_CTL1), D_ALL);
 	MMIO_D(_MMIO(_WRPLL_CTL2), D_ALL);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0fb9030b89f1..e1db85768084 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10394,13 +10394,13 @@ enum skl_power_gate {
 #define  D_COMP_COMP_DISABLE		(1 << 0)
 
 /* Pipe WM_LINETIME - watermark line time */
-#define _PIPE_WM_LINETIME_A		0x45270
-#define _PIPE_WM_LINETIME_B		0x45274
-#define PIPE_WM_LINETIME(pipe) _MMIO_PIPE(pipe, _PIPE_WM_LINETIME_A, _PIPE_WM_LINETIME_B)
-#define   PIPE_WM_LINETIME_MASK			(0x1ff)
-#define   PIPE_WM_LINETIME_TIME(x)		((x))
-#define   PIPE_WM_LINETIME_IPS_LINETIME_MASK	(0x1ff << 16)
-#define   PIPE_WM_LINETIME_IPS_LINETIME(x)	((x) << 16)
+#define _WM_LINETIME_A		0x45270
+#define _WM_LINETIME_B		0x45274
+#define WM_LINETIME(pipe) _MMIO_PIPE(pipe, _WM_LINETIME_A, _WM_LINETIME_B)
+#define  HSW_LINETIME_MASK	REG_GENMASK(8, 0)
+#define  HSW_LINETIME(x)	REG_FIELD_PREP(HSW_LINETIME_MASK, (x))
+#define  HSW_IPS_LINETIME_MASK	REG_GENMASK(24, 16)
+#define  HSW_IPS_LINETIME(x)	REG_FIELD_PREP(HSW_IPS_LINETIME_MASK, (x))
 
 /* SFUSE_STRAP */
 #define SFUSE_STRAP			_MMIO(0xc2014)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f21eb9250d23..f6893b771078 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2764,31 +2764,31 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
 }
 
 static u32
-hsw_compute_linetime_wm(const struct intel_crtc_state *crtc_state)
+hsw_linetime_wm(const struct intel_crtc_state *crtc_state)
 {
-	const struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(crtc_state->base.state);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->base.adjusted_mode;
-	u32 linetime, ips_linetime;
 
 	if (!crtc_state->base.active)
 		return 0;
-	if (WARN_ON(adjusted_mode->crtc_clock == 0))
-		return 0;
-	if (WARN_ON(intel_state->cdclk.logical.cdclk == 0))
-		return 0;
 
-	/* The WM are computed with base on how long it takes to fill a single
-	 * row at the given clock rate, multiplied by 8.
-	 * */
-	linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-				     adjusted_mode->crtc_clock);
-	ips_linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-					 intel_state->cdclk.logical.cdclk);
+	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
+				 adjusted_mode->crtc_clock);
+}
+
+static u32
+hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state)
+{
+	const struct intel_atomic_state *state =
+		to_intel_atomic_state(crtc_state->base.state);
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
 
-	return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
-	       PIPE_WM_LINETIME_TIME(linetime);
+	if (!crtc_state->base.active)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
+				 state->cdclk.logical.cdclk);
 }
 
 static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
@@ -3128,8 +3128,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	ilk_compute_wm_level(dev_priv, intel_crtc, 0, crtc_state,
 			     pristate, sprstate, curstate, &pipe_wm->wm[0]);
 
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		pipe_wm->linetime = hsw_compute_linetime_wm(crtc_state);
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		pipe_wm->linetime = hsw_linetime_wm(crtc_state);
+		pipe_wm->ips_linetime = hsw_ips_linetime_wm(crtc_state);
+	}
 
 	if (!ilk_validate_pipe_wm(dev_priv, pipe_wm))
 		return -EINVAL;
@@ -3329,7 +3331,7 @@ static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 				   enum intel_ddb_partitioning partitioning,
 				   struct ilk_wm_values *results)
 {
-	struct intel_crtc *intel_crtc;
+	struct intel_crtc *crtc;
 	int level, wm_lp;
 
 	results->enable_fbc_wm = merged->fbc_wm_enabled;
@@ -3374,15 +3376,17 @@ static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 	}
 
 	/* LP0 register values */
-	for_each_intel_crtc(&dev_priv->drm, intel_crtc) {
-		enum pipe pipe = intel_crtc->pipe;
-		const struct intel_wm_level *r =
-			&intel_crtc->wm.active.ilk.wm[0];
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		enum pipe pipe = crtc->pipe;
+		const struct intel_pipe_wm *pipe_wm = &crtc->wm.active.ilk;
+		const struct intel_wm_level *r = &pipe_wm->wm[0];
 
 		if (WARN_ON(!r->enable))
 			continue;
 
-		results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime;
+		results->wm_linetime[pipe] =
+			HSW_LINETIME(pipe_wm->linetime) |
+			HSW_IPS_LINETIME(pipe_wm->ips_linetime);
 
 		results->wm_pipe[pipe] =
 			(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -3535,11 +3539,11 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
-		I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results->wm_linetime[0]);
+		I915_WRITE(WM_LINETIME(PIPE_A), results->wm_linetime[0]);
 	if (dirty & WM_DIRTY_LINETIME(PIPE_B))
-		I915_WRITE(PIPE_WM_LINETIME(PIPE_B), results->wm_linetime[1]);
+		I915_WRITE(WM_LINETIME(PIPE_B), results->wm_linetime[1]);
 	if (dirty & WM_DIRTY_LINETIME(PIPE_C))
-		I915_WRITE(PIPE_WM_LINETIME(PIPE_C), results->wm_linetime[2]);
+		I915_WRITE(WM_LINETIME(PIPE_C), results->wm_linetime[2]);
 
 	if (dirty & WM_DIRTY_DDB) {
 		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -5598,7 +5602,7 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 	if ((state->wm_results.dirty_pipes & BIT(crtc->pipe)) == 0)
 		return;
 
-	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
+	I915_WRITE(WM_LINETIME(pipe), HSW_LINETIME(pipe_wm->linetime));
 }
 
 static void skl_initial_wm(struct intel_atomic_state *state,
@@ -5740,7 +5744,8 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 	if (!crtc->active)
 		return;
 
-	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
+	val = I915_READ(WM_LINETIME(pipe));
+	out->linetime = REG_FIELD_GET(HSW_LINETIME_MASK, val);
 }
 
 void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
@@ -5782,7 +5787,7 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 
 	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
+		hw->wm_linetime[pipe] = I915_READ(WM_LINETIME(pipe));
 
 	memset(active, 0, sizeof(*active));
 
@@ -5801,7 +5806,10 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 		active->wm[0].pri_val = (tmp & WM0_PIPE_PLANE_MASK) >> WM0_PIPE_PLANE_SHIFT;
 		active->wm[0].spr_val = (tmp & WM0_PIPE_SPRITE_MASK) >> WM0_PIPE_SPRITE_SHIFT;
 		active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
-		active->linetime = hw->wm_linetime[pipe];
+		active->linetime = REG_FIELD_GET(HSW_LINETIME_MASK,
+						 hw->wm_linetime[pipe]);
+		active->ips_linetime = REG_FIELD_GET(HSW_IPS_LINETIME_MASK,
+						     hw->wm_linetime[pipe]);
 	} else {
 		int level, max_level = ilk_wm_max_level(dev_priv);
 
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Some cleanup near the SKL wm/ddb area (rev2)
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (8 preceding siblings ...)
  2019-10-11 21:35 ` ✗ Fi.CI.BUILD: failure for drm/i915: Some cleanup near the SKL wm/ddb area Patchwork
@ 2019-10-14 18:19 ` Patchwork
  2019-10-15  3:21 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-14 18:19 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Some cleanup near the SKL wm/ddb area (rev2)
URL   : https://patchwork.freedesktop.org/series/67930/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7087 -> Patchwork_14794
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/index.html

Known issues
------------

  Here are the changes found in Patchwork_14794 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@basic-write-cpu-read-gtt:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/fi-icl-u3/igt@gem_mmap_gtt@basic-write-cpu-read-gtt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/fi-icl-u3/igt@gem_mmap_gtt@basic-write-cpu-read-gtt.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-kbl-guc:         [PASS][3] -> [INCOMPLETE][4] ([fdo#112002])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/fi-kbl-guc/igt@i915_selftest@live_gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/fi-kbl-guc/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-write-read:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/fi-icl-u3/igt@gem_mmap_gtt@basic-write-read.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/fi-icl-u3/igt@gem_mmap_gtt@basic-write-read.html

  * igt@i915_selftest@live_execlists:
    - {fi-icl-u4}:        [INCOMPLETE][7] ([fdo#107713]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/fi-icl-u4/igt@i915_selftest@live_execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/fi-icl-u4/igt@i915_selftest@live_execlists.html

  * igt@kms_chamelium@hdmi-edid-read:
    - {fi-icl-u4}:        [FAIL][9] ([fdo#111045]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
  [fdo#111600]: https://bugs.freedesktop.org/show_bug.cgi?id=111600
  [fdo#112002]: https://bugs.freedesktop.org/show_bug.cgi?id=112002


Participating hosts (52 -> 44)
------------------------------

  Missing    (8): fi-ilk-m540 fi-tgl-u fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-skl-6600u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7087 -> Patchwork_14794

  CI-20190529: 20190529
  CI_DRM_7087: a2b54a43afb778438edaa8c4e13d2c7cc0a6909a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5225: 991ce4eede1c52f76378aebf162a13c20d6f6293 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14794: 824abde942f8fcef3ca2ae9d1116cbcc0a5859aa @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

824abde942f8 drm/i915: Nuke skl wm.dirty_pipes bitmask
c1459a6918f8 drm/i915: Move linetime wms into the crtc state
7a9995b69471 drm/i915: Polish WM_LINETIME register stuff
b7f58101c4e5 drm/i915: Streamline skl_commit_modeset_enables()
0e684dc72d96 drm/i915: Don't set undefined bits in dirty_pipes
daba17a895b9 drm/i915: Make dirty_pipes refer to pipes
cf6f9a8c5c4c drm/i915: Nuke 'realloc_pipes'
73c0b3dc5dda drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Some cleanup near the SKL wm/ddb area (rev2)
  2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
                   ` (9 preceding siblings ...)
  2019-10-14 18:19 ` ✓ Fi.CI.BAT: success for drm/i915: Some cleanup near the SKL wm/ddb area (rev2) Patchwork
@ 2019-10-15  3:21 ` Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-15  3:21 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Some cleanup near the SKL wm/ddb area (rev2)
URL   : https://patchwork.freedesktop.org/series/67930/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7087_full -> Patchwork_14794_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_14794_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110854])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb1/igt@gem_exec_balancer@smoke.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb3/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +16 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb4/igt@gem_exec_schedule@fifo-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb8/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb5/igt@gem_exec_schedule@wide-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb4/igt@gem_exec_schedule@wide-bsd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103359] / [fdo#108686] / [k.org#198133])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-glk8/igt@gem_tiled_swapping@non-threaded.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-glk1/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-snb:          [PASS][9] -> [DMESG-WARN][10] ([fdo#111870])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-snb4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-apl7/igt@gem_workarounds@suspend-resume-context.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-apl6/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][13] -> [FAIL][14] ([fdo#105363])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-glk6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#108341])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb2/igt@kms_psr@no_drrs.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109441])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb1/igt@kms_psr@psr2_sprite_plane_move.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - {shard-tglb}:       [INCOMPLETE][25] ([fdo#111832]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-tglb1/igt@gem_ctx_isolation@rcs0-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-tglb6/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_exec_create@madvise:
    - {shard-tglb}:       [INCOMPLETE][27] ([fdo#111747]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-tglb7/igt@gem_exec_create@madvise.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-tglb7/igt@gem_exec_create@madvise.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][29] ([fdo#111325]) -> [PASS][30] +7 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb4/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb8/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-snb:          [TIMEOUT][31] -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-snb7/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-snb7/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [DMESG-WARN][33] ([fdo#111870]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-snb5/igt@gem_userptr_blits@sync-unmap.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-snb7/igt@gem_userptr_blits@sync-unmap.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [DMESG-WARN][35] ([fdo#108566]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-apl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-apl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-iclb:         [INCOMPLETE][37] ([fdo#107713] / [fdo#109507]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb7/igt@kms_flip@flip-vs-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb2/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +4 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - {shard-tglb}:       [FAIL][41] ([fdo#103167]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-stridechange.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-stridechange.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-iclb:         [INCOMPLETE][43] ([fdo#107713] / [fdo#110042]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][45] ([fdo#108145]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][47] ([fdo#109642] / [fdo#111068]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb5/igt@kms_psr2_su@page_flip.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [SKIP][49] ([fdo#109441]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb1/igt@kms_psr@psr2_sprite_render.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb2/igt@kms_psr@psr2_sprite_render.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][51] ([fdo#109276]) -> [PASS][52] +18 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb3/igt@prime_busy@hang-bsd2.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb1/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][53] ([fdo#111329]) -> [SKIP][54] ([fdo#109276])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [SKIP][55] ([fdo#109276]) -> [FAIL][56] ([fdo#111330]) +3 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7087/shard-iclb5/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110042]: https://bugs.freedesktop.org/show_bug.cgi?id=110042
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111672]: https://bugs.freedesktop.org/show_bug.cgi?id=111672
  [fdo#111747]: https://bugs.freedesktop.org/show_bug.cgi?id=111747
  [fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7087 -> Patchwork_14794

  CI-20190529: 20190529
  CI_DRM_7087: a2b54a43afb778438edaa8c4e13d2c7cc0a6909a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5225: 991ce4eede1c52f76378aebf162a13c20d6f6293 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14794: 824abde942f8fcef3ca2ae9d1116cbcc0a5859aa @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14794/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes()
  2019-10-11 20:09 ` [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes() Ville Syrjala
@ 2019-10-16 14:25   ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-10-16 14:25 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> changed==true just means we have some crtcs in the state. All the
> stuff following this only operates on crtcs in the state anyway so
> there is no point in having this bool.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 30 ++++--------------------------
>  1 file changed, 4 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index b306e2338f5a..49568270a89d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5424,35 +5424,14 @@ skl_print_wm_changes(struct
> intel_atomic_state *state)
>  }
>  
>  static int
> -skl_ddb_add_affected_pipes(struct intel_atomic_state *state, bool
> *changed)
> +skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
>  {
>  	struct drm_device *dev = state->base.dev;
>  	const struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *crtc_state;
>  	u32 realloc_pipes = pipes_modified(state);
> -	int ret, i;
> -
> -	/*
> -	 * When we distrust bios wm we always need to recompute to set
> the
> -	 * expected DDB allocations for each CRTC.
> -	 */
> -	if (dev_priv->wm.distrust_bios_wm)
> -		(*changed) = true;
> -
> -	/*
> -	 * If this transaction isn't actually touching any CRTC's,
> don't
> -	 * bother with watermark calculation.  Note that if we pass
> this
> -	 * test, we're guaranteed to hold at least one CRTC state
> mutex,
> -	 * which means we can safely use values like dev_priv-
> >active_pipes
> -	 * since any racing commits that want to update them would need
> to
> -	 * hold _all_ CRTC state mutexes.
> -	 */
> -	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
> -		(*changed) = true;
> -
> -	if (!*changed)
> -		return 0;
> +	int ret;
>  
>  	/*
>  	 * If this is our first atomic update following hardware
> readout,
> @@ -5576,14 +5555,13 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  	struct intel_crtc_state *new_crtc_state;
>  	struct intel_crtc_state *old_crtc_state;
>  	struct skl_ddb_values *results = &state->wm_results;
> -	bool changed = false;
>  	int ret, i;
>  
>  	/* Clear all dirty flags */
>  	results->dirty_pipes = 0;
>  
> -	ret = skl_ddb_add_affected_pipes(state, &changed);
> -	if (ret || !changed)
> +	ret = skl_ddb_add_affected_pipes(state);
> +	if (ret)
>  		return ret;
>  
>  	/*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] drm/i915: Nuke 'realloc_pipes'
  2019-10-11 20:09 ` [PATCH 2/8] drm/i915: Nuke 'realloc_pipes' Ville Syrjala
@ 2019-10-16 14:27   ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-10-16 14:27 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The 'realloc_pipes' bitmask is pointless. It is either:
> a) the set of pipes which are already part of the state,
>    in which case adding them again is entirely redundant
> b) the set of all pipes which we then add to the state
> 
> Also the fact that 'realloc_pipes' uses the crtc indexes is
> going to bite is at some point so best get rid of it quick.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++-----------------
> --
>  1 file changed, 21 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 49568270a89d..3536c2e975e7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5235,19 +5235,6 @@ bool skl_ddb_allocation_overlaps(const struct
> skl_ddb_entry *ddb,
>  	return false;
>  }
>  
> -static u32
> -pipes_modified(struct intel_atomic_state *state)
> -{
> -	struct intel_crtc *crtc;
> -	struct intel_crtc_state *crtc_state;
> -	u32 i, ret = 0;
> -
> -	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
> -		ret |= drm_crtc_mask(&crtc->base);
> -
> -	return ret;
> -}
> -
>  static int
>  skl_ddb_add_affected_planes(const struct intel_crtc_state
> *old_crtc_state,
>  			    struct intel_crtc_state *new_crtc_state)
> @@ -5423,14 +5410,26 @@ skl_print_wm_changes(struct
> intel_atomic_state *state)
>  	}
>  }
>  
> +static int intel_add_all_pipes(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		struct intel_crtc_state *crtc_state;
> +
> +		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
>  {
> -	struct drm_device *dev = state->base.dev;
> -	const struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *crtc;
> -	struct intel_crtc_state *crtc_state;
> -	u32 realloc_pipes = pipes_modified(state);
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	int ret;
>  
>  	/*
> @@ -5440,7 +5439,7 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * ensure a full DDB recompute.
>  	 */
>  	if (dev_priv->wm.distrust_bios_wm) {
> -		ret = drm_modeset_lock(&dev-
> >mode_config.connection_mutex,
> +		ret = drm_modeset_lock(&dev_priv-
> >drm.mode_config.connection_mutex,
>  				       state->base.acquire_ctx);
>  		if (ret)
>  			return ret;
> @@ -5471,18 +5470,11 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * to grab the lock on *all* CRTC's.
>  	 */
>  	if (state->active_pipe_changes || state->modeset) {
> -		realloc_pipes = ~0;
>  		state->wm_results.dirty_pipes = ~0;
> -	}
>  
> -	/*
> -	 * We're not recomputing for the pipes not included in the
> commit, so
> -	 * make sure we start with the current state.
> -	 */
> -	for_each_intel_crtc_mask(dev, crtc, realloc_pipes) {
> -		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> -		if (IS_ERR(crtc_state))
> -			return PTR_ERR(crtc_state);
> +		ret = intel_add_all_pipes(state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	return 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/8] drm/i915: Make dirty_pipes refer to pipes
  2019-10-11 20:09 ` [PATCH 3/8] drm/i915: Make dirty_pipes refer to pipes Ville Syrjala
@ 2019-10-16 14:28   ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-10-16 14:28 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Despite the its name dirty_pipes refers to crtc indexes. Let's
> change its behaviout to match the name.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  9 +++------
>  drivers/gpu/drm/i915/intel_pm.c              | 13 ++++++-------
>  2 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a146ec02a0c1..b9b51bd0c956 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13883,7 +13883,6 @@ static void skl_commit_modeset_enables(struct
> intel_atomic_state *state)
>  	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
>  	unsigned int updated = 0;
>  	bool progress;
> -	enum pipe pipe;
>  	int i;
>  	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
>  	u8 required_slices = state->wm_results.ddb.enabled_slices;
> @@ -13908,12 +13907,10 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
>  		progress = false;
>  
>  		for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
> +			enum pipe pipe = crtc->pipe;
>  			bool vbl_wait = false;
> -			unsigned int cmask = drm_crtc_mask(&crtc-
> >base);
> -
> -			pipe = crtc->pipe;
>  
> -			if (updated & cmask || !new_crtc_state-
> >base.active)
> +			if (updated & BIT(crtc->pipe) ||
> !new_crtc_state->base.active)
>  				continue;
>  
>  			if
> (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> @@ -13921,7 +13918,7 @@ static void skl_commit_modeset_enables(struct
> intel_atomic_state *state)
>  							INTEL_NUM_PIPES
> (dev_priv), i))
>  				continue;
>  
> -			updated |= cmask;
> +			updated |= BIT(pipe);
>  			entries[i] = new_crtc_state->wm.skl.ddb;
>  
>  			/*
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 3536c2e975e7..2b71d52a4ede 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5575,7 +5575,7 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  		if (!skl_pipe_wm_equals(crtc,
>  					&old_crtc_state-
> >wm.skl.optimal,
>  					&new_crtc_state-
> >wm.skl.optimal))
> -			results->dirty_pipes |= drm_crtc_mask(&crtc-
> >base);
> +			results->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	ret = skl_compute_ddb(state);
> @@ -5595,7 +5595,7 @@ static void skl_atomic_update_crtc_wm(struct
> intel_atomic_state *state,
>  	struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
>  	enum pipe pipe = crtc->pipe;
>  
> -	if (!(state->wm_results.dirty_pipes & drm_crtc_mask(&crtc-
> >base)))
> +	if ((state->wm_results.dirty_pipes & BIT(crtc->pipe)) == 0)
>  		return;
>  
>  	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
> @@ -5604,12 +5604,11 @@ static void skl_atomic_update_crtc_wm(struct
> intel_atomic_state *state,
>  static void skl_initial_wm(struct intel_atomic_state *state,
>  			   struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> >base.crtc);
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct skl_ddb_values *results = &state->wm_results;
>  
> -	if ((results->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
> == 0)
> +	if ((results->dirty_pipes & BIT(crtc->pipe)) == 0)
>  		return;
>  
>  	mutex_lock(&dev_priv->wm.wm_mutex);
> @@ -5758,7 +5757,7 @@ void skl_wm_get_hw_state(struct
> drm_i915_private *dev_priv)
>  		skl_pipe_wm_get_hw_state(crtc, &crtc_state-
> >wm.skl.optimal);
>  
>  		if (crtc->active)
> -			hw->dirty_pipes |= drm_crtc_mask(&crtc->base);
> +			hw->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	if (dev_priv->active_pipes) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 8/8] drm/i915: Nuke skl wm.dirty_pipes bitmask
@ 2019-11-29  9:18     ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-11-29  9:18 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The dirty_pipes bitmask is now unused. Get rid of it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 -
>  drivers/gpu/drm/i915/intel_pm.c | 35 +----------------------------
> ----
>  2 files changed, 1 insertion(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index eba9055c4405..c16ba85e2ec8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -901,7 +901,6 @@ struct skl_ddb_allocation {
>  };
>  
>  struct skl_ddb_values {
> -	unsigned dirty_pipes;
>  	struct skl_ddb_allocation ddb;
>  };

As you know, I have also removed skl_ddb_allocation
however as your patch is not yet on drm-tip, I guess
I can't remove skl_ddb_values for now, probably I will
just start using dev_priv for enabled_slices for now.
If your patch set lands before mine I will then nuke it.

So, Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>


>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 390d1f8d45ba..ccbb732cff44 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5130,23 +5130,6 @@ static bool skl_plane_wm_equals(struct
> drm_i915_private *dev_priv,
>  	return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm);
>  }
>  
> -static bool skl_pipe_wm_equals(struct intel_crtc *crtc,
> -			       const struct skl_pipe_wm *wm1,
> -			       const struct skl_pipe_wm *wm2)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum plane_id plane_id;
> -
> -	for_each_plane_id_on_crtc(crtc, plane_id) {
> -		if (!skl_plane_wm_equals(dev_priv,
> -					 &wm1->planes[plane_id],
> -					 &wm2->planes[plane_id]))
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
>  static inline bool skl_ddb_entries_overlap(const struct
> skl_ddb_entry *a,
>  					   const struct skl_ddb_entry
> *b)
>  {
> @@ -5403,8 +5386,6 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * to grab the lock on *all* CRTC's.
>  	 */
>  	if (state->active_pipe_changes || state->modeset) {
> -		state->wm_results.dirty_pipes = INTEL_INFO(dev_priv)-
> >pipe_mask;
> -
>  		ret = intel_add_all_pipes(state);
>  		if (ret)
>  			return ret;
> @@ -5479,12 +5460,8 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *new_crtc_state;
>  	struct intel_crtc_state *old_crtc_state;
> -	struct skl_ddb_values *results = &state->wm_results;
>  	int ret, i;
>  
> -	/* Clear all dirty flags */
> -	results->dirty_pipes = 0;
> -
>  	ret = skl_ddb_add_affected_pipes(state);
>  	if (ret)
>  		return ret;
> @@ -5492,8 +5469,7 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  	/*
>  	 * Calculate WM's for all pipes that are part of this
> transaction.
>  	 * Note that skl_ddb_add_affected_pipes may have added more
> CRTC's that
> -	 * weren't otherwise being modified (and set bits in
> dirty_pipes) if
> -	 * pipe allocations had to change.
> +	 * weren't otherwise being modified if pipe allocations had to
> change.
>  	 */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
>  					    new_crtc_state, i) {
> @@ -5504,11 +5480,6 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  		ret = skl_wm_add_affected_planes(state, crtc);
>  		if (ret)
>  			return ret;
> -
> -		if (!skl_pipe_wm_equals(crtc,
> -					&old_crtc_state-
> >wm.skl.optimal,
> -					&new_crtc_state-
> >wm.skl.optimal))
> -			results->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	ret = skl_compute_ddb(state);
> @@ -5644,7 +5615,6 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc
> *crtc,
>  
>  void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
>  {
> -	struct skl_ddb_values *hw = &dev_priv->wm.skl_hw;
>  	struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *crtc_state;
> @@ -5654,9 +5624,6 @@ void skl_wm_get_hw_state(struct
> drm_i915_private *dev_priv)
>  		crtc_state = to_intel_crtc_state(crtc->base.state);
>  
>  		skl_pipe_wm_get_hw_state(crtc, &crtc_state-
> >wm.skl.optimal);
> -
> -		if (crtc->active)
> -			hw->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	if (dev_priv->active_pipes) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [PATCH 8/8] drm/i915: Nuke skl wm.dirty_pipes bitmask
@ 2019-11-29  9:18     ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-11-29  9:18 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The dirty_pipes bitmask is now unused. Get rid of it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 -
>  drivers/gpu/drm/i915/intel_pm.c | 35 +----------------------------
> ----
>  2 files changed, 1 insertion(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index eba9055c4405..c16ba85e2ec8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -901,7 +901,6 @@ struct skl_ddb_allocation {
>  };
>  
>  struct skl_ddb_values {
> -	unsigned dirty_pipes;
>  	struct skl_ddb_allocation ddb;
>  };

As you know, I have also removed skl_ddb_allocation
however as your patch is not yet on drm-tip, I guess
I can't remove skl_ddb_values for now, probably I will
just start using dev_priv for enabled_slices for now.
If your patch set lands before mine I will then nuke it.

So, Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>


>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 390d1f8d45ba..ccbb732cff44 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5130,23 +5130,6 @@ static bool skl_plane_wm_equals(struct
> drm_i915_private *dev_priv,
>  	return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm);
>  }
>  
> -static bool skl_pipe_wm_equals(struct intel_crtc *crtc,
> -			       const struct skl_pipe_wm *wm1,
> -			       const struct skl_pipe_wm *wm2)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum plane_id plane_id;
> -
> -	for_each_plane_id_on_crtc(crtc, plane_id) {
> -		if (!skl_plane_wm_equals(dev_priv,
> -					 &wm1->planes[plane_id],
> -					 &wm2->planes[plane_id]))
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
>  static inline bool skl_ddb_entries_overlap(const struct
> skl_ddb_entry *a,
>  					   const struct skl_ddb_entry
> *b)
>  {
> @@ -5403,8 +5386,6 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * to grab the lock on *all* CRTC's.
>  	 */
>  	if (state->active_pipe_changes || state->modeset) {
> -		state->wm_results.dirty_pipes = INTEL_INFO(dev_priv)-
> >pipe_mask;
> -
>  		ret = intel_add_all_pipes(state);
>  		if (ret)
>  			return ret;
> @@ -5479,12 +5460,8 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *new_crtc_state;
>  	struct intel_crtc_state *old_crtc_state;
> -	struct skl_ddb_values *results = &state->wm_results;
>  	int ret, i;
>  
> -	/* Clear all dirty flags */
> -	results->dirty_pipes = 0;
> -
>  	ret = skl_ddb_add_affected_pipes(state);
>  	if (ret)
>  		return ret;
> @@ -5492,8 +5469,7 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  	/*
>  	 * Calculate WM's for all pipes that are part of this
> transaction.
>  	 * Note that skl_ddb_add_affected_pipes may have added more
> CRTC's that
> -	 * weren't otherwise being modified (and set bits in
> dirty_pipes) if
> -	 * pipe allocations had to change.
> +	 * weren't otherwise being modified if pipe allocations had to
> change.
>  	 */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
>  					    new_crtc_state, i) {
> @@ -5504,11 +5480,6 @@ skl_compute_wm(struct intel_atomic_state
> *state)
>  		ret = skl_wm_add_affected_planes(state, crtc);
>  		if (ret)
>  			return ret;
> -
> -		if (!skl_pipe_wm_equals(crtc,
> -					&old_crtc_state-
> >wm.skl.optimal,
> -					&new_crtc_state-
> >wm.skl.optimal))
> -			results->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	ret = skl_compute_ddb(state);
> @@ -5644,7 +5615,6 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc
> *crtc,
>  
>  void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
>  {
> -	struct skl_ddb_values *hw = &dev_priv->wm.skl_hw;
>  	struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *crtc_state;
> @@ -5654,9 +5624,6 @@ void skl_wm_get_hw_state(struct
> drm_i915_private *dev_priv)
>  		crtc_state = to_intel_crtc_state(crtc->base.state);
>  
>  		skl_pipe_wm_get_hw_state(crtc, &crtc_state-
> >wm.skl.optimal);
> -
> -		if (crtc->active)
> -			hw->dirty_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	if (dev_priv->active_pipes) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/8] drm/i915: Don't set undefined bits in dirty_pipes
@ 2019-11-29  9:24     ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-11-29  9:24 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> skl_commit_modeset_enables() straight up compares dirty_pipes
> with a bitmask of already committed pipes. If we set bits in
> dirty_pipes for non-existent pipes that comparison will never
> work right. So let's limit ourselves to bits that exist.
> 
> And we'll do the same for the active_pipes_changed bitmask.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 2b71d52a4ede..f21eb9250d23 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5444,7 +5444,7 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  		if (ret)
>  			return ret;
>  
> -		state->active_pipe_changes = ~0;
> +		state->active_pipe_changes = INTEL_INFO(dev_priv)-
> >pipe_mask;
>  
>  		/*
>  		 * We usually only initialize state->active_pipes if we
> @@ -5470,7 +5470,7 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * to grab the lock on *all* CRTC's.
>  	 */
>  	if (state->active_pipe_changes || state->modeset) {
> -		state->wm_results.dirty_pipes = ~0;
> +		state->wm_results.dirty_pipes = INTEL_INFO(dev_priv)-
> >pipe_mask;
>  
>  		ret = intel_add_all_pipes(state);
>  		if (ret)

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [PATCH 4/8] drm/i915: Don't set undefined bits in dirty_pipes
@ 2019-11-29  9:24     ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-11-29  9:24 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> skl_commit_modeset_enables() straight up compares dirty_pipes
> with a bitmask of already committed pipes. If we set bits in
> dirty_pipes for non-existent pipes that comparison will never
> work right. So let's limit ourselves to bits that exist.
> 
> And we'll do the same for the active_pipes_changed bitmask.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 2b71d52a4ede..f21eb9250d23 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5444,7 +5444,7 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  		if (ret)
>  			return ret;
>  
> -		state->active_pipe_changes = ~0;
> +		state->active_pipe_changes = INTEL_INFO(dev_priv)-
> >pipe_mask;
>  
>  		/*
>  		 * We usually only initialize state->active_pipes if we
> @@ -5470,7 +5470,7 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * to grab the lock on *all* CRTC's.
>  	 */
>  	if (state->active_pipe_changes || state->modeset) {
> -		state->wm_results.dirty_pipes = ~0;
> +		state->wm_results.dirty_pipes = INTEL_INFO(dev_priv)-
> >pipe_mask;
>  
>  		ret = intel_add_all_pipes(state);
>  		if (ret)

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [PATCH 5/8] drm/i915: Streamline skl_commit_modeset_enables()
  2019-10-11 20:09 ` [PATCH 5/8] drm/i915: Streamline skl_commit_modeset_enables() Ville Syrjala
@ 2019-12-03 13:47   ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-12-03 13:47 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> skl_commit_modeset_enables() is a bit of mess. Let's streamline
> it by simply tracking which pipes still need to be updated.
> As a bonus we get rid of the state->wm_results.dirty_pipes usage.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 39 +++++++++---------
> --
>  1 file changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index b9b51bd0c956..9eab67bbf61d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13881,17 +13881,19 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc *crtc;
>  	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> -	unsigned int updated = 0;
> -	bool progress;
> -	int i;
>  	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
>  	u8 required_slices = state->wm_results.ddb.enabled_slices;
>  	struct skl_ddb_entry entries[I915_MAX_PIPES] = {};
> +	u8 dirty_pipes = 0;


> +	int i;
>  
> -	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i)

Really "love" this medieval C standard thing for that nice "i" variable
:)

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> +	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
>  		/* ignore allocations for crtc's that have been turned
> off. */
> -		if (new_crtc_state->base.active)
> +		if (new_crtc_state->base.active) {
>  			entries[i] = old_crtc_state->wm.skl.ddb;
> +			dirty_pipes |= BIT(crtc->pipe);
> +		}
> +	}
>  
>  	/* If 2nd DBuf slice required, enable it here */
>  	if (INTEL_GEN(dev_priv) >= 11 && required_slices >
> hw_enabled_slices)
> @@ -13903,14 +13905,12 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
>  	 * never overlap with eachother inbetween CRTC updates.
> Otherwise we'll
>  	 * cause pipe underruns and other bad stuff.
>  	 */
> -	do {
> -		progress = false;
> -
> -		for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
> +	while (dirty_pipes) {
> +		for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
> +						    new_crtc_state, i)
> {
>  			enum pipe pipe = crtc->pipe;
> -			bool vbl_wait = false;
>  
> -			if (updated & BIT(crtc->pipe) ||
> !new_crtc_state->base.active)
> +			if ((dirty_pipes & BIT(pipe)) == 0)
>  				continue;
>  
>  			if
> (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> @@ -13918,8 +13918,11 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
>  							INTEL_NUM_PIPES
> (dev_priv), i))
>  				continue;
>  
> -			updated |= BIT(pipe);
>  			entries[i] = new_crtc_state->wm.skl.ddb;
> +			dirty_pipes &= ~BIT(pipe);
> +
> +			intel_update_crtc(crtc, state, old_crtc_state,
> +					  new_crtc_state);
>  
>  			/*
>  			 * If this is an already active pipe, it's DDB
> changed,
> @@ -13930,18 +13933,10 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
>  			if (!skl_ddb_entry_equal(&new_crtc_state-
> >wm.skl.ddb,
>  						 &old_crtc_state-
> >wm.skl.ddb) &&
>  			    !new_crtc_state->base.active_changed &&
> -			    state->wm_results.dirty_pipes != updated)
> -				vbl_wait = true;
> -
> -			intel_update_crtc(crtc, state, old_crtc_state,
> -					  new_crtc_state);
> -
> -			if (vbl_wait)
> +			    dirty_pipes)
>  				intel_wait_for_vblank(dev_priv, pipe);
> -
> -			progress = true;
>  		}
> -	} while (progress);
> +	}
>  
>  	/* If 2nd DBuf slice is no more required disable it */
>  	if (INTEL_GEN(dev_priv) >= 11 && required_slices <
> hw_enabled_slices)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [PATCH v2 6/8] drm/i915: Polish WM_LINETIME register stuff
  2019-10-14 14:56   ` [PATCH v2 " Ville Syrjala
@ 2019-12-03 14:02     ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 22+ messages in thread
From: Lisovskiy, Stanislav @ 2019-12-03 14:02 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Mon, 2019-10-14 at 17:56 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's store the normal and IPS linetime watermarks individually,
> and while at it we'll pimp the register definitions as well.
> 
> v2: Deal with gvt
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  .../drm/i915/display/intel_display_types.h    |  3 +-
>  drivers/gpu/drm/i915/gvt/handlers.c           |  6 +-
>  drivers/gpu/drm/i915/i915_reg.h               | 14 ++--
>  drivers/gpu/drm/i915/intel_pm.c               | 72 ++++++++++-------
> --
>  4 files changed, 52 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..841952332c7e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -631,7 +631,8 @@ struct intel_crtc_scaler_state {
>  
>  struct intel_pipe_wm {
>  	struct intel_wm_level wm[5];
> -	u32 linetime;
> +	u16 linetime;
> +	u16 ips_linetime;
>  	bool fbc_wm_enabled;
>  	bool pipe_enabled;
>  	bool sprites_enabled;
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 45a9124e53b6..f2ad477ede6b 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -2397,9 +2397,9 @@ static int init_generic_mmio_info(struct
> intel_gvt *gvt)
>  	MMIO_F(_MMIO(0x7144c), 0xc, 0, 0, 0, D_PRE_SKL, NULL, NULL);
>  	MMIO_F(_MMIO(0x7244c), 0xc, 0, 0, 0, D_PRE_SKL, NULL, NULL);
>  
> -	MMIO_D(PIPE_WM_LINETIME(PIPE_A), D_ALL);
> -	MMIO_D(PIPE_WM_LINETIME(PIPE_B), D_ALL);
> -	MMIO_D(PIPE_WM_LINETIME(PIPE_C), D_ALL);
> +	MMIO_D(WM_LINETIME(PIPE_A), D_ALL);
> +	MMIO_D(WM_LINETIME(PIPE_B), D_ALL);
> +	MMIO_D(WM_LINETIME(PIPE_C), D_ALL);
>  	MMIO_D(SPLL_CTL, D_ALL);
>  	MMIO_D(_MMIO(_WRPLL_CTL1), D_ALL);
>  	MMIO_D(_MMIO(_WRPLL_CTL2), D_ALL);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 0fb9030b89f1..e1db85768084 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10394,13 +10394,13 @@ enum skl_power_gate {
>  #define  D_COMP_COMP_DISABLE		(1 << 0)
>  
>  /* Pipe WM_LINETIME - watermark line time */
> -#define _PIPE_WM_LINETIME_A		0x45270
> -#define _PIPE_WM_LINETIME_B		0x45274
> -#define PIPE_WM_LINETIME(pipe) _MMIO_PIPE(pipe, _PIPE_WM_LINETIME_A,
> _PIPE_WM_LINETIME_B)
> -#define   PIPE_WM_LINETIME_MASK			(0x1ff)
> -#define   PIPE_WM_LINETIME_TIME(x)		((x))
> -#define   PIPE_WM_LINETIME_IPS_LINETIME_MASK	(0x1ff << 16)
> -#define   PIPE_WM_LINETIME_IPS_LINETIME(x)	((x) << 16)
> +#define _WM_LINETIME_A		0x45270
> +#define _WM_LINETIME_B		0x45274
> +#define WM_LINETIME(pipe) _MMIO_PIPE(pipe, _WM_LINETIME_A,
> _WM_LINETIME_B)
> +#define  HSW_LINETIME_MASK	REG_GENMASK(8, 0)
> +#define  HSW_LINETIME(x)	REG_FIELD_PREP(HSW_LINETIME_MASK, (x))
> +#define  HSW_IPS_LINETIME_MASK	REG_GENMASK(24, 16)
> +#define  HSW_IPS_LINETIME(x)	REG_FIELD_PREP(HSW_IPS_LINETIME_MASK,
> (x))
>  
>  /* SFUSE_STRAP */
>  #define SFUSE_STRAP			_MMIO(0xc2014)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index f21eb9250d23..f6893b771078 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2764,31 +2764,31 @@ static void ilk_compute_wm_level(const struct
> drm_i915_private *dev_priv,
>  }
>  
>  static u32
> -hsw_compute_linetime_wm(const struct intel_crtc_state *crtc_state)
> +hsw_linetime_wm(const struct intel_crtc_state *crtc_state)
>  {
> -	const struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(crtc_state->base.state);
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->base.adjusted_mode;
> -	u32 linetime, ips_linetime;
>  
>  	if (!crtc_state->base.active)
>  		return 0;
> -	if (WARN_ON(adjusted_mode->crtc_clock == 0))
> -		return 0;
> -	if (WARN_ON(intel_state->cdclk.logical.cdclk == 0))
> -		return 0;
>  
> -	/* The WM are computed with base on how long it takes to fill a
> single
> -	 * row at the given clock rate, multiplied by 8.
> -	 * */
> -	linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000
> * 8,
> -				     adjusted_mode->crtc_clock);
> -	ips_linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal *
> 1000 * 8,
> -					 intel_state-
> >cdclk.logical.cdclk);
> +	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> +				 adjusted_mode->crtc_clock);
> +}
> +
> +static u32
> +hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state)
> +{
> +	const struct intel_atomic_state *state =
> +		to_intel_atomic_state(crtc_state->base.state);
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
>  
> -	return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
> -	       PIPE_WM_LINETIME_TIME(linetime);
> +	if (!crtc_state->base.active)
> +		return 0;
> +
> +	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> +				 state->cdclk.logical.cdclk);
>  }
>  
>  static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
> @@ -3128,8 +3128,10 @@ static int ilk_compute_pipe_wm(struct
> intel_crtc_state *crtc_state)
>  	ilk_compute_wm_level(dev_priv, intel_crtc, 0, crtc_state,
>  			     pristate, sprstate, curstate, &pipe_wm-
> >wm[0]);
>  
> -	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -		pipe_wm->linetime =
> hsw_compute_linetime_wm(crtc_state);
> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> +		pipe_wm->linetime = hsw_linetime_wm(crtc_state);
> +		pipe_wm->ips_linetime =
> hsw_ips_linetime_wm(crtc_state);
> +	}
>  
>  	if (!ilk_validate_pipe_wm(dev_priv, pipe_wm))
>  		return -EINVAL;
> @@ -3329,7 +3331,7 @@ static void ilk_compute_wm_results(struct
> drm_i915_private *dev_priv,
>  				   enum intel_ddb_partitioning
> partitioning,
>  				   struct ilk_wm_values *results)
>  {
> -	struct intel_crtc *intel_crtc;
> +	struct intel_crtc *crtc;
>  	int level, wm_lp;
>  
>  	results->enable_fbc_wm = merged->fbc_wm_enabled;
> @@ -3374,15 +3376,17 @@ static void ilk_compute_wm_results(struct
> drm_i915_private *dev_priv,
>  	}
>  
>  	/* LP0 register values */
> -	for_each_intel_crtc(&dev_priv->drm, intel_crtc) {
> -		enum pipe pipe = intel_crtc->pipe;
> -		const struct intel_wm_level *r =
> -			&intel_crtc->wm.active.ilk.wm[0];
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		enum pipe pipe = crtc->pipe;
> +		const struct intel_pipe_wm *pipe_wm = &crtc-
> >wm.active.ilk;
> +		const struct intel_wm_level *r = &pipe_wm->wm[0];
>  
>  		if (WARN_ON(!r->enable))
>  			continue;
>  
> -		results->wm_linetime[pipe] = intel_crtc-
> >wm.active.ilk.linetime;
> +		results->wm_linetime[pipe] =
> +			HSW_LINETIME(pipe_wm->linetime) |
> +			HSW_IPS_LINETIME(pipe_wm->ips_linetime);
>  
>  		results->wm_pipe[pipe] =
>  			(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
> @@ -3535,11 +3539,11 @@ static void ilk_write_wm_values(struct
> drm_i915_private *dev_priv,
>  		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
>  
>  	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
> -		I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results-
> >wm_linetime[0]);
> +		I915_WRITE(WM_LINETIME(PIPE_A), results-
> >wm_linetime[0]);
>  	if (dirty & WM_DIRTY_LINETIME(PIPE_B))
> -		I915_WRITE(PIPE_WM_LINETIME(PIPE_B), results-
> >wm_linetime[1]);
> +		I915_WRITE(WM_LINETIME(PIPE_B), results-
> >wm_linetime[1]);
>  	if (dirty & WM_DIRTY_LINETIME(PIPE_C))
> -		I915_WRITE(PIPE_WM_LINETIME(PIPE_C), results-
> >wm_linetime[2]);
> +		I915_WRITE(WM_LINETIME(PIPE_C), results-
> >wm_linetime[2]);
>  
>  	if (dirty & WM_DIRTY_DDB) {
>  		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -5598,7 +5602,7 @@ static void skl_atomic_update_crtc_wm(struct
> intel_atomic_state *state,
>  	if ((state->wm_results.dirty_pipes & BIT(crtc->pipe)) == 0)
>  		return;
>  
> -	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
> +	I915_WRITE(WM_LINETIME(pipe), HSW_LINETIME(pipe_wm->linetime));
>  }
>  
>  static void skl_initial_wm(struct intel_atomic_state *state,
> @@ -5740,7 +5744,8 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc
> *crtc,
>  	if (!crtc->active)
>  		return;
>  
> -	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> +	val = I915_READ(WM_LINETIME(pipe));
> +	out->linetime = REG_FIELD_GET(HSW_LINETIME_MASK, val);
>  }
>  
>  void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
> @@ -5782,7 +5787,7 @@ static void ilk_pipe_wm_get_hw_state(struct
> intel_crtc *crtc)
>  
>  	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -		hw->wm_linetime[pipe] =
> I915_READ(PIPE_WM_LINETIME(pipe));
> +		hw->wm_linetime[pipe] = I915_READ(WM_LINETIME(pipe));
>  
>  	memset(active, 0, sizeof(*active));
>  
> @@ -5801,7 +5806,10 @@ static void ilk_pipe_wm_get_hw_state(struct
> intel_crtc *crtc)
>  		active->wm[0].pri_val = (tmp & WM0_PIPE_PLANE_MASK) >>
> WM0_PIPE_PLANE_SHIFT;
>  		active->wm[0].spr_val = (tmp & WM0_PIPE_SPRITE_MASK) >>
> WM0_PIPE_SPRITE_SHIFT;
>  		active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
> -		active->linetime = hw->wm_linetime[pipe];
> +		active->linetime = REG_FIELD_GET(HSW_LINETIME_MASK,
> +						 hw-
> >wm_linetime[pipe]);
> +		active->ips_linetime =
> REG_FIELD_GET(HSW_IPS_LINETIME_MASK,
> +						     hw-
> >wm_linetime[pipe]);
>  	} else {
>  		int level, max_level = ilk_wm_max_level(dev_priv);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-12-03 14:03 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 20:09 [PATCH 0/8] drm/i915: Some cleanup near the SKL wm/ddb area Ville Syrjala
2019-10-11 20:09 ` [PATCH 1/8] drm/i915: Nuke the useless changed param from skl_ddb_add_affected_pipes() Ville Syrjala
2019-10-16 14:25   ` Lisovskiy, Stanislav
2019-10-11 20:09 ` [PATCH 2/8] drm/i915: Nuke 'realloc_pipes' Ville Syrjala
2019-10-16 14:27   ` Lisovskiy, Stanislav
2019-10-11 20:09 ` [PATCH 3/8] drm/i915: Make dirty_pipes refer to pipes Ville Syrjala
2019-10-16 14:28   ` Lisovskiy, Stanislav
2019-10-11 20:09 ` [PATCH 4/8] drm/i915: Don't set undefined bits in dirty_pipes Ville Syrjala
2019-11-29  9:24   ` Lisovskiy, Stanislav
2019-11-29  9:24     ` [Intel-gfx] " Lisovskiy, Stanislav
2019-10-11 20:09 ` [PATCH 5/8] drm/i915: Streamline skl_commit_modeset_enables() Ville Syrjala
2019-12-03 13:47   ` [Intel-gfx] " Lisovskiy, Stanislav
2019-10-11 20:09 ` [PATCH 6/8] drm/i915: Polish WM_LINETIME register stuff Ville Syrjala
2019-10-14 14:56   ` [PATCH v2 " Ville Syrjala
2019-12-03 14:02     ` [Intel-gfx] " Lisovskiy, Stanislav
2019-10-11 20:09 ` [PATCH 7/8] drm/i915: Move linetime wms into the crtc state Ville Syrjala
2019-10-11 20:09 ` [PATCH 8/8] drm/i915: Nuke skl wm.dirty_pipes bitmask Ville Syrjala
2019-11-29  9:18   ` Lisovskiy, Stanislav
2019-11-29  9:18     ` [Intel-gfx] " Lisovskiy, Stanislav
2019-10-11 21:35 ` ✗ Fi.CI.BUILD: failure for drm/i915: Some cleanup near the SKL wm/ddb area Patchwork
2019-10-14 18:19 ` ✓ Fi.CI.BAT: success for drm/i915: Some cleanup near the SKL wm/ddb area (rev2) Patchwork
2019-10-15  3:21 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.