From cc8bc0db20659fde7ce94684bf6abcf172218ab6 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Mon, 19 Jul 2021 15:24:57 +0200 Subject: backend/drm: stop restoring CRTCs on exit This is the cause of the spurious "drmHandleEvent failed" messages at exit. restore_drm_outputs calls handle_drm_event in a loop without checking whether the FD is readable, so drmHandleEvent ends up with a short read (0 bytes) and returns an error. The loop's goal is to wait for all queued page-flip events to complete, to allow drmModeSetCrtc calls to succeed without EBUSY. The drmModeSetCrtc calls are supposed to restore whatever KMS state we were started with. But it's not clear from my PoV that restoring the KMS state on exit is desirable. KMS clients are supposed to save and restore the (full) KMS state on VT switch, but not on exit. Leaving our KMS state on exit avoids unnecessary modesets and allows flicker-free transitions between clients. See [1] for more details, and note that with Pekka we've concluded that a new flag to reset some KMS props to their default value on compositor start-up is the best way forward. As a side note, Weston doesn't restore the CRTC by does disable the cursor plane on exit (see drm_output_deinit_planes, I still think disabling the cursor plane shouldn't be necessary on exit). Additionally, restore_drm_outputs only a subset of the KMS state. Gamma and other atomic properties aren't accounted for. If the previous KMS client had some outputs disabled, restore_drm_outputs would restore a garbage mode. [1]: https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html --- backend/drm/backend.c | 2 -- backend/drm/drm.c | 45 --------------------------------------------- include/backend/drm/drm.h | 3 --- 3 files changed, 50 deletions(-) diff --git a/backend/drm/backend.c b/backend/drm/backend.c index eb22d1a3..39b9d221 100644 --- a/backend/drm/backend.c +++ b/backend/drm/backend.c @@ -33,8 +33,6 @@ static void backend_destroy(struct wlr_backend *backend) { struct wlr_drm_backend *drm = get_drm_backend_from_backend(backend); - restore_drm_outputs(drm); - struct wlr_drm_connector *conn, *next; wl_list_for_each_safe(conn, next, &drm->outputs, link) { destroy_drm_connector(conn); diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 7ea80cc9..be3c3289 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -1306,10 +1306,6 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { "%s-%"PRIu32, conn_get_name(drm_conn->connector_type), drm_conn->connector_type_id); - if (curr_enc) { - wlr_conn->old_crtc = drmModeGetCrtc(drm->fd, curr_enc->crtc_id); - } - wl_list_insert(drm->outputs.prev, &wlr_conn->link); wlr_log(WLR_INFO, "Found connector '%s'", wlr_conn->name); } else { @@ -1556,46 +1552,6 @@ int handle_drm_event(int fd, uint32_t mask, void *data) { return 1; } -void restore_drm_outputs(struct wlr_drm_backend *drm) { - uint64_t to_close = (UINT64_C(1) << wl_list_length(&drm->outputs)) - 1; - - struct wlr_drm_connector *conn; - wl_list_for_each(conn, &drm->outputs, link) { - if (conn->state == WLR_DRM_CONN_CONNECTED) { - conn->state = WLR_DRM_CONN_CLEANUP; - } - } - - time_t timeout = time(NULL) + 5; - - while (to_close && time(NULL) < timeout) { - handle_drm_event(drm->fd, 0, drm); - size_t i = 0; - struct wlr_drm_connector *conn; - wl_list_for_each(conn, &drm->outputs, link) { - if (conn->state != WLR_DRM_CONN_CLEANUP || !conn->pending_page_flip_crtc) { - to_close &= ~(UINT64_C(1) << i); - } - i++; - } - } - - if (to_close) { - wlr_log(WLR_ERROR, "Timed out stopping output renderers"); - } - - wl_list_for_each(conn, &drm->outputs, link) { - drmModeCrtc *crtc = conn->old_crtc; - if (!crtc) { - continue; - } - - drmModeSetCrtc(drm->fd, crtc->crtc_id, crtc->buffer_id, crtc->x, crtc->y, - &conn->id, 1, &crtc->mode); - drmModeSetCursor(drm->fd, crtc->crtc_id, 0, 0, 0); - } -} - static void disconnect_drm_connector(struct wlr_drm_connector *conn) { if (conn->state == WLR_DRM_CONN_DISCONNECTED) { return; @@ -1611,7 +1567,6 @@ static void disconnect_drm_connector(struct wlr_drm_connector *conn) { void destroy_drm_connector(struct wlr_drm_connector *conn) { disconnect_drm_connector(conn); - drmModeFreeCrtc(conn->old_crtc); wl_list_remove(&conn->link); free(conn); } diff --git a/include/backend/drm/drm.h b/include/backend/drm/drm.h index 378e9d20..8c0b0777 100644 --- a/include/backend/drm/drm.h +++ b/include/backend/drm/drm.h @@ -123,8 +123,6 @@ struct wlr_drm_connector { int cursor_width, cursor_height; int cursor_hotspot_x, cursor_hotspot_y; - drmModeCrtc *old_crtc; - struct wl_list link; /* CRTC ID if a page-flip is pending, zero otherwise. @@ -142,7 +140,6 @@ struct wlr_drm_backend *get_drm_backend_from_backend( bool check_drm_features(struct wlr_drm_backend *drm); bool init_drm_resources(struct wlr_drm_backend *drm); void finish_drm_resources(struct wlr_drm_backend *drm); -void restore_drm_outputs(struct wlr_drm_backend *drm); void scan_drm_connectors(struct wlr_drm_backend *state); int handle_drm_event(int fd, uint32_t mask, void *data); void destroy_drm_connector(struct wlr_drm_connector *conn); -- cgit v1.2.3