On Wed, Sep 13, 2017 at 11:25:54AM +0200, Maarten Lankhorst wrote:
> Commit b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.") removed
> the call to wait_for_vblanks and replaced it with flip_done.
>
> Unfortunately legacy_cursor_update was unset too late, and the
> replacement call drm_atomic_helper_wait_for_flip_done() was
> a noop. Make sure that its unset before setup_commit() is
> called to fix this issue.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Fixes: b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
> Testcase: kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Reported-by: Marta Löfstedt <marta.lofstedt@xxxxxxxxx>
> Cc: Marta Löfstedt <marta.lofstedt@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ba76b2db939c..c1da48d77c8e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12412,23 +12412,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;
I'd prefer to see intel_ types.
> + 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
> @@ -12437,19 +12428,32 @@ static int intel_atomic_commit(struct drm_device *dev,
> * updates happen during the correct frames. Gen9+ have
> * double buffered watermarks and so shouldn't need this.
> *
> - * Do this after drm_atomic_helper_setup_commit() and
> - * intel_atomic_prepare_commit() because we still want
> - * to skip the flip and fb cleanup waits. Although that
> - * does risk yanking the mapping from under the display
> - * engine.
> + * Unset state->legacy_cursor_update before the call to
> + * drm_atomic_helper_setup_commit() because otherwise
> + * drm_atomic_helper_wait_for_flip_done() is a noop and
> + * we get FIFO underruns because we didn't wait
> + * for vblank.
> *
> * 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;
> + }
Hmm. I don't think we can rely on need_postvbl_update at this point.
Pre-g4x code won't set it. We already made the decision to not use the
fast path, so explicitly setting legacy_cursor_update=false should still
be fine, no?
> +
> + 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);
>
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
