[Intel-gfx] [PATCH] Revert "drm/i915: Use atomic commits for legacy page_flips"

[Chris Wilson]

On Tue, Jun 28, 2016 at 04:44:42PM +0200, Maarten Lankhorst wrote:
> Op 28-06-16 om 10:35 schreef Chris Wilson:
> > On Tue, Jun 28, 2016 at 09:56:47AM +0200, Maarten Lankhorst wrote:
> >> Op 24-06-16 om 14:44 schreef Chris Wilson:
> >>> This reverts commit ee042aa40b66d18d465206845b0752c6a617ba3f.
> >>>
> >>> Something appears to be off in the timing, but as far as I can tell it
> >>> is not along the event delivery path. The net effect appears to be
> >>> rendering flicker (the current render buffer appears on the scanout,
> >>> with what appears to be active rendering for a fraction of a frame) and
> >>> is causing me a headache.
> >>>
> >>> The cursor is also being stalled by page flips, causing a "heavy mouse"
> >>> and jitter.
> >>>
> >>> Reported-and-tested-by: Steven Newbury <steve at snewbury.org.uk>
> >>> Reported-by:  Rafael Ristovski  <rafael.ristovski at gmail.com>
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96593
> >>> Testcase: igt/kms_cursor_legacy/basic-flip-vs-cursor
> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_display.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index 1141b86..d4a7872 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -11646,7 +11646,6 @@ void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe)
> >>>  	spin_unlock(&dev->event_lock);
> >>>  }
> >>>  
> >>> -__maybe_unused
> >>>  static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >>>  				struct drm_framebuffer *fb,
> >>>  				struct drm_pending_vblank_event *event,
> >>> @@ -14008,7 +14007,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
> >>>  	.set_config = drm_atomic_helper_set_config,
> >>>  	.set_property = drm_atomic_helper_crtc_set_property,
> >>>  	.destroy = intel_crtc_destroy,
> >>> -	.page_flip = drm_atomic_helper_page_flip,
> >>> +	.page_flip = intel_crtc_page_flip,
> >>>  	.atomic_duplicate_state = intel_crtc_duplicate_state,
> >>>  	.atomic_destroy_state = intel_crtc_destroy_state,
> >>>  };
> >> This still wouldn't fix updates based on nonblocking atomic commits, though probably less likely.
> >>
> >> Does this bug get fixed by "drm/i915: Don't stall too much for cursor vs. pageflip" ?
> > Only one side, cursor still stalls flips (or rather a flip can miss 10s of
> > vblanks under load), and the wrong buffers are still visible.
> > -Chris
> Since this only affects legacy page flips the fix is fine for now till we fix the root cause.
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

I've pushed this now. Daniel has undoubtably found the cause of the
tearing bug, but he is also right that with one old/new state mixup
there is likely more. Then there is the challenge of getting
nonblocking cursors and unstalling flips.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre