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.lankhorst@xxxxxxxxxxxxxxx>
>> ...
>> @@ -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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
