Commit fc508eabe2ed808c4b2258bdbbab5940b8c9a0f8

Anthony Pesch 2021-05-23T15:59:20

kmsdrm: remove redundant modeset_pending flag this variable was added in commit 2067a7db8e4a36ba40ab34a55b3166ca28638a60 and ultimately tracks if this is a surface's first present. checking if the current bo is NULL provides the same functionality and cuts down on a redundant piece of state potentially getting out of sync in the future

diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c
index 7b28c55..55a53df 100644
--- a/src/video/kmsdrm/SDL_kmsdrmopengles.c
+++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c
@@ -141,64 +141,58 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
         return 0;
     }
 
-    /* Do we have a modeset pending? If so, configure the new mode on the CRTC.
-       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) {
-
+    if (!windata->bo) {
+        /* On the first swap, immediately present the new front buffer. Before
+           drmModePageFlip can be used the CRTC has to be configured to use
+           the current connector and mode with drmModeSetCrtc */
         ret = KMSDRM_drmModeSetCrtc(viddata->drm_fd,
           dispdata->crtc->crtc_id, fb_info->fb_id, 0, 0,
           &dispdata->connector->connector_id, 1, &dispdata->mode);
 
-        dispdata->modeset_pending = SDL_FALSE;
-
         if (ret) {
             SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not set videomode on CRTC.");
             return 0;
         }
+    } else {
+        /* On subsequent swaps, queue the new front buffer to be flipped during
+           the next vertical blank
+
+           Remember: drmModePageFlip() never blocks, it just issues the flip,
+           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
+           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 && viddata->async_pageflip_support) {
+            flip_flags |= DRM_MODE_PAGE_FLIP_ASYNC;
+        }
 
-        /* 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.
-       Remember: drmModePageFlip() never blocks, it just issues the flip,
-       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
-       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 && viddata->async_pageflip_support) {
-        flip_flags |= DRM_MODE_PAGE_FLIP_ASYNC;
-    }
-
-    ret = KMSDRM_drmModePageFlip(viddata->drm_fd, dispdata->crtc->crtc_id,
-             fb_info->fb_id, flip_flags, &windata->waiting_for_flip);
+        ret = KMSDRM_drmModePageFlip(viddata->drm_fd, dispdata->crtc->crtc_id,
+                 fb_info->fb_id, flip_flags, &windata->waiting_for_flip);
 
-    if (ret == 0) {
-        windata->waiting_for_flip = SDL_TRUE;
-    } else {
-        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not queue pageflip: %d", ret);
-    }
+        if (ret == 0) {
+            windata->waiting_for_flip = SDL_TRUE;
+        } else {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not queue pageflip: %d", ret);
+        }
 
-    /* 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 (windata->double_buffer) {
-        if (!KMSDRM_WaitPageflip(_this, windata)) {
-            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Immediate wait for previous pageflip failed");
-            return 0;
+        /* 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 (windata->double_buffer) {
+            if (!KMSDRM_WaitPageflip(_this, windata)) {
+                SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Immediate wait for previous pageflip failed");
+                return 0;
+            }
         }
     }
 
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index 3f9de06..56c5ef9 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -551,7 +551,6 @@ void KMSDRM_AddDisplay (_THIS, drmModeConnector *connector, drmModeRes *resource
 
     /* Initialize some of the members of the new display's driverdata
        to sane values. */
-    dispdata->modeset_pending = SDL_FALSE;
     dispdata->cursor_bo = NULL;
 
     /* Since we create and show the default cursor on KMSDRM_InitMouse(),
@@ -1073,7 +1072,6 @@ KMSDRM_SetDisplayMode(_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode)
     /* Take note of the new mode to be set, and leave the CRTC modeset pending
        so it's done in SwapWindow. */
     dispdata->mode = conn->modes[modedata->mode_index];
-    dispdata->modeset_pending = SDL_TRUE; 
 
     for (i = 0; i < viddata->num_windows; i++) {
         SDL_Window *window = viddata->windows[i];
@@ -1256,9 +1254,6 @@ KMSDRM_CreateWindow(_THIS, SDL_Window * window)
             dispdata->mode = dispdata->original_mode;
         }
 
-        /* Take note to do the modesettng on the CRTC in SwapWindow. */
-        dispdata->modeset_pending = SDL_TRUE;
-
         /* Create the window surfaces with the size we have just chosen.
            Needs the window diverdata in place. */
         if ((ret = KMSDRM_CreateSurfaces(_this, window))) {
@@ -1348,7 +1343,6 @@ KMSDRM_ReconfigureWindow( _THIS, SDL_Window * window)
     /* Recreate the GBM (and EGL) surfaces, and mark the CRTC mode/fb setting
        as pending so it's done on SwapWindow.  */
     KMSDRM_CreateSurfaces(_this, window);
-    dispdata->modeset_pending = SDL_TRUE;
 
     /* Tell app about the size we have determined for the window,
        so SDL pre-scales to that size for us. */
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.h b/src/video/kmsdrm/SDL_kmsdrmvideo.h
index 18e90ab..c82a849 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.h
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.h
@@ -80,8 +80,6 @@ typedef struct SDL_DisplayData
     uint64_t cursor_w, cursor_h;
 
     SDL_bool default_cursor_init;
-    SDL_bool modeset_pending;
-
 } SDL_DisplayData;
 
 typedef struct SDL_WindowData