Op 26-04-16 om 01:14 schreef Patrik Jakobsson: > On Tue, Apr 19, 2016 at 09:52:23AM +0200, Maarten Lankhorst wrote: >> Instead of calling prepare_flip right before calling finish_page_flip >> do everything from prepare_page_flip in finish_page_flip. >> >> Putting prepare and finish page_flip in a single step removes the need >> for INTEL_FLIP_COMPLETE, so it can be removed. This simplifies the code >> slightly. >> >> Changes since v1: >> - Invert if case to simplify code. >> - Add missing barrier. >> - Reword commit message. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com> >> ... >> @@ -11057,28 +11015,48 @@ 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_PENDING && >> + page_flip_finished(intel_crtc)) >> + 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); >> +} >> + > Do we really need a _plane version of this function? > intel_complete_page_flips() > and ilk+ irq handlers are the only ones using it and the irq handlers claim > there's a 1:1 plane-pipe mapping anyway. That single call in > intel_complete_page_flips() already have the crtc and can easily do the > dev_priv->plane_to_crtc_mapping[plane] there if it's really needed. On earlier generations there was no fixed mapping, but for ilk+ yeah should be removable. > Btw, intel_complete_page_flips() is only called from intel_finish_reset() so > one > could question it's usefulness as well. intel_complete_page_flips can be removed later on too, it isn't required for mmio flips since the requests will complete anyway. >> static inline void intel_mark_page_flip_active(struct intel_unpin_work >> *work) >> { >> /* Ensure that the work item is consistent when activating it ... */ >> @@ -11523,8 +11501,8 @@ static bool __intel_pageflip_stall_check(struct >> drm_device *dev, >> /* ensure that the unpin work is consistent wrt ->pending. */ >> smp_mb__after_atomic(); >> >> - if (pending != INTEL_FLIP_PENDING) >> - return pending == INTEL_FLIP_COMPLETE; >> + if (pending == INTEL_FLIP_INACTIVE) >> + return false; > With INTEL_FLIP_COMPLETE removed I would prefer ->pending to just be true or > false. I thought of re-using it when adding more than 1 flip to the queue, but I probably won't need it then. Wouldn't be a bad idea to remove it. :)
~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx