- 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@xxxxxxxxxxxxxxx> Cc: rafael.ristovski@xxxxxxxxx Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
