drm/i915: Rip out legacy page_flip completion/irq handling
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 20 Jul 2017 17:57:51 +0000 (19:57 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 20 Jul 2017 20:45:14 +0000 (22:45 +0200)
All these races and things are now solved through the vblank evasion
trick, plus event handling is done using normal vblank even processing
and drm_crtc_arm_vblank_event. We can get rid of all this complexity.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170720175754.30751-5-daniel.vetter@ffwll.ch
drivers/gpu/drm/i915/i915_irq.c
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_drv.h

index d228857..fed2894 100644 (file)
@@ -1708,18 +1708,6 @@ static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
        }
 }
 
-static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
-                                    enum pipe pipe)
-{
-       bool ret;
-
-       ret = drm_handle_vblank(&dev_priv->drm, pipe);
-       if (ret)
-               intel_finish_page_flip_mmio(dev_priv, pipe);
-
-       return ret;
-}
-
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
                                        u32 iir, u32 pipe_stats[I915_MAX_PIPES])
 {
@@ -1784,12 +1772,8 @@ static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
        enum pipe pipe;
 
        for_each_pipe(dev_priv, pipe) {
-               if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
-                   intel_pipe_handle_vblank(dev_priv, pipe))
-                       intel_check_page_flip(dev_priv, pipe);
-
-               if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
-                       intel_finish_page_flip_cs(dev_priv, pipe);
+               if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+                       drm_handle_vblank(&dev_priv->drm, pipe);
 
                if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
                        i9xx_pipe_crc_irq_handler(dev_priv, pipe);
@@ -2241,19 +2225,14 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
                DRM_ERROR("Poison interrupt\n");
 
        for_each_pipe(dev_priv, pipe) {
-               if (de_iir & DE_PIPE_VBLANK(pipe) &&
-                   intel_pipe_handle_vblank(dev_priv, pipe))
-                       intel_check_page_flip(dev_priv, pipe);
+               if (de_iir & DE_PIPE_VBLANK(pipe))
+                       drm_handle_vblank(&dev_priv->drm, pipe);
 
                if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
                        intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
 
                if (de_iir & DE_PIPE_CRC_DONE(pipe))
                        i9xx_pipe_crc_irq_handler(dev_priv, pipe);
-
-               /* plane/pipes map 1:1 on ilk+ */
-               if (de_iir & DE_PLANE_FLIP_DONE(pipe))
-                       intel_finish_page_flip_cs(dev_priv, pipe);
        }
 
        /* check event from PCH */
@@ -2292,13 +2271,8 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
                intel_opregion_asle_intr(dev_priv);
 
        for_each_pipe(dev_priv, pipe) {
-               if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)) &&
-                   intel_pipe_handle_vblank(dev_priv, pipe))
-                       intel_check_page_flip(dev_priv, pipe);
-
-               /* plane/pipes map 1:1 on ilk+ */
-               if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
-                       intel_finish_page_flip_cs(dev_priv, pipe);
+               if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)))
+                       drm_handle_vblank(&dev_priv->drm, pipe);
        }
 
        /* check event from PCH */
