On Wed, Mar 23, 2016 at 02:24:28PM +0100, Maarten Lankhorst wrote:
> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
> is no longer useful.
> 
> Signed-off-by: Maarten Lankhorst <[email protected]>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  3 --
>  drivers/gpu/drm/i915/i915_irq.c      | 18 ++-----
>  drivers/gpu/drm/i915/intel_display.c | 96 
> ++++++++++++++----------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 -
>  4 files changed, 40 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 310043d73bf7..4a0078c17cd1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -588,9 +588,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, 
> void *data)
>                       if (pending == INTEL_FLIP_INACTIVE) {
>                               seq_printf(m, "Flip ioctl preparing on pipe %c 
> (plane %c)\n",
>                                          pipe, plane);
> -                     } else if (pending >= INTEL_FLIP_COMPLETE) {
> -                             seq_printf(m, "Flip queued on pipe %c (plane 
> %c)\n",
> -                                        pipe, plane);
>                       } else {
>                               seq_printf(m, "Flip pending (waiting for vsync) 
> on pipe %c (plane %c)\n",
>                                          pipe, plane);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a55a7cc317f8..9d4089f6c91b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct 
> drm_device *dev, u32 iir)
>                   intel_pipe_handle_vblank(dev, pipe))
>                       intel_check_page_flip(dev, pipe);
>  
> -             if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
> -                     intel_prepare_page_flip(dev, pipe);
> +             if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
>                       intel_finish_page_flip(dev, pipe);
> -             }
>  
>               if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>                       i9xx_pipe_crc_irq_handler(dev, pipe);
> @@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device 
> *dev, u32 de_iir)
>                       i9xx_pipe_crc_irq_handler(dev, pipe);
>  
>               /* plane/pipes map 1:1 on ilk+ */
> -             if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> -                     intel_prepare_page_flip(dev, pipe);
> +             if (de_iir & DE_PLANE_FLIP_DONE(pipe))
>                       intel_finish_page_flip_plane(dev, pipe);
> -             }
>       }
>  
>       /* check event from PCH */
> @@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device 
> *dev, u32 de_iir)
>                       intel_check_page_flip(dev, pipe);
>  
>               /* plane/pipes map 1:1 on ilk+ */
> -             if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
> -                     intel_prepare_page_flip(dev, pipe);
> +             if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
>                       intel_finish_page_flip_plane(dev, pipe);
> -             }
>       }
>  
>       /* check event from PCH */
> @@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, 
> u32 master_ctl)
>               else
>                       flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
>  
> -             if (flip_done) {
> -                     intel_prepare_page_flip(dev, pipe);
> +             if (flip_done)
>                       intel_finish_page_flip_plane(dev, pipe);
> -             }
>  
>               if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>                       hsw_pipe_crc_irq_handler(dev, pipe);
> @@ -3984,7 +3976,6 @@ 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;
>  
> @@ -4175,7 +4166,6 @@ 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;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 6303438df9f2..c15c08dc251c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3138,7 +3138,6 @@ static void intel_complete_page_flips(struct drm_device 
> *dev)
>               struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>               enum plane plane = intel_crtc->plane;
>  
> -             intel_prepare_page_flip(dev, plane);
>               intel_finish_page_flip_plane(dev, plane);
>       }
>  }
> @@ -10825,53 +10824,6 @@ static void intel_unpin_work_fn(struct work_struct 
> *__work)
>       kfree(work);
>  }
>  
> -static void do_intel_finish_page_flip(struct drm_device *dev,
> -                                   struct drm_crtc *crtc)
> -{
> -     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -     struct intel_unpin_work *work;
> -     unsigned long flags;
> -
> -     /* Ignore early vblank irqs */
> -     if (intel_crtc == NULL)
> -             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 = intel_crtc->unpin_work;
> -
> -     /* Ensure we don't miss a work->pending update ... */
> -     smp_rmb();
> -
> -     if (work == NULL || atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
> -             spin_unlock_irqrestore(&dev->event_lock, flags);
> -             return;
> -     }
> -
> -     page_flip_completed(intel_crtc);
> -
> -     spin_unlock_irqrestore(&dev->event_lock, flags);
> -}
> -
> -void intel_finish_page_flip(struct drm_device *dev, int pipe)
> -{
> -     struct drm_i915_private *dev_priv = dev->dev_private;
> -     struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> -
> -     do_intel_finish_page_flip(dev, crtc);
> -}
> -
> -void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> -{
> -     struct drm_i915_private *dev_priv = dev->dev_private;
> -     struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
> -
> -     do_intel_finish_page_flip(dev, crtc);
> -}
> -
>  /* Is 'a' after or equal to 'b'? */
>  static bool g4x_flip_count_after_eq(u32 a, u32 b)
>  {
> @@ -10924,28 +10876,52 @@ static bool page_flip_finished(struct intel_crtc 
> *crtc)
>                                   crtc->unpin_work->flip_count);
>  }
>  
> -void intel_prepare_page_flip(struct drm_device *dev, int plane)
> +static void do_intel_finish_page_flip(struct drm_device *dev,
> +                                   struct drm_crtc *crtc)
>  {
> -     struct drm_i915_private *dev_priv = dev->dev_private;
> -     struct intel_crtc *intel_crtc =
> -             to_intel_crtc(dev_priv->plane_to_crtc_mapping[plane]);
> +     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +     struct intel_unpin_work *work;
>       unsigned long flags;
>  
> +     /* Ignore early vblank irqs */
> +     if (intel_crtc == NULL)
> +             return;
>  
>       /*
>        * This is called both by irq handlers and the reset code (to complete
>        * lost pageflips) so needs the full irqsave spinlocks.
> -      *
> -      * NB: An MMIO update of the plane base pointer will also
> -      * generate a page-flip completion irq, i.e. every modeset
> -      * is also accompanied by a spurious intel_prepare_page_flip().
>        */
>       spin_lock_irqsave(&dev->event_lock, flags);
> -     if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
> -             atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
> +     work = intel_crtc->unpin_work;
> +
> +     if (work == NULL ||
> +         atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
> +        !page_flip_finished(intel_crtc)) {
> +             spin_unlock_irqrestore(&dev->event_lock, flags);
> +             return;
> +     }
> +
> +     page_flip_completed(intel_crtc);
> +
>       spin_unlock_irqrestore(&dev->event_lock, flags);
>  }
>  
> +void intel_finish_page_flip(struct drm_device *dev, int pipe)
> +{
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +
> +     do_intel_finish_page_flip(dev, crtc);
> +}
> +
> +void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> +{
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
> +
> +     do_intel_finish_page_flip(dev, crtc);
> +}

