| Message ID | 1466772243-21879-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@snewbury.org.uk> > Reported-by: Rafael Ristovski <rafael.ristovski@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@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Maarten Lankhorst <maarten.lankhorst@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" ?
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@snewbury.org.uk> > > Reported-by: Rafael Ristovski <rafael.ristovski@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@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Maarten Lankhorst <maarten.lankhorst@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
Hi Chris, On 24 June 2016 at 22:44, Chris Wilson <chris@chris-wilson.co.uk> wrote: > 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. Does this change at all with the fixup in https://lists.freedesktop.org/archives/intel-gfx/2016-June/099466.html ? I was seeing no waits whatsoever for rendering, causing active rendering to be visible towards positive x and negative y (top-right corner) with this. > The cursor is also being stalled by page flips, causing a "heavy mouse" > and jitter. This I can't attest to though. Cheers, Daniel
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@snewbury.org.uk> >>> Reported-by: Rafael Ristovski <rafael.ristovski@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@chris-wilson.co.uk> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Cc: Maarten Lankhorst <maarten.lankhorst@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@linux.intel.com>
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@snewbury.org.uk> > >>> Reported-by: Rafael Ristovski <rafael.ristovski@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@chris-wilson.co.uk> > >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>> Cc: Maarten Lankhorst <maarten.lankhorst@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@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
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 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@snewbury.org.uk> Reported-by: Rafael Ristovski <rafael.ristovski@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@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)