Attachment 110912 Details for Bug 85888 – [PATCH] drm/i915: Don't call intel_prepare_page_flip() multiple times on gen2-4

[patch] [PATCH] drm/i915: Don't call intel_prepare_page_flip() multiple times on gen2-4

0001-drm-i915-Don-t-call-intel_prepare_page_flip-multiple.patch (text/plain), 3.29 KB, created by Ville Syrjala on 2014-12-16 16:42:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ville Syrjala

Created: 2014-12-16 16:42:54 UTC

Size: 3.29 KB

patch

obsolete

>From ef1ab395c6b9d9bddb798a6df1a4f5320c3561b2 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
>Date: Thu, 4 Dec 2014 15:29:07 +0200
>Subject: [PATCH] drm/i915: Don't call intel_prepare_page_flip() multiple times
> on gen2-4
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>The flip stall detector kicks in when pending>=INTEL_FLIP_COMPLETE. That
>means if we first call intel_prepare_page_flip() but don't call
>intel_finish_page_flip(), the next stall check will erroneosly think
>the page flip was somehow stuck.
>
>With enough debug spew emitted from the interupt handler my 830 hangs
>when this happens. My theory is that the previous vblank interrupt gets
>sufficiently delayed that the handler will see the pending bit set in
>IIR, but ISR still has the bit set as well (ie. the flip was processed
>by CS but didn't complete yet). In this case the handler will proceed
>to call intel_check_page_flip() immediately after
>intel_prepare_page_flip(). It then tries to print a backtrace for the
>stuck flip WARN, which apparetly results in way too much debug spew
>delaying interrupt processing further. That then seems to cause an
>endless loop in the interrupt handler, and the machine is dead until
>the watchdog kicks in and reboots. At least limiting the number of
>iterations of the loop in the interrupt handler also prevented the
>hang.
>
>So it seems better to not call intel_prepare_page_flip() without
>immediately calling intel_finish_page_flip(). The IIR/ISR trickery
>avoids races here so this is a perfectly safe thing to do.
>
>Cc: stable@vger.kernel.org
>Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88381
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85888
>Signed-off-by: Ville SyrjÃ¤lÃ¤ <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_irq.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index 5d83773..aa3180c 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -3731,8 +3731,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
> 	if ((iir & flip_pending) == 0)
> 		goto check_page_flip;
> 
>-	intel_prepare_page_flip(dev, plane);
>-
> 	/* We detect FlipDone by looking for the change in PendingFlip from '1'
> 	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
> 	 * asserted following the MI_DISPLAY_FLIP, but ISR is deasserted, hence
>@@ -3742,6 +3740,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
> 	if (I915_READ16(ISR) & flip_pending)
> 		goto check_page_flip;
> 
>+	intel_prepare_page_flip(dev, plane);
> 	intel_finish_page_flip(dev, pipe);
> 	return true;
> 
>@@ -3913,8 +3912,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
> 	if ((iir & flip_pending) == 0)
> 		goto check_page_flip;
> 
>-	intel_prepare_page_flip(dev, plane);
>-
> 	/* We detect FlipDone by looking for the change in PendingFlip from '1'
> 	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
> 	 * asserted following the MI_DISPLAY_FLIP, but ISR is deasserted, hence
>@@ -3924,6 +3921,7 @@ static bool i915_handle_vblank(struct drm_device *dev,
> 	if (I915_READ(ISR) & flip_pending)
> 		goto check_page_flip;
> 
>+	intel_prepare_page_flip(dev, plane);
> 	intel_finish_page_flip(dev, pipe);
> 	return true;
> 
>-- 
>2.0.4
>

Actions: View | Splinter Review

Attachments on bug 85888: 108925 | 109779 | 109866 | 110912 | 132962

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.