kmsdrm: greatly improve comments in SwapBuffersFenced() for future reference.

src/video/kmsdrm/SDL_kmsdrmopengles.c

@@ -100,7 +100,13 @@ static EGLSyncKHR create_fence(int fd, _THIS)
 	return fence;
 }
 
-static int
+/***********************************************************************************/
+/* Comments about buffer access protection mechanism (=fences) are the ones boxed. */
+/* Also, DON'T remove the asserts: if a fence-related call fails, it's better that */
+/* program exits immediately, or we could leave KMS waiting for a failed/missing   */
+/* fence forevever.                                                                */
+/***********************************************************************************/
+int
 KMSDRM_GLES_SwapWindowFenced(_THIS, SDL_Window * window)
 {
     SDL_WindowData *windata = ((SDL_WindowData *) window->driverdata);
@@ -108,36 +114,47 @@ KMSDRM_GLES_SwapWindowFenced(_THIS, SDL_Window * window)
     KMSDRM_FBInfo *fb;
     KMSDRM_PlaneInfo info = {0};
 
-    /*************************************************************************/
-    /* Block for telling KMS to wait for GPU rendering of the current frame  */
-    /* before applying the KMS changes requested in the atomic ioctl.        */
-    /*************************************************************************/
-    /* 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. */
+    /******************************************************************/
+    /* Create the GPU-side FENCE OBJECT. It will be inserted into the */
+    /* GL CMDSTREAM exactly at the end of the gl commands that form a */
+    /* frame.(KMS will have to wait on it before doing a pageflip.)   */
+    /******************************************************************/
     dispdata->gpu_fence = create_fence(EGL_NO_NATIVE_FENCE_FD_ANDROID, _this);
     assert(dispdata->gpu_fence);
 
-    /* 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. */
+    /******************************************************************/
+    /* eglSwapBuffers flushes the fence down the GL CMDSTREAM, so we  */
+    /* know for sure it's there now.                                  */
+    /* Also it marks, at EGL level, the buffer that we want to become */
+    /* the new front buffer. (Remember that won't really happen until */
+    /* we request a pageflip at the KMS level and it completes.       */
+    /******************************************************************/
     if (! _this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface)) {
         return SDL_EGL_SetError("Failed to swap EGL buffers", "eglSwapBuffers");
     }
-
-    /* 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);
+    /******************************************************************/
+    /* EXPORT the GPU-side FENCE OBJECT to the fence INPUT FD, so we  */
+    /* can pass it into the kernel. Atomic ioctl will pass the        */
+    /* in-fence fd into the kernel, thus telling KMS that it has to   */
+    /* wait for GPU to finish rendering the frame (remember where we  */
+    /* put the fence in the GL CMDSTREAM) before doing the changes    */
+    /* requested in the atomic ioct (the pageflip in this case).      */
+    /* (We export the GPU-side FENCE OJECT to the fence INPUT FD now, */
+    /* not sooner, because now we are sure that the GPU-side fence is */
+    /* in the CMDSTREAM to be lifted when the CMDSTREAM to this point */
+    /* is completed).                                                 */
+    /******************************************************************/
+    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);
 
-    /***************/
-    /* Block ends. */
-    /***************/
-
-    /* Lock the buffer that is marked by eglSwapBuffers() to become the next front buffer (so it can not
-       be chosen by EGL as back buffer to draw on), and get a handle to it to request the pageflip on it.
-       REMEMBER that gbm_surface_lock_front_buffer() ALWAYS has to be called after eglSwapBuffers(). */
+    /* Lock the buffer that is marked by eglSwapBuffers() to become the
+       next front buffer (so it can not be chosen by EGL as back buffer
+       to draw on), and get a handle to it to request the pageflip on it.
+       REMEMBER that gbm_surface_lock_front_buffer() ALWAYS has to be
+       called after eglSwapBuffers(). */
     windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
     if (!windata->next_bo) {
 	return SDL_SetError("Failed to lock frontbuffer");
@@ -162,56 +179,77 @@ KMSDRM_GLES_SwapWindowFenced(_THIS, SDL_Window * window)
         return SDL_SetError("Failed to request prop changes for setting plane buffer and CRTC");
     }
 
