Skip to contentSkip to conversation

Start of skl watermark cleanup

PATCH In review Lyude 4 participants 16 messages Oct 14, 2016 – Oct 18, 2016

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Lyude

author
#0 lore raw

v3 cover Start of skl watermark cleanup

While it (mostly) works, the code for handling watermarks on Skylake has been kind of ugly for a while. As well a lot of it isn't that friendly to atomic transactions, Lots of copy paste, redundant wm values, etc. While this isn't a full cleanup, it's a good start. As well, we add a couple of features for making debugging watermarks a little easier.

Rebased for latest nightly, new r-bs added + some changes

Lyude (10):
drm/i915/skl: Move per-pipe ddb allocations into crtc states drm/i915/skl: Remove linetime from skl_wm_values drm/i915/gen9: Make skl_wm_level per-plane drm/i915/gen9: Cleanup skl_pipe_wm_active_state drm/i915/gen9: Get rid of redundant watermark values drm/i915/gen9: Add ddb changes to atomic debug output drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable drm/i915/gen9: Add skl_wm_level_equals() drm/i915/gen9: Actually verify WM levels in verify_wm_state() drm/i915/gen9: Don't wrap strings in verify_wm_state()

 drivers/gpu/drm/i915/i915_drv.h      |  10 +-
 drivers/gpu/drm/i915/intel_display.c | 138 ++++++++---
 drivers/gpu/drm/i915/intel_drv.h     |  24 +-
 drivers/gpu/drm/i915/intel_pm.c      | 466 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 5 files changed, 355 insertions(+), 291 deletions(-)
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#1 lore raw

PATCH v3 1/10 drm/i915/skl: Move per-pipe ddb allocations into crtc states

First part of cleaning up all of the skl watermark code. This moves the structures for storing the ddb allocations of each pipe into intel_crtc_state, along with moving the structures for storing the current ddb allocations active on hardware into intel_crtc.

Changes since v1:
- Don't replace alloc->start = alloc->end = 0;

Signed-off-by Lyude <[email protected]>
Reviewed-by Maarten Lankhorst <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  8 +++++---
 drivers/gpu/drm/i915/intel_pm.c      | 40 +++++++++++++++---------------------
 4 files changed, 30 insertions(+), 35 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +0 -1
index fe875b2..ac4287f99 100644
@@ -1641,7 +1641,6 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
 }
 
 struct skl_ddb_allocation {
-	struct skl_ddb_entry pipe[I915_MAX_PIPES];
 	struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
 	struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
 };
drivers/gpu/drm/i915/intel_display.c +9 -7
index 60c699e..7a68cc3 100644
@@ -14246,12 +14246,11 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
 			     unsigned int *crtc_vblank_mask)
 {
 	struct drm_device *dev = state->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_crtc *crtc;
+	struct intel_crtc *intel_crtc;
 	struct drm_crtc_state *old_crtc_state;
-	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
-	struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+	struct intel_crtc_state *cstate;
 	unsigned int updated = 0;
 	bool progress;
 	enum pipe pipe;

@@ -14269,12 +14268,14 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
 		for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
 			bool vbl_wait = false;
 			unsigned int cmask = drm_crtc_mask(crtc);
-			pipe = to_intel_crtc(crtc)->pipe;
+
+			intel_crtc = to_intel_crtc(crtc);
+			cstate = to_intel_crtc_state(crtc->state);
+			pipe = intel_crtc->pipe;
 
 			if (updated & cmask || !crtc->state->active)
 				continue;
-			if (skl_ddb_allocation_overlaps(state, cur_ddb, new_ddb,
-							pipe))
+			if (skl_ddb_allocation_overlaps(state, intel_crtc))
 				continue;
 
 			updated |= cmask;

@@ -14285,7 +14286,8 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
 			 * then we need to wait for a vblank to pass for the
 			 * new ddb allocation to take effect.
 			 */
-			if (!skl_ddb_allocation_equals(cur_ddb, new_ddb, pipe) &&
+			if (!skl_ddb_entry_equal(&cstate->wm.skl.ddb,
+						 &intel_crtc->hw_ddb) &&
 			    !crtc->state->active_changed &&
 			    intel_state->wm_results.dirty_pipes != updated)
 				vbl_wait = true;
drivers/gpu/drm/i915/intel_drv.h +5 -3
index 5bc1154..10a0cf2 100644
@@ -496,6 +496,7 @@ struct intel_crtc_wm_state {
 		struct {
 			/* gen9+ only needs 1-step wm programming */
 			struct skl_pipe_wm optimal;
+			struct skl_ddb_entry ddb;
 
 			/* cached plane data rate */
 			unsigned plane_data_rate[I915_MAX_PLANES];

@@ -733,6 +734,9 @@ struct intel_crtc {
 		bool cxsr_allowed;
 	} wm;
 
+	/* gen9+: ddb allocation currently being used */
+	struct skl_ddb_entry hw_ddb;
+
 	int scanline_offset;
 
 	struct {

@@ -1755,9 +1759,7 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
 			       const struct skl_ddb_allocation *new,
 			       enum pipe pipe);
 bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
-				 const struct skl_ddb_allocation *old,
-				 const struct skl_ddb_allocation *new,
-				 enum pipe pipe);
+				 struct intel_crtc *intel_crtc);
 void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
 			 const struct skl_wm_values *wm);
 void skl_write_plane_wm(struct intel_crtc *intel_crtc,
drivers/gpu/drm/i915/intel_pm.c +16 -24
index 48b3904..66586af 100644
@@ -3062,7 +3062,6 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 	struct drm_crtc *for_crtc = cstate->base.crtc;
 	unsigned int pipe_size, ddb_size;
 	int nth_active_pipe;
-	int pipe = to_intel_crtc(for_crtc)->pipe;
 
 	if (WARN_ON(!state) || !cstate->base.active) {
 		alloc->start = 0;

@@ -3090,7 +3089,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 	 * we currently hold.
 	 */
 	if (!intel_state->active_pipe_changes) {
-		*alloc = dev_priv->wm.skl_hw.ddb.pipe[pipe];
+		*alloc = to_intel_crtc(for_crtc)->hw_ddb;
 		return;
 	}
 

@@ -3358,7 +3357,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	struct drm_plane *plane;
 	struct drm_plane_state *pstate;
 	enum pipe pipe = intel_crtc->pipe;
-	struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
+	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
 	uint16_t alloc_size, start, cursor_blocks;
 	uint16_t *minimum = cstate->wm.skl.minimum_blocks;
 	uint16_t *y_minimum = cstate->wm.skl.minimum_y_blocks;

@@ -3374,7 +3373,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		return 0;
 
 	if (!cstate->base.active) {
-		ddb->pipe[pipe].start = ddb->pipe[pipe].end = 0;
+		alloc->start = alloc->end = 0;
 		return 0;
 	}
 

@@ -3901,14 +3900,6 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
 			    &wm->ddb.plane[pipe][PLANE_CURSOR]);
 }
 
-bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
-			       const struct skl_ddb_allocation *new,
-			       enum pipe pipe)
-{
-	return new->pipe[pipe].start == old->pipe[pipe].start &&
-	       new->pipe[pipe].end == old->pipe[pipe].end;
-}
-
 static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
 					   const struct skl_ddb_entry *b)
 {

@@ -3916,22 +3907,22 @@ static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
 }
 
 bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
