Bug 102675 - [BDW] *ERROR* CPU pipe A|B|C FIFO underrun - Regression - Bisected
Summary: [BDW] *ERROR* CPU pipe A|B|C FIFO underrun - Regression - Bisected
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: DRI git
Hardware: Other All
: medium normal
Assignee: Maarten Lankhorst
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard: PatchSubmitted
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-12 12:36 UTC by Marta Löfstedt
Modified: 2018-03-02 15:41 UTC (History)
1 user (show)

See Also:
i915 platform: BDW
i915 features: display/atomic


Attachments

Description Marta Löfstedt 2017-09-12 12:36:21 UTC
After patchset 
https://patchwork.freedesktop.org/series/29538/

I am getting a lot of FIFO underruns on my BDW NUCi5. This can start during boot up, but if not it is 100% reproducible with igt@kms_cursor_crc@cursor-256x256-random.

Also, this appear to be reproduced on the snb-shards from CI_DRM_3061, when this patch-set was merged. For example igt@kms_cursor_crc@cursor-256x256-random and igt@kms_cursor_crc@cursor-256x256-onscreen, stop flip-flopping and started to always dmesg-warn on FIFO underrun- I.e. BUG 102373 is at least partially duplicate to this issue.
Comment 1 Marta Löfstedt 2017-09-12 12:49:24 UTC
I have tried reverting the whole patch-set but then the machine lock up while running the tests. So, maybe there is more stuff on the drm-misc that build on top of the patch-set.

Anyways drm-tip,
git reset 3ff558e78bc is GOOD
git reset b44d5c0c105 is BAD
Comment 2 Marta Löfstedt 2017-09-13 06:49:12 UTC
Maarten Lankhorst suggested to test with the following:

"
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f77e73c72662..f096c55b35fa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12514,23 +12514,14 @@ static int intel_atomic_commit(struct drm_device *dev,
 {
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret = 0;
-
-	ret = drm_atomic_helper_setup_commit(state, nonblock);
-	if (ret)
-		return ret;
+	struct drm_crtc_state *new_crtc_state;
+	struct drm_crtc *crtc;
+	int i, ret = 0;
 
 	drm_atomic_state_get(state);
 	i915_sw_fence_init(&intel_state->commit_ready,
 			   intel_atomic_commit_ready);
 
-	ret = intel_atomic_prepare_commit(dev, state);
-	if (ret) {
-		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
-		i915_sw_fence_commit(&intel_state->commit_ready);
-		return ret;
-	}
-
 	/*
 	 * The intel_legacy_cursor_update() fast path takes care
 	 * of avoiding the vblank waits for simple cursor
@@ -12548,10 +12539,23 @@ static int intel_atomic_commit(struct drm_device *dev,
 	 * FIXME doing watermarks and fb cleanup from a vblank worker
 	 * (assuming we had any) would solve these problems.
 	 */
-	if (INTEL_GEN(dev_priv) < 9)
-		state->legacy_cursor_update = false;
+	if (INTEL_GEN(dev_priv) < 9 && state->legacy_cursor_update) {
+		for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
+			if (to_intel_crtc_state(new_crtc_state)->wm.need_postvbl_update)
+				state->legacy_cursor_update = false;
+	}
+
+	ret = intel_atomic_prepare_commit(dev, state);
+	if (ret) {
+		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
+		i915_sw_fence_commit(&intel_state->commit_ready);
+		return ret;
+	}
+
+	ret = drm_atomic_helper_setup_commit(state, nonblock);
+	if (!ret)
+		ret = drm_atomic_helper_swap_state(state, true);
 
-	ret = drm_atomic_helper_swap_state(state, true);
 	if (ret) {
 		i915_sw_fence_commit(&intel_state->commit_ready);
 
@@ -12579,7 +12583,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 	else
 		intel_atomic_commit_tail(state);
 
-
 	return 0;
 }
"

With above I can't reproduce the issue.
Comment 3 Maarten Lankhorst 2017-09-13 09:34:33 UTC
https://patchwork.freedesktop.org/series/30273/
Comment 4 Jani Saarinen 2017-09-27 12:31:32 UTC
New series: https://patchwork.freedesktop.org/series/30587/
Under ack / comment.
Comment 5 Marta Löfstedt 2017-10-04 10:21:05 UTC
Maartens patch was merged in: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_3170/

At least:
igt@kms_cursor_crc@cursor-256x256-[dpms|onscreen|random] are now passing for the snb-shards.
Comment 6 Maarten Lankhorst 2017-10-04 11:22:50 UTC
commit 3cf50c63a76177e0bbe0e46e1abe4eb263128ba4
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date:   Tue Sep 19 14:14:18 2017 +0200

    drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
Comment 7 Elizabeth 2018-03-02 15:41:47 UTC
Assuming that this is completely fixed since no new sights has been reported. Otherwise, please reopen.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.