I don't think you need the _plane() variant anywhere actually, so might
as well kill it.

And since this patch is non-controversial I'd move it to the head of the
series.

> +
>  static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
>  {
>       /* Ensure that the work item is consistent when activating it ... */
> @@ -11395,8 +11371,8 @@ static bool __intel_pageflip_stall_check(struct 
> drm_device *dev,
>       u32 pending;
>  
>       pending = atomic_read(&work->pending);
> -     if (pending != INTEL_FLIP_PENDING)
> -             return pending == INTEL_FLIP_COMPLETE;
> +     if (pending == INTEL_FLIP_INACTIVE)
> +             return false;
>  
>       if (work->flip_ready_vblank == 0) {
>               if (work->flip_queued_req &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 7f0779783851..e26293ead94f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -944,7 +944,6 @@ struct intel_unpin_work {
>       atomic_t pending;
>  #define INTEL_FLIP_INACTIVE  0
>  #define INTEL_FLIP_PENDING   1
> -#define INTEL_FLIP_COMPLETE  2
>       u32 flip_count;
>       u32 gtt_offset;
>       struct drm_i915_gem_request *flip_queued_req;
> @@ -1155,7 +1154,6 @@ struct drm_framebuffer *
>  __intel_framebuffer_create(struct drm_device *dev,
>                          struct drm_mode_fb_cmd2 *mode_cmd,
>                          struct drm_i915_gem_object *obj);
> -void intel_prepare_page_flip(struct drm_device *dev, int plane);
>  void intel_finish_page_flip(struct drm_device *dev, int pipe);
>  void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
>  void intel_check_page_flip(struct drm_device *dev, int pipe);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to