Skip to content

Commit

Permalink
kmsdrm: Add error control to plane prop setting function. Do most pla…
Browse files Browse the repository at this point in the history 
…ne prop setting with a single function.
  • Loading branch information
@vanfanel
vanfanel committed Aug 23, 2020
1 parent e06e9c3 commit 0d593d7
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 136 deletions.
115 changes: 45 additions & 70 deletions src/video/kmsdrm/SDL_kmsdrmmouse.c
View file
Expand Up @@ -43,58 +43,6 @@ static int KMSDRM_WarpMouseGlobal(int x, int y);
/* Atomic helper functions block. */
/**********************************/

int
drm_atomic_setcursor(KMSDRM_CursorData *curdata, int x, int y)
{
KMSDRM_FBInfo *fb;
SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0);

/* 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();

if (curdata)
{
if (!dispdata->cursor_plane) {
setup_plane(curdata->video, &(dispdata->cursor_plane), DRM_PLANE_TYPE_CURSOR);
/* "curdata->video" is the _THIS pointer, which points to an SDL_Display, as passed from kmsdrmmouse.c */
}

fb = KMSDRM_FBFromBO(curdata->video, curdata->bo);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "FB_ID", fb->fb_id);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_ID", dispdata->crtc->crtc->crtc_id);

add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_X", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_Y", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_W", curdata->w << 16);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_H", curdata->h << 16);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_X", x);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_Y", y);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_W", curdata->w);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_H", curdata->h);
}
else if (dispdata->cursor_plane) /* Don't go further if plane has already been freed. */
{
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "FB_ID", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_ID", 0);

add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_X", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_Y", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_W", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "SRC_H", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_W", 0);
add_plane_property(dispdata->atomic_req, dispdata->cursor_plane, "CRTC_H", 0);

free_plane(&dispdata->cursor_plane);
}

/* GPU <-> DISPLAY synchronization is done ON the display_plane,
so no need to set any fence props here, we do that only on the main
display plane. */

return 0;
}

int
drm_atomic_movecursor(KMSDRM_CursorData *curdata, int x, int y)
{
Expand Down Expand Up @@ -140,7 +88,6 @@ void alpha_premultiply_ARGB8888 (uint32_t *pixel) {
(*pixel) = (((uint32_t)A << 24) | ((uint32_t)R << 16) | ((uint32_t)G << 8)) | ((uint32_t)B << 0);
}


static SDL_Cursor *
KMSDRM_CreateDefaultCursor(void)
{
Expand Down Expand Up @@ -169,7 +116,7 @@ KMSDRM_CreateCursor(SDL_Surface * surface, int hot_x, int hot_y)
SDL_assert(surface->pitch == surface->w * 4);

if (!KMSDRM_gbm_device_is_format_supported(viddata->gbm_dev, GBM_FORMAT_ARGB8888, GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE)) {
printf("Unsupported pixel format for cursor\n");
SDL_SetError("Unsupported pixel format for cursor");
return NULL;
}

Expand Down Expand Up @@ -291,6 +238,8 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
KMSDRM_CursorData *curdata;
SDL_VideoDisplay *display = NULL;
SDL_DisplayData *dispdata = NULL;
KMSDRM_FBInfo *fb;
KMSDRM_PlaneInfo info = {0};
int ret;

mouse = SDL_GetMouse();
Expand All @@ -305,18 +254,21 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
}
}

