kmsdrm: wait for pending atomic commits before restoring videomode and crtc->buffer on VideoQuit, and simplify double-buffer SwapWindow() implementation.

src/video/kmsdrm/SDL_kmsdrmopengles.c

@@ -101,7 +101,7 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window)
     dispdata->gpu_fence = create_fence(EGL_NO_NATIVE_FENCE_FD_ANDROID, _this);
     assert(dispdata->gpu_fence);
 
-    /* Mark in the EGL level the buffer that we want to become the new front buffer.
+    /* Mark, at EGL level, the buffer that we want to become the new front buffer.
        However, it won't really happen until we request a pageflip at the KMS level and it completes. */
     _this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface);
 
@@ -187,33 +187,18 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window)
     int ret;
     uint32_t flags = 0;
 
+    /* In double-buffer mode, atomic commit will always be synchronous/blocking (ie: won't return until
+       the requested changes are done).
+       Also, there's no need to fence KMS or the GPU, because we won't be entering game loop again
+       (hence not building or executing a new cmdstring) until pageflip is done. */
+
     /* Do we need to set video mode this time? If yes, pass the right flag and issue a blocking atomic ioctl. */
     if (dispdata->modeset_pending) {
         flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
         dispdata->modeset_pending = SDL_FALSE;
     }
-    else {
-        flags |= DRM_MODE_ATOMIC_NONBLOCK;
-    }
 
-    /* Create the fence that will be inserted in the cmdstream exactly at the end
-       of the gl commands that form a frame. KMS will have to wait on it before doing a pageflip.
-       (NOTE this fence is not really neeeded in double-buffer mode because the program will be
-       blocked in eglClientWaitSyncKMHR() until pageflip completes, but we need an in-fence FD anyway
-       to issue the atomic ioctl).
-      */
-    dispdata->gpu_fence = create_fence(EGL_NO_NATIVE_FENCE_FD_ANDROID, _this);
-    assert(dispdata->gpu_fence);
-
-    /* It's safe to get the gpu_fence FD now, because eglSwapBuffers flushes it down the cmdstream, 
-       so it's now in place in the cmdstream.
-       Atomic ioctl will pass the in-fence fd into the kernel, telling KMS that it has to wait for GPU to
-       finish rendering the frame before doing the changes requested in the atomic ioct (pageflip in this case). */
-    dispdata->kms_in_fence_fd = _this->egl_data->eglDupNativeFenceFDANDROID(_this->egl_data->egl_display, dispdata->gpu_fence);
-    _this->egl_data->eglDestroySyncKHR(_this->egl_data->egl_display, dispdata->gpu_fence);
-    assert(dispdata->kms_in_fence_fd != -1);
-
-    /* Mark in the EGL level the buffer that we want to become the new front buffer.
+    /* Mark, at EGL level, the buffer that we want to become the new front buffer.
        However, it won't really happen until we request a pageflip at the KMS level and it completes. */
     _this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface);
 
@@ -245,24 +230,6 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window)
     /* Take note of current front buffer, so we can free it next time we come here. */
     windata->bo = windata->next_bo;
 
-    /* Import the kms fence from the out fence fd. We need it to tell GPU to wait for pageflip to complete. */
-    dispdata->kms_fence = create_fence(dispdata->kms_out_fence_fd, _this);
-    assert(dispdata->kms_fence);
-
-    /* Reset out fence FD value because the fence is now away from us, on the driver side. */
-    dispdata->kms_out_fence_fd = -1;
-
-    /* Wait for pageflip to complete, and destroy kms_fence. */
-    if (dispdata->kms_fence) {
-	EGLint status;
-
-	do {
-	    status = _this->egl_data->eglClientWaitSyncKHR(_this->egl_data->egl_display, dispdata->kms_fence, 0, EGL_FOREVER_KHR);
-	} while (status != EGL_CONDITION_SATISFIED_KHR);
-
-	_this->egl_data->eglDestroySyncKHR(_this->egl_data->egl_display, dispdata->kms_fence);
-    }
-
     return ret;
 }
 

src/video/kmsdrm/SDL_kmsdrmvideo.c

@@ -1051,27 +1051,14 @@ cleanup:
     return ret;
 }
 
-/* Fn to restore original video mode and crtc buffer on quit, using the atomic interface. */
-/*int
-restore_video (_THIS)
-{
-    SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
-
-    ret = drm_atomic_commit(_this, fb->fb_id, flags);
-
-}*/
-
 void
 KMSDRM_VideoQuit(_THIS)
 {
+    int ret;
     SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
     SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
     SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "KMSDRM_VideoQuit()");
 
-    if (_this->gl_config.driver_loaded) {
-        SDL_GL_UnloadLibrary();
-    }
-
     /* Clear out the window list */
     SDL_free(viddata->windows);
     viddata->windows = NULL;
@@ -1082,15 +1069,41 @@ KMSDRM_VideoQuit(_THIS)
     if (viddata->drm_fd >= 0 && dispdata && dispdata->connector && dispdata->crtc) {
         uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
 
-        int ret = drm_atomic_commit(_this, dispdata->crtc->buffer_id, flags);
+        /***********************************************************/
+        /* Atomic block for video mode and crt->buffer restoration */
+        /***********************************************************/
+
+	/* It could happen that we will get here after an async atomic commit (as it's in triple buffer
+           SwapWindow()) and we don't want to issue another atomic commit before previous one is completed. */
+	if (dispdata->kms_fence) {
+	    EGLint status;
+
+	    do {
+		status = _this->egl_data->eglClientWaitSyncKHR(_this->egl_data->egl_display,
+                                              dispdata->kms_fence, 0, EGL_FOREVER_KHR);
+	    } while (status != EGL_CONDITION_SATISFIED_KHR);
+
+	    _this->egl_data->eglDestroySyncKHR(_this->egl_data->egl_display, dispdata->kms_fence);
+	}
+
+        /* Issue sync/blocking atomic commit that restores original video mode and points crtc to original buffer. */
+        ret = drm_atomic_commit(_this, dispdata->crtc->buffer_id, flags);
+        /*********************/
+        /* Atomic block ends */
+        /*********************/
 
         if (ret != 0) {
             SDL_LogWarn(SDL_LOG_CATEGORY_VIDEO, "Could not restore original videomode");
         }
     }
-    /****************/
-    /* Atomic block */
-    /****************/
+
+    if (_this->gl_config.driver_loaded) {
+        SDL_GL_UnloadLibrary();
+    }
+
+    /**************************************************/
+    /* Atomic block for freeing up property pointers. */
+    /**************************************************/
     if (dispdata && dispdata->connector_props_info) {
         SDL_free(dispdata->connector_props_info); 
         dispdata->connector_props_info = NULL;
@@ -1106,6 +1119,7 @@ KMSDRM_VideoQuit(_THIS)
     /*********************/
     /* Atomic block ends */
     /*********************/
+
     if (dispdata && dispdata->connector) {
         KMSDRM_drmModeFreeConnector(dispdata->connector);
         dispdata->connector = NULL;