On Wed, Jun 22, 2016 at 03:47:15PM +0200, Daniel Vetter wrote:
> - 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
Testcase: igt/kms_cursor_legacy/basic-cursor-vs-flip
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: rafael.ristov...@gmail.com
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
> 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);
This looks like a hack. Ideally the computed dependencies for the cursor
update would be nil, right?
> 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;
> + }
This + the bifurcation of hw_done makes sense, but again shouldn't it
fall out that !need_post_state is just a series of noops?
I tried it on my workstation in the hope that the flicker was somehow a
result of the cursor stalls. Sadly not. :(
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
