[PATCH 0/6] Watermarks small fixes/improvements

[PATCH 0/6] Watermarks small fixes/improvements

[Paulo Zanoni]

I'm investigating ICL watermarks failures and these are some of the immediate
problems I was able to find in the watermarks code. I don't think they're enough
to fix the problems our CI is able to reproduce, but I do think these changes
are worth having.

Paulo Zanoni (6):
  drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+
  drm/i915: fix the transition minimums for gen9+ watermarks
  drm/i915: fix the watermark result selection on glk/gen10+
  drm/i915: transition WMs ask for Selected Result Blocks
  drm/i915: don't write PLANE_BUF_CFG twice every time
  drm/i915: promote ddb update message to DRM_DEBUG_KMS

 drivers/gpu/drm/i915/intel_pm.c | 100 ++++++++++++++++++++++++----------------
 1 file changed, 61 insertions(+), 39 deletions(-)

-- 
2.14.4

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

[PATCH 1/6] drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+

[Paulo Zanoni]

BSpec does not show these WAs as applicable to GLK, and for CNL it
only shows them applicable for a super early pre-production stepping
we shouldn't be caring about anymore. Remove these so we can avoid
them on ICL too.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 43 ++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1392aa56a55a..910551e04d16 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4687,28 +4687,31 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	res_lines = div_round_up_fixed16(selected_result,
 					 wp->plane_blocks_per_line);
 
-	/* Display WA #1125: skl,bxt,kbl,glk */
-	if (level == 0 && wp->rc_surface)
-		res_blocks += fixed16_to_u32_round_up(wp->y_tile_minimum);
+	if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) {
+		/* Display WA #1125: skl,bxt,kbl */
+		if (level == 0 && wp->rc_surface)
+			res_blocks +=
+				fixed16_to_u32_round_up(wp->y_tile_minimum);
+
+		/* Display WA #1126: skl,bxt,kbl */
+		if (level >= 1 && level <= 7) {
+			if (wp->y_tiled) {
+				res_blocks +=
+				    fixed16_to_u32_round_up(wp->y_tile_minimum);
+				res_lines += wp->y_min_scanlines;
+			} else {
+				res_blocks++;
+			}
 
-	/* Display WA #1126: skl,bxt,kbl,glk */
-	if (level >= 1 && level <= 7) {
-		if (wp->y_tiled) {
-			res_blocks += fixed16_to_u32_round_up(
-							wp->y_tile_minimum);
-			res_lines += wp->y_min_scanlines;
-		} else {
-			res_blocks++;
+			/*
+			 * Make sure result blocks for higher latency levels are
+			 * atleast as high as level below the current level.
+			 * Assumption in DDB algorithm optimization for special
+			 * cases. Also covers Display WA #1125 for RC.
+			 */
+			if (result_prev->plane_res_b > res_blocks)
+				res_blocks = result_prev->plane_res_b;
 		}
-
-		/*
-		 * Make sure result blocks for higher latency levels are atleast
-		 * as high as level below the current level.
-		 * Assumption in DDB algorithm optimization for special cases.
-		 * Also covers Display WA #1125 for RC.
-		 */
-		if (result_prev->plane_res_b > res_blocks)
-			res_blocks = result_prev->plane_res_b;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 11) {
-- 
2.14.4

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

[PATCH 2/6] drm/i915: fix the transition minimums for gen9+ watermarks

[Paulo Zanoni]

The transition minimum is 14 blocks for gens 9 and 10, and 4 blocks
for gen 11. This minimum value is supposed to be added to the
configurable trans_amount. This matches both BSpec and additional
information provided by our HW engineers.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 910551e04d16..cab86690a0ba 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4878,8 +4878,8 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
 	if (!dev_priv->ipc_enabled)
 		goto exit;
 
-	trans_min = 0;
-	if (INTEL_GEN(dev_priv) >= 10)
+	trans_min = 14;
+	if (INTEL_GEN(dev_priv) >= 11)
 		trans_min = 4;
 
 	trans_offset_b = trans_min + trans_amount;
-- 
2.14.4

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

[PATCH 3/6] drm/i915: fix the watermark result selection on glk/gen10+

[Paulo Zanoni]

On these platforms we're supposed to unconditonally pick the method 2
result instead of the minimum.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cab86690a0ba..40ce99c455f3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4672,15 +4672,24 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	} else {
 		if ((wp->cpp * cstate->base.adjusted_mode.crtc_htotal /
 		     wp->dbuf_block_size < 1) &&
-		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1))
+		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) {
 			selected_result = method2;
-		else if (ddb_allocation >=
-			 fixed16_to_u32_round_up(wp->plane_blocks_per_line))
-			selected_result = min_fixed16(method1, method2);
-		else if (latency >= wp->linetime_us)
-			selected_result = min_fixed16(method1, method2);
-		else
+		} else if (ddb_allocation >=
+			 fixed16_to_u32_round_up(wp->plane_blocks_per_line)) {
+			if (INTEL_GEN(dev_priv) == 9 &&
+			    !IS_GEMINILAKE(dev_priv))
+				selected_result = min_fixed16(method1, method2);
+			else
+				selected_result = method2;
+		} else if (latency >= wp->linetime_us) {
+			if (INTEL_GEN(dev_priv) == 9 &&
+			    !IS_GEMINILAKE(dev_priv))
+				selected_result = min_fixed16(method1, method2);
+			else
+				selected_result = method2;
+		} else {
 			selected_result = method1;
+		}
 	}
 
 	res_blocks = fixed16_to_u32_round_up(selected_result) + 1;
-- 
2.14.4

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

[PATCH 4/6] drm/i915: transition WMs ask for Selected Result Blocks

[Paulo Zanoni]

The transition watermarks ask for Selected Result Blocks (the real
value), not Result Blocks (the integer value). Given how ceilings are
applied in both the non-transition and the transition watermarks
calculations, we can get away with assuming that Selected Result
Blocks is actually Result Blocks minus 1 without any rounding errors.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 40ce99c455f3..14f13a371989 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4874,7 +4874,7 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
 	const struct drm_i915_private *dev_priv = to_i915(dev);
 	uint16_t trans_min, trans_y_tile_min;
 	const uint16_t trans_amount = 10; /* This is configurable amount */
-	uint16_t trans_offset_b, res_blocks;
+	uint16_t wm0_sel_res_b, trans_offset_b, res_blocks;
 
 	if (!cstate->base.active)
 		goto exit;
@@ -4893,13 +4893,25 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
 
 	trans_offset_b = trans_min + trans_amount;
 
+	/*
+	 * The spec asks for Selected Result Blocks for wm0 (the real value),
+	 * not Result Blocks (the integer value). Pay attention to the capital
+	 * letters. The value wm_l0->plane_res_b is actually Result Blocks, but
+	 * since Result Blocks is the ceiling of Selected Result Blocks plus 1,
+	 * and since we later will have to get the ceiling of the sum in the
+	 * transition watermarks calculation, we can just pretend Selected
+	 * Result Blocks is Result Blocks minus 1 and it should work for the
+	 * current platforms.
+	 */
+	wm0_sel_res_b = wm_l0->plane_res_b - 1;
+
 	if (wp->y_tiled) {
 		trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
 							wp->y_tile_minimum);