-				 const struct skl_ddb_allocation *old,
-				 const struct skl_ddb_allocation *new,
-				 enum pipe pipe)
+				 struct intel_crtc *intel_crtc)
 {
-	struct drm_device *dev = state->dev;
-	struct intel_crtc *intel_crtc;
-	enum pipe otherp;
+	struct drm_crtc *other_crtc;
+	struct drm_crtc_state *other_cstate;
+	struct intel_crtc *other_intel_crtc;
+	const struct skl_ddb_entry *ddb =
+		&to_intel_crtc_state(intel_crtc->base.state)->wm.skl.ddb;
+	int i;
 
-	for_each_intel_crtc(dev, intel_crtc) {
-		otherp = intel_crtc->pipe;
+	for_each_crtc_in_state(state, other_crtc, other_cstate, i) {
+		other_intel_crtc = to_intel_crtc(other_crtc);
 
-		if (otherp == pipe)
+		if (other_intel_crtc == intel_crtc)
 			continue;
 
-		if (skl_ddb_entries_overlap(&new->pipe[pipe],
-					    &old->pipe[otherp]))
+		if (skl_ddb_entries_overlap(ddb, &other_intel_crtc->hw_ddb))
 			return true;
 	}
 

@@ -4096,7 +4087,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
 	memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
 	       sizeof(dst->plane_trans[pipe]));
 
-	dst->ddb.pipe[pipe] = src->ddb.pipe[pipe];
 	memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
 	       sizeof(dst->ddb.y_plane[pipe]));
 	memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],

@@ -4204,6 +4194,8 @@ static void skl_update_wm(struct drm_crtc *crtc)
 
 	skl_copy_wm_for_pipe(hw_vals, results, pipe);
 
+	intel_crtc->hw_ddb = cstate->wm.skl.ddb;
+
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#2 lore raw

PATCH v3 2/10 drm/i915/skl: Remove linetime from skl_wm_values

In reply to Lyude

Next part of cleaning up the watermark code for skl. This is easy, since it seems that we never actually needed to keep track of the linetime in the skl_wm_values struct anyway.

Signed-off-by Lyude <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Reviewed-by Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h      | 1 -
 drivers/gpu/drm/i915/intel_display.c | 6 ++++--
 drivers/gpu/drm/i915/intel_pm.c      | 7 +------
 3 files changed, 5 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +0 -1
index ac4287f99..e56d4a4 100644
@@ -1648,7 +1648,6 @@ struct skl_ddb_allocation {
 struct skl_wm_values {
 	unsigned dirty_pipes;
 	struct skl_ddb_allocation ddb;
-	uint32_t wm_linetime[I915_MAX_PIPES];
 	uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
 	uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
 };
drivers/gpu/drm/i915/intel_display.c +4 -2
index 7a68cc3..ced70ad 100644
@@ -14844,6 +14844,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *intel_cstate =
+		to_intel_crtc_state(crtc->state);
 	struct intel_crtc_state *old_intel_state =
 		to_intel_crtc_state(old_crtc_state);
 	bool modeset = needs_modeset(crtc->state);

@@ -14860,13 +14862,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 		intel_color_load_luts(crtc->state);
 	}
 
-	if (to_intel_crtc_state(crtc->state)->update_pipe)
+	if (intel_cstate->update_pipe)
 		intel_update_pipe_config(intel_crtc, old_intel_state);
 	else if (INTEL_GEN(dev_priv) >= 9) {
 		skl_detach_scalers(intel_crtc);
 
 		I915_WRITE(PIPE_WM_LINETIME(pipe),
-			   dev_priv->wm.skl_hw.wm_linetime[pipe]);
+			   intel_cstate->wm.skl.optimal.linetime);
 	}
 }
drivers/gpu/drm/i915/intel_pm.c +1 -6
index 66586af..a7d5721 100644
@@ -3845,8 +3845,6 @@ static void skl_compute_wm_results(struct drm_device *dev,
 		temp |= PLANE_WM_EN;
 
 	r->plane_trans[pipe][PLANE_CURSOR] = temp;
-
-	r->wm_linetime[pipe] = p_wm->linetime;
 }
 
 static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,

@@ -4081,7 +4079,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
 		     struct skl_wm_values *src,
 		     enum pipe pipe)
 {
-	dst->wm_linetime[pipe] = src->wm_linetime[pipe];
 	memcpy(dst->plane[pipe], src->plane[pipe],
 	       sizeof(dst->plane[pipe]));
 	memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],

@@ -4332,8 +4329,6 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 
 	max_level = ilk_wm_max_level(dev_priv);
 
