On 06/13/16 17:06, Daniel Vetter wrote: > On Mon, Jun 13, 2016 at 10:54:37AM +0900, Michel Dänzer wrote: >> On 10.06.2016 23:43, Daniel Vetter wrote: >>> On Fri, Jun 10, 2016 at 05:57:12PM +0900, Michel Dänzer wrote: >>>> From: Michel Dänzer <michel.daenzer@xxxxxxx> >>>> >>>> If userspace wants a page flip to take effect during vblank sequence n, >>>> it has to wait for vblank seqno n-1 before calling the >>>> DRM_IOCTL_MODE_PAGE_FLIP ioctl. >>>> >>>> This change makes sure that we do not program the flip to the hardware >>>> before the end of vblank seqno n-1 in this case, to prevent the flip >>>> from taking effect too early. >>>> >>>> On the other hand, if the DRM_IOCTL_MODE_PAGE_FLIP ioctl is called >>>> during vblank, but userspace didn't wait for the current vblank seqno >>>> before, this change would still allow the flip to be programmed during >>>> the current vblank seqno. >>> >>> This just sounds like you're sending vblank events out a bit too early. >>> And watching vblank waits that userspace does works, but it's fragile, >>> add-hoc and I don't really jump in joy about adding that to the vblank >>> core. Is there no way you can adjust sending out the vblank events >>> similarly, to make sure userspace can never sneak in a pageflip too early? >> >> What you call "too early" is actually "during the vertical blank period >> waited for". IMHO only notifying userspace of a vertical blank period >> when it's already over would defeat the purpose. > > Afaiui the rules are: > - The timestamp for vblank event needs to agree with whatever oml_sync > requries. > - The event delivery itself needs to be consistent with what page_flip > takes, i.e. if userspace sees an event and immediately issues a > page_flip then it should not be able to hit the same vblank with that > pageflip. > - The event needs to be after the old buffers are not longer used and can > be reused for rendering. That's only relevant for DRM_IOCTL_MODE_PAGE_FLIP, not DRM_IOCTL_WAIT_VBLANK. > - There's no requirement at all that the event gets delivered at a > specific point in the vblank, hardware is too different for that to work As the name implies, the purpose of DRM_IOCTL_WAIT_VBLANK is to wait for a vertical blank period. If that doesn't work as intended with some hardware, that's tough luck but not really my problem. :) > - that kind of precision is why we have a separate timestamp. I'm afraid this last item gives away that you're relatively new to this code. ;) The timestamp was originally literally just the current gettimeofday when the wait finished (the original DRM_IOCTL_WAIT_VBLANK ioctl didn't have any asynchronous notification functionality). It was relatively recently that Mario changed the timestamp to correspond to the end of the vertical blank period / start of scanout of the next frame, presumably due to your first rule above. > I assume you're goal is to not delay page_flips unecessarily, without > breaking requirement 2 here. Imo a simpler fix would be to delay the > vblank handling to end of vblank. Fixes everything without hacks, [...] Except it breaks the original purpose of the wait for vblank functionality, which is to wait for the beginning of a vertical blank period. [0] You're focusing too much on page flips and suggesting to throw out the vblank baby with the bathwater. I really don't see the big issue which would justify that. [0] As an analogy, how useful would e.g. calendar notifications be if they arrived at the end of the events they're about? "Hey, that meeting you were supposed to attend? It just finished!" -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