-		res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
+		res_blocks = max(wm0_sel_res_b, trans_y_tile_min) +
 				trans_offset_b;
 	} else {
-		res_blocks = wm_l0->plane_res_b + trans_offset_b;
+		res_blocks = wm0_sel_res_b + trans_offset_b;
 
 		/* WA BUG:1938466 add one block for non y-tile planes */
 		if (IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
-- 
2.14.4

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

[PATCH 5/6] drm/i915: don't write PLANE_BUF_CFG twice every time

[Paulo Zanoni]

We were writing to PLANE_BUF_CFG(pipe, plane_id) twice for every
platform, and we were even using different values on the gen10- planar
case. The first write is useless since it just gets replaced with the
next one, so kill it.

There's a lot to improve in the DDB code, but let's start by avoiding
the double write.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 14f13a371989..53b4a9a2de69 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5040,8 +5040,6 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
 	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
 			   &wm->trans_wm);
 
-	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
-			    &ddb->plane[pipe][plane_id]);
 	/* FIXME: add proper NV12 support for ICL. */
 	if (INTEL_GEN(dev_priv) >= 11)
 		return skl_ddb_entry_write(dev_priv,
-- 
2.14.4

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

[PATCH 6/6] drm/i915: promote ddb update message to DRM_DEBUG_KMS

[Paulo Zanoni]

This message is currently marked as DRM_DEBUG_ATOMIC. I would like it
to be DRM_DEBUG_KMS since it is more KMS than atomic, and this will
also make the message appear in the CI logs, which may or may not help
us with some FIFO underrun bugs.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@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 53b4a9a2de69..6cacddd16010 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5233,11 +5233,11 @@ skl_print_wm_changes(const struct drm_atomic_state *state)
 			if (skl_ddb_entry_equal(old, new))
 				continue;
 
-			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] ddb (%d - %d) -> (%d - %d)\n",
-					 intel_plane->base.base.id,
-					 intel_plane->base.name,
-					 old->start, old->end,
-					 new->start, new->end);
+			DRM_DEBUG_KMS("[PLANE:%d:%s] ddb (%d - %d) -> (%d - %d)\n",
+				      intel_plane->base.base.id,
+				      intel_plane->base.name,
+				      old->start, old->end,
+				      new->start, new->end);
 		}
 	}
 }
-- 
2.14.4

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

✗ Fi.CI.SPARSE: warning for Watermarks small fixes/improvements

[Patchwork]

== Series Details ==

Series: Watermarks small fixes/improvements
URL   : https://patchwork.freedesktop.org/series/50579/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+
Okay!

Commit: drm/i915: fix the transition minimums for gen9+ watermarks
Okay!

Commit: drm/i915: fix the watermark result selection on glk/gen10+
Okay!

Commit: drm/i915: transition WMs ask for Selected Result Blocks
-O:drivers/gpu/drm/i915/intel_pm.c:4899:30: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:4899:30: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4911:30: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:4911:30: warning: expression using sizeof(void)

Commit: drm/i915: don't write PLANE_BUF_CFG twice every time
Okay!

Commit: drm/i915: promote ddb update message to DRM_DEBUG_KMS
Okay!

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

✓ Fi.CI.BAT: success for Watermarks small fixes/improvements

[Patchwork]

== Series Details ==

