Commit 427c96ec11df3059c8295db5576a930c95bf93f7

Manuel Alfayate Corchete 2020-12-29T14:24:38

[KMS/DRM] Rework some functions.

diff --git a/src/video/kmsdrm/SDL_kmsdrmmouse.c b/src/video/kmsdrm/SDL_kmsdrmmouse.c
index c472fc4..ff5bfc8 100644
--- a/src/video/kmsdrm/SDL_kmsdrmmouse.c
+++ b/src/video/kmsdrm/SDL_kmsdrmmouse.c
@@ -252,7 +252,7 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
                 info.plane = dispdata->cursor_plane;
                 /* The rest of the members are zeroed. */
                 drm_atomic_set_plane_props(&info);
-                if (drm_atomic_commit(display->device, SDL_TRUE))
+                if (drm_atomic_commit(display->device, SDL_TRUE, SDL_FALSE))
                     return SDL_SetError("Failed atomic commit in KMSDRM_ShowCursor.");
                 }
                 return 0;
@@ -324,7 +324,7 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
 
     drm_atomic_set_plane_props(&info);
 
-    if (drm_atomic_commit(display->device, SDL_TRUE)) {
+    if (drm_atomic_commit(display->device, SDL_TRUE, SDL_FALSE)) {
         ret = SDL_SetError("Failed atomic commit in KMSDRM_ShowCursor.");
         goto cleanup;
     }
@@ -491,7 +491,7 @@ KMSDRM_DeinitMouse(_THIS)
 	drm_atomic_set_plane_props(&info);
 	/* Wait until the cursor is unset from the cursor plane
 	   before destroying it's BO. */
-	if (drm_atomic_commit(video_device, SDL_TRUE)) {
+	if (drm_atomic_commit(video_device, SDL_TRUE, SDL_FALSE)) {
 	    SDL_SetError("Failed atomic commit in KMSDRM_DenitMouse.");
 	}
 	/* ..and finally destroy the cursor DRM BO! */
diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c
index a828f3a..19126cb 100644
--- a/src/video/kmsdrm/SDL_kmsdrmopengles.c
+++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c
@@ -224,7 +224,6 @@ KMSDRM_GLES_SwapWindowFenced(_THIS, SDL_Window * window)
         uint32_t blob_id;
         SDL_VideoData *viddata = (SDL_VideoData *)_this->driverdata;
 
-        dispdata->atomic_flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
         add_connector_property(dispdata->atomic_req, dispdata->connector, "CRTC_ID", dispdata->crtc->crtc->crtc_id);
         KMSDRM_drmModeCreatePropertyBlob(viddata->drm_fd, &dispdata->mode, sizeof(dispdata->mode), &blob_id);
         add_crtc_property(dispdata->atomic_req, dispdata->crtc, "MODE_ID", blob_id);
@@ -237,10 +236,15 @@ KMSDRM_GLES_SwapWindowFenced(_THIS, SDL_Window * window)
     /* 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)) {
+    if (drm_atomic_commit(_this, SDL_FALSE, dispdata->modeset_pending)) {
         return SDL_SetError("Failed to issue atomic commit on pageflip");
     }
 
+    /* If we had a pending modesetting, we have done it by now.  */
+    if (dispdata->modeset_pending) {
+        dispdata->modeset_pending = SDL_FALSE;
+    }
+
     /* Release the previous front buffer so EGL can chose it as back buffer
        and render on it again. */
     if (windata->bo) {
@@ -283,22 +287,23 @@ KMSDRM_GLES_SwapWindowDoubleBuffered(_THIS, SDL_Window * window)
     KMSDRM_FBInfo *fb;
     KMSDRM_PlaneInfo info = {0};
 
-    /****************************************************************************************************/
-    /* 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, so we don't need to    */
-    /* protect the KMS/GPU access to the buffer.                                                        */
-    /****************************************************************************************************/ 
+    /**********************************************************************************/
+    /* 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, 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.
-       However, it won't really happen until we request a pageflip at the KMS level and it completes. */
+       It 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");
     }
 
-    /* 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. */
+    /* 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. */
     windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
     if (!windata->next_bo) {
         return SDL_SetError("Failed to lock frontbuffer");
@@ -324,20 +329,25 @@ KMSDRM_GLES_SwapWindowDoubleBuffered(_THIS, SDL_Window * window)
     /* Do we have a pending modesetting? If so, set the necessary 
        props so it's included in the incoming atomic commit. */
     if (dispdata->modeset_pending) {
-        SDL_VideoData *viddata = (SDL_VideoData *)_this->driverdata;
         uint32_t blob_id;
-        dispdata->atomic_flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
+
+        SDL_VideoData *viddata = (SDL_VideoData *)_this->driverdata;
+
         add_connector_property(dispdata->atomic_req, dispdata->connector, "CRTC_ID", dispdata->crtc->crtc->crtc_id);
         KMSDRM_drmModeCreatePropertyBlob(viddata->drm_fd, &dispdata->mode, sizeof(dispdata->mode), &blob_id);
         add_crtc_property(dispdata->atomic_req, dispdata->crtc, "MODE_ID", blob_id);
         add_crtc_property(dispdata->atomic_req, dispdata->crtc, "active", 1);
-        dispdata->modeset_pending = SDL_FALSE;
     }
 
     /* Issue the one and only atomic commit where all changes will be requested!. 
        Blocking for double buffering: won't return until completed. */
-    if (drm_atomic_commit(_this, SDL_TRUE)) {
-        return SDL_SetError("Failed to issue atomic commit");
+    if (drm_atomic_commit(_this, SDL_TRUE, dispdata->modeset_pending)) {
+        return SDL_SetError("Failed to issue atomic commit on pageflip");
+    }
+
+    /* If we had a pending modesetting, we have done it by now.  */
+    if (dispdata->modeset_pending) {
+        dispdata->modeset_pending = SDL_FALSE;
     }
 
     /* Release last front buffer so EGL can chose it as back buffer and render on it again. */
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index ff235b1..3c30e20 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -486,8 +486,8 @@ void get_planes_info(_THIS)
 #endif
 
 /* Get the plane_id of a plane that is of the specified plane type (primary,
-   overlay, cursor...) and can use the CRTC we have chosen previously. */
-static int get_plane_id(_THIS, uint32_t plane_type)
+   overlay, cursor...) and can use specified CRTC. */
+static int get_plane_id(_THIS, unsigned int crtc_id, uint32_t plane_type)
 {
     drmModeRes *resources = NULL;
     drmModePlaneResPtr plane_resources = NULL;
@@ -497,14 +497,13 @@ static int get_plane_id(_THIS, uint32_t plane_type)
     int found = 0;
 
     SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
-    SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
 
     resources = KMSDRM_drmModeGetResources(viddata->drm_fd);
 
     /* Get the crtc_index for the current CRTC.
        It's needed to find out if a plane supports the CRTC. */
     for (i = 0; i < resources->count_crtcs; i++) {
-        if (resources->crtcs[i] == dispdata->crtc->crtc->crtc_id) {
+        if (resources->crtcs[i] == crtc_id) {
             crtc_index = i;
             break;
         }
@@ -565,6 +564,7 @@ setup_plane(_THIS, struct plane **plane, uint32_t plane_type)
 {
     uint32_t plane_id;
     SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
+    SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
     int ret = 0;
 
     *plane = SDL_calloc(1, sizeof(**plane));
@@ -573,8 +573,8 @@ setup_plane(_THIS, struct plane **plane, uint32_t plane_type)
         goto cleanup;
     }
 
-    /* Get plane ID. */
-    plane_id = get_plane_id(_this, plane_type);
+    /* Get plane ID for a given CRTC and plane type. */
+    plane_id = get_plane_id(_this, dispdata->crtc->crtc->crtc_id, plane_type);
 
     if (!plane_id) {
         ret = SDL_SetError("Invalid Plane ID");
@@ -671,19 +671,28 @@ drm_atomic_set_plane_props(struct KMSDRM_PlaneInfo *info)
     add_plane_property(dispdata->atomic_req, info->plane, "CRTC_Y", info->crtc_y);
 }
 
-int drm_atomic_commit(_THIS, SDL_bool blocking)
+int drm_atomic_commit(_THIS, SDL_bool blocking, SDL_bool allow_modeset)
 {
     SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
     SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
+    uint32_t atomic_flags = 0;
     int ret;
 
-    if (!blocking)
-        dispdata->atomic_flags |= DRM_MODE_ATOMIC_NONBLOCK;
+    if (!blocking) {
+        atomic_flags |= DRM_MODE_ATOMIC_NONBLOCK;
+    }
+
+    if (allow_modeset) {
+        atomic_flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
+    }
 
-    /* Never issue a new atomic commit if previous has not yet completed, or it will error. */
+    /* Never issue a new atomic commit if previous has not yet completed,
+       or it will error. */
     drm_atomic_waitpending(_this);
 
-    ret = KMSDRM_drmModeAtomicCommit(viddata->drm_fd, dispdata->atomic_req, dispdata->atomic_flags, NULL);
+    ret = KMSDRM_drmModeAtomicCommit(viddata->drm_fd, dispdata->atomic_req,
+              atomic_flags, NULL);
+
     if (ret) {
         SDL_SetError("Atomic commit failed, returned %d.", ret);
         /* Uncomment this for fast-debugging */
@@ -701,7 +710,6 @@ int drm_atomic_commit(_THIS, SDL_bool blocking)
 out:
     KMSDRM_drmModeAtomicFree(dispdata->atomic_req);
     dispdata->atomic_req = NULL;
-    dispdata->atomic_flags = 0;
 
     return ret;
 }
@@ -739,7 +747,7 @@ KMSDRM_Available(void)
     if (ret >= 0)
         return 1;
 
-    return ret;
+   return ret;
 }
 
 static void
@@ -886,7 +894,6 @@ VideoBootStrap KMSDRM_bootstrap = {
     KMSDRM_CreateDevice
 };
 
-
 static void
 KMSDRM_FBDestroyCallback(struct gbm_bo *bo, void *data)
 {
@@ -1010,7 +1017,6 @@ int KMSDRM_DisplayDataInit (_THIS, SDL_DisplayData *dispdata) {
     int ret = 0;
     unsigned i,j;
 
-    dispdata->atomic_flags = 0;
     dispdata->atomic_req = NULL;
     dispdata->kms_fence = NULL;
     dispdata->gpu_fence = NULL;
@@ -1326,6 +1332,30 @@ KMSDRM_DestroySurfaces(_THIS, SDL_Window *window)
     SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
     KMSDRM_PlaneInfo plane_info = {0};
 
+    /* TODO : Continue investigating why this doesn't work. We should do this instead
+       of making the display plane point to the TTY console, which isn't there
+       after creating and destroying a Vulkan window. */
+#if 0
+    /* Disconnect the connector from the CRTC (remember: several connectors
+       can read a CRTC), deactivate the CRTC, and set the PRIMARY PLANE props
+       CRTC_ID and FB_ID to 0. Then we can destroy the GBM buffers and surface. */
+    add_connector_property(dispdata->atomic_req, dispdata->connector , "CRTC_ID", 0);
+    add_crtc_property(dispdata->atomic_req, dispdata->crtc , "MODE_ID", 0); 
+    add_crtc_property(dispdata->atomic_req, dispdata->crtc , "active", 0); 
+
+    plane_info.plane = dispdata->display_plane;
+    plane_info.crtc_id = 0;
+    plane_info.fb_id = 0;
+
+    drm_atomic_set_plane_props(&plane_info);
+
+    /* Issue atomic commit that is blocking and allows modesetting. */
+    if (drm_atomic_commit(_this, SDL_TRUE, SDL_TRUE)) {
+        SDL_SetError("Failed to issue atomic commit on surfaces destruction.");
+    }
+#endif
+
+#if 1
     /************************************************************/
     /* Make the display plane point to the original TTY buffer. */
     /************************************************************/
@@ -1340,9 +1370,10 @@ KMSDRM_DestroySurfaces(_THIS, SDL_Window *window)
 
     drm_atomic_set_plane_props(&plane_info);
 
-    if (drm_atomic_commit(_this, SDL_TRUE)) {
+    if (drm_atomic_commit(_this, SDL_TRUE, SDL_FALSE)) {
         SDL_SetError("Failed to issue atomic commit on surfaces destruction.");
     }
+#endif
 
     /***************************/
     /* Destroy the EGL surface */
@@ -1467,6 +1498,7 @@ KMSDRM_DestroyWindow(_THIS, SDL_Window *window)
         if (_this->egl_data) {
             SDL_EGL_UnloadLibrary(_this);
         }
+
         /* Free display plane, and destroy GBM device. */
         KMSDRM_GBMDeinit(_this, dispdata);
     }
@@ -1706,10 +1738,12 @@ KMSDRM_SetDisplayMode(_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode)
         return SDL_SetError("Mode doesn't have an associated index");
     }
 
-    /* Take note of the new mode. It will be used in SwapWindow to
-       set the props needed for mode setting. */
+    /* Take note of the new mode to be set. */
     dispdata->mode = conn->modes[modedata->mode_index];
 
+    /* Take note that we have to change mode in SwapWindow(). We have to do it there
+       because we need a buffer of the new size so the commit that contains the
+       mode change can be completed OK.  */
     dispdata->modeset_pending = SDL_TRUE;
 
     for (i = 0; i < viddata->num_windows; i++) {
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.h b/src/video/kmsdrm/SDL_kmsdrmvideo.h
index 1023c1d..7b7f7bb 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.h
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.h
@@ -84,7 +84,6 @@ typedef struct SDL_DisplayData
 {
     drmModeModeInfo mode;
     drmModeModeInfo preferred_mode;
-    uint32_t atomic_flags;
 
     plane *display_plane;
     plane *cursor_plane;
@@ -173,7 +172,7 @@ KMSDRM_FBInfo *KMSDRM_FBFromBO(_THIS, struct gbm_bo *bo);
 void drm_atomic_set_plane_props(struct KMSDRM_PlaneInfo *info); 
 
 void drm_atomic_waitpending(_THIS);
-int drm_atomic_commit(_THIS, SDL_bool blocking);
+int drm_atomic_commit(_THIS, SDL_bool blocking, SDL_bool allow_modeset);
 int add_plane_property(drmModeAtomicReq *req, struct plane *plane,
                              const char *name, uint64_t value);
 int add_crtc_property(drmModeAtomicReq *req, struct crtc *crtc,