/* Hide cursor */
/**********************************/
/* if cursor == NULL, HIDE cursor */
/**********************************/
if (!cursor) {
/* Hide CURRENT cursor, a cursor that is already on screen
and SDL stores in mouse->cur_cursor. */
if (mouse->cur_cursor && mouse->cur_cursor->driverdata) {
curdata = (KMSDRM_CursorData *) mouse->cur_cursor->driverdata;
if (curdata->video) {

ret = drm_atomic_setcursor(0, 0, 0);
if (dispdata && dispdata->cursor_plane) {
info.plane = dispdata->cursor_plane; /* The rest of the members are zeroed. */
ret = drm_atomic_set_plane_props(&info);
/* Free the plane on which the cursor was being shown. */
free_plane(&dispdata->cursor_plane);

if (ret) {
SDL_SetError("Could not hide current cursor with drm_atomic_setcursor).");
SDL_SetError("Could not hide current cursor.");
return ret;
}

Expand All @@ -328,8 +280,10 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
return SDL_SetError("Couldn't find cursor to hide.");
}


/* If cursor != NULL, show new cursor on display */
/************************************************/
/* If cursor != NULL, DO show cursor on display */
/************************************************/

if (!display) {
return SDL_SetError("Could not get display for mouse.");
}
Expand All @@ -338,15 +292,33 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
}

curdata = (KMSDRM_CursorData *) cursor->driverdata;

if (!curdata || !curdata->bo) {
return SDL_SetError("Cursor not initialized properly.");
}

curdata->crtc_id = dispdata->crtc->crtc->crtc_id;
curdata->video = video_device;
curdata->plane = dispdata->cursor_plane;
curdata->video = video_device;

/* Init cursor plane, if we haven't yet. */
if (!dispdata->cursor_plane) {
setup_plane(curdata->video, &(dispdata->cursor_plane), DRM_PLANE_TYPE_CURSOR);
}

fb = KMSDRM_FBFromBO(curdata->video, curdata->bo);

info.plane = dispdata->cursor_plane;
info.crtc_id = curdata->crtc_id;
info.fb_id = fb->fb_id;
info.src_w = curdata->w;
info.src_h = curdata->h;
info.crtc_x = mouse->x - curdata->hot_x;
info.crtc_y = mouse->y - curdata->hot_y;
info.crtc_w = curdata->w;
info.crtc_h = curdata->h;

/* DO show cursor */
ret = drm_atomic_setcursor(curdata, mouse->x - curdata->hot_x, mouse->y - curdata->hot_y);
ret = drm_atomic_set_plane_props(&info);

if (ret) {
SDL_SetError("KMSDRM_SetCursor failed.");
Expand All @@ -356,12 +328,12 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor)
return 0;
}

/* Unset the cursor from the cusror plane, and ONLY WHEN THAT'S DONE,
/* Unset the cursor from the cursor plane, and ONLY WHEN THAT'S DONE,
DONE FOR REAL, and not only requested, destroy it by destroying the curso BO.
Destroying the cursor BO is an special an delicate situation,
because drm_atomic_setcursor() returns immediately, and we DON'T
because drm_atomic_set_plane_props() returns immediately, and we DON'T
want to get to gbm_bo_destroy() before the prop changes requested
in drm_atomic_setcursor() have effectively been done. So we
in drm_atomic_set_plane_props() have effectively been done. So we
issue a BLOCKING atomic_commit here to avoid that situation.
REMEMBER you yan issue an atomic_commit whenever you want, and
the changes requested until that moment (for any planes, crtcs, etc.)
Expand All @@ -371,12 +343,14 @@ KMSDRM_FreeCursor(SDL_Cursor * cursor)
{
KMSDRM_CursorData *curdata = NULL;
SDL_VideoDevice *video = NULL;
KMSDRM_PlaneInfo info = {0};

if (cursor) {
curdata = (KMSDRM_CursorData *) cursor->driverdata;
video = curdata->video;
if (video && curdata->bo) {
drm_atomic_setcursor(0, 0, 0);
if (video && curdata->bo && curdata->plane) {
info.plane = curdata->plane; /* The other members are zeroed. */
drm_atomic_set_plane_props(&info);
/* Wait until the cursor is unset from the cursor plane before destroying it's BO. */
drm_atomic_commit(video, SDL_TRUE);
KMSDRM_gbm_bo_destroy(curdata->bo);
Expand Down Expand Up @@ -427,6 +401,7 @@ KMSDRM_WarpMouseGlobal(int x, int y)
} else {
return SDL_SetError("No mouse or current cursor.");
}
return 0;
}

void
Expand Down
1 change: 1 addition & 0 deletions src/video/kmsdrm/SDL_kmsdrmmouse.h
View file
Expand Up @@ -33,6 +33,7 @@
typedef struct _KMSDRM_CursorData
{
struct gbm_bo *bo;
struct plane *plane;
uint32_t crtc_id;
int hot_x, hot_y;
int w, h;
Expand Down
43 changes: 34 additions & 9 deletions src/video/kmsdrm/SDL_kmsdrmopengles.c
View file
Expand Up @@ -79,6 +79,7 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window)
SDL_WindowData *windata = ((SDL_WindowData *) window->driverdata);
SDL_DisplayData *dispdata = (SDL_DisplayData *) SDL_GetDisplayForWindow(window)->driverdata;
KMSDRM_FBInfo *fb;
KMSDRM_PlaneInfo info = {0};
int ret;

/*************************************************************************/
Expand Down Expand Up @@ -108,25 +109,37 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window)
/***************/

/* 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. */
be chosen by EGL as back buffer to draw on), and get a handle to it to request the pageflip on it.
REMEMBER that gbm_surface_lock_front_buffer() ALWAYS has to be called after eglSwapBuffers(). */
windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
if (!windata->next_bo) {
return SDL_SetError("Failed to lock frontbuffer on GBM surface destruction");
return SDL_SetError("Failed to lock frontbuffer");
}
fb = KMSDRM_FBFromBO(_this, windata->next_bo);
if (!fb) {
return SDL_SetError("Failed to get a new framebuffer on GBM surface destruction");
return SDL_SetError("Failed to get a new framebuffer from BO");
}

/* Add the pageflip to te request list. */
drm_atomic_setbuffer(_this, dispdata->display_plane, fb->fb_id, dispdata->crtc->crtc->crtc_id);
/* Add the pageflip to the request list. */
info.plane = dispdata->display_plane;
info.crtc_id = dispdata->crtc->crtc->crtc_id;
info.fb_id = fb->fb_id;
info.src_w = dispdata->mode.hdisplay;
info.src_h = dispdata->mode.vdisplay;
info.crtc_w = dispdata->mode.hdisplay;
info.crtc_h = dispdata->mode.vdisplay;

ret = drm_atomic_set_plane_props(&info);
if (ret) {
return SDL_SetError("Failed to request prop changes for setting plane buffer and CRTC");
}

/* 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);
if (ret) {
return SDL_SetError("failed to issue atomic commit on GBM surface destruction");
return SDL_SetError("Failed to issue atomic commit on pageflip");
}

/* Release the last front buffer so EGL can chose it as back buffer and render on it again. */
Expand Down Expand Up @@ -165,6 +178,7 @@ 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;
KMSDRM_PlaneInfo info = {0};
int ret;

/* In double-buffer mode, atomic commit will always be synchronous/blocking (ie: won't return until
Expand All @@ -187,15 +201,26 @@ KMSDRM_GLES_SwapWindowDB(_THIS, SDL_Window * window)
return SDL_SetError("Failed to get a new framebuffer BO");
}

/* Add the pageflip to te request list. */
drm_atomic_setbuffer(_this, dispdata->display_plane, fb->fb_id, dispdata->crtc->crtc->crtc_id);
/* Add the pageflip to the request list. */
info.plane = dispdata->display_plane;
info.crtc_id = dispdata->crtc->crtc->crtc_id;
info.fb_id = fb->fb_id;
info.src_w = dispdata->mode.hdisplay;
info.src_h = dispdata->mode.vdisplay;
info.crtc_w = dispdata->mode.hdisplay;
info.crtc_h = dispdata->mode.vdisplay;

ret = drm_atomic_set_plane_props(&info);
if (ret) {
return SDL_SetError("Failed to request prop changes for setting plane buffer and CRTC");
}

/* 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 issue 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. */
Expand Down

0 comments on commit 0d593d7

Please sign in to comment.