[PATCH] drm/i915: Don't stall too much for cursor vs. pageflip

* [PATCH] drm/i915: Don't stall too much for cursor vs. pageflip
@ 2016-06-22 13:47 Daniel Vetter
  2016-06-22 14:15 ` ✗ Ro.CI.BAT: warning for " Patchwork
  2016-06-22 20:29 ` [PATCH] " Chris Wilson
  0 siblings, 2 replies; 3+ messages in thread
From: Daniel Vetter @ 2016-06-22 13:47 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter, rafael.ristovski

- We don't need to wait for the previous commit to have fully
  completed, the waiting for hw_done in swap_state is sufficient.

- We need to make sure that the pure page_flip signals hw_done early
  enough. This is done through a gross hack by recomputing stuff. I
  think it'd be better to fix this by moving things around a bit,
  keeping separate state pointers where needed (e.g. hw verifier) and
  for the wm optimization, by extracting that into a cancellable work
  item.

But legacy cursor vs. legacy page-flip is a very restricted use case,
and we need to make it work meanwhile.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96593
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: rafael.ristovski@gmail.com
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

---

Totally untested. I think we want at least an igt that runs legacy
page flips against a storm of cursor movements and checks that the
curosr movements never stalls. I think we can be slightly more lenient
with cursor bo updates. Storms with those tend to happen when X starts
up, or changes configurations. Not when moving it around. But I might
be mistaken on that one, iirc at least some older versions of
modesetting had a silly flag set which resulted in the cursor getting
disabled, changed and then re-enabled on each update "to avoid
tearing" or something like that. But I think that was fixed meanwhile.

Need to hand this off to Maarten since I'm going on vacation this
Friday for 2 weeks, and just a bit of time left to wind down various
things.

Thanks, Daniel
---
 drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc33bf278cc2..ff66b73fa412 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13706,6 +13706,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	struct drm_plane *plane;
 	struct drm_plane_state *plane_state;
 	bool hw_check = intel_state->modeset;
+	bool need_post_state;
 	unsigned long put_domains[I915_MAX_PIPES] = {};
 	unsigned crtc_vblank_mask = 0;
 	int i, ret;
@@ -13724,7 +13725,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		WARN_ON(ret);
 	}
 
-	drm_atomic_helper_wait_for_dependencies(state);
+	if (!state->legacy_cursor_update)
+		drm_atomic_helper_wait_for_dependencies(state);
 
 	if (intel_state->modeset) {
 		memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
@@ -13821,6 +13823,17 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 			crtc_vblank_mask |= 1 << i;
 	}
 
+	need_post_state = false;
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		intel_cstate = to_intel_crtc_state(crtc->state);
+
+		if (intel_cstate->wm.need_postvbl_update)
+			need_post_state = true;
+
+		if (needs_modeset(crtc->state) || intel_cstate->update_pipe)
+			need_post_state = true;
+	}
+
 	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
 	 * already, but still need the state for the delayed optimization. To
 	 * fix this:
@@ -13830,6 +13843,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	 * - switch over to the vblank wait helper in the core after that since
 	 *   we don't need out special handling any more.
 	 */
+	if (!need_post_state)
+		drm_atomic_helper_commit_hw_done(state);
+
 	if (!state->legacy_cursor_update)
 		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
 
@@ -13856,7 +13872,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state);
 	}
 
-	drm_atomic_helper_commit_hw_done(state);
+	if (need_post_state)
+		drm_atomic_helper_commit_hw_done(state);
 
 	if (intel_state->modeset)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
-- 
2.8.1

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

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