-    /* Set the IN_FENCE and OUT_FENCE props only here, since this is the only place
-       on which we're interested in managing who and when should access the buffers
-       that the display plane uses, and that's what these props are for. */
+    /*****************************************************************/
+    /* Tell the display (KMS) that it will have to wait on the fence */
+    /* for the GPU-side FENCE.                                       */
+    /*                                                               */
+    /* Since KMS is a kernel thing, we have to pass an FD into       */
+    /* the kernel, and get another FD out of the kernel.             */
+    /*                                                               */
+    /* 1) To pass the GPU-side fence into the kernel, we set the     */
+    /* INPUT FD as the IN_FENCE_FD prop of the PRIMARY PLANE.        */
+    /* This FD tells KMS (the kernel) to wait for the GPU-side fence.*/
+    /*                                                               */
+    /* 2) To get the KMS-side fence out of the kernel, we set the    */
+    /* OUTPUT FD as the OUT_FEWNCE_FD prop of the CRTC.              */
+    /* This FD will be later imported as a FENCE OBJECT which will be*/
+    /* used to tell the GPU to wait for KMS to complete the changes  */
+    /* requested in atomic_commit (the pageflip in this case).       */ 
+    /*****************************************************************/
     if (dispdata->kms_in_fence_fd != -1)
     {
-	if (add_crtc_property(dispdata->atomic_req, dispdata->crtc, "OUT_FENCE_PTR",
-			          VOID2U64(&dispdata->kms_out_fence_fd)) < 0)
-            return SDL_SetError("Failed to set CRTC OUT_FENCE_PTR prop");
-	if (add_plane_property(dispdata->atomic_req, dispdata->display_plane, "IN_FENCE_FD", dispdata->kms_in_fence_fd) < 0)
+	if (add_plane_property(dispdata->atomic_req, dispdata->display_plane,
+               "IN_FENCE_FD", dispdata->kms_in_fence_fd) < 0)
             return SDL_SetError("Failed to set plane IN_FENCE_FD prop");
+	if (add_crtc_property(dispdata->atomic_req, dispdata->crtc,
+               "OUT_FENCE_PTR", VOID2U64(&dispdata->kms_out_fence_fd)) < 0)
+            return SDL_SetError("Failed to set CRTC OUT_FENCE_PTR prop");
     }
 
-    /* Issue the one and only atomic commit where all changes will be requested!.
-       We need e a non-blocking atomic commit for triple buffering, because we 
-       must not block on this atomic commit so we can re-enter program loop once more. */
+    /*****************************************************************/   
+    /* Issue a non-blocking atomic commit: for triple buffering,     */
+    /* this must not block so the game can start building another    */
+    /* frame, even if the just-requested pageflip hasnt't completed. */
+    /*****************************************************************/   
     if (drm_atomic_commit(_this, SDL_FALSE)) {
         return SDL_SetError("Failed to issue atomic commit on pageflip");
     }
 
-    /* Release the last front buffer so EGL can chose it as back buffer and render on it again. */
+    /* Release the previous front buffer so EGL can chose it as back buffer
+       and render on it again. */
     if (windata->bo) {
 	KMSDRM_gbm_surface_release_buffer(windata->gs, windata->bo);
     }
 
-    /* Take note of current front buffer, so we can free it next time we come here. */
+    /* Take note of the buffer about to become front buffer, so next
+       time we come here we can free it like we just did with the previous
+       front buffer. */
     windata->bo = windata->next_bo;
 
-    /**************************************************************************/
-    /* Block for telling GPU to wait for completion of requested KMS changes  */
-    /* before starting cmdstream execution (=next frame rendering).           */
-    /**************************************************************************/
-
-    /* Import the kms fence from the out fence fd. We need it to tell GPU to wait for pageflip to complete. */
+    /****************************************************************/
+    /* Import the KMS-side FENCE OUTPUT FD from the kernel to the   */
+    /* KMS-side FENCE OBJECT so we can use use it to fence the GPU. */
+    /****************************************************************/
     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. */
+    /****************************************************************/
+    /* "Delete" the fence OUTPUT FD, because we already have the    */
+    /* KMS FENCE OBJECT, the fence itself is away from us, on the   */
+    /* kernel side.                                                 */
+    /****************************************************************/
     dispdata->kms_out_fence_fd = -1;
 
-    /* Tell the GPU to wait until the requested pageflip has completed. */
+    /*****************************************************************/
+    /* Tell the GPU to wait on the fence for the KMS-side FENCE,     */
+    /* which means waiting until the requested pageflip is completed.*/
+    /*****************************************************************/
     _this->egl_data->eglWaitSyncKHR(_this->egl_data->egl_display, dispdata->kms_fence, 0);
 
-    /***************/
-    /* Block ends. */
-    /***************/
-
     return 0;
 }
 
-static int
+int
 KMSDRM_GLES_SwapWindowDoubleBuffered(_THIS, SDL_Window * window)
 {
     SDL_WindowData *windata = ((SDL_WindowData *) window->driverdata);
@@ -220,10 +258,11 @@ KMSDRM_GLES_SwapWindowDoubleBuffered(_THIS, SDL_Window * window)
     KMSDRM_PlaneInfo info = {0};
 
     /****************************************************************************************************/
-    /* In double-buffer mode, atomic commit will always be synchronous/blocking (ie: won't return until */
+    /* In double-buffer mode, atomic_commit will always be synchronous/blocking (ie: won't return until */
     /* the requested changes are really 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.                        */
+    /* (hence not building or executing a new cmdstring) until pageflip is done, so we don't need to    */
+    /* protect the KMS/GPU access to the buffer.                                                        */
     /****************************************************************************************************/ 
 
     /* Mark, at EGL level, the buffer that we want to become the new front buffer.
@@ -312,7 +351,6 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window)
     return windata->swap_window(_this, window);
 }
 
-
 /***************************************/
 /* End of Atomic functions block       */
 /***************************************/