-	hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
-
 	for (level = 0; level <= max_level; level++) {
 		for (i = 0; i < intel_num_planes(intel_crtc); i++)
 			hw->plane[pipe][i][level] =

@@ -4350,7 +4345,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 
 	hw->dirty_pipes |= drm_crtc_mask(crtc);
 
-	active->linetime = hw->wm_linetime[pipe];
+	active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
 
 	for (level = 0; level <= max_level; level++) {
 		for (i = 0; i < intel_num_planes(intel_crtc); i++) {
Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#3 lore raw

PATCH v3 3/10 drm/i915/gen9: Make skl_wm_level per-plane

In reply to Lyude

Having skl_wm_level contain all of the watermarks for each plane is annoying since it prevents us from having any sort of object to represent a single watermark level, something we take advantage of in the next commit to cut down on all of the copy paste code in here.

Changes since v1:
- Style nitpicks
- Fix accidental usage of i vs. PLANE_CURSOR
- Split out skl_pipe_wm_active_state simplification into seperate patch

Signed-off-by Lyude <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Reviewed-by Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h  |   6 +-
 drivers/gpu/drm/i915/intel_drv.h |   6 +-
 drivers/gpu/drm/i915/intel_pm.c  | 207 +++++++++++++++++++--------------------
 3 files changed, 111 insertions(+), 108 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +3 -3
index e56d4a4..4d1133f 100644
@@ -1653,9 +1653,9 @@ struct skl_wm_values {
 };
 
 struct skl_wm_level {
-	bool plane_en[I915_MAX_PLANES];
-	uint16_t plane_res_b[I915_MAX_PLANES];
-	uint8_t plane_res_l[I915_MAX_PLANES];
+	bool plane_en;
+	uint16_t plane_res_b;
+	uint8_t plane_res_l;
 };
 
 /*
drivers/gpu/drm/i915/intel_drv.h +5 -1
index 10a0cf2..84301d3 100644
@@ -468,9 +468,13 @@ struct intel_pipe_wm {
 	bool sprites_scaled;
 };
 
-struct skl_pipe_wm {
+struct skl_plane_wm {
 	struct skl_wm_level wm[8];
 	struct skl_wm_level trans_wm;
+};
+
+struct skl_pipe_wm {
+	struct skl_plane_wm planes[I915_MAX_PLANES];
 	uint32_t linetime;
 };
drivers/gpu/drm/i915/intel_pm.c +103 -104

293 lines not on this page · raw message

Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#4 lore raw

PATCH v2 4/10 drm/i915/gen9: Cleanup skl_pipe_wm_active_state

In reply to Lyude

This function is a wreck, let's help it get its life back together and cleanup all of the copy pasta here.

Signed-off-by Lyude <[email protected]>
Reviewed-by Maarten Lankhorst <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_pm.c | 52 +++++++++++------------------------------
 1 file changed, 14 insertions(+), 38 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +14 -38
index 1bdccc9..2d63a37 100644
@@ -4271,46 +4271,22 @@ static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
 static void skl_pipe_wm_active_state(uint32_t val,
 				     struct skl_pipe_wm *active,
 				     bool is_transwm,
-				     bool is_cursor,
 				     int i,
 				     int level)
 {
+	struct skl_plane_wm *plane_wm = &active->planes[i];
 	bool is_enabled = (val & PLANE_WM_EN) != 0;
 
 	if (!is_transwm) {
-		if (!is_cursor) {
-			active->planes[i].wm[level].plane_en = is_enabled;
-			active->planes[i].wm[level].plane_res_b =
-				val & PLANE_WM_BLOCKS_MASK;
-			active->planes[i].wm[level].plane_res_l =
-				(val >> PLANE_WM_LINES_SHIFT) &
-				PLANE_WM_LINES_MASK;
-		} else {
-			active->planes[PLANE_CURSOR].wm[level].plane_en =
-				is_enabled;
-			active->planes[PLANE_CURSOR].wm[level].plane_res_b =
-				val & PLANE_WM_BLOCKS_MASK;
-			active->planes[PLANE_CURSOR].wm[level].plane_res_l =
-				(val >> PLANE_WM_LINES_SHIFT) &
-				PLANE_WM_LINES_MASK;
-		}
+		plane_wm->wm[level].plane_en = is_enabled;
+		plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+		plane_wm->wm[level].plane_res_l =
+			(val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
 	} else {
-		if (!is_cursor) {
-			active->planes[i].trans_wm.plane_en = is_enabled;
-			active->planes[i].trans_wm.plane_res_b =
-				val & PLANE_WM_BLOCKS_MASK;
-			active->planes[i].trans_wm.plane_res_l =
-				(val >> PLANE_WM_LINES_SHIFT) &
-				PLANE_WM_LINES_MASK;
-		} else {
-			active->planes[PLANE_CURSOR].trans_wm.plane_en =
-				is_enabled;
-			active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
-				val & PLANE_WM_BLOCKS_MASK;
-			active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
-				(val >> PLANE_WM_LINES_SHIFT) &
-				PLANE_WM_LINES_MASK;
-		}
+		plane_wm->trans_wm.plane_en = is_enabled;
+		plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+		plane_wm->trans_wm.plane_res_l =
+			(val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
 	}
 }
 

@@ -4349,20 +4325,20 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 	for (level = 0; level <= max_level; level++) {
 		for (i = 0; i < intel_num_planes(intel_crtc); i++) {
 			temp = hw->plane[pipe][i][level];
-			skl_pipe_wm_active_state(temp, active, false,
-						false, i, level);
+			skl_pipe_wm_active_state(temp, active, false, i, level);
 		}
 		temp = hw->plane[pipe][PLANE_CURSOR][level];
-		skl_pipe_wm_active_state(temp, active, false, true, i, level);
+		skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
+					 level);
 	}
 
 	for (i = 0; i < intel_num_planes(intel_crtc); i++) {
 		temp = hw->plane_trans[pipe][i];
-		skl_pipe_wm_active_state(temp, active, true, false, i, 0);
+		skl_pipe_wm_active_state(temp, active, true, i, 0);
 	}
 
 	temp = hw->plane_trans[pipe][PLANE_CURSOR];
-	skl_pipe_wm_active_state(temp, active, true, true, i, 0);
+	skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
 
 	intel_crtc->wm.active.skl = *active;
 }
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#5 lore raw

PATCH v3 5/10 drm/i915/gen9: Get rid of redundant watermark values

In reply to Lyude

Now that we've make skl_wm_levels make a little more sense, we can remove all of the redundant wm information. Up until now we'd been storing two copies of all of the skl watermarks: one being the skl_pipe_wm structs, the other being the global wm struct in drm_i915_private containing the raw register values. This is confusing and problematic, since it means we're prone to accidentally letting the two copies go out of sync. So, get rid of all of the functions responsible for computing the register values and just use a single helper, skl_write_wm_level(), to convert and write the new watermarks on the fly.

Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl Changes since v2:
- Fix usage of wrong cstate

Signed-off-by Lyude <[email protected]>
Reviewed-by Maarten Lankhorst <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h      |   2 -
 drivers/gpu/drm/i915/intel_display.c |  14 ++-
 drivers/gpu/drm/i915/intel_drv.h     |   6 +-
 drivers/gpu/drm/i915/intel_pm.c      | 204 ++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 5 files changed, 91 insertions(+), 143 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +0 -2
index 4d1133f..929b643 100644
@@ -1648,8 +1648,6 @@ struct skl_ddb_allocation {
 struct skl_wm_values {
 	unsigned dirty_pipes;
 	struct skl_ddb_allocation ddb;
-	uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
-	uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
 };
 
 struct skl_wm_level {
drivers/gpu/drm/i915/intel_display.c +11 -3
index ced70ad..18e82b4 100644
@@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	const struct skl_plane_wm *p_wm =
+		&crtc_state->wm.skl.optimal.planes[0];
 	int pipe = intel_crtc->pipe;
 	u32 plane_ctl;
 	unsigned int rotation = plane_state->base.rotation;

@@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_y = src_y;
 
 	if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
-		skl_write_plane_wm(intel_crtc, wm, 0);
+		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
 
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);

@@ -3448,6 +3450,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+	const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0];
 	int pipe = intel_crtc->pipe;
 
 	/*

@@ -3455,7 +3459,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	 * plane's visiblity isn't actually changing neither is its watermarks.
 	 */
 	if (!crtc->primary->state->visible)
-		skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
+		skl_write_plane_wm(intel_crtc, p_wm,
+				   &dev_priv->wm.skl_results.ddb, 0);
 
 	I915_WRITE(PLANE_CTL(pipe, 0), 0);
 	I915_WRITE(PLANE_SURF(pipe, 0), 0);

@@ -10826,12 +10831,15 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	const struct skl_plane_wm *p_wm =
+		&cstate->wm.skl.optimal.planes[PLANE_CURSOR];
 	int pipe = intel_crtc->pipe;
 	uint32_t cntl = 0;
 
 	if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
-		skl_write_cursor_wm(intel_crtc, wm);
+		skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
 
 	if (plane_state && plane_state->base.visible) {
 		cntl = MCURSOR_GAMMA_ENABLE;
drivers/gpu/drm/i915/intel_drv.h +4 -2
index 84301d3..a85ce2c 100644
@@ -1765,9 +1765,11 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
 bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
 				 struct intel_crtc *intel_crtc);
 void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
-			 const struct skl_wm_values *wm);
+			 const struct skl_plane_wm *wm,
+			 const struct skl_ddb_allocation *ddb);
 void skl_write_plane_wm(struct intel_crtc *intel_crtc,
-			const struct skl_wm_values *wm,
+			const struct skl_plane_wm *wm,
+			const struct skl_ddb_allocation *ddb,
 			int plane);
 uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
 bool ilk_disable_lp_wm(struct drm_device *dev);
drivers/gpu/drm/i915/intel_pm.c +70 -134

311 lines not on this page · raw message

drivers/gpu/drm/i915/intel_sprite.c +6 -2
index c7d9a20..e7d2cff 100644
@@ -208,6 +208,8 @@ skl_update_plane(struct drm_plane *drm_plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
+	const struct skl_plane_wm *p_wm =
+		&crtc_state->wm.skl.optimal.planes[plane];
 	u32 plane_ctl;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 surf_addr = plane_state->main.offset;

@@ -232,7 +234,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
 	if (wm->dirty_pipes & drm_crtc_mask(crtc))
-		skl_write_plane_wm(intel_crtc, wm, plane);
+		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, plane);
 
 	if (key->flags) {
 		I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);

@@ -289,6 +291,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
 

@@ -298,7 +301,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	 */
 	if (!dplane->state->visible)
 		skl_write_plane_wm(to_intel_crtc(crtc),
-				   &dev_priv->wm.skl_results, plane);
+				   &cstate->wm.skl.optimal.planes[plane],
+				   &dev_priv->wm.skl_results.ddb, plane);
 
 	I915_WRITE(PLANE_CTL(pipe, plane), 0);
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#6 lore raw

