[KMS/DRM] Refactor, improve and re-comment async pageflips code.

src/video/kmsdrm/SDL_kmsdrmopengles.c

@@ -154,13 +154,14 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
 
     /* Issue pageflip on the next front buffer.
        Remember: drmModePageFlip() never blocks, it just issues the flip,
-       which will be done during the next vblank.
-       Since it will return EBUSY if we call it again without having
-       completed the last issued flip, we must pass the
+       which will be done during the next vblank, or immediately if
+       we pass the DRM_MODE_PAGE_FLIP_ASYNC flag.
+       Since calling drmModePageFlip() will return EBUSY if we call it
+       without having completed the last issued flip, we must pass the
        DRM_MODE_PAGE_FLIP_ASYNC if we don't block on EGL (egl_swapinterval = 0).
-       That makes it flip immediately, without waiting for the next vblank,
-       so even if we don't block on EGL, it will have flipped when we
-       get back here. */
+       That makes it flip immediately, without waiting for the next vblank
+       to do so, so even if we don't block on EGL, the flip will have completed
+       when we get here again. */
 
     if (_this->egl_data->egl_swapinterval == 0) {
         flip_flags |= DRM_MODE_PAGE_FLIP_ASYNC;
@@ -173,11 +174,15 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
 	windata->waiting_for_flip = SDL_TRUE;
     } else {
 	SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not queue pageflip: %d", ret);
-	printf("Could not queue pageflip: %s\n", strerror(errno));
     }
 
-    /* If we are in double-buffer mode, wait immediately for vsync
-       (as if we only had two buffers),
+    /* Wait immediately for vsync (as if we only had two buffers).
+       Even if we are already doing a WaitPageFlip at the begining of this
+       function, this is NOT redundant because here we wait immediately
+       after submitting the image to the screen, reducing lag, and if
+       we have waited here, there won't be a pending pageflip so the
+       WaitPageFlip at the beggining of this function will be a no-op.
+       Just leave it here and don't worry. 
        Run your SDL2 program with "SDL_KMSDRM_DOUBLE_BUFFER=1 <program_name>"
        to enable this. */
     if (_this->egl_data->egl_swapinterval == 1 && windata->double_buffer) {

src/video/kmsdrm/SDL_kmsdrmvideo.c

@@ -45,8 +45,9 @@
 #include "SDL_kmsdrmvulkan.h"
 #include <sys/stat.h>
 #include <dirent.h>
-#include <errno.h>
 #include <poll.h>
+#include <time.h>
+#include <errno.h>
 
 #ifdef __OpenBSD__
 #define KMSDRM_DRI_PATH "/dev/"
@@ -340,11 +341,10 @@ KMSDRM_FlipHandler(int fd, unsigned int frame, unsigned int sec, unsigned int us
 
 SDL_bool
 KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) {
+
     SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
     drmEventContext ev = {0};
     struct pollfd pfd = {0};
-    /* If the pageflip hasn't completed after 10 seconds, it nevel will. */
-    uint32_t timeout = 10000;
 
     ev.version = DRM_EVENT_CONTEXT_VERSION;
     ev.page_flip_handler = KMSDRM_FlipHandler;
@@ -352,30 +352,52 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) {
     pfd.fd = viddata->drm_fd;
     pfd.events = POLLIN;
 
+    /* Stay on loop until we get the desired event
+       We need the while the loop because we could be in a situation where:
+       -We get events on the FD in time, thus not on exiting on return number 1.
+       -These events are not errors, thus not exiting on return number 2.
+       -These events are of POLLIN type, thus not exiting on return number 3,
+        but if the event is not the pageflip we are waiting for, we arrive at the end
+        of the loop and do loop re-entry, hoping the next event will be the pageflip.
+
+        So we could erroneously exit the function without the pageflip event to arrive
+        if it wasn't for the while loop!
+
+      Vblank events, for example, hit the FD and they are POLLIN events too (POLLIN
+      means "there's data to read on the FD"), but they are not the pageflip event
+      we are waiting for, so the drmEventHandle() doesn't run the flip handler, and
+      since waiting_for_flip is set on the pageflip handle, it's not set and we stay
+      on the loop.
+    */
     while (windata->waiting_for_flip) {
-        pfd.revents = 0;
+
+        pfd.revents  = 0;
 
         /* poll() waits for events arriving on the FD, and returns < 0 if timeout
-           passes with no events.  */
-        if (poll(&pfd, 1, timeout) < 0) {
+           passes with no events.
+           We wait forever (timeout = -1), but even if we DO get an event,
+           we have yet to see if it's of the required type. */
+        if (poll(&pfd, 1, -1) < 0) {
             SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "DRM poll error");
-            return SDL_FALSE;
+            return SDL_FALSE; /* Return number 1. */
         }
 
         if (pfd.revents & (POLLHUP | POLLERR)) {
             /* An event arrived on the FD in time, but it's an error. */
             SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "DRM poll hup or error");
-            return SDL_FALSE;
+            return SDL_FALSE; /* Return number 2. */
         }
 
         if (pfd.revents & POLLIN) {
             /* There is data to read on the FD!
-               Is the event a pageflip? If so, drmHandleEvent will
-               unset windata->waiting_for_flip */
+               Is the event a pageflip? We know it is a pageflip if it matches the
+               event we are passing in &ev. If it is, drmHandleEvent() will unset
+               windata->waiting_for_flip and we will get out of the "while" loop.
+               If it's not, we keep iterating on the loop. */
             KMSDRM_drmHandleEvent(viddata->drm_fd, &ev);
         } else {
             SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Dropping frame while waiting_for_flip");
-            return SDL_FALSE;
+            return SDL_FALSE; /* Return number 3. */
         }
     }