From 1adadc7702c7f82db9140a1ae2a7ecb5ec66e62a Mon Sep 17 00:00:00 2001 From: Manuel Alfayate Corchete Date: Thu, 14 Jan 2021 10:18:40 +0100 Subject: [PATCH] [KMS/DRM] Adjust come return values. Improve comments. --- src/video/kmsdrm/SDL_kmsdrmopengles.c | 24 ++++++++++++++++-------- src/video/kmsdrm/SDL_kmsdrmvideo.c | 10 +++++----- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c index d9e14b037..1b0714442 100644 --- a/src/video/kmsdrm/SDL_kmsdrmopengles.c +++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c @@ -100,6 +100,7 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) { /* Wait for confirmation that the next front buffer has been flipped, at which point the previous front buffer can be released */ if (!KMSDRM_WaitPageFlip(_this, windata)) { + SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Wait for previous pageflip failed"); return 0; } @@ -115,7 +116,7 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) { This won't happen until pagelip completes. */ if (!(_this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface))) { - SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed."); + SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed"); return 0; } @@ -124,19 +125,20 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) { from drawing into it!) */ windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs); if (!windata->next_bo) { - SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock GBM surface front buffer"); + SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock front buffer on GBM surface"); return 0; } /* Get an actual usable fb for the next front buffer. */ fb_info = KMSDRM_FBFromBO(_this, windata->next_bo); if (!fb_info) { + SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not get a framebuffer"); return 0; } /* Do we have a modeset pending? If so, configure the new mode on the CRTC. - Has to be done before the upcoming pageflip issue, so the buffer with the - new size is big enough so the CRTC doesn't read out of bounds. */ + 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) { ret = KMSDRM_drmModeSetCrtc(viddata->drm_fd, @@ -147,9 +149,12 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) { if (ret) { SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not set videomode on CRTC."); + return 0; } - return 0; + /* 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. @@ -185,11 +190,14 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) { Just leave it here and don't worry. Run your SDL2 program with "SDL_KMSDRM_DOUBLE_BUFFER=1 " to enable this. */ - if (_this->egl_data->egl_swapinterval == 1 && windata->double_buffer) { - KMSDRM_WaitPageFlip(_this, windata); + if (windata->double_buffer) { + if (!KMSDRM_WaitPageFlip(_this, windata)) { + SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Immediate wait for previous pageflip failed"); + return 0; + } } - return 0; + return 1; } SDL_EGL_MakeCurrent_impl(KMSDRM) diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c index a82813bfc..ebf643a01 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.c +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c @@ -351,7 +351,7 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) { pfd.fd = viddata->drm_fd; pfd.events = POLLIN; - /* Stay on loop until we get the desired event + /* Stay on the while loop until we get the desired event. We need the while the loop because we could be in a situation where: -We get events on the FD in time, thus not on exiting on return number 1. -These events are not errors, thus not exiting on return number 2. @@ -359,10 +359,10 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) { but if the event is not the pageflip we are waiting for, we arrive at the end of the loop and do loop re-entry, hoping the next event will be the pageflip. - So we could erroneously exit the function without the pageflip event to arrive - if it wasn't for the while loop! + If it wasn't for the while loop, we could erroneously exit the function + without the pageflip event to arrive! - Vblank events, for example, hit the FD and they are POLLIN events too (POLLIN + For example, vblank events hit the FD and they are POLLIN events too (POLLIN means "there's data to read on the FD"), but they are not the pageflip event we are waiting for, so the drmEventHandle() doesn't run the flip handler, and since waiting_for_flip is set on the pageflip handle, it's not set and we stay @@ -390,7 +390,7 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata) { if (pfd.revents & POLLIN) { /* There is data to read on the FD! Is the event a pageflip? We know it is a pageflip if it matches the - event we are passing in &ev. If it is, drmHandleEvent() will unset + event we are passing in &ev. If it does, drmHandleEvent() will unset windata->waiting_for_flip and we will get out of the "while" loop. If it's not, we keep iterating on the loop. */ KMSDRM_drmHandleEvent(viddata->drm_fd, &ev);