@@ -2479,7 +2453,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
        }
 
        for_each_pipe(dev_priv, pipe) {
-               u32 flip_done, fault_errors;
+               u32 fault_errors;
 
                if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
                        continue;
@@ -2493,18 +2467,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
                ret = IRQ_HANDLED;
                I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
 
-               if (iir & GEN8_PIPE_VBLANK &&
-                   intel_pipe_handle_vblank(dev_priv, pipe))
-                       intel_check_page_flip(dev_priv, pipe);
-
-               flip_done = iir;
-               if (INTEL_GEN(dev_priv) >= 9)
-                       flip_done &= GEN9_PIPE_PLANE1_FLIP_DONE;
-               else
-                       flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
-
-               if (flip_done)
-                       intel_finish_page_flip_cs(dev_priv, pipe);
+               if (iir & GEN8_PIPE_VBLANK)
+                       drm_handle_vblank(&dev_priv->drm, pipe);
 
                if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
                        hsw_pipe_crc_irq_handler(dev_priv, pipe);
@@ -3675,34 +3639,6 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 /*
  * Returns true when a page flip has completed.
  */
-static bool i8xx_handle_vblank(struct drm_i915_private *dev_priv,
-                              int plane, int pipe, u32 iir)
-{
-       u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
-
-       if (!intel_pipe_handle_vblank(dev_priv, pipe))
-               return false;
-
-       if ((iir & flip_pending) == 0)
-               goto check_page_flip;
-
-       /* 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
-        * the flip is completed (no longer pending). Since this doesn't raise
-        * an interrupt per se, we watch for the change at vblank.
-        */
-       if (I915_READ16(ISR) & flip_pending)
-               goto check_page_flip;
-
-       intel_finish_page_flip_cs(dev_priv, pipe);
-       return true;
-
-check_page_flip:
-       intel_check_page_flip(dev_priv, pipe);
-       return false;
-}
-
 static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 {
        struct drm_device *dev = arg;
@@ -3710,9 +3646,6 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
        u16 iir, new_iir;
        u32 pipe_stats[2];
        int pipe;
-       u16 flip_mask =
-               I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-               I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
        irqreturn_t ret;
 
        if (!intel_irqs_enabled(dev_priv))
@@ -3726,7 +3659,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
        if (iir == 0)
                goto out;
 
-       while (iir & ~flip_mask) {
+       while (iir) {
                /* Can't rely on pipestat interrupt bit in iir as it might
                 * have been cleared after the pipestat interrupt was received.
                 * It doesn't set the bit in iir again, but it still produces
@@ -3748,7 +3681,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
                }
                spin_unlock(&dev_priv->irq_lock);
 
-               I915_WRITE16(IIR, iir & ~flip_mask);
+               I915_WRITE16(IIR, iir);
                new_iir = I915_READ16(IIR); /* Flush posted writes */
 
                if (iir & I915_USER_INTERRUPT)
@@ -3759,9 +3692,8 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
                        if (HAS_FBC(dev_priv))
                                plane = !plane;
 
-                       if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
-                           i8xx_handle_vblank(dev_priv, plane, pipe, iir))
-                               flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
+                       if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+                               drm_handle_vblank(&dev_priv->drm, pipe);
 
                        if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
                                i9xx_pipe_crc_irq_handler(dev_priv, pipe);
@@ -3861,45 +3793,11 @@ static int i915_irq_postinstall(struct drm_device *dev)
        return 0;
 }
 
-/*
- * Returns true when a page flip has completed.
- */
-static bool i915_handle_vblank(struct drm_i915_private *dev_priv,
-                              int plane, int pipe, u32 iir)
-{
-       u32 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
-
-       if (!intel_pipe_handle_vblank(dev_priv, pipe))
-               return false;
-
-       if ((iir & flip_pending) == 0)
-               goto check_page_flip;
-
-       /* 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
-        * the flip is completed (no longer pending). Since this doesn't raise
-        * an interrupt per se, we watch for the change at vblank.
-        */
-       if (I915_READ(ISR) & flip_pending)
-               goto check_page_flip;
-
-       intel_finish_page_flip_cs(dev_priv, pipe);
-       return true;
-
-check_page_flip:
-       intel_check_page_flip(dev_priv, pipe);
-       return false;
-}
-
 static irqreturn_t i915_irq_handler(int irq, void *arg)
 {
        struct drm_device *dev = arg;
        struct drm_i915_private *dev_priv = to_i915(dev);
        u32 iir, new_iir, pipe_stats[I915_MAX_PIPES];
-       u32 flip_mask =
-               I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-               I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
        int pipe, ret = IRQ_NONE;
 
        if (!intel_irqs_enabled(dev_priv))
@@ -3910,7 +3808,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 
        iir = I915_READ(IIR);
        do {
-               bool irq_received = (iir & ~flip_mask) != 0;
+               bool irq_received = (iir) != 0;
                bool blc_event = false;
 
                /* Can't rely on pipestat interrupt bit in iir as it might
@@ -3945,7 +3843,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
                                i9xx_hpd_irq_handler(dev_priv, hotplug_status);
                }
 
-               I915_WRITE(IIR, iir & ~flip_mask);
+               I915_WRITE(IIR, iir);
                new_iir = I915_READ(IIR); /* Flush posted writes */
 
                if (iir & I915_USER_INTERRUPT)
@@ -3956,9 +3854,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
                        if (HAS_FBC(dev_priv))
                                plane = !plane;
 
-                       if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
-                           i915_handle_vblank(dev_priv, plane, pipe, iir))
-                               flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
+                       if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+                               drm_handle_vblank(&dev_priv->drm, pipe);
 
                        if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
                                blc_event = true;
@@ -3991,7 +3888,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
                 */
                ret = IRQ_HANDLED;
                iir = new_iir;
-       } while (iir & ~flip_mask);
+       } while (iir);
 
        enable_rpm_wakeref_asserts(dev_priv);
 
@@ -4126,9 +4023,6 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
        u32 iir, new_iir;
        u32 pipe_stats[I915_MAX_PIPES];
        int ret = IRQ_NONE, pipe;
-       u32 flip_mask =
-               I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
-               I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 
        if (!intel_irqs_enabled(dev_priv))
                return IRQ_NONE;
@@ -4139,7 +4033,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
        iir = I915_READ(IIR);
 
        for (;;) {
-               bool irq_received = (iir & ~flip_mask) != 0;
+               bool irq_received = (iir) != 0;
                bool blc_event = false;
 
                /* Can't rely on pipestat interrupt bit in iir as it might
@@ -4177,7 +4071,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
                                i9xx_hpd_irq_handler(dev_priv, hotplug_status);
                }
 
-               I915_WRITE(IIR, iir & ~flip_mask);
+               I915_WRITE(IIR, iir);
                new_iir = I915_READ(IIR); /* Flush posted writes */
 
                if (iir & I915_USER_INTERRUPT)
@@ -4186,9 +4080,8 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
                        notify_ring(dev_priv->engine[VCS]);
 
                for_each_pipe(dev_priv, pipe) {
-                       if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
-                           i915_handle_vblank(dev_priv, pipe, pipe, iir))
-                               flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe);
+                       if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+                               drm_handle_vblank(&dev_priv->drm, pipe);
 
                        if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
                                blc_event = true;
index 928af81..0d8cf76 100644 (file)
@@ -3405,14 +3405,6 @@ static void skylake_disable_primary_plane(struct intel_plane *primary,
        spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void intel_complete_page_flips(struct drm_i915_private *dev_priv)
-{
-       struct intel_crtc *crtc;
-
-       for_each_intel_crtc(&dev_priv->drm, crtc)
-               intel_finish_page_flip_cs(dev_priv, crtc->pipe);
-}
-
 static int
 __intel_display_resume(struct drm_device *dev,
                       struct drm_atomic_state *state,
@@ -3521,13 +3513,6 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
        if (!state)
                goto unlock;
 
-       /*
-        * Flips in the rings will be nuked by the reset,
-        * so complete all pending flips so that user space
-        * will get its events and not get stuck.
-        */
-       intel_complete_page_flips(dev_priv);
-
        dev_priv->modeset_restore_state = NULL;
 
        /* reset doesn't touch the display */
@@ -10121,140 +10106,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
        kfree(intel_crtc);
 }
 
-/* Is 'a' after or equal to 'b'? */
-static bool g4x_flip_count_after_eq(u32 a, u32 b)
-{
-       return !((a - b) & 0x80000000);
-}
-
-static bool __pageflip_finished_cs(struct intel_crtc *crtc,
-                                  struct intel_flip_work *work)
-{
-       struct drm_device *dev = crtc->base.dev;
-       struct drm_i915_private *dev_priv = to_i915(dev);
-
-       if (abort_flip_on_reset(crtc))
-               return true;
-
-       /*
-        * The relevant registers doen't exist on pre-ctg.
-        * As the flip done interrupt doesn't trigger for mmio
-        * flips on gmch platforms, a flip count check isn't
-        * really needed there. But since ctg has the registers,
-        * include it in the check anyway.
-        */
-       if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv))
-               return true;
-
-       /*
-        * BDW signals flip done immediately if the plane
-        * is disabled, even if the plane enable is already
-        * armed to occur at the next vblank :(
-        */
-
-       /*
-        * A DSPSURFLIVE check isn't enough in case the mmio and CS flips
-        * used the same base address. In that case the mmio flip might
-        * have completed, but the CS hasn't even executed the flip yet.
-        *
-        * A flip count check isn't enough as the CS might have updated
-        * the base address just after start of vblank, but before we
-        * managed to process the interrupt. This means we'd complete the
-        * CS flip too soon.
-        *
-        * Combining both checks should get us a good enough result. It may
-        * still happen that the CS flip has been executed, but has not
-        * yet actually completed. But in case the base address is the same
-        * anyway, we don't really care.
-        */
-       return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
-               crtc->flip_work->gtt_offset &&
-               g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
-                                   crtc->flip_work->flip_count);
-}
-
-static bool
-__pageflip_finished_mmio(struct intel_crtc *crtc,
-                              struct intel_flip_work *work)
-{
-       /*
-        * MMIO work completes when vblank is different from
-        * flip_queued_vblank.
-        *
-        * Reset counter value doesn't matter, this is handled by
-        * i915_wait_request finishing early, so no need to handle
-        * reset here.
-        */
-       return intel_crtc_get_vblank_counter(crtc) != work->flip_queued_vblank;
-}
-
-
-static bool pageflip_finished(struct intel_crtc *crtc,
-                             struct intel_flip_work *work)
-{
-       if (!atomic_read(&work->pending))
-               return false;
-
-       smp_rmb();
-
-       if (is_mmio_work(work))
-               return __pageflip_finished_mmio(crtc, work);
-       else
-               return __pageflip_finished_cs(crtc, work);
-}
-
-void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe)
-{
-       struct drm_device *dev = &dev_priv->drm;
-       struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
-       struct intel_flip_work *work;
-       unsigned long flags;
-
-       /* Ignore early vblank irqs */
-       if (!crtc)
-               return;
-
-       /*
-        * This is called both by irq handlers and the reset code (to complete
-        * lost pageflips) so needs the full irqsave spinlocks.
-        */
-       spin_lock_irqsave(&dev->event_lock, flags);
-       work = crtc->flip_work;
-
-       if (work != NULL &&
-           !is_mmio_work(work) &&
-           pageflip_finished(crtc, work))
-               page_flip_completed(crtc);
-
-       spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
-void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe)
-{
-       struct drm_device *dev = &dev_priv->drm;
-       struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
-       struct intel_flip_work *work;
-       unsigned long flags;
-
-       /* Ignore early vblank irqs */
-       if (!crtc)
-               return;
-
-       /*
-        * This is called both by irq handlers and the reset code (to complete
-        * lost pageflips) so needs the full irqsave spinlocks.
-        */
-       spin_lock_irqsave(&dev->event_lock, flags);
-       work = crtc->flip_work;
-
-       if (work != NULL &&
-           is_mmio_work(work) &&
-           pageflip_finished(crtc, work))
-               page_flip_completed(crtc);
-
-       spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
 static inline void intel_mark_page_flip_active(struct intel_crtc *crtc,
                                               struct intel_flip_work *work)
 {
@@ -10265,72 +10116,6 @@ static inline void intel_mark_page_flip_active(struct intel_crtc *crtc,
        atomic_set(&work->pending, 1);
 }
 
-static bool __pageflip_stall_check_cs(struct drm_i915_private *dev_priv,
-                                     struct intel_crtc *intel_crtc,
-                                     struct intel_flip_work *work)
-{
-       u32 addr, vblank;
-
-       if (!atomic_read(&work->pending))
-               return false;
-
-       smp_rmb();
-
-       vblank = intel_crtc_get_vblank_counter(intel_crtc);
-       if (work->flip_ready_vblank == 0) {
-               if (work->flip_queued_req &&
-                   !i915_gem_request_completed(work->flip_queued_req))
-                       return false;
-
-               work->flip_ready_vblank = vblank;
-       }
-
-       if (vblank - work->flip_ready_vblank < 3)
-               return false;
-
-       /* Potential stall - if we see that the flip has happened,
-        * assume a missed interrupt. */
-       if (INTEL_GEN(dev_priv) >= 4)
-               addr = I915_HI_DISPBASE(I915_READ(DSPSURF(intel_crtc->plane)));
-       else
-               addr = I915_READ(DSPADDR(intel_crtc->plane));
-
-       /* There is a potential issue here with a false positive after a flip
-        * to the same address. We could address this by checking for a
-        * non-incrementing frame counter.
-        */
-       return addr == work->gtt_offset;
-}
-
-void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe)
-{
-       struct drm_device *dev = &dev_priv->drm;
-       struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
-       struct intel_flip_work *work;
-
-       WARN_ON(!in_interrupt());
-
-       if (crtc == NULL)
-               return;
-
-       spin_lock(&dev->event_lock);
-       work = crtc->flip_work;
-
-       if (work != NULL && !is_mmio_work(work) &&
-           __pageflip_stall_check_cs(dev_priv, crtc, work)) {
-               WARN_ONCE(1,
-                         "Kicking stuck page flip: queued at %d, now %d\n",
-                       work->flip_queued_vblank, intel_crtc_get_vblank_counter(crtc));
-               page_flip_completed(crtc);
-               work = NULL;
-       }
-
-       if (work != NULL && !is_mmio_work(work) &&
-           intel_crtc_get_vblank_counter(crtc) - work->flip_queued_vblank > 1)
-               intel_queue_rps_boost_for_request(work->flip_queued_req);
-       spin_unlock(&dev->event_lock);
-}
-
 /**
  * intel_wm_need_update - Check whether watermarks need updating
  * @plane: drm plane
index 4f9775a..4b7766e 100644 (file)
@@ -1408,9 +1408,6 @@ void intel_unpin_fb_vma(struct i915_vma *vma);
 struct drm_framebuffer *
 intel_framebuffer_create(struct drm_i915_gem_object *obj,
                         struct drm_mode_fb_cmd2 *mode_cmd);
-void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe);
-void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe);
-void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe);
 int intel_prepare_plane_fb(struct drm_plane *plane,
                           struct drm_plane_state *new_state);
 void intel_cleanup_plane_fb(struct drm_plane *plane,