PATCH v4 5/10 drm/i915/gen9: Get rid of redundant watermark values

From: "[email protected]" <[email protected]>

Now that we've make skl_wm_levels make a little more sense, we can remove all of the redundant wm information. Up until now we'd been storing two copies of all of the skl watermarks: one being the skl_pipe_wm structs, the other being the global wm struct in drm_i915_private containing the raw register values. This is confusing and problematic, since it means we're prone to accidentally letting the two copies go out of sync. So, get rid of all of the functions responsible for computing the register values and just use a single helper, skl_write_wm_level(), to convert and write the new watermarks on the fly.

Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl Changes since v2:
- Fix usage of wrong cstate
Changes since v3 (by Paulo):
- Rebase

Reviewed-by Maarten Lankhorst <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]> (v2)
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
Signed-off-by Lyude <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h      |   2 -
 drivers/gpu/drm/i915/intel_display.c |  14 ++-
 drivers/gpu/drm/i915/intel_drv.h     |   6 +-
 drivers/gpu/drm/i915/intel_pm.c      | 203 ++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 5 files changed, 90 insertions(+), 143 deletions(-)

I just merged my fixes, and that caused conflicts on the part of this patch that touches intel_can_enable_sagv(). So I decided to just resolve the conflicts since I created them.

drivers/gpu/drm/i915/i915_drv.h +0 -2
index 33c44c6..456bab9 100644
@@ -1648,8 +1648,6 @@ struct skl_ddb_allocation {
 struct skl_wm_values {
 	unsigned dirty_pipes;
 	struct skl_ddb_allocation ddb;
-	uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
-	uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
 };
 
 struct skl_wm_level {
drivers/gpu/drm/i915/intel_display.c +11 -3
index 9395878..0e18cf47 100644
@@ -3385,6 +3385,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	const struct skl_plane_wm *p_wm =
+		&crtc_state->wm.skl.optimal.planes[0];
 	int pipe = intel_crtc->pipe;
 	u32 plane_ctl;
 	unsigned int rotation = plane_state->base.rotation;

@@ -3421,7 +3423,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_y = src_y;
 
 	if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
-		skl_write_plane_wm(intel_crtc, wm, 0);
+		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
 
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);

@@ -3455,6 +3457,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+	const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0];
 	int pipe = intel_crtc->pipe;
 
 	/*

@@ -3462,7 +3466,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	 * plane's visiblity isn't actually changing neither is its watermarks.
 	 */
 	if (!crtc->primary->state->visible)
-		skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
+		skl_write_plane_wm(intel_crtc, p_wm,
+				   &dev_priv->wm.skl_results.ddb, 0);
 
 	I915_WRITE(PLANE_CTL(pipe, 0), 0);
 	I915_WRITE(PLANE_SURF(pipe, 0), 0);

@@ -10833,12 +10838,15 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	const struct skl_plane_wm *p_wm =
+		&cstate->wm.skl.optimal.planes[PLANE_CURSOR];
 	int pipe = intel_crtc->pipe;
 	uint32_t cntl = 0;
 
 	if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
-		skl_write_cursor_wm(intel_crtc, wm);
+		skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
 
 	if (plane_state && plane_state->base.visible) {
 		cntl = MCURSOR_GAMMA_ENABLE;
drivers/gpu/drm/i915/intel_drv.h +4 -2
index 5760420..a158d7e 100644
@@ -1773,9 +1773,11 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
 bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
 				 struct intel_crtc *intel_crtc);
 void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
-			 const struct skl_wm_values *wm);
+			 const struct skl_plane_wm *wm,
+			 const struct skl_ddb_allocation *ddb);
 void skl_write_plane_wm(struct intel_crtc *intel_crtc,
-			const struct skl_wm_values *wm,
+			const struct skl_plane_wm *wm,
+			const struct skl_ddb_allocation *ddb,
 			int plane);
 uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
 bool ilk_disable_lp_wm(struct drm_device *dev);