Series: Watermarks small fixes/improvements
URL   : https://patchwork.freedesktop.org/series/50579/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4933 -> Patchwork_10368 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128, fdo#107139)

    igt@kms_flip@basic-plain-flip:
      fi-ilk-650:         PASS -> DMESG-WARN (fdo#106387)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (44 -> 40) ==

  Additional (2): fi-glk-j4005 fi-snb-2520m 
  Missing    (6): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 fi-pnv-d510 


== Build changes ==

    * Linux: CI_DRM_4933 -> Patchwork_10368

  CI_DRM_4933: 6b7a44d1597791524f46d7ea17620db54dffdc8c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4669: 5f40e617cd9c1e089f4a2d79c53a417d891e3e3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10368: cb6ffc487ae82bef586cce9617a7f4b150118efc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

cb6ffc487ae8 drm/i915: promote ddb update message to DRM_DEBUG_KMS
ba4585dfc75c drm/i915: don't write PLANE_BUF_CFG twice every time
bc9212a3b57e drm/i915: transition WMs ask for Selected Result Blocks
c1245720e828 drm/i915: fix the watermark result selection on glk/gen10+
47839a11088f drm/i915: fix the transition minimums for gen9+ watermarks
99b1569417cb drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+

== Logs ==

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

✓ Fi.CI.IGT: success for Watermarks small fixes/improvements

[Patchwork]

== Series Details ==

Series: Watermarks small fixes/improvements
URL   : https://patchwork.freedesktop.org/series/50579/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4933_full -> Patchwork_10368_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10368_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10368_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10368_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-blt:
      shard-hsw:          PASS -> SKIP

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          PASS -> INCOMPLETE (fdo#108074)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-hang-newfb-render-a:
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_color@pipe-a-ctm-max:
      shard-apl:          PASS -> FAIL (fdo#108147)

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
      shard-hsw:          PASS -> FAIL (fdo#105767)

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#106538, fdo#105763)

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-wc:
      shard-skl:          PASS -> FAIL (fdo#105682) +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-glk:          PASS -> FAIL (fdo#103167) +2

    igt@kms_plane@pixel-format-pipe-a-planes:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#106885)

    {igt@kms_plane_alpha_blend@pipe-a-coverage-7efc}:
      shard-skl:          PASS -> FAIL (fdo#108145)

    {igt@kms_plane_alpha_blend@pipe-b-coverage-7efc}:
      shard-skl:          NOTRUN -> FAIL (fdo#108146)

    {igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb}:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +3

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_rotation_crc@exhaust-fences:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#105748)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@kms_sysfs_edid_timing:
      shard-skl:          NOTRUN -> FAIL (fdo#100047)

    
    ==== Possible fixes ====

    igt@gem_cpu_reloc@full:
      shard-skl:          INCOMPLETE (fdo#108073) -> PASS

    igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-b:
      shard-apl:          DMESG-WARN (fdo#103558, fdo#105602) -> PASS +1

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-glk:          FAIL (fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-apl:          FAIL (fdo#103191, fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-64x64-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#105454, fdo#106509) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane@plane-position-covered-pipe-b-planes:
      shard-apl:          FAIL (fdo#103166) -> PASS

    {igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb}:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-glk:          DMESG-WARN (fdo#106538, fdo#105763) -> PASS +3

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@pm_rpm@pm-tiling:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS +1

    
  {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#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105748 https://bugs.freedesktop.org/show_bug.cgi?id=105748
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
  fdo#108147 https://bugs.freedesktop.org/show_bug.cgi?id=108147
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4933 -> Patchwork_10368

  CI_DRM_4933: 6b7a44d1597791524f46d7ea17620db54dffdc8c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4669: 5f40e617cd9c1e089f4a2d79c53a417d891e3e3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10368: cb6ffc487ae82bef586cce9617a7f4b150118efc @ 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_10368/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+

[Matt Roper]

On Thu, Oct 04, 2018 at 04:15:55PM -0700, Paulo Zanoni wrote:
> BSpec does not show these WAs as applicable to GLK, and for CNL it
> only shows them applicable for a super early pre-production stepping
> we shouldn't be caring about anymore. Remove these so we can avoid
> them on ICL too.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

From a quick grep, it looks like there's a couple other CNL A0-specific
workarounds in the codebase (in the watermark code).  If we don't want
to handle CNL A0 in this patch, then I think we should first land a
patch that removes the others and updates
intel_detect_preproduction_hw() to make it explicit that this is no
longer a supported stepping.

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 43 ++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1392aa56a55a..910551e04d16 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4687,28 +4687,31 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	res_lines = div_round_up_fixed16(selected_result,
>  					 wp->plane_blocks_per_line);
>  
> -	/* Display WA #1125: skl,bxt,kbl,glk */
> -	if (level == 0 && wp->rc_surface)
> -		res_blocks += fixed16_to_u32_round_up(wp->y_tile_minimum);
> +	if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) {
> +		/* Display WA #1125: skl,bxt,kbl */

Although the code is right, should we just write "gen9" (or gen9display)
in this comment (and the one for 1126)?  That's how the bspec lists it
(GEN9:All), and removes the ambiguity of whether "kbl" in this context
is also supposed to cover CFL, AML, WHL (which it does).


Matt

> +		if (level == 0 && wp->rc_surface)
> +			res_blocks +=
> +				fixed16_to_u32_round_up(wp->y_tile_minimum);
> +
> +		/* Display WA #1126: skl,bxt,kbl */
> +		if (level >= 1 && level <= 7) {
> +			if (wp->y_tiled) {
> +				res_blocks +=
> +				    fixed16_to_u32_round_up(wp->y_tile_minimum);
> +				res_lines += wp->y_min_scanlines;
> +			} else {
> +				res_blocks++;
> +			}
>  
> -	/* Display WA #1126: skl,bxt,kbl,glk */
> -	if (level >= 1 && level <= 7) {
> -		if (wp->y_tiled) {
> -			res_blocks += fixed16_to_u32_round_up(
> -							wp->y_tile_minimum);
> -			res_lines += wp->y_min_scanlines;
> -		} else {
> -			res_blocks++;
> +			/*
> +			 * Make sure result blocks for higher latency levels are
> +			 * atleast as high as level below the current level.
> +			 * Assumption in DDB algorithm optimization for special
> +			 * cases. Also covers Display WA #1125 for RC.
> +			 */
> +			if (result_prev->plane_res_b > res_blocks)
> +				res_blocks = result_prev->plane_res_b;
>  		}
> -
> -		/*
> -		 * Make sure result blocks for higher latency levels are atleast
> -		 * as high as level below the current level.
> -		 * Assumption in DDB algorithm optimization for special cases.
> -		 * Also covers Display WA #1125 for RC.
> -		 */
> -		if (result_prev->plane_res_b > res_blocks)
> -			res_blocks = result_prev->plane_res_b;
>  	}
>  
>  	if (INTEL_GEN(dev_priv) >= 11) {
> -- 
> 2.14.4
> 
> _______________________________________________
> 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

Re: [PATCH 2/6] drm/i915: fix the transition minimums for gen9+ watermarks

[Matt Roper]

On Thu, Oct 04, 2018 at 04:15:56PM -0700, Paulo Zanoni wrote:
> The transition minimum is 14 blocks for gens 9 and 10, and 4 blocks
> for gen 11. This minimum value is supposed to be added to the
> configurable trans_amount. This matches both BSpec and additional
> information provided by our HW engineers.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Matches what I see in the bspec.

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 910551e04d16..cab86690a0ba 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4878,8 +4878,8 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
>  	if (!dev_priv->ipc_enabled)
>  		goto exit;
>  
> -	trans_min = 0;
> -	if (INTEL_GEN(dev_priv) >= 10)
> +	trans_min = 14;
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		trans_min = 4;
>  
>  	trans_offset_b = trans_min + trans_amount;
> -- 
> 2.14.4
> 
> _______________________________________________
> 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

Re: [PATCH 3/6] drm/i915: fix the watermark result selection on glk/gen10+

[Matt Roper]

On Thu, Oct 04, 2018 at 04:15:57PM -0700, Paulo Zanoni wrote:
> On these platforms we're supposed to unconditonally pick the method 2
> result instead of the minimum.

In addition to this, it looks like the calculations for method 1 and
method 2 need a slight update.  gen10/gen11 adds an extra "+1" to the
end of the method1 calculation and also to the interm_pbpl used in
various method 2 calculations.


Matt

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index cab86690a0ba..40ce99c455f3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4672,15 +4672,24 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	} else {
>  		if ((wp->cpp * cstate->base.adjusted_mode.crtc_htotal /
>  		     wp->dbuf_block_size < 1) &&
> -		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1))
> +		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) {
>  			selected_result = method2;
> -		else if (ddb_allocation >=
> -			 fixed16_to_u32_round_up(wp->plane_blocks_per_line))
> -			selected_result = min_fixed16(method1, method2);
> -		else if (latency >= wp->linetime_us)
> -			selected_result = min_fixed16(method1, method2);
> -		else
> +		} else if (ddb_allocation >=
> +			 fixed16_to_u32_round_up(wp->plane_blocks_per_line)) {
> +			if (INTEL_GEN(dev_priv) == 9 &&
> +			    !IS_GEMINILAKE(dev_priv))
> +				selected_result = min_fixed16(method1, method2);
> +			else
> +				selected_result = method2;
> +		} else if (latency >= wp->linetime_us) {
> +			if (INTEL_GEN(dev_priv) == 9 &&
> +			    !IS_GEMINILAKE(dev_priv))
> +				selected_result = min_fixed16(method1, method2);
> +			else
> +				selected_result = method2;
> +		} else {
>  			selected_result = method1;
> +		}
>  	}
>  
>  	res_blocks = fixed16_to_u32_round_up(selected_result) + 1;
> -- 
> 2.14.4
> 
> _______________________________________________
> 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

Re: [PATCH 4/6] drm/i915: transition WMs ask for Selected Result Blocks

[Matt Roper]

On Thu, Oct 04, 2018 at 04:15:58PM -0700, Paulo Zanoni wrote:
> The transition watermarks ask for Selected Result Blocks (the real
> value), not Result Blocks (the integer value). Given how ceilings are
> applied in both the non-transition and the transition watermarks
> calculations, we can get away with assuming that Selected Result
> Blocks is actually Result Blocks minus 1 without any rounding errors.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Matches current bspec.

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

I wonder if we should make trans_amount configurable via debugfs so that
we can experiment with the impact of transitional watermarks.


Matt

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 40ce99c455f3..14f13a371989 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4874,7 +4874,7 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
>  	const struct drm_i915_private *dev_priv = to_i915(dev);
>  	uint16_t trans_min, trans_y_tile_min;
>  	const uint16_t trans_amount = 10; /* This is configurable amount */
> -	uint16_t trans_offset_b, res_blocks;
> +	uint16_t wm0_sel_res_b, trans_offset_b, res_blocks;
>  
>  	if (!cstate->base.active)
>  		goto exit;
> @@ -4893,13 +4893,25 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
>  
>  	trans_offset_b = trans_min + trans_amount;
>  
> +	/*
> +	 * The spec asks for Selected Result Blocks for wm0 (the real value),
> +	 * not Result Blocks (the integer value). Pay attention to the capital
> +	 * letters. The value wm_l0->plane_res_b is actually Result Blocks, but
> +	 * since Result Blocks is the ceiling of Selected Result Blocks plus 1,
> +	 * and since we later will have to get the ceiling of the sum in the
> +	 * transition watermarks calculation, we can just pretend Selected
> +	 * Result Blocks is Result Blocks minus 1 and it should work for the
> +	 * current platforms.
> +	 */
> +	wm0_sel_res_b = wm_l0->plane_res_b - 1;
> +
>  	if (wp->y_tiled) {
>  		trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
>  							wp->y_tile_minimum);
> -		res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
> +		res_blocks = max(wm0_sel_res_b, trans_y_tile_min) +
>  				trans_offset_b;
>  	} else {
> -		res_blocks = wm_l0->plane_res_b + trans_offset_b;
> +		res_blocks = wm0_sel_res_b + trans_offset_b;
>  
>  		/* WA BUG:1938466 add one block for non y-tile planes */
>  		if (IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
> -- 
> 2.14.4
> 
> _______________________________________________
> 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

Re: [PATCH 3/6] drm/i915: fix the watermark result selection on glk/gen10+

[Matt Roper]

On Tue, Oct 09, 2018 at 04:55:15PM -0700, Matt Roper wrote:
> On Thu, Oct 04, 2018 at 04:15:57PM -0700, Paulo Zanoni wrote:
> > On these platforms we're supposed to unconditonally pick the method 2
> > result instead of the minimum.
> 
> In addition to this, it looks like the calculations for method 1 and
> method 2 need a slight update.  gen10/gen11 adds an extra "+1" to the
> end of the method1 calculation and also to the interm_pbpl used in
> various method 2 calculations.
> 

Actually, it looks like you've already made these updates in a previous
patch; disregard this comment.

The rest of this patch looks good.

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

> 
> Matt
> 
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 23 ++++++++++++++++-------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index cab86690a0ba..40ce99c455f3 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4672,15 +4672,24 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> >  	} else {
> >  		if ((wp->cpp * cstate->base.adjusted_mode.crtc_htotal /
> >  		     wp->dbuf_block_size < 1) &&
> > -		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1))
> > +		     (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) {
> >  			selected_result = method2;
> > -		else if (ddb_allocation >=
> > -			 fixed16_to_u32_round_up(wp->plane_blocks_per_line))
> > -			selected_result = min_fixed16(method1, method2);
> > -		else if (latency >= wp->linetime_us)
> > -			selected_result = min_fixed16(method1, method2);
> > -		else
> > +		} else if (ddb_allocation >=
> > +			 fixed16_to_u32_round_up(wp->plane_blocks_per_line)) {
> > +			if (INTEL_GEN(dev_priv) == 9 &&
> > +			    !IS_GEMINILAKE(dev_priv))
> > +				selected_result = min_fixed16(method1, method2);
> > +			else
> > +				selected_result = method2;
> > +		} else if (latency >= wp->linetime_us) {
> > +			if (INTEL_GEN(dev_priv) == 9 &&
> > +			    !IS_GEMINILAKE(dev_priv))
> > +				selected_result = min_fixed16(method1, method2);
> > +			else
> > +				selected_result = method2;
> > +		} else {
> >  			selected_result = method1;
> > +		}
> >  	}
> >  
> >  	res_blocks = fixed16_to_u32_round_up(selected_result) + 1;
> > -- 
> > 2.14.4
> > 
> > _______________________________________________
> > 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

-- 
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

Re: [PATCH 5/6] drm/i915: don't write PLANE_BUF_CFG twice every time

[Matt Roper]

On Thu, Oct 04, 2018 at 04:15:59PM -0700, Paulo Zanoni wrote:
> We were writing to PLANE_BUF_CFG(pipe, plane_id) twice for every
> platform, and we were even using different values on the gen10- planar
> case. The first write is useless since it just gets replaced with the
> next one, so kill it.
> 
> There's a lot to improve in the DDB code, but let's start by avoiding
> the double write.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Looks like leftover code that was intended for removal in commit
b879d58ff31ba ("drm/i915/skl+: refactor WM calculation for NV12").

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

One other comment below.

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 14f13a371989..53b4a9a2de69 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5040,8 +5040,6 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
>  	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
>  			   &wm->trans_wm);
>  
> -	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> -			    &ddb->plane[pipe][plane_id]);
>  	/* FIXME: add proper NV12 support for ICL. */
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		return skl_ddb_entry_write(dev_priv,

Neither this function nor skl_ddb_entry_write() have return values, so
I'm surprised this doesn't raise a compiler warning.  Maybe while we're
touching this area of the code we should drop the 'return' here and
convert the next 'if' to an 'else if.'


Matt

> -- 
> 2.14.4
> 
> _______________________________________________
> 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

Re: [PATCH 6/6] drm/i915: promote ddb update message to DRM_DEBUG_KMS

[Matt Roper]

On Thu, Oct 04, 2018 at 04:16:00PM -0700, Paulo Zanoni wrote:
> This message is currently marked as DRM_DEBUG_ATOMIC. I would like it
> to be DRM_DEBUG_KMS since it is more KMS than atomic, and this will
> also make the message appear in the CI logs, which may or may not help
> us with some FIFO underrun bugs.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@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 53b4a9a2de69..6cacddd16010 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5233,11 +5233,11 @@ skl_print_wm_changes(const struct drm_atomic_state *state)
>  			if (skl_ddb_entry_equal(old, new))
>  				continue;
>  
> -			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] ddb (%d - %d) -> (%d - %d)\n",
> -					 intel_plane->base.base.id,
> -					 intel_plane->base.name,
> -					 old->start, old->end,
> -					 new->start, new->end);
> +			DRM_DEBUG_KMS("[PLANE:%d:%s] ddb (%d - %d) -> (%d - %d)\n",
> +				      intel_plane->base.base.id,
> +				      intel_plane->base.name,
> +				      old->start, old->end,
> +				      new->start, new->end);
>  		}
>  	}
>  }
> -- 
> 2.14.4
> 
> _______________________________________________
> 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

Re: [PATCH 1/6] drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+

[Paulo Zanoni]

Em Ter, 2018-10-09 às 16:55 -0700, Matt Roper escreveu:
> On Thu, Oct 04, 2018 at 04:15:55PM -0700, Paulo Zanoni wrote:
> > BSpec does not show these WAs as applicable to GLK, and for CNL it
> > only shows them applicable for a super early pre-production
> > stepping
> > we shouldn't be caring about anymore. Remove these so we can avoid
> > them on ICL too.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> From a quick grep, it looks like there's a couple other CNL A0-
> specific
> workarounds in the codebase (in the watermark code).  If we don't
> want
> to handle CNL A0 in this patch, then I think we should first land a
> patch that removes the others and updates
> intel_detect_preproduction_hw() to make it explicit that this is no
> longer a supported stepping.

This patch (and series) is potentially a "bug fix" due to changes in
real-world not-A0 hardware (although I didn't mark this one as a fix
since I doubt it will close anything in Bugzilla). The patch to remove
the implementation of WAs in CNL:A0 is not a bug fix, but a rework.
Fixes should always come before the reworks in order to facilitate
backporting efforts.

I do intend to propose patches removing the CNL A0 watermarks code
(along with other reworks I have in mind), but I wanted to sort the
bugs first.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 43 ++++++++++++++++++++++-------
> > ------------
> >  1 file changed, 23 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 1392aa56a55a..910551e04d16 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4687,28 +4687,31 @@ static int skl_compute_plane_wm(const
> > struct drm_i915_private *dev_priv,
> >  	res_lines = div_round_up_fixed16(selected_result,
> >  					 wp-
> > >plane_blocks_per_line);
> >  
> > -	/* Display WA #1125: skl,bxt,kbl,glk */
> > -	if (level == 0 && wp->rc_surface)
> > -		res_blocks += fixed16_to_u32_round_up(wp-
> > >y_tile_minimum);
> > +	if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) {
> > +		/* Display WA #1125: skl,bxt,kbl */
> 
> Although the code is right, should we just write "gen9" (or
> gen9display)
> in this comment (and the one for 1126)?  That's how the bspec lists
> it
> (GEN9:All), and removes the ambiguity of whether "kbl" in this
> context
> is also supposed to cover CFL, AML, WHL (which it does).

Although that makes sense, it doesn't seem to follow the current
(undocumented?) standard we have for display WAs. I can totally do
this, but I would like some more opinions first. CC'ing the
maintainers.

Thanks for the reviews,
Paulo
> 
> 
> Matt
> 
> > +		if (level == 0 && wp->rc_surface)
> > +			res_blocks +=
> > +				fixed16_to_u32_round_up(wp-
> > >y_tile_minimum);
> > +
> > +		/* Display WA #1126: skl,bxt,kbl */
> > +		if (level >= 1 && level <= 7) {
> > +			if (wp->y_tiled) {
> > +				res_blocks +=
> > +				    fixed16_to_u32_round_up(wp-
> > >y_tile_minimum);
> > +				res_lines += wp->y_min_scanlines;
> > +			} else {
> > +				res_blocks++;
> > +			}
> >  
> > -	/* Display WA #1126: skl,bxt,kbl,glk */
> > -	if (level >= 1 && level <= 7) {
> > -		if (wp->y_tiled) {
> > -			res_blocks += fixed16_to_u32_round_up(
> > -							wp-
> > >y_tile_minimum);
> > -			res_lines += wp->y_min_scanlines;
> > -		} else {
> > -			res_blocks++;
> > +			/*
> > +			 * Make sure result blocks for higher
> > latency levels are
> > +			 * atleast as high as level below the
> > current level.
> > +			 * Assumption in DDB algorithm
> > optimization for special
> > +			 * cases. Also covers Display WA #1125 for
> > RC.
> > +			 */
> > +			if (result_prev->plane_res_b > res_blocks)
> > +				res_blocks = result_prev-
> > >plane_res_b;
> >  		}
> > -
> > -		/*
> > -		 * Make sure result blocks for higher latency
> > levels are atleast
> > -		 * as high as level below the current level.
> > -		 * Assumption in DDB algorithm optimization for
> > special cases.
> > -		 * Also covers Display WA #1125 for RC.
> > -		 */
> > -		if (result_prev->plane_res_b > res_blocks)
> > -			res_blocks = result_prev->plane_res_b;
> >  	}
> >  
> >  	if (INTEL_GEN(dev_priv) >= 11) {
> > -- 
> > 2.14.4
> > 
> > _______________________________________________
> > 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