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