All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] skl+ watermark stuff
@ 2018-12-21 17:14 Ville Syrjala
  2018-12-21 17:14 ` [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+ Ville Syrjala
                   ` (11 more replies)
  0 siblings, 12 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

A few fixes to the watermark/ddb stuff. Noticed while trying to fix
icl. That one is still busted but at least we should more or less
match the spec now. Included a few cleanups etc. at the end as well.

Ville Syrjälä (9):
  drm/i915: Don't ignore level 0 lines watermark for glk+
  drm/i915: Reinstate an early latency==0 check for skl+
  drm/i915: Fix bits vs. bytes mixup in dbuf block size computation
  drm/i915: Fix > vs >= mismatch in watermark/ddb calculations
  drm/i915: Account for minimum ddb allocation restrictions
  drm/i915: Pass dev_priv to skl_needs_memory_bw_wa()
  drm/i915: Drop the definite article in front of SAGV
  drm/i915: Drop the pointless linetime==0 check
  drm/i915: Use IS_GEN9_LP() for the linetime w/a check

 drivers/gpu/drm/i915/i915_drv.h |   1 +
 drivers/gpu/drm/i915/intel_pm.c | 123 ++++++++++++++++++++------------
 2 files changed, 79 insertions(+), 45 deletions(-)

-- 
2.19.2

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

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

