| Message ID | 1460539104-15661-4-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote: > Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE > is no longer useful. What's the deal with "use atomic_read"? I couldn't find where that is different from the old code. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > 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 df8073a2ffbe..c3b029e7bffd 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -589,9 +589,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 679f08c944ef..5ed2f73a7ea9 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); > @@ -4024,7 +4016,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; > > @@ -4215,7 +4206,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 618e034a7a5e..dc42335409b3 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3101,7 +3101,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); > } > } > @@ -10925,53 +10924,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 this is not needed anymore, should the matching smp_wmb() from intel_mark_page_flip_active() be removed? > - > - 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) > { > @@ -11024,28 +10976,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)) { This line is misaligned. Also, if the condition is was inverted you could save the extra unlock and return. Ander > + 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); > +} > + > static inline void intel_mark_page_flip_active(struct intel_unpin_work *work) > { > /* Ensure that the work item is consistent when activating it ... */ > @@ -11495,8 +11471,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 ce2a6d985bbe..a74edecfd179 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -946,7 +946,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; > @@ -1159,7 +1158,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);
Op 15-04-16 om 14:21 schreef Ander Conselvan De Oliveira: > On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote: >> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE >> is no longer useful. > What's the deal with "use atomic_read"? I couldn't find where that is different > from the old code. This could probably use some rewording, what I believe was that with the removal of prepare_page_flip, do_intel_finish_page_flip only needs a single atomic_read to complete. >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> 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 df8073a2ffbe..c3b029e7bffd 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -589,9 +589,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 679f08c944ef..5ed2f73a7ea9 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); >> @@ -4024,7 +4016,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; >> >> @@ -4215,7 +4206,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 618e034a7a5e..dc42335409b3 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3101,7 +3101,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); >> } >> } >> @@ -10925,53 +10924,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 this is not needed anymore, should the matching smp_wmb() from > intel_mark_page_flip_active() be removed? > > >> - >> - 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) >> { >> @@ -11024,28 +10976,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)) { > This line is misaligned. Also, if the condition is was inverted you could save > the extra unlock and return. > > > Ander > >> + 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); >> +} >> + >> static inline void intel_mark_page_flip_active(struct intel_unpin_work *work) >> { >> /* Ensure that the work item is consistent when activating it ... */ >> @@ -11495,8 +11471,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 ce2a6d985bbe..a74edecfd179 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -946,7 +946,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; >> @@ -1159,7 +1158,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);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index df8073a2ffbe..c3b029e7bffd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -589,9 +589,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 679f08c944ef..5ed2f73a7ea9 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); @@ -4024,7 +4016,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; @@ -4215,7 +4206,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 618e034a7a5e..dc42335409b3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3101,7 +3101,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); } } @@ -10925,53 +10924,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) { @@ -11024,28 +10976,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); +} + static inline void intel_mark_page_flip_active(struct intel_unpin_work *work) { /* Ensure that the work item is consistent when activating it ... */ @@ -11495,8 +11471,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 ce2a6d985bbe..a74edecfd179 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -946,7 +946,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; @@ -1159,7 +1158,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);
Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE is no longer useful. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- 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(-)