[Intel-gfx] [PATCH 4/6] drm/i915: Allow mmio updates on all platforms.

[Maarten Lankhorst]

Op 24-03-16 om 16:32 schreef Ville Syrjälä:
> On Thu, Mar 24, 2016 at 04:19:13PM +0100, Maarten Lankhorst wrote:
>> Op 24-03-16 om 15:48 schreef Ville Syrjälä:
>>> On Thu, Mar 24, 2016 at 03:42:28PM +0100, Maarten Lankhorst wrote:
>>>> Op 24-03-16 om 15:26 schreef Ville Syrjälä:
>>>>> On Thu, Mar 24, 2016 at 09:35:04AM +0100, Maarten Lankhorst wrote:
>>>>>> Op 23-03-16 om 16:07 schreef Ville Syrjälä:
>>>>>>> On Wed, Mar 23, 2016 at 02:24:30PM +0100, Maarten Lankhorst wrote:
>>>>>>>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
>>>>>>>> and unpinning. Use flip_queued_req to hold the wait request in the
>>>>>>>> mmio case and allow the vblank interrupt to complete mmio work to
>>>>>>>> have mmio flips run correctly on g4 and earlier.
>>>>>>> Before you actually go and trust drm_vblank_count() you should make it
>>>>>>> race free.
>>>>>> How about adding the below to the patch?
>>>>> You can't just mix the hw and sw counter. Using the hw counter would be
>>>>> neat because it doesn't require so much work to be race-free, but that
>>>>> leaves out gen2 which I don't like. My atomic code used the hw counter
>>>>> though, but I had plans to fall back to the sw counter on gen2
>>>>> eventually.
>>>>>
>>>> So I dug in a little bit more..
>>>>
>>>> MMIO updates don't require accurate vblank count for anything, so even if it was completely removed it would work.
>>> Yes they do if you want to use the vblank irq for completing them.
>>>
>> Why? We only need a vblank irq, after the mmio updates are done we set an atomic that indicates mmio is done, which is checked in the vblank irq.
> You can race with the irq handler, especially on gen2/3 where the vblank
> irq is actually the frame start irq that is delayed by at least ~1
> scanline. So you absolutely need to know on which frame actually
> submitted the flip so as not to complete it prematurely.
In that case won't you get a DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n"); anyway?