Commit 1adadc7702c7f82db9140a1ae2a7ecb5ec66e62a

Manuel Alfayate Corchete 2021-01-14T10:18:40

[KMS/DRM] Adjust come return values. Improve comments.

diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c
index d9e14b0..1b07144 100644
--- a/src/video/kmsdrm/SDL_kmsdrmopengles.c
+++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c
@@ -100,6 +100,7 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
     /* Wait for confirmation that the next front buffer has been flipped, at which
        point the previous front buffer can be released */
     if (!KMSDRM_WaitPageFlip(_this, windata)) {
+        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Wait for previous pageflip failed");
         return 0;
     }
 
@@ -115,7 +116,7 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
        This won't happen until pagelip completes. */
     if (!(_this->egl_data->eglSwapBuffers(_this->egl_data->egl_display,
                                            windata->egl_surface))) {
-        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed.");
+        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed");
         return 0;
     }
 
@@ -124,19 +125,20 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
        from drawing into it!) */
     windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
     if (!windata->next_bo) {
-        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock GBM surface front buffer");
+        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock front buffer on GBM surface");
         return 0;
     }
 
     /* Get an actual usable fb for the next front buffer. */
     fb_info = KMSDRM_FBFromBO(_this, windata->next_bo);
     if (!fb_info) {
+        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not get a framebuffer");
         return 0;
     }
 
     /* Do we have a modeset pending? If so, configure the new mode on the CRTC.
-       Has to be done before the upcoming pageflip issue, so the buffer with the
-       new size is big enough so the CRTC doesn't read out of bounds. */
+       Has to be done before next pageflip issues, so the buffer with the
+       new size is big enough for preventing CRTC from reading out of bounds. */
     if (dispdata->modeset_pending) {
 
         ret = KMSDRM_drmModeSetCrtc(viddata->drm_fd,
@@ -147,9 +149,12 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
 
         if (ret) {
             SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not set videomode on CRTC.");
+            return 0;
         }
 
-        return 0;
+        /* It's OK to return now if we have done a drmModeSetCrtc(),
+           it has done the pageflip and blocked until it was done. */
+        return 1;
     }
 
     /* Issue pageflip on the next front buffer.
@@ -185,11 +190,14 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
        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) {
-	KMSDRM_WaitPageFlip(_this, windata);
+    if (windata->double_buffer) {
+	if (!KMSDRM_WaitPageFlip(_this, windata)) {
+	    SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Immediate wait for previous pageflip failed");
+	    return 0;
+	}
     }
 
-    return 0;
+    return 1;
 }
 
 SDL_EGL_MakeCurrent_impl(KMSDRM)
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index a82813b..ebf643a 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -351,7 +351,7 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) {
     pfd.fd = viddata->drm_fd;
     pfd.events = POLLIN;
 
-    /* Stay on loop until we get the desired event
+    /* Stay on the while 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.
@@ -359,10 +359,10 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) {
         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!
+        If it wasn't for the while loop, we could erroneously exit the function
+        without the pageflip event to arrive!
 
-      Vblank events, for example, hit the FD and they are POLLIN events too (POLLIN
+      For example, vblank events 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
@@ -390,7 +390,7 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) {
         if (pfd.revents & POLLIN) {
             /* There is data to read on the FD!
                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
+               event we are passing in &ev. If it does, 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);