From eade05ca03a01bb42e897d7c5338b8a1be4bdf31 Mon Sep 17 00:00:00 2001 From: Manuel Alfayate Corchete Date: Mon, 24 Aug 2020 12:51:20 +0200 Subject: [PATCH] kmsdrm: Finetune integer type usage. Add some comments. --- src/video/kmsdrm/SDL_kmsdrmmouse.c | 2 +- src/video/kmsdrm/SDL_kmsdrmmouse.h | 4 +-- src/video/kmsdrm/SDL_kmsdrmvideo.c | 49 +++++++++++++++++------------- src/video/kmsdrm/SDL_kmsdrmvideo.h | 23 +++++++------- 4 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/video/kmsdrm/SDL_kmsdrmmouse.c b/src/video/kmsdrm/SDL_kmsdrmmouse.c index 2d628617b..dc9e37e59 100644 --- a/src/video/kmsdrm/SDL_kmsdrmmouse.c +++ b/src/video/kmsdrm/SDL_kmsdrmmouse.c @@ -44,7 +44,7 @@ static int KMSDRM_WarpMouseGlobal(int x, int y); /**********************************/ int -drm_atomic_movecursor(KMSDRM_CursorData *curdata, int x, int y) +drm_atomic_movecursor(KMSDRM_CursorData *curdata, uint16_t x, uint16_t y) { SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); diff --git a/src/video/kmsdrm/SDL_kmsdrmmouse.h b/src/video/kmsdrm/SDL_kmsdrmmouse.h index 8bb12005b..ebc472880 100644 --- a/src/video/kmsdrm/SDL_kmsdrmmouse.h +++ b/src/video/kmsdrm/SDL_kmsdrmmouse.h @@ -35,8 +35,8 @@ typedef struct _KMSDRM_CursorData struct gbm_bo *bo; struct plane *plane; uint32_t crtc_id; - int hot_x, hot_y; - int w, h; + uint16_t hot_x, hot_y; + uint16_t w, h; /* The video devide implemented on SDL_kmsdrmvideo.c * to be used as _THIS pointer in SDL_kmsdrmvideo.c * functions that need it. */ diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c index aff994e1f..768b9affd 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.c +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c @@ -68,7 +68,7 @@ check_modesetting(int devindex) resources->count_connectors, resources->count_encoders, resources->count_crtcs); if (resources->count_connectors > 0 && resources->count_encoders > 0 && resources->count_crtcs > 0) { - for (int i = 0; i < resources->count_connectors; i++) { + for (unsigned int i = 0; i < resources->count_connectors; i++) { drmModeConnector *conn = KMSDRM_drmModeGetConnector(drm_fd, resources->connectors[i]); if (!conn) { @@ -95,9 +95,9 @@ check_modesetting(int devindex) return available; } -static int get_dricount(void) +static unsigned int get_dricount(void) { - int devcount = 0; + unsigned int devcount = 0; struct dirent *res; struct stat sb; DIR *folder; @@ -117,7 +117,7 @@ static int get_dricount(void) folder = opendir(KMSDRM_DRI_PATH); if (folder) { while ((res = readdir(folder))) { - int len = SDL_strlen(res->d_name); + size_t len = SDL_strlen(res->d_name); if (len > 4 && SDL_strncmp(res->d_name, "card", 4) == 0) { devcount++; } @@ -131,8 +131,8 @@ static int get_dricount(void) static int get_driindex(void) { - const int devcount = get_dricount(); - int i; + const unsigned int devcount = get_dricount(); + unsigned int i; for (i = 0; i < devcount; i++) { if (check_modesetting(i)) { @@ -314,13 +314,13 @@ void get_planes_info(_THIS) /* Get the plane_id of a plane that is of the specified plane type (primary, overlay, cursor...) and can use the CRTC we have chosen previously. */ -static uint32_t get_plane_id(_THIS, uint32_t plane_type) +static int get_plane_id(_THIS, uint32_t plane_type) { drmModeRes *resources = NULL; drmModePlaneResPtr plane_resources = NULL; uint32_t i, j; - uint32_t crtc_index = 0; - uint32_t ret = -EINVAL; + unsigned int crtc_index = 0; + int ret = -EINVAL; int found = 0; SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); @@ -414,7 +414,7 @@ setup_plane(_THIS, struct plane **plane, uint32_t plane_type) (*plane)->props_info = SDL_calloc((*plane)->props->count_props, sizeof((*plane)->props_info)); - for (int i = 0; i < (*plane)->props->count_props; i++) { + for (unsigned int i = 0; i < (*plane)->props->count_props; i++) { (*plane)->props_info[i] = KMSDRM_drmModeGetProperty(viddata->drm_fd, (*plane)->props->props[i]); } @@ -495,10 +495,10 @@ drm_atomic_set_plane_props(struct KMSDRM_PlaneInfo *info) if (add_plane_property(dispdata->atomic_req, info->plane, "CRTC_Y", info->crtc_y) < 0) return SDL_SetError("Failed to set plane CRTC_Y prop"); - /* Only set the IN_FENCE aqnd OUT_FENCE props if we're operationg on the display plane, + /* Set the IN_FENCE and OUT_FENCE props only if we're operating on the display plane, since that's the only plane for which we manage who and when should access the buffers it uses. */ - if ((info->plane == dispdata->display_plane) && (dispdata->kms_in_fence_fd != -1)) + if (info->plane == dispdata->display_plane && dispdata->kms_in_fence_fd != -1) { if (add_crtc_property(dispdata->atomic_req, dispdata->crtc, "OUT_FENCE_PTR", VOID2U64(&dispdata->kms_out_fence_fd)) < 0) @@ -904,7 +904,7 @@ KMSDRM_VideoInit(_THIS) } /* Iterate on the available connectors to find a connected connector. */ - for (int i = 0; i < resources->count_connectors; i++) { + for (unsigned int i = 0; i < resources->count_connectors; i++) { drmModeConnector *conn = KMSDRM_drmModeGetConnector(viddata->drm_fd, resources->connectors[i]); if (!conn) { @@ -927,7 +927,7 @@ KMSDRM_VideoInit(_THIS) } /* Try to find the connector's current encoder */ - for (int i = 0; i < resources->count_encoders; i++) { + for (unsigned int i = 0; i < resources->count_encoders; i++) { encoder = KMSDRM_drmModeGetEncoder(viddata->drm_fd, resources->encoders[i]); if (!encoder) { @@ -945,7 +945,7 @@ KMSDRM_VideoInit(_THIS) if (!encoder) { /* No encoder was connected, find the first supported one */ - for (int i = 0, j; i < resources->count_encoders; i++) { + for (unsigned int i = 0, j; i < resources->count_encoders; i++) { encoder = KMSDRM_drmModeGetEncoder(viddata->drm_fd, resources->encoders[i]); if (!encoder) { @@ -979,7 +979,7 @@ KMSDRM_VideoInit(_THIS) /* If no CRTC was connected to the encoder, find the first CRTC that is supported by the encoder, and use that. */ if (!dispdata->crtc->crtc) { - for (int i = 0; i < resources->count_crtcs; i++) { + for (unsigned int i = 0; i < resources->count_crtcs; i++) { if (encoder->possible_crtcs & (1 << i)) { encoder->crtc_id = resources->crtcs[i]; dispdata->crtc->crtc = KMSDRM_drmModeGetCrtc(viddata->drm_fd, encoder->crtc_id); @@ -1059,7 +1059,7 @@ KMSDRM_VideoInit(_THIS) dispdata->crtc->props_info = SDL_calloc(dispdata->crtc->props->count_props, sizeof(dispdata->crtc->props_info)); - for (int i = 0; i < dispdata->crtc->props->count_props; i++) { + for (unsigned int i = 0; i < dispdata->crtc->props->count_props; i++) { dispdata->crtc->props_info[i] = KMSDRM_drmModeGetProperty(viddata->drm_fd, dispdata->crtc->props->props[i]); } @@ -1071,7 +1071,7 @@ KMSDRM_VideoInit(_THIS) dispdata->connector->props_info = SDL_calloc(dispdata->connector->props->count_props, sizeof(dispdata->connector->props_info)); - for (int i = 0; i < dispdata->connector->props->count_props; i++) { + for (unsigned int i = 0; i < dispdata->connector->props->count_props; i++) { dispdata->connector->props_info[i] = KMSDRM_drmModeGetProperty(viddata->drm_fd, dispdata->connector->props->props[i]); } @@ -1194,6 +1194,13 @@ KMSDRM_GetDisplayModes(_THIS, SDL_VideoDisplay * display) int KMSDRM_SetDisplayMode(_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode) { + /************************************************************************/ + /* DO NOT add dynamic videomode changes unless you can REALLY test */ + /* on all available KMS drivers and fix them in-kernel, and also test */ + /* all SDL2 software: things will fail one way or another, and it */ + /* greatly increases backend complexiity thus compromising it's */ + /* maintenance. It's NOT as easy as reconstructing GBM and EGL surfaces.*/ + /************************************************************************/ return 0; } @@ -1237,7 +1244,7 @@ KMSDRM_CreateWindow(_THIS, SDL_Window * window) seem odd to support multiple fullscreen windows, some apps create an extra window as a dummy surface when working with multiple contexts */ if (viddata->num_windows >= viddata->max_windows) { - int new_max_windows = viddata->max_windows + 1; + unsigned int new_max_windows = viddata->max_windows + 1; viddata->windows = (SDL_Window **)SDL_realloc(viddata->windows, new_max_windows * sizeof(SDL_Window *)); viddata->max_windows = new_max_windows; @@ -1274,11 +1281,11 @@ KMSDRM_DestroyWindow(_THIS, SDL_Window * window) /* Remove from the internal window list */ viddata = windata->viddata; - for (int i = 0; i < viddata->num_windows; i++) { + for (unsigned int i = 0; i < viddata->num_windows; i++) { if (viddata->windows[i] == window) { viddata->num_windows--; - for (int j = i; j < viddata->num_windows; j++) { + for (unsigned int j = i; j < viddata->num_windows; j++) { viddata->windows[j] = viddata->windows[j + 1]; } diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.h b/src/video/kmsdrm/SDL_kmsdrmvideo.h index 10d70062c..c4c3a4006 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.h +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.h @@ -49,8 +49,8 @@ typedef struct SDL_VideoData struct gbm_device *gbm_dev; SDL_Window **windows; - int max_windows; - int num_windows; + unsigned int max_windows; + unsigned int num_windows; } SDL_VideoData; struct plane { @@ -119,20 +119,21 @@ typedef struct KMSDRM_FBInfo uint32_t fb_id; /* DRM framebuffer ID */ } KMSDRM_FBInfo; -/* Info passed to set_plane_props calls. */ +/* Info passed to set_plane_props calls. hdisplay and vdisplay in a drm mode are uint16_t, + so that's what we use for sizes and positions here. IDs are uint32_t as always. */ typedef struct KMSDRM_PlaneInfo { struct plane *plane; uint32_t fb_id; uint32_t crtc_id; - int src_x; - int src_y; - int src_w; - int src_h; - int crtc_x; - int crtc_y; - int crtc_w; - int crtc_h; + uint16_t src_x; + uint16_t src_y; + uint16_t src_w; + uint16_t src_h; + uint16_t crtc_x; + uint16_t crtc_y; + uint16_t crtc_w; + uint16_t crtc_h; } KMSDRM_PlaneInfo; /* Helper functions */