* [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-28  8:40   ` Lisovskiy, Stanislav
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+ Ville Syrjala
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

On glk+ the level 0 lines watermark actually matters. Do not ignore it.
And while at it let's change things so that we always program a
consistnet 0 to the register when the lines watermarks is ignored
by the hardware.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2a6ffb8b975a..d132ef10fa60 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4675,6 +4675,15 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
 	return 0;
 }
 
+static bool skl_wm_has_lines(struct drm_i915_private *dev_priv, int level)
+{
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+		return true;
+
+	/* The number of lines are ignored for the level 0 watermark. */
+	return level > 0;
+}
+
 static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 				 const struct intel_plane_state *intel_pstate,
 				 int level,
@@ -4757,8 +4766,10 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 		}
 	}
 
-	/* The number of lines are ignored for the level 0 watermark. */
-	if (level > 0 && res_lines > 31)
+	if (!skl_wm_has_lines(dev_priv, level))
+		res_lines = 0;
+
+	if (res_lines > 31)
 		return;
 
 	/*
-- 
2.19.2

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

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

* [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
  2018-12-21 17:14 ` [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+ Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-28  8:34   ` Lisovskiy, Stanislav
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation Ville Syrjala
                   ` (9 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

I thought we could remove all the early latency==0 checks
and rely on skl_wm_method{1,2}() checking for it. But
skl_compute_plane_wm() applies a bunch of workarounds to bump
up the latency before calling those guys so clearly it won't
end up doing the right thing. Also not sure if the calculations
based on the method1/2 results are safe agaisnt overflows so
it might not work all that well in any case. Let's put the
early check back.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d132ef10fa60..0aac7e7b660f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 		to_intel_atomic_state(cstate->base.state);
 	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
 
+	if (latency == 0)
+		return;
+
 	/* Display WA #1141: kbl,cfl */
 	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
 	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
-- 
2.19.2

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

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

* [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
  2018-12-21 17:14 ` [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+ Ville Syrjala
  2018-12-21 17:14 ` [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+ Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-28  8:43   ` Lisovskiy, Stanislav
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 4/9] drm/i915: Fix > vs >= mismatch in watermark/ddb calculations Ville Syrjala
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

The spec used to say "8bpp" which someone took to mean 8 bytes per
pixel when in fact it was supposed to be 8 bits per pixel. The
spec has been updated to make it more clear now. Fix the code
to match.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0aac7e7b660f..55a1c577f060 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4618,7 +4618,7 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
 							     intel_pstate);
 
 	if (INTEL_GEN(dev_priv) >= 11 &&
-	    fb->modifier == I915_FORMAT_MOD_Yf_TILED && wp->cpp == 8)
+	    fb->modifier == I915_FORMAT_MOD_Yf_TILED && wp->cpp == 1)
 		wp->dbuf_block_size = 256;
 	else
 		wp->dbuf_block_size = 512;
-- 
2.19.2

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

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

* [PATCH 4/9] drm/i915: Fix > vs >= mismatch in watermark/ddb calculations
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 5/9] drm/i915: Account for minimum ddb allocation restrictions Ville Syrjala
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

Bspec says we have to reject the watermark if it's >= the ddb
allocation. Fix the code to reject the == case as it should.
For transition watermarks we can just use >=, for the rest
we'll do +1 when calculating the minimum ddb allocation size.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 55a1c577f060..3c5cba31f055 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4371,8 +4371,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 				continue;
 
 			wm = &cstate->wm.skl.optimal.planes[plane_id];
-			blocks += wm->wm[level].plane_res_b;
-			blocks += wm->uv_wm[level].plane_res_b;
+			blocks += wm->wm[level].plane_res_b + 1;
+			blocks += wm->uv_wm[level].plane_res_b + 1;
 		}
 
 		if (blocks < alloc_size) {
@@ -4413,7 +4413,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		extra = min_t(u16, alloc_size,
 			      DIV64_U64_ROUND_UP(alloc_size * rate,
 						 total_data_rate));
-		total[plane_id] = wm->wm[level].plane_res_b + extra;
+		total[plane_id] = wm->wm[level].plane_res_b + 1 + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
 
@@ -4424,7 +4424,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		extra = min_t(u16, alloc_size,
 			      DIV64_U64_ROUND_UP(alloc_size * rate,
 						 total_data_rate));
-		uv_total[plane_id] = wm->uv_wm[level].plane_res_b + extra;
+		uv_total[plane_id] = wm->uv_wm[level].plane_res_b + 1 + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
 	}
@@ -4477,7 +4477,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	 */
 	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
 		wm = &cstate->wm.skl.optimal.planes[plane_id];
-		if (wm->trans_wm.plane_res_b > total[plane_id])
+		if (wm->trans_wm.plane_res_b >= total[plane_id])
 			memset(&wm->trans_wm, 0, sizeof(wm->trans_wm));
 	}
 
-- 
2.19.2

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

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

* [PATCH 5/9] drm/i915: Account for minimum ddb allocation restrictions
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 4/9] drm/i915: Fix > vs >= mismatch in watermark/ddb calculations Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 6/9] drm/i915: Pass dev_priv to skl_needs_memory_bw_wa() Ville Syrjala
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

On icl+ bspec tells us to calculate a separate minimum ddb
allocation from the blocks watermark. Both have to be checked
against the actual ddb allocation, but since we do things the
other way around we'll just calculat the minimum acceptable
ddb allocation by taking the maximum of the two values.

We'll also replace the memcmp() with a full trawl over the
the watermarks so that it'll ignore the min_ddb_alloc
because we can't directly read that out from the hw. I suppose
we could reconstruct it from the other values, but I was
too lazy to do that now.

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 | 53 +++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 815db160b966..7668d7197994 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1108,6 +1108,7 @@ struct skl_ddb_values {
 };
 
 struct skl_wm_level {
+	uint16_t min_ddb_alloc;
 	uint16_t plane_res_b;
 	uint8_t plane_res_l;
 	bool plane_en;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3c5cba31f055..7464552c05f4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4371,8 +4371,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 				continue;
 
 			wm = &cstate->wm.skl.optimal.planes[plane_id];
-			blocks += wm->wm[level].plane_res_b + 1;
-			blocks += wm->uv_wm[level].plane_res_b + 1;
+			blocks += wm->wm[level].min_ddb_alloc;
+			blocks += wm->uv_wm[level].min_ddb_alloc;
 		}
 
 		if (blocks < alloc_size) {
@@ -4413,7 +4413,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		extra = min_t(u16, alloc_size,
 			      DIV64_U64_ROUND_UP(alloc_size * rate,
 						 total_data_rate));
-		total[plane_id] = wm->wm[level].plane_res_b + 1 + extra;
+		total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
 
@@ -4424,7 +4424,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		extra = min_t(u16, alloc_size,
 			      DIV64_U64_ROUND_UP(alloc_size * rate,
 						 total_data_rate));
-		uv_total[plane_id] = wm->uv_wm[level].plane_res_b + 1 + extra;
+		uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
 		alloc_size -= extra;
 		total_data_rate -= rate;
 	}
@@ -4696,7 +4696,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	uint32_t latency = dev_priv->wm.skl_latency[level];
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
-	uint32_t res_blocks, res_lines;
+	uint32_t res_blocks, res_lines, min_ddb_alloc = 0;
 	struct intel_atomic_state *state =
 		to_intel_atomic_state(cstate->base.state);
 	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
@@ -4769,6 +4769,24 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 		}
 	}
 
+	if (INTEL_GEN(dev_priv) >= 11) {
+		if (wp->y_tiled) {
+			int extra_lines;
+
+			if (res_lines % wp->y_min_scanlines == 0)
+				extra_lines = wp->y_min_scanlines;
+			else
+				extra_lines = wp->y_min_scanlines * 2 -
+					res_lines % wp->y_min_scanlines;
+
+			min_ddb_alloc = mul_round_up_u32_fixed16(res_lines + extra_lines,
+								 wp->plane_blocks_per_line);
+		} else {
+			min_ddb_alloc = res_blocks +
+				DIV_ROUND_UP(res_blocks, 10);
+		}
+	}
+
 	if (!skl_wm_has_lines(dev_priv, level))
 		res_lines = 0;
 
@@ -4783,6 +4801,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	 */
 	result->plane_res_b = res_blocks;
 	result->plane_res_l = res_lines;
+	/* Bspec says: value >= plane ddb allocation -> invalid, hence the +1 here */
+	result->min_ddb_alloc = max(min_ddb_alloc, res_blocks) + 1;
 	result->plane_en = true;
 }
 
@@ -5133,6 +5153,23 @@ 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 wm1->linetime == wm2->linetime;
+}
+
 static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
 					   const struct skl_ddb_entry *b)
 {
@@ -5159,16 +5196,14 @@ static int skl_update_pipe_wm(struct intel_crtc_state *cstate,
 			      struct skl_pipe_wm *pipe_wm, /* out */
 			      bool *changed /* out */)
 {
+	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
 	int ret;
 
 	ret = skl_build_pipe_wm(cstate, pipe_wm);
 	if (ret)
 		return ret;
 
-	if (!memcmp(old_pipe_wm, pipe_wm, sizeof(*pipe_wm)))
-		*changed = false;
-	else
-		*changed = true;
+	*changed = !skl_pipe_wm_equals(crtc, old_pipe_wm, pipe_wm);
 
 	return 0;
 }
-- 
2.19.2

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

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

* [PATCH 6/9] drm/i915: Pass dev_priv to skl_needs_memory_bw_wa()
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (4 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 5/9] drm/i915: Account for minimum ddb allocation restrictions Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 7/9] drm/i915: Drop the definite article in front of SAGV Ville Syrjala
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

skl_needs_memory_bw_wa() doesn't look at the passed in state at all.
Possibly it should, but for now let's make life simpler by just
passing in dev_priv.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7464552c05f4..40cb18c61e11 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3633,14 +3633,9 @@ static u8 intel_enabled_dbuf_slices_num(struct drm_i915_private *dev_priv)
  * FIXME: We still don't have the proper code detect if we need to apply the WA,
  * so assume we'll always need it in order to avoid underruns.
  */
-static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
+static bool skl_needs_memory_bw_wa(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-
-	if (IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv))
-		return true;
-
-	return false;
+	return IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv);
 }
 
 static bool
@@ -3792,7 +3787,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 
 		latency = dev_priv->wm.skl_latency[level];
 
-		if (skl_needs_memory_bw_wa(intel_state) &&
+		if (skl_needs_memory_bw_wa(dev_priv) &&
 		    plane->base.state->fb->modifier ==
 		    I915_FORMAT_MOD_X_TILED)
 			latency += 15;
@@ -4580,9 +4575,6 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
 	const struct drm_plane_state *pstate = &intel_pstate->base;
 	const struct drm_framebuffer *fb = pstate->fb;
 	uint32_t interm_pbpl;
-	struct intel_atomic_state *state =
-		to_intel_atomic_state(cstate->base.state);
-	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
 
 	/* only NV12 format has two planes */
 	if (color_plane == 1 && fb->format->format != DRM_FORMAT_NV12) {
@@ -4643,7 +4635,7 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
 		wp->y_min_scanlines = 4;
 	}
 
-	if (apply_memory_bw_wa)
+	if (skl_needs_memory_bw_wa(dev_priv))
 		wp->y_min_scanlines *= 2;
 
 	wp->plane_bytes_per_line = wp->width * wp->cpp;
@@ -4697,9 +4689,6 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t selected_result;
 	uint32_t res_blocks, res_lines, min_ddb_alloc = 0;
-	struct intel_atomic_state *state =
-		to_intel_atomic_state(cstate->base.state);
-	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
 
 	if (latency == 0)
 		return;
@@ -4710,7 +4699,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	    dev_priv->ipc_enabled)
 		latency += 4;
 
-	if (apply_memory_bw_wa && wp->x_tiled)
+	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)
 		latency += 15;
 
 	method1 = skl_wm_method1(dev_priv, wp->plane_pixel_rate,
-- 
2.19.2

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

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

* [PATCH 7/9] drm/i915: Drop the definite article in front of SAGV
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (5 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 6/9] drm/i915: Pass dev_priv to skl_needs_memory_bw_wa() Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2018-12-21 17:40   ` Rodrigo Vivi
  2018-12-21 17:14 ` [PATCH 8/9] drm/i915: Drop the pointless linetime==0 check Ville Syrjala
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

The spec doesn't use a definite article in front of SAGV. The
rules regarding articles and initialisms are super fuzzy, but
at least to my ears it sounds much more natural to not have
the article. Perhaps because I tend to pronounce it as
"sag-vee" instead of spelling out the letters one at a time.
Actually I might still prefer to leave out the article if I
did spell them out.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 40cb18c61e11..0843990ebf9f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3667,25 +3667,25 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
 	if (dev_priv->sagv_status == I915_SAGV_ENABLED)
 		return 0;
 
-	DRM_DEBUG_KMS("Enabling the SAGV\n");
+	DRM_DEBUG_KMS("Enabling SAGV\n");
 	mutex_lock(&dev_priv->pcu_lock);
 
 	ret = sandybridge_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,
 				      GEN9_SAGV_ENABLE);
 
-	/* We don't need to wait for the SAGV when enabling */
+	/* We don't need to wait for SAGV when enabling */
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	/*
 	 * Some skl systems, pre-release machines in particular,
-	 * don't actually have an SAGV.
+	 * don't actually have SAGV.
 	 */
 	if (IS_SKYLAKE(dev_priv) && ret == -ENXIO) {
 		DRM_DEBUG_DRIVER("No SAGV found on system, ignoring\n");
 		dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
 		return 0;
 	} else if (ret < 0) {
-		DRM_ERROR("Failed to enable the SAGV\n");
+		DRM_ERROR("Failed to enable SAGV\n");
 		return ret;
 	}
 
@@ -3704,7 +3704,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
 	if (dev_priv->sagv_status == I915_SAGV_DISABLED)
 		return 0;
 
-	DRM_DEBUG_KMS("Disabling the SAGV\n");
+	DRM_DEBUG_KMS("Disabling SAGV\n");
 	mutex_lock(&dev_priv->pcu_lock);
 
 	/* bspec says to keep retrying for at least 1 ms */
@@ -3716,14 +3716,14 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
 
 	/*
 	 * Some skl systems, pre-release machines in particular,
-	 * don't actually have an SAGV.
+	 * don't actually have SAGV.
 	 */
 	if (IS_SKYLAKE(dev_priv) && ret == -ENXIO) {
 		DRM_DEBUG_DRIVER("No SAGV found on system, ignoring\n");
 		dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
 		return 0;
 	} else if (ret < 0) {
-		DRM_ERROR("Failed to disable the SAGV (%d)\n", ret);
+		DRM_ERROR("Failed to disable SAGV (%d)\n", ret);
 		return ret;
 	}
 
@@ -3754,7 +3754,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 		sagv_block_time_us = 10;
 
 	/*
-	 * SKL+ workaround: bspec recommends we disable the SAGV when we have
+	 * SKL+ workaround: bspec recommends we disable SAGV when we have
 	 * more then one pipe enabled
 	 *
 	 * If there are no active CRTCs, no additional checks need be performed
@@ -3795,7 +3795,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 		/*
 		 * If any of the planes on this pipe don't enable wm levels that
 		 * incur memory latencies higher than sagv_block_time_us we
-		 * can't enable the SAGV.
+		 * can't enable SAGV.
 		 */
 		if (latency < sagv_block_time_us)
 			return false;
-- 
2.19.2

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

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

* [PATCH 8/9] drm/i915: Drop the pointless linetime==0 check
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (6 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 7/9] drm/i915: Drop the definite article in front of SAGV Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:14 ` [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check Ville Syrjala
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

0*whatever==0 so this check is pointless. Remove it.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0843990ebf9f..3c351a21a0fa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4825,10 +4825,6 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
 	uint32_t linetime_wm;
 
 	linetime_us = intel_get_linetime_us(cstate);
-
-	if (is_fixed16_zero(linetime_us))
-		return 0;
-
 	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
 
 	/* Display WA #1135: bxt:ALL GLK:ALL */
-- 
2.19.2

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

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

* [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (7 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 8/9] drm/i915: Drop the pointless linetime==0 check Ville Syrjala
@ 2018-12-21 17:14 ` Ville Syrjala
  2018-12-21 17:39   ` Rodrigo Vivi
  2019-01-29 23:54   ` Matt Roper
  2018-12-21 17:49 ` ✗ Fi.CI.SPARSE: warning for skl+ watermark stuff Patchwork
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-12-21 17:14 UTC (permalink / raw)
  To: intel-gfx

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

IS_GLK||IS_BXT == IS_GEN9_LP

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3c351a21a0fa..b5e8ac51ef1c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4827,9 +4827,8 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
 	linetime_us = intel_get_linetime_us(cstate);
 	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
 
-	/* Display WA #1135: bxt:ALL GLK:ALL */
-	if ((IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) &&
-	    dev_priv->ipc_enabled)
+	/* Display WA #1135: BXT:ALL GLK:ALL */
+	if (IS_GEN9_LP(dev_priv) && dev_priv->ipc_enabled)
 		linetime_wm /= 2;
 
 	return linetime_wm;
-- 
2.19.2

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

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

* Re: [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check
  2018-12-21 17:14 ` [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check Ville Syrjala
@ 2018-12-21 17:39   ` Rodrigo Vivi
  2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Rodrigo Vivi @ 2018-12-21 17:39 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx


On Fri, Dec 21, 2018 at 07:14:36PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> IS_GLK||IS_BXT == IS_GEN9_LP
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

(I wont be able to review the entire series, just quickly
glancing the obvious ones before going out on vacation)

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3c351a21a0fa..b5e8ac51ef1c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4827,9 +4827,8 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
>  	linetime_us = intel_get_linetime_us(cstate);
>  	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
>  
> -	/* Display WA #1135: bxt:ALL GLK:ALL */
> -	if ((IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) &&
> -	    dev_priv->ipc_enabled)
> +	/* Display WA #1135: BXT:ALL GLK:ALL */
> +	if (IS_GEN9_LP(dev_priv) && dev_priv->ipc_enabled)
>  		linetime_wm /= 2;
>  
>  	return linetime_wm;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 7/9] drm/i915: Drop the definite article in front of SAGV
  2018-12-21 17:14 ` [PATCH 7/9] drm/i915: Drop the definite article in front of SAGV Ville Syrjala
@ 2018-12-21 17:40   ` Rodrigo Vivi
  0 siblings, 0 replies; 27+ messages in thread
From: Rodrigo Vivi @ 2018-12-21 17:40 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:34PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The spec doesn't use a definite article in front of SAGV. The
> rules regarding articles and initialisms are super fuzzy, but
> at least to my ears it sounds much more natural to not have
> the article. Perhaps because I tend to pronounce it as
> "sag-vee" instead of spelling out the letters one at a time.
> Actually I might still prefer to leave out the article if I
> did spell them out.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 40cb18c61e11..0843990ebf9f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3667,25 +3667,25 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
>  	if (dev_priv->sagv_status == I915_SAGV_ENABLED)
>  		return 0;
>  
> -	DRM_DEBUG_KMS("Enabling the SAGV\n");
> +	DRM_DEBUG_KMS("Enabling SAGV\n");
>  	mutex_lock(&dev_priv->pcu_lock);
>  
>  	ret = sandybridge_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,
>  				      GEN9_SAGV_ENABLE);
>  
> -	/* We don't need to wait for the SAGV when enabling */
> +	/* We don't need to wait for SAGV when enabling */
>  	mutex_unlock(&dev_priv->pcu_lock);
>  
>  	/*
>  	 * Some skl systems, pre-release machines in particular,
> -	 * don't actually have an SAGV.
> +	 * don't actually have SAGV.
>  	 */
>  	if (IS_SKYLAKE(dev_priv) && ret == -ENXIO) {
>  		DRM_DEBUG_DRIVER("No SAGV found on system, ignoring\n");
>  		dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
>  		return 0;
>  	} else if (ret < 0) {
> -		DRM_ERROR("Failed to enable the SAGV\n");
> +		DRM_ERROR("Failed to enable SAGV\n");
>  		return ret;
>  	}
>  
> @@ -3704,7 +3704,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
>  	if (dev_priv->sagv_status == I915_SAGV_DISABLED)
>  		return 0;
>  
> -	DRM_DEBUG_KMS("Disabling the SAGV\n");
> +	DRM_DEBUG_KMS("Disabling SAGV\n");
>  	mutex_lock(&dev_priv->pcu_lock);
>  
>  	/* bspec says to keep retrying for at least 1 ms */
> @@ -3716,14 +3716,14 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
>  
>  	/*
>  	 * Some skl systems, pre-release machines in particular,
> -	 * don't actually have an SAGV.
> +	 * don't actually have SAGV.
>  	 */
>  	if (IS_SKYLAKE(dev_priv) && ret == -ENXIO) {
>  		DRM_DEBUG_DRIVER("No SAGV found on system, ignoring\n");
>  		dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
>  		return 0;
>  	} else if (ret < 0) {
> -		DRM_ERROR("Failed to disable the SAGV (%d)\n", ret);
> +		DRM_ERROR("Failed to disable SAGV (%d)\n", ret);
>  		return ret;
>  	}
>  
> @@ -3754,7 +3754,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  		sagv_block_time_us = 10;
>  
>  	/*
> -	 * SKL+ workaround: bspec recommends we disable the SAGV when we have
> +	 * SKL+ workaround: bspec recommends we disable SAGV when we have
>  	 * more then one pipe enabled
>  	 *
>  	 * If there are no active CRTCs, no additional checks need be performed
> @@ -3795,7 +3795,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  		/*
>  		 * If any of the planes on this pipe don't enable wm levels that
>  		 * incur memory latencies higher than sagv_block_time_us we
> -		 * can't enable the SAGV.
> +		 * can't enable SAGV.
>  		 */
>  		if (latency < sagv_block_time_us)
>  			return false;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.SPARSE: warning for skl+ watermark stuff
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (8 preceding siblings ...)
  2018-12-21 17:14 ` [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check Ville Syrjala
@ 2018-12-21 17:49 ` Patchwork
  2018-12-21 18:05 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-12-21 23:15 ` ✓ Fi.CI.IGT: " Patchwork
  11 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-12-21 17:49 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: skl+ watermark stuff
URL   : https://patchwork.freedesktop.org/series/54424/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Don't ignore level 0 lines watermark for glk+
Okay!

Commit: drm/i915: Reinstate an early latency==0 check for skl+
Okay!

Commit: drm/i915: Fix bits vs. bytes mixup in dbuf block size computation
Okay!

Commit: drm/i915: Fix > vs >= mismatch in watermark/ddb calculations
-O:drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)

Commit: drm/i915: Account for minimum ddb allocation restrictions
-O:drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_pm.c:4892:30: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_pm.c:4892:30: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_pm.c:6670:24: warning: too many warnings
+drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4413:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4424:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4805:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4805:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4892:30: warning: too many warnings
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3551:16: warning: expression using sizeof(void)

Commit: drm/i915: Pass dev_priv to skl_needs_memory_bw_wa()
Okay!

Commit: drm/i915: Drop the definite article in front of SAGV
Okay!

Commit: drm/i915: Drop the pointless linetime==0 check
Okay!

Commit: drm/i915: Use IS_GEN9_LP() for the linetime w/a check
Okay!

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

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

* ✓ Fi.CI.BAT: success for skl+ watermark stuff
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (9 preceding siblings ...)
  2018-12-21 17:49 ` ✗ Fi.CI.SPARSE: warning for skl+ watermark stuff Patchwork
@ 2018-12-21 18:05 ` Patchwork
  2018-12-21 23:15 ` ✓ Fi.CI.IGT: " Patchwork
  11 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-12-21 18:05 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: skl+ watermark stuff
URL   : https://patchwork.freedesktop.org/series/54424/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5339 -> Patchwork_11148
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54424/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-kbl-7560u:       PASS -> INCOMPLETE [fdo#108767]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> INCOMPLETE [fdo#102657]

  * {igt@runner@aborted}:
    - fi-icl-u3:          NOTRUN -> FAIL [fdo#108315]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#107362] -> PASS

  * igt@vgem_basic@mmap:
    - fi-snb-2520m:       DMESG-WARN -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]
    - fi-icl-u2:          INCOMPLETE [fdo#108315] -> DMESG-FAIL [fdo#108569]

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

  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767


Participating hosts (49 -> 42)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y 


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

    * Linux: CI_DRM_5339 -> Patchwork_11148

  CI_DRM_5339: c6cfab3d1c61d968f483554fc063faf1ee1063fa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4753: 0bc683ea2dcf270b5287ffb4a6510fdff44e9390 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11148: 701f01d8eb8da344f1afcfbe9646b53db89500e1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

701f01d8eb8d drm/i915: Use IS_GEN9_LP() for the linetime w/a check
aefcbb0f9265 drm/i915: Drop the pointless linetime==0 check
09afa37b1405 drm/i915: Drop the definite article in front of SAGV
fee0ba572c40 drm/i915: Pass dev_priv to skl_needs_memory_bw_wa()
9a6e49f3b92f drm/i915: Account for minimum ddb allocation restrictions
c32a52ac83dc drm/i915: Fix > vs >= mismatch in watermark/ddb calculations
8b4572b9954d drm/i915: Fix bits vs. bytes mixup in dbuf block size computation
b47f650fffe7 drm/i915: Reinstate an early latency==0 check for skl+
367c59fbd99a drm/i915: Don't ignore level 0 lines watermark for glk+

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for skl+ watermark stuff
  2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
                   ` (10 preceding siblings ...)
  2018-12-21 18:05 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-12-21 23:15 ` Patchwork
  11 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-12-21 23:15 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: skl+ watermark stuff
URL   : https://patchwork.freedesktop.org/series/54424/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5339_full -> Patchwork_11148_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-bsd:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_exec_whisper@normal:
    - shard-skl:          PASS -> TIMEOUT [fdo#108592]

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@kms_atomic_transition@plane-toggle-modeset-transition:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +8

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_chv_cursor_fail@pipe-b-256x256-right-edge:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +5

  * igt@kms_color@pipe-b-ctm-green-to-red:
    - shard-skl:          PASS -> FAIL [fdo#107201]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-skl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +23

  * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-ytiled:
    - shard-iclb:         PASS -> WARN [fdo#108336] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-wc:
    - shard-skl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-iclb:         PASS -> FAIL [fdo#105683] / [fdo#108040]
    - shard-skl:          NOTRUN -> FAIL [fdo#105683]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107724] +2

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-apl:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
    - shard-skl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#103166] / [fdo#107724]

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#108950]

  * igt@kms_setmode@basic:
    - shard-hsw:          PASS -> FAIL [fdo#99912]

  * igt@kms_sysfs_edid_timing:
    - shard-skl:          NOTRUN -> FAIL [fdo#100047]

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-hsw:          PASS -> FAIL [fdo#104894]

  * igt@pm_backlight@fade_with_suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#107847]

  * igt@pm_rpm@basic-rte:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@gem-execbuf-stress-pc8:
    - shard-skl:          SKIP -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-iclb:         SKIP -> INCOMPLETE [fdo#108840]

  * igt@pm_rpm@universal-planes:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108654] / [fdo#108756]

  * {igt@runner@aborted}:
    - shard-iclb:         NOTRUN -> ( 2 FAIL ) [fdo#108654] / [fdo#108756] / [fdo#108866]

  
#### Possible fixes ####

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-snb:          DMESG-WARN [fdo#107956] -> PASS
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_color@pipe-a-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - shard-skl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-kbl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +25

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-glk:          FAIL [fdo#103167] -> PASS +2
    - shard-iclb:         DMESG-FAIL [fdo#107724] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +6

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] / [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-iclb:         FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +2

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         FAIL [fdo#100047] -> PASS

  * igt@pm_rpm@cursor-dpms:
    - shard-iclb:         DMESG-WARN [fdo#108654] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

  * igt@i915_suspend@shrink:
    - shard-skl:          DMESG-WARN [fdo#108784] -> INCOMPLETE [fdo#106886]

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> DMESG-WARN [fdo#103558] / [fdo#105602]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> DMESG-FAIL [fdo#103166] / [fdo#103558] / [fdo#105602]

  * igt@pm_backlight@fade_with_suspend:
    - shard-iclb:         FAIL [fdo#107847] -> DMESG-FAIL [fdo#107724]

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

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108592]: https://bugs.freedesktop.org/show_bug.cgi?id=108592
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108866]: https://bugs.freedesktop.org/show_bug.cgi?id=108866
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5339 -> Patchwork_11148

  CI_DRM_5339: c6cfab3d1c61d968f483554fc063faf1ee1063fa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4753: 0bc683ea2dcf270b5287ffb4a6510fdff44e9390 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11148: 701f01d8eb8da344f1afcfbe9646b53db89500e1 @ 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_11148/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+
  2018-12-21 17:14 ` [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+ Ville Syrjala
@ 2019-01-28  8:34   ` Lisovskiy, Stanislav
  2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Lisovskiy, Stanislav @ 2019-01-28  8:34 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2018-12-21 at 19:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I thought we could remove all the early latency==0 checks
> and rely on skl_wm_method{1,2}() checking for it. But
> skl_compute_plane_wm() applies a bunch of workarounds to bump
> up the latency before calling those guys so clearly it won't
> end up doing the right thing. Also not sure if the calculations
> based on the method1/2 results are safe agaisnt overflows so
> it might not work all that well in any case. Let's put the
> early check back.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index d132ef10fa60..0aac7e7b660f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct
> intel_crtc_state *cstate,
>  		to_intel_atomic_state(cstate->base.state);
>  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
>  
> +	if (latency == 0)
> +		return;
> +
>  	/* Display WA #1141: kbl,cfl */
>  	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
>  	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&

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

-- 
Best Regards,

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

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

* Re: [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+
  2018-12-21 17:14 ` [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+ Ville Syrjala
@ 2019-01-28  8:40   ` Lisovskiy, Stanislav
  2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Lisovskiy, Stanislav @ 2019-01-28  8:40 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2018-12-21 at 19:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On glk+ the level 0 lines watermark actually matters. Do not ignore
> it.
> And while at it let's change things so that we always program a
> consistnet 0 to the register when the lines watermarks is ignored
> by the hardware.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 2a6ffb8b975a..d132ef10fa60 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4675,6 +4675,15 @@ skl_compute_plane_wm_params(const struct
> intel_crtc_state *cstate,
>  	return 0;
>  }
>  
> +static bool skl_wm_has_lines(struct drm_i915_private *dev_priv, int
> level)
> +{
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> +		return true;
> +
> +	/* The number of lines are ignored for the level 0
> watermark. */
> +	return level > 0;
> +}
> +
>  static void skl_compute_plane_wm(const struct intel_crtc_state
> *cstate,
>  				 const struct intel_plane_state
> *intel_pstate,
>  				 int level,
> @@ -4757,8 +4766,10 @@ static void skl_compute_plane_wm(const struct
> intel_crtc_state *cstate,
>  		}
>  	}
>  
> -	/* The number of lines are ignored for the level 0
> watermark. */
> -	if (level > 0 && res_lines > 31)
> +	if (!skl_wm_has_lines(dev_priv, level))
> +		res_lines = 0;
> +
> +	if (res_lines > 31)
>  		return;
>  
>  	/*

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

-- 
Best Regards,

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

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

* Re: [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation
  2018-12-21 17:14 ` [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation Ville Syrjala
@ 2019-01-28  8:43   ` Lisovskiy, Stanislav
  2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Lisovskiy, Stanislav @ 2019-01-28  8:43 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Fri, 2018-12-21 at 19:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The spec used to say "8bpp" which someone took to mean 8 bytes per
> pixel when in fact it was supposed to be 8 bits per pixel. The
> spec has been updated to make it more clear now. Fix the code
> to match.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 0aac7e7b660f..55a1c577f060 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4618,7 +4618,7 @@ skl_compute_plane_wm_params(const struct
> intel_crtc_state *cstate,
>  							     intel_p
> state);
>  
>  	if (INTEL_GEN(dev_priv) >= 11 &&
> -	    fb->modifier == I915_FORMAT_MOD_Yf_TILED && wp->cpp ==
> 8)
> +	    fb->modifier == I915_FORMAT_MOD_Yf_TILED && wp->cpp ==
> 1)
>  		wp->dbuf_block_size = 256;
>  	else
>  		wp->dbuf_block_size = 512;

Interesting how this small misunderstanding could affect our bugs
previously.

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

-- 
Best Regards,

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

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

* Re: [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+
  2018-12-21 17:14 ` [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+ Ville Syrjala
  2019-01-28  8:40   ` Lisovskiy, Stanislav
@ 2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:28PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On glk+ the level 0 lines watermark actually matters. Do not ignore it.
> And while at it let's change things so that we always program a
> consistnet 0 to the register when the lines watermarks is ignored
> by the hardware.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks like this just got added to the bspec on Dec 18th, 2018.

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2a6ffb8b975a..d132ef10fa60 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4675,6 +4675,15 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
>  	return 0;
>  }
>  
> +static bool skl_wm_has_lines(struct drm_i915_private *dev_priv, int level)
> +{
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> +		return true;
> +
> +	/* The number of lines are ignored for the level 0 watermark. */
> +	return level > 0;
> +}
> +
>  static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  				 const struct intel_plane_state *intel_pstate,
>  				 int level,
> @@ -4757,8 +4766,10 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  		}
>  	}
>  
> -	/* The number of lines are ignored for the level 0 watermark. */
> -	if (level > 0 && res_lines > 31)
> +	if (!skl_wm_has_lines(dev_priv, level))
> +		res_lines = 0;
> +
> +	if (res_lines > 31)
>  		return;
>  
>  	/*
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+
  2018-12-21 17:14 ` [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+ Ville Syrjala
  2019-01-28  8:34   ` Lisovskiy, Stanislav
@ 2019-01-29 23:54   ` Matt Roper
  2019-01-30 14:25     ` Ville Syrjälä
  1 sibling, 1 reply; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:29PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I thought we could remove all the early latency==0 checks
> and rely on skl_wm_method{1,2}() checking for it. But
> skl_compute_plane_wm() applies a bunch of workarounds to bump
> up the latency before calling those guys so clearly it won't
> end up doing the right thing. Also not sure if the calculations
> based on the method1/2 results are safe agaisnt overflows so
> it might not work all that well in any case. Let's put the
> early check back.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Should we remove the tests from skl_wm_method{1,2}() now?  I suppose
someone could still use the debugfs interface to set a latency value of
exactly (UINT32_MAX - workaround amount) to make latency wrap around and
hit 0, but I'm not sure if that's really any worse than if they shoot
themselves in the foot by setting a too-low non-zero latency.  I don't
think we divide by latency anywhere.

Either way,

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


> ---
>  drivers/gpu/drm/i915/intel_pm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d132ef10fa60..0aac7e7b660f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  		to_intel_atomic_state(cstate->base.state);
>  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
>  
> +	if (latency == 0)
> +		return;
> +
>  	/* Display WA #1141: kbl,cfl */
>  	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
>  	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation
  2018-12-21 17:14 ` [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation Ville Syrjala
  2019-01-28  8:43   ` Lisovskiy, Stanislav
@ 2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:30PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The spec used to say "8bpp" which someone took to mean 8 bytes per
> pixel when in fact it was supposed to be 8 bits per pixel. The
> spec has been updated to make it more clear now. Fix the code
> to match.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0aac7e7b660f..55a1c577f060 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4618,7 +4618,7 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
>  							     intel_pstate);
>  
>  	if (INTEL_GEN(dev_priv) >= 11 &&
> -	    fb->modifier == I915_FORMAT_MOD_Yf_TILED && wp->cpp == 8)
> +	    fb->modifier == I915_FORMAT_MOD_Yf_TILED && wp->cpp == 1)
>  		wp->dbuf_block_size = 256;
>  	else
>  		wp->dbuf_block_size = 512;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/9] drm/i915: Fix > vs >= mismatch in watermark/ddb calculations
  2018-12-21 17:14 ` [PATCH 4/9] drm/i915: Fix > vs >= mismatch in watermark/ddb calculations Ville Syrjala
@ 2019-01-29 23:54   ` Matt Roper
  0 siblings, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:31PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec says we have to reject the watermark if it's >= the ddb
> allocation. Fix the code to reject the == case as it should.
> For transition watermarks we can just use >=, for the rest
> we'll do +1 when calculating the minimum ddb allocation size.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 55a1c577f060..3c5cba31f055 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4371,8 +4371,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  				continue;
>  
>  			wm = &cstate->wm.skl.optimal.planes[plane_id];
> -			blocks += wm->wm[level].plane_res_b;
> -			blocks += wm->uv_wm[level].plane_res_b;
> +			blocks += wm->wm[level].plane_res_b + 1;
> +			blocks += wm->uv_wm[level].plane_res_b + 1;
>  		}
>  
>  		if (blocks < alloc_size) {
> @@ -4413,7 +4413,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		extra = min_t(u16, alloc_size,
>  			      DIV64_U64_ROUND_UP(alloc_size * rate,
>  						 total_data_rate));
> -		total[plane_id] = wm->wm[level].plane_res_b + extra;
> +		total[plane_id] = wm->wm[level].plane_res_b + 1 + extra;
>  		alloc_size -= extra;
>  		total_data_rate -= rate;
>  
> @@ -4424,7 +4424,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		extra = min_t(u16, alloc_size,
>  			      DIV64_U64_ROUND_UP(alloc_size * rate,
>  						 total_data_rate));
> -		uv_total[plane_id] = wm->uv_wm[level].plane_res_b + extra;
> +		uv_total[plane_id] = wm->uv_wm[level].plane_res_b + 1 + extra;
>  		alloc_size -= extra;
>  		total_data_rate -= rate;
>  	}
> @@ -4477,7 +4477,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	 */
>  	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
>  		wm = &cstate->wm.skl.optimal.planes[plane_id];
> -		if (wm->trans_wm.plane_res_b > total[plane_id])
> +		if (wm->trans_wm.plane_res_b >= total[plane_id])
>  			memset(&wm->trans_wm, 0, sizeof(wm->trans_wm));
>  	}
>  
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/9] drm/i915: Account for minimum ddb allocation restrictions
  2018-12-21 17:14 ` [PATCH 5/9] drm/i915: Account for minimum ddb allocation restrictions Ville Syrjala
@ 2019-01-29 23:54   ` Matt Roper
  0 siblings, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:32PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On icl+ bspec tells us to calculate a separate minimum ddb
> allocation from the blocks watermark. Both have to be checked
> against the actual ddb allocation, but since we do things the
> other way around we'll just calculat the minimum acceptable
> ddb allocation by taking the maximum of the two values.
> 
> We'll also replace the memcmp() with a full trawl over the
> the watermarks so that it'll ignore the min_ddb_alloc
> because we can't directly read that out from the hw. I suppose
> we could reconstruct it from the other values, but I was
> too lazy to do that now.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Matches the bspec.

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

> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 53 +++++++++++++++++++++++++++------
>  2 files changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 815db160b966..7668d7197994 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1108,6 +1108,7 @@ struct skl_ddb_values {
>  };
>  
>  struct skl_wm_level {
> +	uint16_t min_ddb_alloc;
>  	uint16_t plane_res_b;
>  	uint8_t plane_res_l;
>  	bool plane_en;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3c5cba31f055..7464552c05f4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4371,8 +4371,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  				continue;
>  
>  			wm = &cstate->wm.skl.optimal.planes[plane_id];
> -			blocks += wm->wm[level].plane_res_b + 1;
> -			blocks += wm->uv_wm[level].plane_res_b + 1;
> +			blocks += wm->wm[level].min_ddb_alloc;
> +			blocks += wm->uv_wm[level].min_ddb_alloc;
>  		}
>  
>  		if (blocks < alloc_size) {
> @@ -4413,7 +4413,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		extra = min_t(u16, alloc_size,
>  			      DIV64_U64_ROUND_UP(alloc_size * rate,
>  						 total_data_rate));
> -		total[plane_id] = wm->wm[level].plane_res_b + 1 + extra;
> +		total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
>  		alloc_size -= extra;
>  		total_data_rate -= rate;
>  
> @@ -4424,7 +4424,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		extra = min_t(u16, alloc_size,
>  			      DIV64_U64_ROUND_UP(alloc_size * rate,
>  						 total_data_rate));
> -		uv_total[plane_id] = wm->uv_wm[level].plane_res_b + 1 + extra;
> +		uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
>  		alloc_size -= extra;
>  		total_data_rate -= rate;
>  	}
> @@ -4696,7 +4696,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  	uint32_t latency = dev_priv->wm.skl_latency[level];
>  	uint_fixed_16_16_t method1, method2;
>  	uint_fixed_16_16_t selected_result;
> -	uint32_t res_blocks, res_lines;
> +	uint32_t res_blocks, res_lines, min_ddb_alloc = 0;
>  	struct intel_atomic_state *state =
>  		to_intel_atomic_state(cstate->base.state);
>  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
> @@ -4769,6 +4769,24 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  		}
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 11) {
> +		if (wp->y_tiled) {
> +			int extra_lines;
> +
> +			if (res_lines % wp->y_min_scanlines == 0)
> +				extra_lines = wp->y_min_scanlines;
> +			else
> +				extra_lines = wp->y_min_scanlines * 2 -
> +					res_lines % wp->y_min_scanlines;
> +
> +			min_ddb_alloc = mul_round_up_u32_fixed16(res_lines + extra_lines,
> +								 wp->plane_blocks_per_line);
> +		} else {
> +			min_ddb_alloc = res_blocks +
> +				DIV_ROUND_UP(res_blocks, 10);
> +		}
> +	}
> +
>  	if (!skl_wm_has_lines(dev_priv, level))
>  		res_lines = 0;
>  
> @@ -4783,6 +4801,8 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  	 */
>  	result->plane_res_b = res_blocks;
>  	result->plane_res_l = res_lines;
> +	/* Bspec says: value >= plane ddb allocation -> invalid, hence the +1 here */
> +	result->min_ddb_alloc = max(min_ddb_alloc, res_blocks) + 1;
>  	result->plane_en = true;
>  }
>  
> @@ -5133,6 +5153,23 @@ 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 wm1->linetime == wm2->linetime;
> +}
> +
>  static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
>  					   const struct skl_ddb_entry *b)
>  {
> @@ -5159,16 +5196,14 @@ static int skl_update_pipe_wm(struct intel_crtc_state *cstate,
>  			      struct skl_pipe_wm *pipe_wm, /* out */
>  			      bool *changed /* out */)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
>  	int ret;
>  
>  	ret = skl_build_pipe_wm(cstate, pipe_wm);
>  	if (ret)
>  		return ret;
>  
> -	if (!memcmp(old_pipe_wm, pipe_wm, sizeof(*pipe_wm)))
> -		*changed = false;
> -	else
> -		*changed = true;
> +	*changed = !skl_pipe_wm_equals(crtc, old_pipe_wm, pipe_wm);
>  
>  	return 0;
>  }
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/9] drm/i915: Pass dev_priv to skl_needs_memory_bw_wa()
  2018-12-21 17:14 ` [PATCH 6/9] drm/i915: Pass dev_priv to skl_needs_memory_bw_wa() Ville Syrjala
@ 2019-01-29 23:54   ` Matt Roper
  0 siblings, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:33PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> skl_needs_memory_bw_wa() doesn't look at the passed in state at all.
> Possibly it should, but for now let's make life simpler by just
> passing in dev_priv.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7464552c05f4..40cb18c61e11 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3633,14 +3633,9 @@ static u8 intel_enabled_dbuf_slices_num(struct drm_i915_private *dev_priv)
>   * FIXME: We still don't have the proper code detect if we need to apply the WA,
>   * so assume we'll always need it in order to avoid underruns.
>   */
> -static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> +static bool skl_needs_memory_bw_wa(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -
> -	if (IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv))
> -		return true;
> -
> -	return false;
> +	return IS_GEN9_BC(dev_priv) || IS_BROXTON(dev_priv);
>  }
>  
>  static bool
> @@ -3792,7 +3787,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  
>  		latency = dev_priv->wm.skl_latency[level];
>  
> -		if (skl_needs_memory_bw_wa(intel_state) &&
> +		if (skl_needs_memory_bw_wa(dev_priv) &&
>  		    plane->base.state->fb->modifier ==
>  		    I915_FORMAT_MOD_X_TILED)
>  			latency += 15;
> @@ -4580,9 +4575,6 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
>  	const struct drm_plane_state *pstate = &intel_pstate->base;
>  	const struct drm_framebuffer *fb = pstate->fb;
>  	uint32_t interm_pbpl;
> -	struct intel_atomic_state *state =
> -		to_intel_atomic_state(cstate->base.state);
> -	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
>  
>  	/* only NV12 format has two planes */
>  	if (color_plane == 1 && fb->format->format != DRM_FORMAT_NV12) {
> @@ -4643,7 +4635,7 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate,
>  		wp->y_min_scanlines = 4;
>  	}
>  
> -	if (apply_memory_bw_wa)
> +	if (skl_needs_memory_bw_wa(dev_priv))
>  		wp->y_min_scanlines *= 2;
>  
>  	wp->plane_bytes_per_line = wp->width * wp->cpp;
> @@ -4697,9 +4689,6 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  	uint_fixed_16_16_t method1, method2;
>  	uint_fixed_16_16_t selected_result;
>  	uint32_t res_blocks, res_lines, min_ddb_alloc = 0;
> -	struct intel_atomic_state *state =
> -		to_intel_atomic_state(cstate->base.state);
> -	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
>  
>  	if (latency == 0)
>  		return;
> @@ -4710,7 +4699,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  	    dev_priv->ipc_enabled)
>  		latency += 4;
>  
> -	if (apply_memory_bw_wa && wp->x_tiled)
> +	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)
>  		latency += 15;
>  
>  	method1 = skl_wm_method1(dev_priv, wp->plane_pixel_rate,
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 8/9] drm/i915: Drop the pointless linetime==0 check
  2018-12-21 17:14 ` [PATCH 8/9] drm/i915: Drop the pointless linetime==0 check Ville Syrjala
@ 2019-01-29 23:54   ` Matt Roper
  0 siblings, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:35PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 0*whatever==0 so this check is pointless. Remove it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0843990ebf9f..3c351a21a0fa 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4825,10 +4825,6 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
>  	uint32_t linetime_wm;
>  
>  	linetime_us = intel_get_linetime_us(cstate);
> -
> -	if (is_fixed16_zero(linetime_us))
> -		return 0;
> -
>  	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
>  
>  	/* Display WA #1135: bxt:ALL GLK:ALL */
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check
  2018-12-21 17:14 ` [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check Ville Syrjala
  2018-12-21 17:39   ` Rodrigo Vivi
@ 2019-01-29 23:54   ` Matt Roper
  1 sibling, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-01-29 23:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Dec 21, 2018 at 07:14:36PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> IS_GLK||IS_BXT == IS_GEN9_LP
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3c351a21a0fa..b5e8ac51ef1c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4827,9 +4827,8 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate)
>  	linetime_us = intel_get_linetime_us(cstate);
>  	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
>  
> -	/* Display WA #1135: bxt:ALL GLK:ALL */
> -	if ((IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) &&
> -	    dev_priv->ipc_enabled)
> +	/* Display WA #1135: BXT:ALL GLK:ALL */
> +	if (IS_GEN9_LP(dev_priv) && dev_priv->ipc_enabled)
>  		linetime_wm /= 2;
>  
>  	return linetime_wm;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+
  2019-01-29 23:54   ` Matt Roper
@ 2019-01-30 14:25     ` Ville Syrjälä
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2019-01-30 14:25 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

On Tue, Jan 29, 2019 at 03:54:30PM -0800, Matt Roper wrote:
> On Fri, Dec 21, 2018 at 07:14:29PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I thought we could remove all the early latency==0 checks
> > and rely on skl_wm_method{1,2}() checking for it. But
> > skl_compute_plane_wm() applies a bunch of workarounds to bump
> > up the latency before calling those guys so clearly it won't
> > end up doing the right thing. Also not sure if the calculations
> > based on the method1/2 results are safe agaisnt overflows so
> > it might not work all that well in any case. Let's put the
> > early check back.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Should we remove the tests from skl_wm_method{1,2}() now?  I suppose
> someone could still use the debugfs interface to set a latency value of
> exactly (UINT32_MAX - workaround amount) to make latency wrap around and
> hit 0, but I'm not sure if that's really any worse than if they shoot
> themselves in the foot by setting a too-low non-zero latency.  I don't
> think we divide by latency anywhere.

I'd probably prefer to fix the overflows somehow so that the code
could work the same way as the pre-skl code (ie. just check for 0
latency in the method1/2 functions). But that would require some
actual thinking.

> 
> Either way,
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks for the reviews. Series pushed to dinq.

> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d132ef10fa60..0aac7e7b660f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> >  		to_intel_atomic_state(cstate->base.state);
> >  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
> >  
> > +	if (latency == 0)
> > +		return;
> > +
> >  	/* Display WA #1141: kbl,cfl */
> >  	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> >  	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

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

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

end of thread, other threads:[~2019-01-30 14:25 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-21 17:14 [PATCH 0/9] skl+ watermark stuff Ville Syrjala
2018-12-21 17:14 ` [PATCH 1/9] drm/i915: Don't ignore level 0 lines watermark for glk+ Ville Syrjala
2019-01-28  8:40   ` Lisovskiy, Stanislav
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:14 ` [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+ Ville Syrjala
2019-01-28  8:34   ` Lisovskiy, Stanislav
2019-01-29 23:54   ` Matt Roper
2019-01-30 14:25     ` Ville Syrjälä
2018-12-21 17:14 ` [PATCH 3/9] drm/i915: Fix bits vs. bytes mixup in dbuf block size computation Ville Syrjala
2019-01-28  8:43   ` Lisovskiy, Stanislav
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:14 ` [PATCH 4/9] drm/i915: Fix > vs >= mismatch in watermark/ddb calculations Ville Syrjala
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:14 ` [PATCH 5/9] drm/i915: Account for minimum ddb allocation restrictions Ville Syrjala
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:14 ` [PATCH 6/9] drm/i915: Pass dev_priv to skl_needs_memory_bw_wa() Ville Syrjala
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:14 ` [PATCH 7/9] drm/i915: Drop the definite article in front of SAGV Ville Syrjala
2018-12-21 17:40   ` Rodrigo Vivi
2018-12-21 17:14 ` [PATCH 8/9] drm/i915: Drop the pointless linetime==0 check Ville Syrjala
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:14 ` [PATCH 9/9] drm/i915: Use IS_GEN9_LP() for the linetime w/a check Ville Syrjala
2018-12-21 17:39   ` Rodrigo Vivi
2019-01-29 23:54   ` Matt Roper
2018-12-21 17:49 ` ✗ Fi.CI.SPARSE: warning for skl+ watermark stuff Patchwork
2018-12-21 18:05 ` ✓ Fi.CI.BAT: success " Patchwork
2018-12-21 23:15 ` ✓ 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.