From c437729b2198069a4c11f2a5d91e00aaf9aabd4c Mon Sep 17 00:00:00 2001 From: Manuel Alfayate Corchete Date: Sat, 8 Aug 2020 14:27:55 +0200 Subject: [PATCH] kmsdrm: separate requests in different functions so we only need one atomic commit for everything, as expected by atomic design. --- src/video/kmsdrm/SDL_kmsdrmopengles.c | 47 +++----- src/video/kmsdrm/SDL_kmsdrmvideo.c | 156 +++++++++++++++----------- src/video/kmsdrm/SDL_kmsdrmvideo.h | 18 ++- 3 files changed, 117 insertions(+), 104 deletions(-) diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c index f5f93be47..582b1c103 100644 --- a/src/video/kmsdrm/SDL_kmsdrmopengles.c +++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c @@ -80,16 +80,6 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) SDL_DisplayData *dispdata = (SDL_DisplayData *) SDL_GetDisplayForWindow(window)->driverdata; KMSDRM_FBInfo *fb; int ret; - uint32_t flags = 0; - - /* 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; - } /*************************************************************************/ /* Block for telling KMS to wait for GPU rendering of the current frame */ @@ -128,20 +118,14 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) return SDL_SetError("Failed to get a new framebuffer BO"); } - /* Don't issue another atomic ioctl until previous one has completed: it will cause errors. */ - if (dispdata->kms_fence) { - EGLint status; + /* Add the pageflip to te request list. */ + drm_atomic_request_pageflip(_this, fb->fb_id); - do { - status = _this->egl_data->eglClientWaitSyncKHR(_this->egl_data->egl_display, dispdata->kms_fence, 0, EGL_FOREVER_KHR); - } while (status != EGL_CONDITION_SATISFIED_KHR); + /* 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. */ + ret = drm_atomic_commit(_this, SDL_FALSE); - _this->egl_data->eglDestroySyncKHR(_this->egl_data->egl_display, dispdata->kms_fence); - dispdata->kms_fence = NULL; - } - - /* Issue atomic commit, where we request the pageflip. */ - ret = drm_atomic_commit(_this, fb->fb_id, flags); if (ret) { return SDL_SetError("failed to issue atomic commit"); } @@ -180,22 +164,14 @@ int KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window) { SDL_WindowData *windata = ((SDL_WindowData *) window->driverdata); - SDL_DisplayData *dispdata = (SDL_DisplayData *) SDL_GetDisplayForWindow(window)->driverdata; KMSDRM_FBInfo *fb; 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; - } - /* 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); @@ -211,10 +187,15 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window) return SDL_SetError("Failed to get a new framebuffer BO"); } - /* Issue atomic commit, where we request the pageflip. */ - ret = drm_atomic_commit(_this, fb->fb_id, flags); + /* Add the pageflip to te request list. */ + drm_atomic_request_pageflip(_this, fb->fb_id); + + /* Issue the one and only atomic commit where all changes will be requested!. + Blocking for double buffering: won't return until completed. */ + ret = drm_atomic_commit(_this, SDL_TRUE); + if (ret) { - return SDL_SetError("failed to do atomic commit"); + return SDL_SetError("failed to issue atomic commit"); } /* 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 76c037eff..8f417b808 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.c +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c @@ -197,23 +197,23 @@ static int add_crtc_property(drmModeAtomicReq *req, uint32_t obj_id, static int add_plane_property(drmModeAtomicReq *req, uint32_t obj_id, const char *name, uint64_t value) { - SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); - unsigned int i; - int prop_id = -1; + SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); + unsigned int i; + int prop_id = -1; - for (i = 0 ; i < dispdata->plane_props->count_props ; i++) { - if (strcmp(dispdata->plane_props_info[i]->name, name) == 0) { - prop_id = dispdata->plane_props_info[i]->prop_id; - break; - } + for (i = 0 ; i < dispdata->plane_props->count_props ; i++) { + if (strcmp(dispdata->plane_props_info[i]->name, name) == 0) { + prop_id = dispdata->plane_props_info[i]->prop_id; + break; } + } - if (prop_id < 0) { - printf("no plane property: %s\n", name); - return -EINVAL; - } + if (prop_id < 0) { + printf("no plane property: %s\n", name); + return -EINVAL; + } - return KMSDRM_drmModeAtomicAddProperty(req, obj_id, prop_id, value); + return KMSDRM_drmModeAtomicAddProperty(req, obj_id, prop_id, value); } #if 0 @@ -403,51 +403,70 @@ uint32_t get_plane_id(_THIS, drmModeRes *resources) return ret; } -int drm_atomic_commit(_THIS, uint32_t fb_id, uint32_t flags) +void +drm_atomic_request_pageflip(_THIS, uint32_t fb_id) +{ + + SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); + uint32_t plane_id = dispdata->plane->plane_id; + + /* Do we have a set of changes already in the making? If not, allocate a new one. */ + if (!dispdata->atomic_req) + dispdata->atomic_req = KMSDRM_drmModeAtomicAlloc(); + + add_plane_property(dispdata->atomic_req, plane_id, "FB_ID", fb_id); + add_plane_property(dispdata->atomic_req, plane_id, "CRTC_ID", dispdata->crtc_id); + add_plane_property(dispdata->atomic_req, plane_id, "SRC_X", 0); + add_plane_property(dispdata->atomic_req, plane_id, "SRC_Y", 0); + add_plane_property(dispdata->atomic_req, plane_id, "SRC_W", dispdata->mode.hdisplay << 16); + add_plane_property(dispdata->atomic_req, plane_id, "SRC_H", dispdata->mode.vdisplay << 16); + add_plane_property(dispdata->atomic_req, plane_id, "CRTC_X", 0); + add_plane_property(dispdata->atomic_req, plane_id, "CRTC_Y", 0); + add_plane_property(dispdata->atomic_req, plane_id, "CRTC_W", dispdata->mode.hdisplay); + add_plane_property(dispdata->atomic_req, plane_id, "CRTC_H", dispdata->mode.vdisplay); + + if (dispdata->kms_in_fence_fd != -1) { + add_crtc_property(dispdata->atomic_req, dispdata->crtc_id, "OUT_FENCE_PTR", + VOID2U64(&dispdata->kms_out_fence_fd)); + add_plane_property(dispdata->atomic_req, plane_id, "IN_FENCE_FD", dispdata->kms_in_fence_fd); + } + +} + +void +drm_atomic_request_modeset(_THIS) { SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); - uint32_t plane_id = dispdata->plane->plane_id; + uint32_t blob_id; - drmModeAtomicReq *req; + + /* Do we have a set of changes already in the making? If not, allocate a new one. */ + if (!dispdata->atomic_req) + dispdata->atomic_req = KMSDRM_drmModeAtomicAlloc(); + + dispdata->atomic_flags |= DRM_MODE_ATOMIC_ALLOW_MODESET; + + add_connector_property(dispdata->atomic_req, dispdata->connector->connector_id, "CRTC_ID", dispdata->crtc_id); + KMSDRM_drmModeCreatePropertyBlob(viddata->drm_fd, &dispdata->mode, sizeof(dispdata->mode), &blob_id); + add_crtc_property(dispdata->atomic_req, dispdata->crtc_id, "MODE_ID", blob_id); + add_crtc_property(dispdata->atomic_req, dispdata->crtc_id, "ACTIVE", 1); +} + + +int drm_atomic_commit(_THIS, SDL_bool blocking) +{ + SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); + SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); int ret; - req = KMSDRM_drmModeAtomicAlloc(); + if (!blocking) + dispdata->atomic_flags |= DRM_MODE_ATOMIC_NONBLOCK; - if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) { - if (add_connector_property(req, dispdata->connector->connector_id, "CRTC_ID", - dispdata->crtc_id) < 0) - return -1; + /* Never issue a new atomic commit if previous has not yet completed, or it will error. */ + drm_atomic_wait_pending(_this); - if (KMSDRM_drmModeCreatePropertyBlob(viddata->drm_fd, &dispdata->mode, sizeof(dispdata->mode), - &blob_id) != 0) - return -1; - - if (add_crtc_property(req, dispdata->crtc_id, "MODE_ID", blob_id) < 0) - return -1; - - if (add_crtc_property(req, dispdata->crtc_id, "ACTIVE", 1) < 0) - return -1; - } - - add_plane_property(req, plane_id, "FB_ID", fb_id); - add_plane_property(req, plane_id, "CRTC_ID", dispdata->crtc_id); - add_plane_property(req, plane_id, "SRC_X", 0); - add_plane_property(req, plane_id, "SRC_Y", 0); - add_plane_property(req, plane_id, "SRC_W", dispdata->mode.hdisplay << 16); - add_plane_property(req, plane_id, "SRC_H", dispdata->mode.vdisplay << 16); - add_plane_property(req, plane_id, "CRTC_X", 0); - add_plane_property(req, plane_id, "CRTC_Y", 0); - add_plane_property(req, plane_id, "CRTC_W", dispdata->mode.hdisplay); - add_plane_property(req, plane_id, "CRTC_H", dispdata->mode.vdisplay); - - if (dispdata->kms_in_fence_fd != -1) { - add_crtc_property(req, dispdata->crtc_id, "OUT_FENCE_PTR", - VOID2U64(&dispdata->kms_out_fence_fd)); - add_plane_property(req, plane_id, "IN_FENCE_FD", dispdata->kms_in_fence_fd); - } - - ret = KMSDRM_drmModeAtomicCommit(viddata->drm_fd, req, flags, NULL); + ret = KMSDRM_drmModeAtomicCommit(viddata->drm_fd, dispdata->atomic_req, dispdata->atomic_flags, NULL); if (ret) goto out; @@ -457,13 +476,15 @@ int drm_atomic_commit(_THIS, uint32_t fb_id, uint32_t flags) } out: - KMSDRM_drmModeAtomicFree(req); + KMSDRM_drmModeAtomicFree(dispdata->atomic_req); + dispdata->atomic_req = NULL; + dispdata->atomic_flags = 0; return ret; } void -wait_pending_atomic(_THIS) +drm_atomic_wait_pending(_THIS) { SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); @@ -676,7 +697,7 @@ KMSDRM_DestroySurfaces(_THIS, SDL_Window * window) SDL_WindowData *windata = (SDL_WindowData *)window->driverdata; /* Wait for pending atomic commit (like pageflips requested in SwapWindow) to complete. */ - wait_pending_atomic(_this); + drm_atomic_wait_pending(_this); if (windata->bo) { KMSDRM_gbm_surface_release_buffer(windata->gs, windata->bo); @@ -746,9 +767,8 @@ KMSDRM_CreateSurfaces(_THIS, SDL_Window * window) windata->egl_surface_dirty = SDL_FALSE; #endif - /* We take note here about the need to do a modeset in the atomic_commit(), - called in KMSDRM_GLES_SwapWindow(). */ - dispdata->modeset_pending = SDL_TRUE; + /* Add modeset request to the current change request list. */ + drm_atomic_request_modeset(_this); return 0; } @@ -901,12 +921,13 @@ KMSDRM_VideoInit(_THIS) /* Atomic block */ /****************/ - /* Initialize the fences and their fds: */ + /* Initialize the atomic dispdata members. */ + dispdata->atomic_flags = 0; + dispdata->atomic_req = NULL; dispdata->kms_fence = NULL; dispdata->gpu_fence = NULL; dispdata->kms_out_fence_fd = -1, dispdata->kms_in_fence_fd = -1, - dispdata->modeset_pending = SDL_FALSE; /*********************/ /* Atomic block ends */ @@ -1086,20 +1107,18 @@ KMSDRM_VideoQuit(_THIS) viddata->max_windows = 0; viddata->num_windows = 0; - /* Restore original videomode. */ + /* Restore original buffer. */ if (viddata->drm_fd >= 0 && dispdata && dispdata->connector && dispdata->crtc) { - uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET; - /***********************************************************/ - /* Atomic block for video mode and crt->buffer restoration */ + /* Atomic block for original crt->buffer restoration */ /***********************************************************/ - /* We could 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. */ - wait_pending_atomic(_this); - - /* 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); + /* Issue sync/blocking atomic commit that points crtc to original buffer. + SDL_video has already called SetDisplayMode() to set the original display mode at this point. + We are not doing pageflips anymore, so we can't rely on the SwapWindow() atomic commit + so we are explicitly calling it here. */ + drm_atomic_request_pageflip(_this, dispdata->crtc->buffer_id); + ret = drm_atomic_commit(_this, SDL_TRUE); /*********************/ /* Atomic block ends */ @@ -1194,6 +1213,7 @@ KMSDRM_SetDisplayMode(_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode) } dispdata->mode = conn->modes[modedata->mode_index]; + drm_atomic_request_modeset(_this); for (int i = 0; i < viddata->num_windows; i++) { SDL_Window *window = viddata->windows[i]; diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.h b/src/video/kmsdrm/SDL_kmsdrmvideo.h index 374d23b1d..4812e029d 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.h +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.h @@ -60,13 +60,24 @@ typedef struct SDL_DisplayData drmModeModeInfo mode; uint32_t plane_id; + uint32_t cusor_plane_id; uint32_t crtc_id; uint32_t connector_id; + uint32_t atomic_flags; + + /* All changes will be requested via this one and only atomic request, + that will be sent to the kernel in the one and only atomic_commit() call + that takes place in SwapWindow(). */ + drmModeAtomicReq *atomic_req; drmModePlane *plane; drmModeObjectProperties *plane_props; drmModePropertyRes **plane_props_info; + drmModePlane *cursor_plane; + drmModeObjectProperties *cursor_plane_props; + drmModePropertyRes **cursor_plane_props_info; + drmModeCrtc *crtc; drmModeObjectProperties *crtc_props; drmModePropertyRes **crtc_props_info; @@ -81,7 +92,6 @@ typedef struct SDL_DisplayData EGLSyncKHR kms_fence; /* Signaled when kms completes changes requested in atomic iotcl (pageflip, etc). */ EGLSyncKHR gpu_fence; /* Signaled when GPU rendering is done. */ - SDL_bool modeset_pending; } SDL_DisplayData; @@ -107,10 +117,12 @@ typedef struct KMSDRM_FBInfo /* Helper functions */ int KMSDRM_CreateSurfaces(_THIS, SDL_Window * window); KMSDRM_FBInfo *KMSDRM_FBFromBO(_THIS, struct gbm_bo *bo); -SDL_bool KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata, int timeout); /* Atomic functions that are used from SDL_kmsdrmopengles.c */ -int drm_atomic_commit(_THIS, uint32_t fb_id, uint32_t flags); +void drm_atomic_request_modeset(_THIS); +void drm_atomic_request_pageflip(_THIS, uint32_t fb_id); +int drm_atomic_commit(_THIS, SDL_bool blocking); +void drm_atomic_wait_pending(_THIS); /****************************************************************************/ /* SDL_VideoDevice functions declaration */