drivers/gpu/drm/i915/intel_pm.c +69 -134

312 lines not on this page · raw message

drivers/gpu/drm/i915/intel_sprite.c +6 -2
index c7d9a20..e7d2cff 100644
@@ -208,6 +208,8 @@ skl_update_plane(struct drm_plane *drm_plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
+	const struct skl_plane_wm *p_wm =
+		&crtc_state->wm.skl.optimal.planes[plane];
 	u32 plane_ctl;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 surf_addr = plane_state->main.offset;

@@ -232,7 +234,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
 	if (wm->dirty_pipes & drm_crtc_mask(crtc))
-		skl_write_plane_wm(intel_crtc, wm, plane);
+		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, plane);
 
 	if (key->flags) {
 		I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);

@@ -289,6 +291,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
 

@@ -298,7 +301,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	 */
 	if (!dplane->state->visible)
 		skl_write_plane_wm(to_intel_crtc(crtc),
-				   &dev_priv->wm.skl_results, plane);
+				   &cstate->wm.skl.optimal.planes[plane],
+				   &dev_priv->wm.skl_results.ddb, plane);
 
 	I915_WRITE(PLANE_CTL(pipe, plane), 0);
Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#7 lore raw

PATCH v3 6/10 drm/i915/gen9: Add ddb changes to atomic debug output

In reply to Lyude

Finally, add some debugging output for ddb changes in the atomic debug output. This makes it a lot easier to spot bugs from incorrect ddb allocations.

Signed-off-by Lyude <[email protected]>
Reviewed-by Maarten Lankhorst <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_pm.c | 54 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
drivers/gpu/drm/i915/intel_pm.c +54 -0
index 5df5cea..2fe851e 100644
@@ -4044,6 +4044,58 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
 	       sizeof(dst->ddb.plane[pipe]));
 }
 
+static void
+skl_print_wm_changes(const struct drm_atomic_state *state)
+{
+	const struct drm_device *dev = state->dev;
+	const struct drm_i915_private *dev_priv = to_i915(dev);
+	const struct intel_atomic_state *intel_state =
+		to_intel_atomic_state(state);
+	const struct drm_crtc *crtc;
+	const struct drm_crtc_state *cstate;
+	const struct drm_plane *plane;
+	const struct intel_plane *intel_plane;
+	const struct drm_plane_state *pstate;
+	const struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb;
+	const struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+	enum pipe pipe;
+	int id;
+	int i, j;
+
+	for_each_crtc_in_state(state, crtc, cstate, i) {
+		pipe = to_intel_crtc(crtc)->pipe;
+
+		for_each_plane_in_state(state, plane, pstate, j) {
+			const struct skl_ddb_entry *old, *new;
+
+			intel_plane = to_intel_plane(plane);
+			id = skl_wm_plane_id(intel_plane);
+			old = &old_ddb->plane[pipe][id];
+			new = &new_ddb->plane[pipe][id];
+
+			if (intel_plane->pipe != pipe)
+				continue;
+
+			if (skl_ddb_entry_equal(old, new))
+				continue;
+
+			if (id != PLANE_CURSOR) {
+				DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c] ddb (%d - %d) -> (%d - %d)\n",
+						 plane->base.id, id + 1,
+						 pipe_name(pipe),
+						 old->start, old->end,
+						 new->start, new->end);
+			} else {
+				DRM_DEBUG_ATOMIC("[PLANE:%d:cursor %c] ddb (%d - %d) -> (%d - %d)\n",
+						 plane->base.id,
+						 pipe_name(pipe),
+						 old->start, old->end,
+						 new->start, new->end);
+			}
+		}
+	}
+}
+
 static int
 skl_compute_wm(struct drm_atomic_state *state)
 {

@@ -4105,6 +4157,8 @@ skl_compute_wm(struct drm_atomic_state *state)
 		intel_cstate->update_wm_pre = true;
 	}
 
+	skl_print_wm_changes(state);
+
 	return 0;
 }
Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#8 lore raw

PATCH v2 7/10 drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable

In reply to Lyude

There's not much of a reason this should have the locations to read out the hardware state hardcoded, so allow the caller to specify the location and add this function to intel_drv.h. As well, we're going to need this function to be reusable for the next patch.

Changes since v1:
- Fix accidental behavior change in the code that Paulo pointed out

Signed-off-by Lyude <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c  | 28 ++++++++++++++++++----------
 2 files changed, 20 insertions(+), 10 deletions(-)
drivers/gpu/drm/i915/intel_drv.h +2 -0
index a85ce2c..7036310 100644
@@ -1756,6 +1756,8 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
 void skl_wm_get_hw_state(struct drm_device *dev);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */);
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+			      struct skl_pipe_wm *out);
 bool intel_can_enable_sagv(struct drm_atomic_state *state);
 int intel_enable_sagv(struct drm_i915_private *dev_priv);
 int intel_disable_sagv(struct drm_i915_private *dev_priv);
drivers/gpu/drm/i915/intel_pm.c +18 -10
index 2fe851e..6eaeb87 100644
@@ -4288,15 +4288,13 @@ static inline void skl_wm_level_from_reg_val(uint32_t val,
 		PLANE_WM_LINES_MASK;
 }
 
-static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+			      struct skl_pipe_wm *out)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	struct intel_plane *intel_plane;
-	struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
 	struct skl_plane_wm *wm;
 	enum pipe pipe = intel_crtc->pipe;
 	int level, id, max_level;

@@ -4306,7 +4304,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 
 	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
 		id = skl_wm_plane_id(intel_plane);
-		wm = &cstate->wm.skl.optimal.planes[id];
+		wm = &out->planes[id];
 
 		for (level = 0; level <= max_level; level++) {
 			if (id != PLANE_CURSOR)

@@ -4328,20 +4326,30 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	hw->dirty_pipes |= drm_crtc_mask(crtc);
-	active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
-	intel_crtc->wm.active.skl = *active;
+	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
 }
 
 void skl_wm_get_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
 	struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
 	struct drm_crtc *crtc;
