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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 17 Dec 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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 interrupt 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.
>
> v2: Fix typo in commit message (checkpatch)
>
> 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
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.


> ---
>  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
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center