Commit daa752b10ee24f7c6a626ada7eac6f884c4f6ea0

Manuel Alfayate Corchete 2020-09-06T23:19:54

kmsdrm: fix first frame display: no need to wait for SwapWindow() for EGL surface creation.

diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c
index e9c55c9..31204f0 100644
--- a/src/video/kmsdrm/SDL_kmsdrmopengles.c
+++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c
@@ -82,12 +82,16 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window)
     KMSDRM_FBInfo *fb;
     KMSDRM_PlaneInfo info = {0};
 
-    /* Recreate the GBM / EGL surfaces if the window has been reconfigured. */
-    if (windata->egl_surface_dirty) {
-        if (KMSDRM_CreateSurfaces(_this, window)) {
-	    return SDL_SetError("Failed to do pending surfaces creation");
-        }
+    /* Get the EGL context, now that SDL_CreateRenderer() has already been called,
+       and call eglMakeCurrent() on it and the EGL surface. */
+#if SDL_VIDEO_OPENGL_EGL
+    if (windata->egl_context_pending) {
+        EGLContext egl_context;
+        egl_context = (EGLContext)SDL_GL_GetCurrentContext();
+        SDL_EGL_MakeCurrent(_this, windata->egl_surface, egl_context);
+        windata->egl_context_pending = SDL_FALSE;
     }   
+#endif
 
     /*************************************************************************/
     /* Block for telling KMS to wait for GPU rendering of the current frame  */
@@ -210,12 +214,16 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window)
     KMSDRM_FBInfo *fb;
     KMSDRM_PlaneInfo info = {0};
 