+	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *cstate;
 
 	skl_ddb_get_hw_state(dev_priv, ddb);
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		skl_pipe_wm_get_hw_state(crtc);
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		intel_crtc = to_intel_crtc(crtc);
+		cstate = to_intel_crtc_state(crtc->state);
+
+		skl_pipe_wm_get_hw_state(crtc, &cstate->wm.skl.optimal);
+
+		if (intel_crtc->active) {
+			hw->dirty_pipes |= drm_crtc_mask(crtc);
+			intel_crtc->wm.active.skl = cstate->wm.skl.optimal;
+		}
+	}
 
 	if (dev_priv->active_crtcs) {
 		/* Fully recompute DDB on first atomic commit */
Signature · 1 line
2.7.4
Reviewed-by on v2 7/10
#9 lore raw

Re: [PATCH v2 07/10] drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable

Em Sex, 2016-10-14 às 17:31 -0400, Lyude escreveu:

Quoted 9 lines

There's not much of a reason this should have the locations to read out
the hardware state hardcoded, so allow the caller to specify the location and add this function to intel_drv.h. As well, we're going to
need this function to be reusable for the next patch.

Changes since v1:
- Fix accidental behavior change in the code that Paulo pointed out

Reviewed-by Paulo Zanoni <[email protected]>

I just submitted v4 of patch 5 solving the conflicts I created. With that + this review, we can merge this series. If you give me an ack on patch 5 I can just go and merge these, so we can move to Maarten's series and then later to Mahesh's series.

Quoted 98 lines
Signed-off-by Lyude <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>

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

 2 files changed, 20 insertions(+), 10 deletions(-)
drivers/gpu/drm/i915/intel_drv.h +2 -0
index a85ce2c..7036310 100644
@@ -1756,6 +1756,8 @@ void ilk_wm_get_hw_state(struct drm_device
 *dev);
  void skl_wm_get_hw_state(struct drm_device *dev);
  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
  			  struct skl_ddb_allocation *ddb /* out */);
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+			      struct skl_pipe_wm *out);
  bool intel_can_enable_sagv(struct drm_atomic_state *state);
  int intel_enable_sagv(struct drm_i915_private *dev_priv);

 int intel_disable_sagv(struct drm_i915_private *dev_priv);

drivers/gpu/drm/i915/intel_pm.c +0 -0
index 2fe851e..6eaeb87 100644
@@ -4288,15 +4288,13 @@ static inline void
 skl_wm_level_from_reg_val(uint32_t val,
  		PLANE_WM_LINES_MASK;
  }

drivers/gpu/drm/i915/intel_pm.c +2 -1
-static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+			      struct skl_pipe_wm *out)

 {
  struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev);

drivers/gpu/drm/i915/intel_pm.c +0 -1
-	struct skl_wm_values *hw = &dev_priv->wm.skl_hw;

  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

drivers/gpu/drm/i915/intel_pm.c +0 -1
-	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc-
Quoted 1 line

state);

  struct intel_plane *intel_plane;

drivers/gpu/drm/i915/intel_pm.c +0 -1
-	struct skl_pipe_wm *active = &cstate->wm.skl.optimal;

  struct skl_plane_wm *wm;
  enum pipe pipe = intel_crtc->pipe;
  int level, id, max_level;

drivers/gpu/drm/i915/intel_pm.c +0 -0
@@ -4306,7 +4304,7 @@ static void skl_pipe_wm_get_hw_state(struct
 drm_crtc *crtc)


  for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { id = skl_wm_plane_id(intel_plane);

drivers/gpu/drm/i915/intel_pm.c +1 -1
-		wm = &cstate->wm.skl.optimal.planes[id];
+		wm = &out->planes[id];


  for (level = 0; level <= max_level; level++) { if (id != PLANE_CURSOR)

drivers/gpu/drm/i915/intel_pm.c +0 -0
@@ -4328,20 +4326,30 @@ static void skl_pipe_wm_get_hw_state(struct
 drm_crtc *crtc)
  	if (!intel_crtc->active)
  		return;

drivers/gpu/drm/i915/intel_pm.c +1 -3
-	hw->dirty_pipes |= drm_crtc_mask(crtc);
-	active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
-	intel_crtc->wm.active.skl = *active;
+	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));

 }

 void skl_wm_get_hw_state(struct drm_device *dev) {
  struct drm_i915_private *dev_priv = to_i915(dev);

drivers/gpu/drm/i915/intel_pm.c +1 -0
+	struct skl_wm_values *hw = &dev_priv->wm.skl_hw;

  struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb; struct drm_crtc *crtc;

drivers/gpu/drm/i915/intel_pm.c +2 -0
+	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *cstate;


  skl_ddb_get_hw_state(dev_priv, ddb);

drivers/gpu/drm/i915/intel_pm.c +1 -2
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		skl_pipe_wm_get_hw_state(crtc);
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)

{

drivers/gpu/drm/i915/intel_pm.c +4 -0
+		intel_crtc = to_intel_crtc(crtc);
+		cstate = to_intel_crtc_state(crtc->state);
+
+		skl_pipe_wm_get_hw_state(crtc, &cstate-
Quoted 1 line

wm.skl.optimal);

drivers/gpu/drm/i915/intel_pm.c +4 -0
+
+		if (intel_crtc->active) {
+			hw->dirty_pipes |= drm_crtc_mask(crtc);
+			intel_crtc->wm.active.skl = cstate-
Quoted 1 line

wm.skl.optimal;

drivers/gpu/drm/i915/intel_pm.c +2 -0
+		}
+	}
Quoted 3 lines


  if (dev_priv->active_crtcs) {
  /* Fully recompute DDB on first atomic commit */

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#10 lore raw

PATCH v2 8/10 drm/i915/gen9: Add skl_wm_level_equals()

In reply to Lyude

Helper we're going to be using for implementing verification of the wm levels in skl_verify_wm_level().

Signed-off-by Lyude <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c  | 14 ++++++++++++++
 2 files changed, 16 insertions(+)
drivers/gpu/drm/i915/intel_drv.h +2 -0
index 7036310..96963ea 100644
@@ -1761,6 +1761,8 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
 bool intel_can_enable_sagv(struct drm_atomic_state *state);
 int intel_enable_sagv(struct drm_i915_private *dev_priv);
 int intel_disable_sagv(struct drm_i915_private *dev_priv);
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+			 const struct skl_wm_level *l2);
 bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
 			       const struct skl_ddb_allocation *new,
 			       enum pipe pipe);
