[4/8] drm/i915/gen9: fix the WaWmMemoryReadLatency implementation

Commit Message

Zanoni, Paulo R Sept. 7, 2016, 12:52 a.m. UTC

Bspec says:
  "The mailbox response data may not account for memory read latency.
   If the mailbox response data for level 0 is 0us, add 2 microseconds
   to the result for each valid level."

This means we should only do the +2 in case wm[0] == 0, not always.

So split the sanitizing implementation from the WA implementation and
fix the WA implementation.

Signed-off-by: Paulo Zanoni <[e-mail]>
---
 drivers/gpu/drm/i915/intel_pm.c | 42 +++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

Comments

Maarten Lankhorst Sept. 13, 2016, 12:28 p.m. UTC | #1

Op 07-09-16 om 02:52 schreef Paulo Zanoni:
> Bspec says:
>   "The mailbox response data may not account for memory read latency.
>    If the mailbox response data for level 0 is 0us, add 2 microseconds
>    to the result for each valid level."
>
> This means we should only do the +2 in case wm[0] == 0, not always.
>
> So split the sanitizing implementation from the WA implementation and
> fix the WA implementation.
>
> Signed-off-by: Paulo Zanoni <[e-mail]>
This one probably needs a dim fixes $commit, just like patch 6 and 7.

For the rest the changes look sane, so with the review comments in the other patches addressed:
Reviewed-by: Maarten Lankhorst <[e-mail]>

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index baacd95..f8ac928 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2127,32 +2127,34 @@  static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
 				GEN9_MEM_LATENCY_LEVEL_MASK;
 
 		/*
+		 * If a level n (n > 1) has a 0us latency, all levels m (m >= n)
+		 * need to be disabled. We make sure to sanitize the values out
+		 * of the punit to satisfy this requirement.
+		 */
+		for (level = 1; level <= max_level; level++) {
+			if (wm[level] == 0) {
+				for (i = level + 1; i <= max_level; i++)
+					wm[i] = 0;
+				break;
+			}
+		}
+
+		/*
 		 * WaWmMemoryReadLatency:skl
 		 *
 		 * punit doesn't take into account the read latency so we need
-		 * to add 2us to the various latency levels we retrieve from
-		 * the punit.
-		 *   - W0 is a bit special in that it's the only level that
-		 *   can't be disabled if we want to have display working, so
-		 *   we always add 2us there.
-		 *   - For levels >=1, punit returns 0us latency when they are
-		 *   disabled, so we respect that and don't add 2us then
-		 *
-		 * Additionally, if a level n (n > 1) has a 0us latency, all
-		 * levels m (m >= n) need to be disabled. We make sure to
-		 * sanitize the values out of the punit to satisfy this
-		 * requirement.
+		 * to add 2us to the various latency levels we retrieve from the
+		 * punit when level 0 response data us 0us.
 		 */
-		wm[0] += 2;
-		for (level = 1; level <= max_level; level++)
-			if (wm[level] != 0)
+		if (wm[0] == 0) {
+			wm[0] += 2;
+			for (level = 1; level <= max_level; level++) {
+				if (wm[level] == 0)
+					break;
 				wm[level] += 2;
-			else {
-				for (i = level + 1; i <= max_level; i++)
-					wm[i] = 0;
-
-				break;
 			}
+		}
+
 	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		uint64_t sskpd = I915_READ64(MCH_SSKPD);