Skip to content

Commit

Permalink
kmsdrm: Buffer management refactoring. Fixes for compatibility with m…
Browse files Browse the repository at this point in the history 
…ore video drivers.
  • Loading branch information
@vanfanel
vanfanel committed Aug 25, 2020
1 parent eade05c commit d43e666
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 51 deletions.
5 changes: 2 additions & 3 deletions src/video/kmsdrm/SDL_kmsdrmmouse.c
View file
Expand Up @@ -263,6 +263,7 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
if (dispdata && dispdata->cursor_plane) {
info.plane = dispdata->cursor_plane; /* The rest of the members are zeroed. */
ret = drm_atomic_set_plane_props(&info);
//drm_atomic_commit(display->device, SDL_TRUE);
if (ret) {
SDL_SetError("Could not hide current cursor.");
return ret;
Expand Down Expand Up @@ -313,6 +314,7 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
info.crtc_h = curdata->h;

ret = drm_atomic_set_plane_props(&info);
//drm_atomic_commit(display->device, SDL_TRUE);

if (ret) {
SDL_SetError("KMSDRM_ShowCursor failed.");
Expand Down Expand Up @@ -341,7 +343,6 @@ KMSDRM_FreeCursor(SDL_Cursor * cursor)

if (cursor) {
curdata = (KMSDRM_CursorData *) cursor->driverdata;
video = curdata->video;
if (video && curdata->bo && curdata->plane) {
info.plane = curdata->plane; /* The other members are zeroed. */
drm_atomic_set_plane_props(&info);
Expand Down Expand Up @@ -381,7 +382,6 @@ KMSDRM_WarpMouseGlobal(int x, int y)
int ret;

ret = drm_atomic_movecursor(curdata, x, y);
//ret = drm_atomic_commit(curdata->video, SDL_TRUE);

if (ret) {
SDL_SetError("drm_atomic_movecursor() failed.");
Expand Down Expand Up @@ -449,7 +449,6 @@ KMSDRM_MoveCursor(SDL_Cursor * cursor)
cursor movement request, but it cripples the movement to 30FPS, so a future solution
is needed. SDLPoP "QUIT?" menu is an example of this situation. */
ret = drm_atomic_movecursor(curdata, mouse->x, mouse->y);
//ret = drm_atomic_commit(curdata->video, SDL_TRUE);

if (ret) {
SDL_SetError("drm_atomic_movecursor() failed.");
Expand Down
35 changes: 31 additions & 4 deletions src/video/kmsdrm/SDL_kmsdrmopengles.c
View file
Expand Up @@ -82,6 +82,23 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window)
KMSDRM_PlaneInfo info = {0};
int ret;

/* Do we have a pending old surfaces destruction? */
if (dispdata->destroy_surfaces_pending == SDL_TRUE) {

/* Take note that we have not pending old surfaces destruction.
Do it ASAP, DON'T GO into SwapWindowDB() with it enabled or
we will enter recursivity! */
dispdata->destroy_surfaces_pending = SDL_FALSE;

/* Do blocking pageflip, to be sure it's atomic commit is completed
before destroying the old surfaces and buffers. */
KMSDRM_GLES_SwapWindowDB(_this, window);

KMSDRM_DestroyOldSurfaces(_this);

return 0;
}

/*************************************************************************/
/* Block for telling KMS to wait for GPU rendering of the current frame */
/* before applying the KMS changes requested in the atomic ioctl. */
Expand Down Expand Up @@ -181,10 +198,12 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window)
KMSDRM_PlaneInfo info = {0};
int ret;

/* 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. */
/****************************************************************************************************/
/* 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. */
/****************************************************************************************************/

/* 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. */
Expand Down Expand Up @@ -231,6 +250,14 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window)
/* Take note of current front buffer, so we can free it next time we come here. */
windata->bo = windata->next_bo;

/* Do we have a pending old surfaces destruction? */
if (dispdata->destroy_surfaces_pending == SDL_TRUE) {
/* We have just done a blocking pageflip to the new buffers already,
so just do what you are here for... */
KMSDRM_DestroyOldSurfaces(_this);
dispdata->destroy_surfaces_pending = SDL_FALSE;
}

return ret;
}

Expand Down
156 changes: 116 additions & 40 deletions src/video/kmsdrm/SDL_kmsdrmvideo.c
View file
Expand Up @@ -149,7 +149,6 @@ get_driindex(void)

#define VOID2U64(x) ((uint64_t)(unsigned long)(x))

#if 0
static int add_connector_property(drmModeAtomicReq *req, struct connector *connector,
const char *name, uint64_t value)
{
Expand All @@ -170,7 +169,6 @@ static int add_connector_property(drmModeAtomicReq *req, struct connector *conne

return KMSDRM_drmModeAtomicAddProperty(req, connector->connector->connector_id, prop_id, value);
}
#endif

static int add_crtc_property(drmModeAtomicReq *req, struct crtc *crtc,
const char *name, uint64_t value)
Expand Down Expand Up @@ -526,7 +524,7 @@ int drm_atomic_commit(_THIS, SDL_bool blocking)
if (ret) {
SDL_SetError("Atomic commit failed, returned %d.", ret);
/* Uncomment this for fast-debugging */
//printf("ATOMIC COMMIT FAILED: %d.\n", ret);
printf("ATOMIC COMMIT FAILED: %d.\n", ret);
goto out;
}

Expand Down Expand Up @@ -761,44 +759,64 @@ KMSDRM_FBFromBO(_THIS, struct gbm_bo *bo)
/* _this is a SDL_VideoDevice * */
/*****************************************************************************/

/* Just backup the GBM/EGL surfaces pinters, and buffers pointers,
because we are not destroying them until we get a buffer of the new GBM
surface so we can set the FB_ID of the PRIMARY plane to it.
That will happen in SwapWindow(), when we call lock_front_buffer()
on the new GBM surface, so that's where we can destroy the old buffers.
THE IDEA IS THAT THE PRIMARY PLANE IS NEVER LEFT WITHOUT A VALID BUFFER
TO READ, AND ONLY SET IT'S FB_ID/CRTC_ID PROPS TO 0 WHEN PROGRAM IS QUITTING.*/
static void
KMSDRM_DestroySurfaces(_THIS, SDL_Window * window)
KMSDRM_SetPendingSurfacesDestruction(_THIS, SDL_Window * window)
{
SDL_WindowData *windata = (SDL_WindowData *)window->driverdata;
SDL_DisplayData *dispdata = (SDL_DisplayData *) SDL_GetDisplayForWindow(window)->driverdata;

/* CAUTION: Before destroying the GBM ane EGL surfaces, we must disconnect
the display plane from the GBM surface buffer it's reading by setting
it's CRTC_ID and FB_ID props to 0.
*/
KMSDRM_PlaneInfo info = {0};
info.plane = dispdata->display_plane;
/*******************************************************************************/
/* Backup the pointers to the GBM/EGL surfaces and buffers we want to destroy, */
/* so we can destroy them later, even after destroying the SDL_Window. */
/* DO NOT store the old ponters in windata, because it's freed when the window */
/* is destroyed. */
/*******************************************************************************/

dispdata->old_gs = windata->gs;
dispdata->old_bo = windata->bo;
dispdata->old_next_bo = windata->next_bo;
dispdata->old_egl_surface = windata->egl_surface;

/* Take note that we have yet to destroy the old surfaces.
We will do so in SwapWindow(), once we have the new
surfaces and buffers available for the PRIMARY plane. */
dispdata->destroy_surfaces_pending = SDL_TRUE;
}

drm_atomic_set_plane_props(&info);
drm_atomic_commit(_this, SDL_TRUE);
void
KMSDRM_DestroyOldSurfaces(_THIS)
{
SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);

if (windata->bo) {
KMSDRM_gbm_surface_release_buffer(windata->gs, windata->bo);
windata->bo = NULL;
#if SDL_VIDEO_OPENGL_EGL
/* Destroy the old EGL surface. */
if (dispdata->old_egl_surface != EGL_NO_SURFACE) {
SDL_EGL_DestroySurface(_this, dispdata->old_egl_surface);
dispdata->old_egl_surface = EGL_NO_SURFACE;
}
#endif

if (windata->next_bo) {
KMSDRM_gbm_surface_release_buffer(windata->gs, windata->next_bo);
windata->next_bo = NULL;
/* Destroy the old GBM surface and buffers. */
if (dispdata->old_bo) {
KMSDRM_gbm_surface_release_buffer(dispdata->old_gs, dispdata->old_bo);
dispdata->old_bo = NULL;
}

#if SDL_VIDEO_OPENGL_EGL
SDL_EGL_MakeCurrent(_this, EGL_NO_SURFACE, EGL_NO_CONTEXT);

if (windata->egl_surface != EGL_NO_SURFACE) {
SDL_EGL_DestroySurface(_this, windata->egl_surface);
windata->egl_surface = EGL_NO_SURFACE;
if (dispdata->old_next_bo) {
KMSDRM_gbm_surface_release_buffer(dispdata->old_gs, dispdata->old_next_bo);
dispdata->old_next_bo = NULL;
}
#endif

if (windata->gs) {
KMSDRM_gbm_surface_destroy(windata->gs);
windata->gs = NULL;
if (dispdata->old_gs) {
KMSDRM_gbm_surface_destroy(dispdata->old_gs);
dispdata->old_gs = NULL;
}
}

Expand All @@ -814,20 +832,13 @@ KMSDRM_CreateSurfaces(_THIS, SDL_Window * window)
Uint32 surface_flags = GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING;
#if SDL_VIDEO_OPENGL_EGL
EGLContext egl_context;
SDL_EGL_SetRequiredVisualId(_this, surface_fmt);
egl_context = (EGLContext)SDL_GL_GetCurrentContext();
#endif

/* Always try to destroy previous surfaces before creating new ones. */
KMSDRM_DestroySurfaces(_this, window);

if (!KMSDRM_gbm_device_is_format_supported(viddata->gbm_dev, surface_fmt, surface_flags)) {
SDL_LogWarn(SDL_LOG_CATEGORY_VIDEO, "GBM surface format not supported. Trying anyway.");
}

#if SDL_VIDEO_OPENGL_EGL
SDL_EGL_SetRequiredVisualId(_this, surface_fmt);
egl_context = (EGLContext)SDL_GL_GetCurrentContext();
#endif

windata->gs = KMSDRM_gbm_surface_create(viddata->gbm_dev, width, height, surface_fmt, surface_flags);

if (!windata->gs) {
Expand Down Expand Up @@ -1123,7 +1134,72 @@ KMSDRM_VideoQuit(_THIS)
{
SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);
SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "KMSDRM_VideoQuit()");
KMSDRM_PlaneInfo plane_info = {0};

/*******************************************************************************/
/* BLOCK 1 */
/* Clear Old GBM surface and KMS buffers. */
/* We have to set the FB_ID and CRTC_ID props of the PRIMARY PLANE to ZERO */
/* before destroying the GBM buffers it's reading (SDL internals have already */
/* run the DestroyWindow()->SetPendingSurfacesDestruction()() sequence, */
/* so we have pending the destruction of the GBM/EGL surfaces and GBM buffers).*/
/* But there can not be an ACTIVE CRTC without a PLANE, so we also have to */
/* DEACTIVATE the CRTC, and dettach the CONNECTORs from the CRTC, all in the */
/* same atomic commit in which we zero the primary plane FB_ID and CRTC_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();
#define AMDGPU /* Use this for now, for greater compatibility! */
#ifdef AMDGPU

/* AMDGPU won't let FBCON takeover without this. The problem 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;

#else /* This is the correct way, but wait until more chips support FB_ID/CRTC_ID to 0. */

/* This is the *right* thing to do: dettach the CONNECTOR from the CRTC,
deactivate the CRTC, and set the FB_ID and CRTC_ID props of the PRIMARY PLANE
to 0. All this is needed because there can be no active CRTC with no plane.
All this is done in a single blocking atomic commit before destroying the buffers
that the PRIMARY PLANE is reading. */
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, but they *should*. */
plane_info.plane = dispdata->display_plane;

#endif

drm_atomic_set_plane_props(&plane_info);

/* ... And finally issue blocking ATOMIC commit before destroying the old buffers.
We get here with this destruction still pending if the program calls SDL_DestroyWindow(),
which in turn calls KMSDRM_SetPendingSurfacesDestruction(), and then quits instead of
creating a new SDL_Window, thus ending here. */

drm_atomic_commit(_this, SDL_TRUE);

if (dispdata->destroy_surfaces_pending) {
KMSDRM_DestroyOldSurfaces(_this);
dispdata->destroy_surfaces_pending = SDL_FALSE;
}

/************************/
/* BLOCK 1 ENDS. */
/************************/

/* Clear out the window list */
SDL_free(viddata->windows);
Expand Down Expand Up @@ -1169,7 +1245,7 @@ KMSDRM_VideoQuit(_THIS)
/* Free cursor plane (if still not freed) */
free_plane(&dispdata->cursor_plane);

/* Destroy GBM device. GBM surface is destroyed by DestroySurfaces(). */
/* Destroy GBM device. GBM surface is destroyed by DestroyOldSurfaces(). */
if (viddata->gbm_dev) {
KMSDRM_gbm_device_destroy(viddata->gbm_dev);
viddata->gbm_dev = NULL;
Expand Down Expand Up @@ -1293,7 +1369,7 @@ KMSDRM_DestroyWindow(_THIS, SDL_Window * window)
}
}

KMSDRM_DestroySurfaces(_this, window);
KMSDRM_SetPendingSurfacesDestruction(_this, window);

window->driverdata = NULL;

Expand Down
22 changes: 18 additions & 4 deletions src/video/kmsdrm/SDL_kmsdrmvideo.h
View file
Expand Up @@ -78,8 +78,8 @@ typedef struct SDL_DisplayData
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(). */
that will be sent to the kernel in the one and only atomic_commit()
call that takes place in SwapWindow(). */
drmModeAtomicReq *atomic_req;
struct plane *display_plane;
struct plane *cursor_plane;
Expand All @@ -94,6 +94,19 @@ typedef struct SDL_DisplayData

EGLSyncKHR gpu_fence; /* Signaled when GPU rendering is done. */

/* Backup pointers of the GBM surfaces and buffers to be deleted after
SDL_Window destruction, since SDL_Window destruction causes it's
driverdata pointer (windata) to be destroyed, so we have to
keep them here instead. */
struct gbm_surface *old_gs;
struct gbm_bo *old_bo;
struct gbm_bo *old_next_bo;
#if SDL_VIDEO_OPENGL_EGL
EGLSurface old_egl_surface;
#endif

SDL_bool destroy_surfaces_pending;

} SDL_DisplayData;

/* Driverdata info that gives KMSDRM-side support and substance to the SDL_Window. */
Expand All @@ -102,8 +115,8 @@ typedef struct SDL_WindowData
SDL_VideoData *viddata;
/* SDL internals expect EGL surface to be here, and in KMSDRM the GBM surface is
what supports the EGL surface on the driver side, so all these surfaces and buffers
are expected to be here, in the struct pointed by SDL_Window driverdata pointer: this one.
So don't try to move these to dispdata! */
are expected to be here, in the struct pointed by SDL_Window driverdata pointer:
this one. So don't try to move these to dispdata! */
struct gbm_surface *gs;
struct gbm_bo *bo;
struct gbm_bo *next_bo;
Expand Down Expand Up @@ -138,6 +151,7 @@ typedef struct KMSDRM_PlaneInfo

/* Helper functions */
int KMSDRM_CreateSurfaces(_THIS, SDL_Window * window);
void KMSDRM_DestroyOldSurfaces(_THIS);
KMSDRM_FBInfo *KMSDRM_FBFromBO(_THIS, struct gbm_bo *bo);

/* Atomic functions that are used from SDL_kmsdrmopengles.c and SDL_kmsdrmmouse.c */
Expand Down

0 comments on commit d43e666

Please sign in to comment.