drivers/gpu/drm/i915/intel_pm.c +14 -0
index 6eaeb87..fae3ce4 100644
@@ -3857,6 +3857,20 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
 			    &ddb->plane[pipe][PLANE_CURSOR]);
 }
 
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+			 const struct skl_wm_level *l2)
+{
+	if (l1->plane_en != l2->plane_en)
+		return false;
+
+	/* If both planes aren't enabled, the rest shouldn't matter */
+	if (!l1->plane_en)
+		return true;
+
+	return (l1->plane_res_l == l2->plane_res_l &&
+		l1->plane_res_b == l2->plane_res_b);
+}
+
 static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
 					   const struct skl_ddb_entry *b)
 {
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#11 lore raw

PATCH v2 9/10 drm/i915/gen9: Actually verify WM levels in verify_wm_state()

In reply to Lyude

Thanks to Paulo Zanoni for indirectly pointing this out.

Looks like we never actually added any code for checking whether or not we actually wrote watermark levels properly. Let's fix that.

Changes since v1:
- Use %u instead of %d when printing WM state mismatches

Signed-off-by Lyude <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_display.c | 100 +++++++++++++++++++++++++++++------
 1 file changed, 84 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_display.c +84 -16
index 18e82b4..ca12f0e 100644
@@ -13455,30 +13455,66 @@ static void verify_wm_state(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct skl_ddb_allocation hw_ddb, *sw_ddb;
-	struct skl_ddb_entry *hw_entry, *sw_entry;
+	struct skl_pipe_wm hw_wm, *sw_wm;
+	struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
+	struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	const enum pipe pipe = intel_crtc->pipe;
-	int plane;
+	int plane, level, max_level = ilk_wm_max_level(dev_priv);
 
 	if (INTEL_INFO(dev)->gen < 9 || !new_state->active)
 		return;
 
+	skl_pipe_wm_get_hw_state(crtc, &hw_wm);
+	sw_wm = &intel_crtc->wm.active.skl;
+
 	skl_ddb_get_hw_state(dev_priv, &hw_ddb);
 	sw_ddb = &dev_priv->wm.skl_hw.ddb;
 
 	/* planes */
 	for_each_plane(dev_priv, pipe, plane) {
-		hw_entry = &hw_ddb.plane[pipe][plane];
-		sw_entry = &sw_ddb->plane[pipe][plane];
+		hw_plane_wm = &hw_wm.planes[plane];
+		sw_plane_wm = &sw_wm->planes[plane];
 
-		if (skl_ddb_entry_equal(hw_entry, sw_entry))
-			continue;
+		/* Watermarks */
+		for (level = 0; level <= max_level; level++) {
+			if (skl_wm_level_equals(&hw_plane_wm->wm[level],
+						&sw_plane_wm->wm[level]))
+				continue;
+
+			DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+				  pipe_name(pipe), plane + 1, level,
+				  sw_plane_wm->wm[level].plane_en,
+				  sw_plane_wm->wm[level].plane_res_b,
+				  sw_plane_wm->wm[level].plane_res_l,
+				  hw_plane_wm->wm[level].plane_en,
+				  hw_plane_wm->wm[level].plane_res_b,
+				  hw_plane_wm->wm[level].plane_res_l);
+		}
 
-		DRM_ERROR("mismatch in DDB state pipe %c plane %d "
-			  "(expected (%u,%u), found (%u,%u))\n",
-			  pipe_name(pipe), plane + 1,
-			  sw_entry->start, sw_entry->end,
-			  hw_entry->start, hw_entry->end);
+		if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
+					 &sw_plane_wm->trans_wm)) {
+			DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+				  pipe_name(pipe), plane + 1,
+				  sw_plane_wm->trans_wm.plane_en,
+				  sw_plane_wm->trans_wm.plane_res_b,
+				  sw_plane_wm->trans_wm.plane_res_l,
+				  hw_plane_wm->trans_wm.plane_en,
+				  hw_plane_wm->trans_wm.plane_res_b,
+				  hw_plane_wm->trans_wm.plane_res_l);
+		}
+
+		/* DDB */
+		hw_ddb_entry = &hw_ddb.plane[pipe][plane];
+		sw_ddb_entry = &sw_ddb->plane[pipe][plane];
+
+		if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
+			DRM_ERROR("mismatch in DDB state pipe %c plane %d "
+				  "(expected (%u,%u), found (%u,%u))\n",
+				  pipe_name(pipe), plane + 1,
+				  sw_ddb_entry->start, sw_ddb_entry->end,
+				  hw_ddb_entry->start, hw_ddb_entry->end);
+		}
 	}
 
 	/*

@@ -13488,15 +13524,47 @@ static void verify_wm_state(struct drm_crtc *crtc,
 	 * once the plane becomes visible, we can skip this check
 	 */
 	if (intel_crtc->cursor_addr) {
-		hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
-		sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
+		hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
+		sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
+
+		/* Watermarks */
+		for (level = 0; level <= max_level; level++) {
+			if (skl_wm_level_equals(&hw_plane_wm->wm[level],
+						&sw_plane_wm->wm[level]))
+				continue;
+
+			DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+				  pipe_name(pipe), level,
+				  sw_plane_wm->wm[level].plane_en,
+				  sw_plane_wm->wm[level].plane_res_b,
+				  sw_plane_wm->wm[level].plane_res_l,
+				  hw_plane_wm->wm[level].plane_en,
+				  hw_plane_wm->wm[level].plane_res_b,
+				  hw_plane_wm->wm[level].plane_res_l);
+		}
+
+		if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
+					 &sw_plane_wm->trans_wm)) {
+			DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+				  pipe_name(pipe),
+				  sw_plane_wm->trans_wm.plane_en,
+				  sw_plane_wm->trans_wm.plane_res_b,
+				  sw_plane_wm->trans_wm.plane_res_l,
+				  hw_plane_wm->trans_wm.plane_en,
+				  hw_plane_wm->trans_wm.plane_res_b,
+				  hw_plane_wm->trans_wm.plane_res_l);
+		}
+
+		/* DDB */
+		hw_ddb_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
+		sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
 
-		if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
+		if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
 			DRM_ERROR("mismatch in DDB state pipe %c cursor "
 				  "(expected (%u,%u), found (%u,%u))\n",
 				  pipe_name(pipe),
-				  sw_entry->start, sw_entry->end,
-				  hw_entry->start, hw_entry->end);
+				  sw_ddb_entry->start, sw_ddb_entry->end,
+				  hw_ddb_entry->start, hw_ddb_entry->end);
 		}
 	}
 }
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#12 lore raw

PATCH v2 10/10 drm/i915/gen9: Don't wrap strings in verify_wm_state()

In reply to Lyude

Wrapping strings is against the guidelines in Documentation/CodingStyle, chapter 2.

Signed-off-by Lyude <[email protected]>
Reviewed-by Paulo Zanoni <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Matt Roper <[email protected]
Cc Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +2 -4
index ca12f0e..eaec9f3 100644
@@ -13509,8 +13509,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
 		sw_ddb_entry = &sw_ddb->plane[pipe][plane];
 
 		if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
-			DRM_ERROR("mismatch in DDB state pipe %c plane %d "
-				  "(expected (%u,%u), found (%u,%u))\n",
+			DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n",
 				  pipe_name(pipe), plane + 1,
 				  sw_ddb_entry->start, sw_ddb_entry->end,
 				  hw_ddb_entry->start, hw_ddb_entry->end);

