* [PATCH] drm/i915: Do not trigger the pageflip stall check too early @ 2014-12-17 16:41 Chris Wilson 2014-12-17 16:48 ` Chris Wilson 2014-12-18 5:30 ` shuang.he 0 siblings, 2 replies; 7+ messages in thread From: Chris Wilson @ 2014-12-17 16:41 UTC (permalink / raw) To: intel-gfx On gen2-4, we have a separate pageflip prepare/finish phase. The intent of the stall check is to detect when we have incurred a delay, potentially indefinite, after the pageflip is submitted and before the flip is processed by the hardware. However, our notion of INTEL_FLIP_PENDING/INTEL_FLIP_COMPLETE do not tally with how we set the values during prepare/finish and the current stall check erroneously assumes that when the pending value >= COMPLETE, the driver has seen the hardware completion flag. But what the driver actually means, is that it has seen the acknowlegement that the flip is queued and is now pending the completion event. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85888 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 70a75136bc61..0c43d970dbf9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9707,8 +9707,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev, struct intel_unpin_work *work = intel_crtc->unpin_work; u32 addr; - if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) - return true; + if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) + return false; if (!work->enable_stall_check) return false; -- 2.1.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Do not trigger the pageflip stall check too early 2014-12-17 16:41 [PATCH] drm/i915: Do not trigger the pageflip stall check too early Chris Wilson @ 2014-12-17 16:48 ` Chris Wilson 2014-12-17 18:19 ` Ville Syrjälä 2014-12-18 5:30 ` shuang.he 1 sibling, 1 reply; 7+ messages in thread From: Chris Wilson @ 2014-12-17 16:48 UTC (permalink / raw) To: intel-gfx On Wed, Dec 17, 2014 at 04:41:42PM +0000, Chris Wilson wrote: > On gen2-4, we have a separate pageflip prepare/finish phase. The intent > of the stall check is to detect when we have incurred a delay, > potentially indefinite, after the pageflip is submitted and before the > flip is processed by the hardware. However, our notion of > INTEL_FLIP_PENDING/INTEL_FLIP_COMPLETE do not tally with how we set the > values during prepare/finish and the current stall check erroneously > assumes that when the pending value >= COMPLETE, the driver has seen the > hardware completion flag. But what the driver actually means, is that it > has seen the acknowlegement that the flip is queued and is now pending > the completion event. Bah, otoh, as we don't mark the flip as pending before completion on gen5+, we know don't detect when the flip is wedged there after applying this patch. It is quite possible that the work->pending check is entirely bogus. Back to the drawing board. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Do not trigger the pageflip stall check too early 2014-12-17 16:48 ` Chris Wilson @ 2014-12-17 18:19 ` Ville Syrjälä 2014-12-17 18:35 ` Chris Wilson 0 siblings, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2014-12-17 18:19 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Wed, Dec 17, 2014 at 04:48:19PM +0000, Chris Wilson wrote: > On Wed, Dec 17, 2014 at 04:41:42PM +0000, Chris Wilson wrote: > > On gen2-4, we have a separate pageflip prepare/finish phase. The intent > > of the stall check is to detect when we have incurred a delay, > > potentially indefinite, after the pageflip is submitted and before the > > flip is processed by the hardware. However, our notion of > > INTEL_FLIP_PENDING/INTEL_FLIP_COMPLETE do not tally with how we set the > > values during prepare/finish and the current stall check erroneously > > assumes that when the pending value >= COMPLETE, the driver has seen the > > hardware completion flag. But what the driver actually means, is that it > > has seen the acknowlegement that the flip is queued and is now pending > > the completion event. > > Bah, otoh, as we don't mark the flip as pending before completion on > gen5+, we know don't detect when the flip is wedged there after applying > this patch. > > It is quite possible that the work->pending check is entirely bogus. > Back to the drawing board. We just apply my fix. And then we can even get rid of the whole prepare/finish mess. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Do not trigger the pageflip stall check too early 2014-12-17 18:19 ` Ville Syrjälä @ 2014-12-17 18:35 ` Chris Wilson 2014-12-17 20:59 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Chris Wilson @ 2014-12-17 18:35 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Wed, Dec 17, 2014 at 08:19:00PM +0200, Ville Syrjälä wrote: > On Wed, Dec 17, 2014 at 04:48:19PM +0000, Chris Wilson wrote: > > On Wed, Dec 17, 2014 at 04:41:42PM +0000, Chris Wilson wrote: > > > On gen2-4, we have a separate pageflip prepare/finish phase. The intent > > > of the stall check is to detect when we have incurred a delay, > > > potentially indefinite, after the pageflip is submitted and before the > > > flip is processed by the hardware. However, our notion of > > > INTEL_FLIP_PENDING/INTEL_FLIP_COMPLETE do not tally with how we set the > > > values during prepare/finish and the current stall check erroneously > > > assumes that when the pending value >= COMPLETE, the driver has seen the > > > hardware completion flag. But what the driver actually means, is that it > > > has seen the acknowlegement that the flip is queued and is now pending > > > the completion event. > > > > Bah, otoh, as we don't mark the flip as pending before completion on > > gen5+, we know don't detect when the flip is wedged there after applying > > this patch. > > > > It is quite possible that the work->pending check is entirely bogus. > > Back to the drawing board. > > We just apply my fix. And then we can even get rid of the whole > prepare/finish mess. I concur. I was trying to see if there was any value in trusting IIR but not ISR for gen2/3, but that's silly. So with the current flip interrupt handling we really do not have the two phases anymore. https://bugs.freedesktop.org/attachment.cgi?id=110912 Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Do not trigger the pageflip stall check too early 2014-12-17 18:35 ` Chris Wilson @ 2014-12-17 20:59 ` Daniel Vetter 2014-12-17 21:19 ` Ville Syrjälä 0 siblings, 1 reply; 7+ messages in thread From: Daniel Vetter @ 2014-12-17 20:59 UTC (permalink / raw) To: Chris Wilson, Ville Syrjälä, intel-gfx, Jani Nikula On Wed, Dec 17, 2014 at 06:35:04PM +0000, Chris Wilson wrote: > On Wed, Dec 17, 2014 at 08:19:00PM +0200, Ville Syrjälä wrote: > > On Wed, Dec 17, 2014 at 04:48:19PM +0000, Chris Wilson wrote: > > > On Wed, Dec 17, 2014 at 04:41:42PM +0000, Chris Wilson wrote: > > > > On gen2-4, we have a separate pageflip prepare/finish phase. The intent > > > > of the stall check is to detect when we have incurred a delay, > > > > potentially indefinite, after the pageflip is submitted and before the > > > > flip is processed by the hardware. However, our notion of > > > > INTEL_FLIP_PENDING/INTEL_FLIP_COMPLETE do not tally with how we set the > > > > values during prepare/finish and the current stall check erroneously > > > > assumes that when the pending value >= COMPLETE, the driver has seen the > > > > hardware completion flag. But what the driver actually means, is that it > > > > has seen the acknowlegement that the flip is queued and is now pending > > > > the completion event. > > > > > > Bah, otoh, as we don't mark the flip as pending before completion on > > > gen5+, we know don't detect when the flip is wedged there after applying > > > this patch. > > > > > > It is quite possible that the work->pending check is entirely bogus. > > > Back to the drawing board. > > > > We just apply my fix. And then we can even get rid of the whole > > prepare/finish mess. > > I concur. I was trying to see if there was any value in trusting IIR but > not ISR for gen2/3, but that's silly. So with the current flip interrupt > handling we really do not have the two phases anymore. > > https://bugs.freedesktop.org/attachment.cgi?id=110912 > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Jani can you please pick this on up directly? It doesn't seem to have ever reached intel-gfx unfortunately, at least google doesn't find it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Do not trigger the pageflip stall check too early 2014-12-17 20:59 ` Daniel Vetter @ 2014-12-17 21:19 ` Ville Syrjälä 0 siblings, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2014-12-17 21:19 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Wed, Dec 17, 2014 at 09:59:09PM +0100, Daniel Vetter wrote: > On Wed, Dec 17, 2014 at 06:35:04PM +0000, Chris Wilson wrote: > > On Wed, Dec 17, 2014 at 08:19:00PM +0200, Ville Syrjälä wrote: > > > On Wed, Dec 17, 2014 at 04:48:19PM +0000, Chris Wilson wrote: > > > > On Wed, Dec 17, 2014 at 04:41:42PM +0000, Chris Wilson wrote: > > > > > On gen2-4, we have a separate pageflip prepare/finish phase. The intent > > > > > of the stall check is to detect when we have incurred a delay, > > > > > potentially indefinite, after the pageflip is submitted and before the > > > > > flip is processed by the hardware. However, our notion of > > > > > INTEL_FLIP_PENDING/INTEL_FLIP_COMPLETE do not tally with how we set the > > > > > values during prepare/finish and the current stall check erroneously > > > > > assumes that when the pending value >= COMPLETE, the driver has seen the > > > > > hardware completion flag. But what the driver actually means, is that it > > > > > has seen the acknowlegement that the flip is queued and is now pending > > > > > the completion event. > > > > > > > > Bah, otoh, as we don't mark the flip as pending before completion on > > > > gen5+, we know don't detect when the flip is wedged there after applying > > > > this patch. > > > > > > > > It is quite possible that the work->pending check is entirely bogus. > > > > Back to the drawing board. > > > > > > We just apply my fix. And then we can even get rid of the whole > > > prepare/finish mess. > > > > I concur. I was trying to see if there was any value in trusting IIR but > > not ISR for gen2/3, but that's silly. So with the current flip interrupt > > handling we really do not have the two phases anymore. > > > > https://bugs.freedesktop.org/attachment.cgi?id=110912 > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Jani can you please pick this on up directly? It doesn't seem to have ever > reached intel-gfx unfortunately, at least google doesn't find it. It was waiting for test results. Posted now: http://lists.freedesktop.org/archives/intel-gfx/2014-December/057746.html -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Do not trigger the pageflip stall check too early 2014-12-17 16:41 [PATCH] drm/i915: Do not trigger the pageflip stall check too early Chris Wilson 2014-12-17 16:48 ` Chris Wilson @ 2014-12-18 5:30 ` shuang.he 1 sibling, 0 replies; 7+ messages in thread From: shuang.he @ 2014-12-18 5:30 UTC (permalink / raw) To: shuang.he, intel-gfx, chris Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK +1-1 364/366 364/366 SNB 448/450 448/450 IVB 497/498 497/498 BYT 289/289 289/289 HSW 563/564 563/564 BDW 416/417 416/417 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied *ILK igt_kms_pipe_crc_basic_bad-nb-words-1 PASS(2, M26) DMESG_WARN(1, M26) ILK igt_kms_flip_bcs-flip-vs-modeset-interruptible DMESG_WARN(1, M26)PASS(2, M37M26) PASS(1, M26) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-12-18 5:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-17 16:41 [PATCH] drm/i915: Do not trigger the pageflip stall check too early Chris Wilson 2014-12-17 16:48 ` Chris Wilson 2014-12-17 18:19 ` Ville Syrjälä 2014-12-17 18:35 ` Chris Wilson 2014-12-17 20:59 ` Daniel Vetter 2014-12-17 21:19 ` Ville Syrjälä 2014-12-18 5:30 ` shuang.he
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.