-    /* Recreate the GBM / EGL surfaces if the window has been reconfigured. */
-    if (windata->egl_surface_dirty) {
-        if (KMSDRM_CreateSurfaces(_this, window)) {
-	    return SDL_SetError("Failed to do pending surfaces creation");
-        }
+    /* Get the EGL context, now that SDL_CreateRenderer() has already been called,
+       and call eglMakeCurrent() on it and the EGL surface. */
+#if SDL_VIDEO_OPENGL_EGL
+    if (windata->egl_context_pending) {
+        EGLContext egl_context;
+        egl_context = (EGLContext)SDL_GL_GetCurrentContext();
+        SDL_EGL_MakeCurrent(_this, windata->egl_surface, egl_context);
+        windata->egl_context_pending = SDL_FALSE;
     }   
+#endif
 
     /****************************************************************************************************/
     /* In double-buffer mode, atomic commit will always be synchronous/blocking (ie: won't return until */
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index 7d9245f..d441290 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -760,6 +760,71 @@ void
 KMSDRM_DestroySurfaces(_THIS, SDL_Window *window)
 {
     SDL_WindowData *windata = (SDL_WindowData *) window->driverdata;
+    SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
+    KMSDRM_PlaneInfo plane_info = {0};
+
+#if SDL_VIDEO_OPENGL_EGL
+    EGLContext egl_context;
+#endif
+
+    /********************************************************************/
+    /* BLOCK 1: protect the PRIMARY PLANE before destroying the buffers */
+    /* it's using.                                                      */
+    /********************************************************************/
+
+#if AMDGPU_COMPAT
+    /************************************************************************/
+    /*  We can't do the usual CRTC_ID+FB_ID to 0 with AMDGPU, because       */
+    /*  the driver never recovers from the CONNECTOR and CRTC disconnection.*/
+    /*  The with this solution is that crtc->buffer_id is not guaranteed    */
+    /*  to be there...                                                      */
+    /************************************************************************/
+    plane_info.plane = dispdata->display_plane;
+    plane_info.crtc_id = dispdata->crtc->crtc->crtc_id;
+    plane_info.fb_id = dispdata->crtc->crtc->buffer_id;
+    plane_info.src_w = dispdata->mode.hdisplay;
+    plane_info.src_h = dispdata->mode.vdisplay;
+    plane_info.crtc_w = dispdata->mode.hdisplay;
+    plane_info.crtc_h = dispdata->mode.vdisplay;
+
+    drm_atomic_set_plane_props(&plane_info);
+
+    /* Issue blocking atomic commit. */    
+    if (drm_atomic_commit(_this, SDL_TRUE)) {
+        SDL_SetError("Failed to issue atomic commit on DestroyWindow().");
+    }
+#else
+    /*********************************************************************************/
+    /* Disconnect the CONNECTOR from the CRTC (several connectors can read a CRTC),  */
+    /* deactivate the CRTC, and set the PRIMARY PLANE props CRTC_ID and FB_ID to 0.  */
+    /* We have to do this before setting the PLANE CRTC_ID and FB_ID to 0 because    */
+    /* there can be no active CRTC without an active PLANE.                          */
+    /* We can leave all like this if we are exiting the program after the window     */
+    /* destruction, or things will be re-connected again on SwapWindow(), if needed. */
+    /*********************************************************************************/
+    if (add_connector_property(dispdata->atomic_req, dispdata->connector , "CRTC_ID", 0) < 0)
+        SDL_SetError("Failed to set CONNECTOR prop CRTC_ID to zero before buffer destruction");
+
+    if (add_crtc_property(dispdata->atomic_req, dispdata->crtc , "ACTIVE", 0) < 0)
+        SDL_SetError("Failed to set CRTC prop ACTIVE to zero before buffer destruction");
+
+    /* Since we initialize plane_info to all zeros,  ALL PRIMARY PLANE props are set to 0 with this,
+       including FB_ID and CRTC_ID. Not all drivers like FB_ID and CRTC_ID to 0 yet. */
+    plane_info.plane = dispdata->display_plane;
+
+    drm_atomic_set_plane_props(&plane_info);
+
+    /* Issue blocking atomic commit. */    
+    if (drm_atomic_commit(_this, SDL_TRUE)) {
+        SDL_SetError("Failed to issue atomic commit on window surfaces and buffers destruction.");
+    }
+#endif
+
+    /****************************************************************************/
+    /* BLOCK 2: We can finally destroy the window GBM and EGL surfaces, and     */
+    /* GBM buffers now that the buffers are not being used by the PRIMARY PLANE */
+    /* anymore.                                                                 */
+    /****************************************************************************/
 
     /* Destroy the GBM surface and buffers. */
     if (windata->bo) {
@@ -774,7 +839,17 @@ KMSDRM_DestroySurfaces(_THIS, SDL_Window *window)
 
     /* Destroy the EGL surface. */
 #if SDL_VIDEO_OPENGL_EGL
-    SDL_EGL_MakeCurrent(_this, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+    /***************************************************************************/
+    /* In this eglMakeCurrent() call, we disable the current EGL surface       */
+    /* because we're going to destroy it, but DON'T disable the EGL context,   */
+    /* because it won't be enabled again until the programs ask for a pageflip */
+    /* so we get to SwapWindow().                                              */
+    /* If we disable the context until then and a program tries to retrieve    */
+    /* the context version info before calling for a pageflip, the program     */
+    /* will get wrong info and we will be in trouble.                          */
+    /***************************************************************************/
+    egl_context = (EGLContext)SDL_GL_GetCurrentContext();
+    SDL_EGL_MakeCurrent(_this, EGL_NO_SURFACE, egl_context);
 
     if (windata->egl_surface != EGL_NO_SURFACE) {
         SDL_EGL_DestroySurface(_this, windata->egl_surface);
@@ -798,12 +873,6 @@ KMSDRM_CreateSurfaces(_THIS, SDL_Window * window)
     uint32_t surface_flags = GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING;
     uint32_t width, height;
 
-#if SDL_VIDEO_OPENGL_EGL
-    EGLContext egl_context;
-    SDL_EGL_SetRequiredVisualId(_this, surface_fmt);
-    egl_context = (EGLContext)SDL_GL_GetCurrentContext();
-#endif
-
     /* Destroy the surfaces and buffers before creating the new ones. */
     KMSDRM_DestroySurfaces(_this, window);
 
@@ -827,15 +896,20 @@ KMSDRM_CreateSurfaces(_THIS, SDL_Window * window)
     }
 
 #if SDL_VIDEO_OPENGL_EGL
+    /* We can't get the EGL context yet because SDL_CreateRenderer has not been called,
+       but we need an EGL surface NOW, or GL won't be able to render into any surface
+       and we won't see the first frame. */ 
+    SDL_EGL_SetRequiredVisualId(_this, surface_fmt);
     windata->egl_surface = SDL_EGL_CreateSurface(_this, (NativeWindowType)windata->gs);
 
     if (windata->egl_surface == EGL_NO_SURFACE) {
         return SDL_SetError("Could not create EGL window surface");
     }
 
-    SDL_EGL_MakeCurrent(_this, windata->egl_surface, egl_context);
-
-    windata->egl_surface_dirty = 0;
+    /* Take note that we're still missing the EGL contex,
+       so we can get it in SwapWindow, when SDL_CreateRenderer()
+       has already been called. */
+    windata->egl_context_pending = SDL_TRUE;
 #endif
 
     return 0;
@@ -845,65 +919,12 @@ void
 KMSDRM_DestroyWindow(_THIS, SDL_Window *window)
 {
     SDL_WindowData *windata = (SDL_WindowData *) window->driverdata;
-    SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
-    KMSDRM_PlaneInfo plane_info = {0};
-
     SDL_VideoData *viddata;
+
     if (!windata) {
         return;
     }
-#if AMDGPU_COMPAT
-    /************************************************************************/
-    /*  We can't do the usual CRTC_ID+FB_ID to 0 with AMDGPU, because       */
-    /*  the driver never recovers from the CONNECTOR and CRTC disconnection.*/
-    /*  The with this solution is that crtc->buffer_id is not guaranteed    */
-    /*  to be there...                                                      */
-    /************************************************************************/
-    plane_info.plane = dispdata->display_plane;
-    plane_info.crtc_id = dispdata->crtc->crtc->crtc_id;
-    plane_info.fb_id = dispdata->crtc->crtc->buffer_id;
-    plane_info.src_w = dispdata->mode.hdisplay;
-    plane_info.src_h = dispdata->mode.vdisplay;
-    plane_info.crtc_w = dispdata->mode.hdisplay;
-    plane_info.crtc_h = dispdata->mode.vdisplay;
 
-    drm_atomic_set_plane_props(&plane_info);
-
-    /* Issue blocking atomic commit. */    
-    if (drm_atomic_commit(_this, SDL_TRUE)) {
-        SDL_SetError("Failed to issue atomic commit on DestroyWindow().");
-    }
-#else
-    /*********************************************************************************/
-    /* Disconnect the CONNECTOR from the CRTC (several connectors can read a CRTC),  */
-    /* deactivate the CRTC, and set the PRIMARY PLANE props CRTC_ID and FB_ID to 0.  */
-    /* We have to do this before setting the PLANE CRTC_ID and FB_ID to 0 because    */
-    /* there can be no active CRTC without an active PLANE.                          */
-    /* We can leave all like this if we are exiting the program after the window     */
-    /* destruction, or things will be re-connected again on SwapWindow(), if needed. */
-    /*********************************************************************************/
-    if (add_connector_property(dispdata->atomic_req, dispdata->connector , "CRTC_ID", 0) < 0)
-        SDL_SetError("Failed to set CONNECTOR prop CRTC_ID to zero before buffer destruction");
-
-    if (add_crtc_property(dispdata->atomic_req, dispdata->crtc , "ACTIVE", 0) < 0)
-        SDL_SetError("Failed to set CRTC prop ACTIVE to zero before buffer destruction");
-
-    /* Since we initialize plane_info to all zeros,  ALL PRIMARY PLANE props are set to 0 with this,
-       including FB_ID and CRTC_ID. Not all drivers like FB_ID and CRTC_ID to 0 yet. */
-    plane_info.plane = dispdata->display_plane;
-
-    drm_atomic_set_plane_props(&plane_info);
-
-    /* Issue blocking atomic commit. */    
-    if (drm_atomic_commit(_this, SDL_TRUE)) {
-        SDL_SetError("Failed to issue atomic commit on window surfaces and buffers destruction.");
-    }
-#endif
-
-    /****************************************************************************/
-    /* We can finally destroy the window GBM and EGL surfaces, and GBM buffers, */
-    /* now that the buffers are not being used by the PRIMARY PLANE anymore.    */
-    /****************************************************************************/
     KMSDRM_DestroySurfaces(_this, window);
 
     /********************************************/
@@ -957,19 +978,9 @@ KMSDRM_ReconfigureWindow( _THIS, SDL_Window * window) {
         windata->output_x = (dispdata->mode.hdisplay - windata->output_w) / 2;
     }
 
-#if SDL_VIDEO_OPENGL_EGL
-    /* Can't recreate EGL surfaces right now, need to wait until SwapWindow
-       so the EGL context is available. That's because SDL_CreateRenderer(),
-       where the EGL context is created, is always called after SDL_CreateWindow()
-       since SDL_CreateRenderer() takes a window as parameter.
-       On window destruction, SDL_DestroyRenderer() is called before SDL_DestroWindow(),
-       so on SDL_DestroyWindow() the EGL context isn't available anymore. */
-    windata->egl_surface_dirty = SDL_TRUE;
-#else
     if (KMSDRM_CreateSurfaces(_this, window)) {
 	return -1; 
     }   
-#endif
 
     return 0;
 }
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.h b/src/video/kmsdrm/SDL_kmsdrmvideo.h
index bb16237..16bee13 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.h
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.h
@@ -126,7 +126,9 @@ typedef struct SDL_WindowData
     int32_t output_h;
     int32_t output_x;
 
-    SDL_bool egl_surface_dirty;
+    /* This is for deferred eglMakeCurrent() call: we can't call it until
+       the EGL context is available, but we need the EGL surface sooner. */
+    SDL_bool egl_context_pending;
 
 } SDL_WindowData;
 
@@ -152,7 +154,7 @@ typedef struct KMSDRM_PlaneInfo
 } KMSDRM_PlaneInfo;
 
 /* Helper functions */
-int KMSDRM_CreateSurfaces(_THIS, SDL_Window * window);
+int KMSDRM_CreateEGLSurface(_THIS, SDL_Window * window);
 KMSDRM_FBInfo *KMSDRM_FBFromBO(_THIS, struct gbm_bo *bo);
 
 /* Atomic functions that are used from SDL_kmsdrmopengles.c and SDL_kmsdrmmouse.c */