@@ -13560,8 +13559,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
 		sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
 
 		if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
-			DRM_ERROR("mismatch in DDB state pipe %c cursor "
-				  "(expected (%u,%u), found (%u,%u))\n",
+			DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n",
 				  pipe_name(pipe),
 				  sw_ddb_entry->start, sw_ddb_entry->end,
 				  hw_ddb_entry->start, hw_ddb_entry->end);
Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
CI
#13 lore raw

✗ Fi.CI.BAT: warning for Start of skl watermark cleanup (rev3)

In reply to Lyude
== Series Details ==

== Series Details ==

Series: Start of skl watermark cleanup (rev3)
URL : https://patchwork.freedesktop.org/series/13333/
State : warning

== Summary ==

Series 13333v3 Start of skl watermark cleanup
https://patchwork.freedesktop.org/api/1.0/series/13333/revisions/3/mbox/

Test kms_flip:

        Subgroup basic-flip-vs-wf_vblank:
                dmesg-warn -> PASS       (fi-skl-6700k)
                dmesg-warn -> PASS       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                dmesg-warn -> PASS       (fi-ilk-650)
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-skl-6700k)
Test vgem_basic:
        Subgroup unload:
                pass       -> SKIP       (fi-hsw-4770)
                pass       -> SKIP       (fi-bdw-5557u)
                skip       -> PASS       (fi-skl-6770hq)
fi-bdw-5557u     total:246  pass:230  dwarn:0   dfail:0   fail:0   skip:16 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-n2820     total:246  pass:210  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:184  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:230  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37

fi-byt-j1900 failed to connect after reboot

Results at /archive/results/CI_IGT_test/Patchwork_2729/

38ecbe9082e477953fe165265c76e6c6061aeaf6 drm-intel-nightly: 2016y-10m-14d-16h-23m-48s UTC integration manifest e6ec3db drm/i915/gen9: Don't wrap strings in verify_wm_state()
be9c54e drm/i915/gen9: Actually verify WM levels in verify_wm_state()
158bb87 drm/i915/gen9: Add skl_wm_level_equals()
95570fd drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
1f4ad42 drm/i915/gen9: Add ddb changes to atomic debug output
9fd875d drm/i915/gen9: Get rid of redundant watermark values
aaf254a drm/i915/gen9: Cleanup skl_pipe_wm_active_state
6e1087f drm/i915/gen9: Make skl_wm_level per-plane
b1399ab drm/i915/skl: Remove linetime from skl_wm_values
599c303 drm/i915/skl: Move per-pipe ddb allocations into crtc states

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#14 lore raw

Re: ✗ Fi.CI.BAT: warning for Start of skl watermark cleanup (rev3)

Quoted 12 lines

== Series Details ==

Series: Start of skl watermark cleanup (rev3)
URL : https://patchwork.freedesktop.org/series/13333/
State : warning

== Summary ==

Series 13333v3 Start of skl watermark cleanup
https://patchwork.freedesktop.org/api/1.0/series/13333/revisions/3/mbox/

Test kms_flip:

Quoted 15 lines
        Subgroup basic-flip-vs-wf_vblank:
                dmesg-warn -> PASS       (fi-skl-6700k)
                dmesg-warn -> PASS       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                dmesg-warn -> PASS       (fi-ilk-650)
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-skl-6700k)
Test vgem_basic:
        Subgroup unload:
                pass       -> SKIP       (fi-hsw-4770)
                pass       -> SKIP       (fi-bdw-5557u)
                skip       -> PASS       (fi-skl-6770hq)

Known un-stable. Will be worked on still.

Quoted 21 lines
fi-bdw-5557u     total:246  pass:230  dwarn:0   dfail:0   fail:0   skip:16
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30
fi-byt-n2820     total:246  pass:210  dwarn:0   dfail:0   fail:1   skip:35
fi-hsw-4770      total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22
fi-ilk-650       total:246  pass:184  dwarn:0   dfail:0   fail:2   skip:60
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14
fi-skl-6700hq    total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24
fi-skl-6770hq    total:246  pass:230  dwarn:1   dfail:0   fail:1   skip:14
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37

fi-byt-j1900 failed to connect after reboot

Results at /archive/results/CI_IGT_test/Patchwork_2729/

Quoted 13 lines

38ecbe9082e477953fe165265c76e6c6061aeaf6 drm-intel-nightly: 2016y-10m- 14d-16h-23m-48s UTC integration manifest e6ec3db drm/i915/gen9: Don't wrap strings in verify_wm_state() be9c54e drm/i915/gen9: Actually verify WM levels in verify_wm_state()
158bb87 drm/i915/gen9: Add skl_wm_level_equals() 95570fd drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable 1f4ad42 drm/i915/gen9: Add ddb changes to atomic debug output 9fd875d drm/i915/gen9: Get rid of redundant watermark values aaf254a drm/i915/gen9: Cleanup skl_pipe_wm_active_state 6e1087f drm/i915/gen9:
Make skl_wm_level per-plane b1399ab drm/i915/skl: Remove linetime from skl_wm_values
599c303 drm/i915/skl: Move per-pipe ddb allocations into crtc states

Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#15 lore raw
In reply to Lyude

Pushed patches 1-4 in this series (with some very small style changes to make checkpatch happy), drm-intel-nightly also rebuilt.

Quoted 32 lines · Lyude

While it (mostly) works, the code for handling watermarks on Skylake has been
kind of ugly for a while. As well a lot of it isn't that friendly to atomic
transactions, Lots of copy paste, redundant wm values, etc. While this isn't a
full cleanup, it's a good start. As well, we add a couple of features for
making debugging watermarks a little easier.

Rebased for latest nightly, new r-bs added + some changes

Lyude (10):
  drm/i915/skl: Move per-pipe ddb allocations into crtc states drm/i915/skl: Remove linetime from skl_wm_values drm/i915/gen9: Make skl_wm_level per-plane drm/i915/gen9: Cleanup skl_pipe_wm_active_state drm/i915/gen9: Get rid of redundant watermark values drm/i915/gen9: Add ddb changes to atomic debug output drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable drm/i915/gen9: Add skl_wm_level_equals() drm/i915/gen9: Actually verify WM levels in verify_wm_state() drm/i915/gen9: Don't wrap strings in verify_wm_state()

 drivers/gpu/drm/i915/i915_drv.h      |  10 +- drivers/gpu/drm/i915/intel_display.c | 138 ++++++++--- drivers/gpu/drm/i915/intel_drv.h     |  24 +- drivers/gpu/drm/i915/intel_pm.c      | 466 +++++++++++++++++------
------------
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-

 5 files changed, 355 insertions(+), 291 deletions(-)
Signature · 6 lines
Cheers,
	Lyude
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel