Re: [PATCH] don't wait on page flips if none are pending

[Michel D. <mi...@tu...>]

On Fri, 2008-06-20 at 14:27 -0700, Jesse Barnes wrote:
> On Friday, June 20, 2008 2:09 pm Jesse Barnes wrote:
> > Michel, can you take a look at this?  vblank wait is working really well
> > for me with the latest bits, but I found what I think is a page flip
> > related bug on 965.  

No this isn't correct, it also needs to wait for scheduled non-flip
buffer swaps.

Isn't this just a workaround for the lack of the 2D driver patch and not
necessary with it?

> > I've been testing with the attached pre- & post-modeset ioctl patch to the 
> > 2D driver.

That one looks good; if anything, maybe slightly too careful about
omitting redundant ioctl calls, but better safe than sorry I guess. :)


> Oops, I've also been testing with this.

Yeah, that driver workaround should definitely no longer be necessary.


> And looking again at c7ee6cc269c26d8e7ed98a16a272eca63daab201 (the "in
> vblank" handling).  I think my 965 test machine is fast enough that I
> haven't seen much tearing on it yet, but I think for slower machines
> we'll definitely need a real fix.
> 
> My current thinking is that the only non-racy way to handle the "in
> vblank" case is to only use the frame counter to keep the vblank count
> accurate across interrupt disabled periods.  While the interrupt is
> enabled, however, the driver should internally keep a counter going
> based on the vblank interrupt and use that to return a value to the
> core.
> 
> If the radeon driver has to do something similar, we could probably
> just update the core drm_handle_vblank() function to use the atomic
> counter instead, which would save a bunch of duplicated driver code...
> Any thoughts there?

I've been thinking about this as well, should be worth a try.


Something else I think we need to do before merging vblank-rework into
the kernel is to keep the vblank interrupts enabled and use an atomic
counter until the 2D driver calls the modeset ioctl (or a kernel
modesetting driver does the corresponding thing to indicate it handles
this correctly).


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer