From 60137c83bd3c2aa2a30f36b0da5e489b689f82cb Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Sun, 11 Oct 2020 22:31:08 +0200 Subject: [PATCH 1/5] reimplement legacy modesetting --- include/compositor.h | 17 ++ include/modesetting.h | 60 ++++++ src/compositor.c | 442 ++++++++++++++++++++++++++++++++++-------- src/flutter-pi.c | 1 + src/modesetting.c | 230 ++++++++++++++++++++++ 5 files changed, 674 insertions(+), 76 deletions(-) diff --git a/include/compositor.h b/include/compositor.h index 8555e053..84aa40c8 100644 --- a/include/compositor.h +++ b/include/compositor.h @@ -126,6 +126,12 @@ struct compositor { * like usual. */ bool do_blocking_atomic_commits; + + /** + * @brief Whether atomic modesetting should be used. + * If `false`, legacy modesetting is used. + */ + bool use_atomic_modesetting; }; /* @@ -221,6 +227,17 @@ struct rendertarget { int height, int zpos ); + int (*present_legacy)( + struct rendertarget *target, + struct drmdev *drmdev, + uint32_t drm_plane_id, + int offset_x, + int offset_y, + int width, + int height, + int zpos, + bool set_mode + ); }; struct flutterpi_backing_store { diff --git a/include/modesetting.h b/include/modesetting.h index bffbd3ab..272e0e9a 100644 --- a/include/modesetting.h +++ b/include/modesetting.h @@ -88,6 +88,11 @@ int drmdev_configure( const drmModeModeInfo *mode ); +int drmdev_plane_get_type( + struct drmdev *drmdev, + uint32_t plane_id +); + int drmdev_plane_supports_setting_rotation_value( struct drmdev *drmdev, uint32_t plane_id, @@ -166,6 +171,61 @@ int drmdev_atomic_req_commit( void *userdata ); +int drmdev_legacy_set_mode_and_fb( + struct drmdev *drmdev, + uint32_t fb_id +); + +/** + * @brief Do a nonblocking, vblank-synced framebuffer swap. + */ +int drmdev_legacy_primary_plane_pageflip( + struct drmdev *drmdev, + uint32_t fb_id, + void *userdata +); + +/** + * @brief Do a blocking, vblank-synced framebuffer swap. + * Using this in combination with @ref drmdev_legacy_primary_plane_pageflip + * is not a good idea, since it will block until the primary plane pageflip is complete, + * and then block even longer till the overlay plane pageflip completes the vblank after. + */ +int drmdev_legacy_overlay_plane_pageflip( + struct drmdev *drmdev, + uint32_t plane_id, + uint32_t fb_id, + int32_t crtc_x, + int32_t crtc_y, + int32_t crtc_w, + int32_t crtc_h, + uint32_t src_x, + uint32_t src_y, + uint32_t src_w, + uint32_t src_h +); + +int drmdev_legacy_set_connector_property( + struct drmdev *drmdev, + const char *name, + uint64_t value +); + +int drmdev_legacy_set_crtc_property( + struct drmdev *drmdev, + const char *name, + uint64_t value +); + +int drmdev_legacy_set_plane_property( + struct drmdev *drmdev, + uint32_t plane_id, + const char *name, + uint64_t value +); + + + inline static struct drm_connector *__next_connector(const struct drmdev *drmdev, const struct drm_connector *connector) { bool found = connector == NULL; for (int i = 0; i < drmdev->n_connectors; i++) { diff --git a/src/compositor.c b/src/compositor.c index d8c17517..69228f5a 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -50,7 +50,8 @@ struct compositor compositor = { .has_applied_modeset = false, .should_create_window_surface_backing_store = true, .stale_rendertargets = CPSET_INITIALIZER(CPSET_DEFAULT_MAX_SIZE), - .do_blocking_atomic_commits = false + .do_blocking_atomic_commits = false, + .use_atomic_modesetting = false }; static struct view_cb_data *get_cbs_for_view_id_locked(int64_t view_id) { @@ -417,6 +418,105 @@ static int rendertarget_gbm_present( return 0; } +static int rendertarget_gbm_present_legacy( + struct rendertarget *target, + struct drmdev *drmdev, + uint32_t drm_plane_id, + int offset_x, + int offset_y, + int width, + int height, + int zpos, + bool set_mode +) { + struct rendertarget_gbm *gbm_target; + struct gbm_bo *next_front_bo; + uint32_t next_front_fb_id; + bool supported, is_primary; + int ok; + + gbm_target = &target->gbm; + + is_primary = drmdev_plane_get_type(drmdev, drm_plane_id) == DRM_PLANE_TYPE_PRIMARY; + + next_front_bo = gbm_surface_lock_front_buffer(gbm_target->gbm_surface); + next_front_fb_id = gbm_bo_get_drm_fb_id(next_front_bo); + + if (is_primary) { + if (set_mode) { + drmdev_legacy_set_mode_and_fb( + drmdev, + next_front_fb_id + ); + } else { + drmdev_legacy_primary_plane_pageflip( + drmdev, + next_front_fb_id, + NULL + ); + } + } else { + drmdev_legacy_overlay_plane_pageflip( + drmdev, + drm_plane_id, + next_front_fb_id, + 0, + 0, + flutterpi.display.width, + flutterpi.display.height, + 0, + 0, + ((uint16_t) flutterpi.display.width) << 16, + ((uint16_t) flutterpi.display.height) << 16 + ); + } + + ok = drmdev_plane_supports_setting_rotation_value(drmdev, drm_plane_id, DRM_MODE_ROTATE_0, &supported); + if (ok != 0) return ok; + + if (supported) { + drmdev_legacy_set_plane_property(drmdev, drm_plane_id, "rotation", DRM_MODE_ROTATE_0); + } else { + static bool printed = false; + + if (!printed) { + fprintf(stderr, + "[compositor] GPU does not support reflecting the screen in Y-direction.\n" + " This is required for rendering into hardware overlay planes though.\n" + " Any UI that is drawn in overlay planes will look upside down.\n" + ); + printed = true; + } + } + + ok = drmdev_plane_supports_setting_zpos_value(drmdev, drm_plane_id, zpos, &supported); + if (ok != 0) return ok; + + if (supported) { + drmdev_legacy_set_plane_property(drmdev, drm_plane_id, "zpos", zpos); + } else { + static bool printed = false; + + if (!printed) { + fprintf(stderr, + "[compositor] GPU does not supported the desired HW plane order.\n" + " Some UI layers may be invisible.\n" + ); + printed = true; + } + } + + // TODO: move this to the page flip handler. + // We can only be sure the buffer can be released when the buffer swap + // ocurred. + if (gbm_target->current_front_bo != NULL) { + gbm_surface_release_buffer(gbm_target->gbm_surface, gbm_target->current_front_bo); + } + gbm_target->current_front_bo = (struct gbm_bo *) next_front_bo; + + return 0; +} + /** * @brief Create a type of rendertarget that is backed by a GBM Surface, used for rendering into the DRM primary plane. * @@ -447,7 +547,8 @@ static int rendertarget_gbm_new( }, .gl_fbo_id = 0, .destroy = rendertarget_gbm_destroy, - .present = rendertarget_gbm_present + .present = rendertarget_gbm_present, + .present_legacy = rendertarget_gbm_present_legacy }; *out = target; @@ -531,6 +632,99 @@ static int rendertarget_nogbm_present( return 0; } +static int rendertarget_nogbm_present_legacy( + struct rendertarget *target, + struct drmdev *drmdev, + uint32_t drm_plane_id, + int offset_x, + int offset_y, + int width, + int height, + int zpos, + bool set_mode +) { + struct rendertarget_nogbm *nogbm_target; + uint32_t fb_id; + bool supported, is_primary; + int ok; + + nogbm_target = &target->nogbm; + + is_primary = drmdev_plane_get_type(drmdev, drm_plane_id) == DRM_PLANE_TYPE_PRIMARY; + + nogbm_target->current_front_rbo ^= 1; + ok = attach_drm_rbo_to_fbo(nogbm_target->gl_fbo_id, nogbm_target->rbos + nogbm_target->current_front_rbo); + if (ok != 0) return ok; + + fb_id = nogbm_target->rbos[nogbm_target->current_front_rbo ^ 1].drm_fb_id; + + if (is_primary) { + if (set_mode) { + drmdev_legacy_set_mode_and_fb( + drmdev, + fb_id + ); + } else { + drmdev_legacy_primary_plane_pageflip( + drmdev, + fb_id, + NULL + ); + } + } else { + drmdev_legacy_overlay_plane_pageflip( + drmdev, + drm_plane_id, + fb_id, + 0, + 0, + flutterpi.display.width, + flutterpi.display.height, + 0, + 0, + ((uint16_t) flutterpi.display.width) << 16, + ((uint16_t) flutterpi.display.height) << 16 + ); + } + + ok = drmdev_plane_supports_setting_rotation_value(drmdev, drm_plane_id, DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y, &supported); + if (ok != 0) return ok; + + if (supported) { + drmdev_legacy_set_plane_property(drmdev, drm_plane_id, "rotation", DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y); + } else { + static bool printed = false; + + if (!printed) { + fprintf(stderr, + "[compositor] GPU does not support reflecting the screen in Y-direction.\n" + " This is required for rendering into hardware overlay planes though.\n" + " Any UI that is drawn in overlay planes will look upside down.\n" + ); + printed = true; + } + } + + ok = drmdev_plane_supports_setting_zpos_value(drmdev, drm_plane_id, zpos, &supported); + if (ok != 0) return ok; + + if (supported) { + drmdev_legacy_set_plane_property(drmdev, drm_plane_id, "zpos", zpos); + } else { + static bool printed = false; + + if (!printed) { + fprintf(stderr, + "[compositor] GPU does not supported the desired HW plane order.\n" + " Some UI layers may be invisible.\n" + ); + printed = true; + } + } + + return 0; +} + /** * @brief Create a type of rendertarget that is not backed by a GBM-Surface, used for rendering into DRM overlay planes. * @@ -557,6 +751,7 @@ static int rendertarget_nogbm_new( target->compositor = compositor; target->destroy = rendertarget_nogbm_destroy; target->present = rendertarget_nogbm_present; + target->present_legacy = rendertarget_nogbm_present_legacy; eglGetError(); glGetError(); @@ -784,14 +979,30 @@ static bool on_present_layers( ) { struct drmdev_atomic_req *req; struct view_cb_data *cb_data; + struct pointer_set planes; struct compositor *compositor; struct drm_plane *plane; + struct drmdev *drmdev; uint32_t req_flags; + void *planes_storage[32] = {0}; + bool legacy_rendertarget_set_mode = false; + bool schedule_fake_page_flip_event; int ok; compositor = userdata; + drmdev = compositor->drmdev; + schedule_fake_page_flip_event = compositor->do_blocking_atomic_commits; - drmdev_new_atomic_req(compositor->drmdev, &req); + if (compositor->use_atomic_modesetting) { + drmdev_new_atomic_req(compositor->drmdev, &req); + } else { + planes = PSET_INITIALIZER_STATIC(planes_storage, 32); + for_each_plane_in_drmdev(drmdev, plane) { + if (plane->plane->possible_crtcs & drmdev->selected_crtc->bitmask) { + pset_put(&planes, plane); + } + } + } cpset_lock(&compositor->cbs); @@ -800,34 +1011,68 @@ static bool on_present_layers( req_flags = 0 /* DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK*/; if (compositor->has_applied_modeset == false) { - ok = drmdev_atomic_req_put_modeset_props(req, &req_flags); - if (ok != 0) return false; + if (compositor->use_atomic_modesetting) { + ok = drmdev_atomic_req_put_modeset_props(req, &req_flags); + if (ok != 0) return false; + } else { + legacy_rendertarget_set_mode = true; + schedule_fake_page_flip_event = true; + } int64_t max_zpos = 0; - for_each_unreserved_plane_in_atomic_req(req, plane) { - if (plane->type == DRM_PLANE_TYPE_CURSOR) { - // make sure the cursor is in front of everything - int64_t max_zpos; - bool supported; + if (compositor->use_atomic_modesetting) { + for_each_unreserved_plane_in_atomic_req(req, plane) { + if (plane->type == DRM_PLANE_TYPE_CURSOR) { + // make sure the cursor is in front of everything + int64_t max_zpos; + bool supported; + + ok = drmdev_plane_get_max_zpos_value(req->drmdev, plane->plane->plane_id, &max_zpos); + if (ok != 0) { + printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_get_max_zpos_value: %s\n", strerror(ok)); + continue; + } + + ok = drmdev_plane_supports_setting_zpos_value(req->drmdev, plane->plane->plane_id, max_zpos, &supported); + if (ok != 0) { + printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_supports_setting_zpos_value: %s\n", strerror(ok)); + continue; + } - ok = drmdev_plane_get_max_zpos_value(req->drmdev, plane->plane->plane_id, &max_zpos); - if (ok != 0) { - printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_get_max_zpos_value: %s\n", strerror(ok)); - continue; - } - - ok = drmdev_plane_supports_setting_zpos_value(req->drmdev, plane->plane->plane_id, max_zpos, &supported); - if (ok != 0) { - printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_supports_setting_zpos_value: %s\n", strerror(ok)); - continue; + if (supported) { + drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "zpos", max_zpos); + } else { + printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_supports_setting_zpos_value: %s\n", strerror(ok)); + continue; + } } + } + } else { + for_each_pointer_in_pset(&planes, plane) { + if (plane->type == DRM_PLANE_TYPE_CURSOR) { + // make sure the cursor is in front of everything + int64_t max_zpos; + bool supported; + + ok = drmdev_plane_get_max_zpos_value(drmdev, plane->plane->plane_id, &max_zpos); + if (ok != 0) { + printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_get_max_zpos_value: %s\n", strerror(ok)); + continue; + } + + ok = drmdev_plane_supports_setting_zpos_value(drmdev, plane->plane->plane_id, max_zpos, &supported); + if (ok != 0) { + printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_supports_setting_zpos_value: %s\n", strerror(ok)); + continue; + } - if (supported) { - drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "zpos", max_zpos); - } else { - printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_supports_setting_zpos_value: %s\n", strerror(ok)); - continue; + if (supported) { + drmdev_legacy_set_plane_property(drmdev, plane->plane->plane_id, "zpos", max_zpos); + } else { + printf("[compositor] Could not move cursor to front. Mouse cursor may be invisible. drmdev_plane_supports_setting_zpos_value: %s\n", strerror(ok)); + continue; + } } } } @@ -983,31 +1228,56 @@ static bool on_present_layers( } int64_t min_zpos; - for_each_unreserved_plane_in_atomic_req(req, plane) { - if (plane->type == DRM_PLANE_TYPE_PRIMARY) { - ok = drmdev_plane_get_min_zpos_value(req->drmdev, plane->plane->plane_id, &min_zpos); - if (ok != 0) { - min_zpos = 0; + if (compositor->use_atomic_modesetting) { + for_each_unreserved_plane_in_atomic_req(req, plane) { + if (plane->type == DRM_PLANE_TYPE_PRIMARY) { + ok = drmdev_plane_get_min_zpos_value(req->drmdev, plane->plane->plane_id, &min_zpos); + if (ok != 0) { + min_zpos = 0; + } + break; + } + } + } else { + for_each_pointer_in_pset(&planes, plane) { + if (plane->type == DRM_PLANE_TYPE_PRIMARY) { + ok = drmdev_plane_get_min_zpos_value(drmdev, plane->plane->plane_id, &min_zpos); + if (ok != 0) { + min_zpos = 0; + } + break; } - break; } } for (int i = 0; i < layers_count; i++) { if (layers[i]->type == kFlutterLayerContentTypeBackingStore) { - for_each_unreserved_plane_in_atomic_req(req, plane) { - // choose a plane which has an "intrinsic" zpos that matches - // the zpos we want the plane to have. - // (Since planes are buggy and we can't rely on the zpos we explicitly - // configure the plane to have to be actually applied to the hardware. - // In short, assigning a different value to the zpos property won't always - // take effect.) - if ((i == 0) && (plane->type == DRM_PLANE_TYPE_PRIMARY)) { - drmdev_atomic_req_reserve_plane(req, plane); - break; - } else if ((i != 0) && (plane->type == DRM_PLANE_TYPE_OVERLAY)) { - drmdev_atomic_req_reserve_plane(req, plane); - break; + if (compositor->use_atomic_modesetting) { + for_each_unreserved_plane_in_atomic_req(req, plane) { + // choose a plane which has an "intrinsic" zpos that matches + // the zpos we want the plane to have. + // (Since planes are buggy and we can't rely on the zpos we explicitly + // configure the plane to have to be actually applied to the hardware. + // In short, assigning a different value to the zpos property won't always + // take effect.) + if ((i == 0) && (plane->type == DRM_PLANE_TYPE_PRIMARY)) { + ok = drmdev_atomic_req_reserve_plane(req, plane); + break; + } else if ((i != 0) && (plane->type == DRM_PLANE_TYPE_OVERLAY)) { + ok = drmdev_atomic_req_reserve_plane(req, plane); + break; + } + } + } else { + for_each_pointer_in_pset(&planes, plane) { + if ((i == 0) && (plane->type == DRM_PLANE_TYPE_PRIMARY)) { + break; + } else if ((i != 0) && (plane->type == DRM_PLANE_TYPE_OVERLAY)) { + break; + } + } + if (plane != NULL) { + pset_remove(&planes, plane); } } if (plane == NULL) { @@ -1018,18 +1288,32 @@ static bool on_present_layers( struct flutterpi_backing_store *store = layers[i]->backing_store->user_data; struct rendertarget *target = store->target; - ok = target->present( - target, - req, - plane->plane->plane_id, - 0, - 0, - compositor->drmdev->selected_mode->hdisplay, - compositor->drmdev->selected_mode->vdisplay, - i + min_zpos - ); - if (ok != 0) { - fprintf(stderr, "[compositor] Could not present backing store. rendertarget->present: %s\n", strerror(ok)); + if (compositor->use_atomic_modesetting) { + ok = target->present( + target, + req, + plane->plane->plane_id, + 0, + 0, + compositor->drmdev->selected_mode->hdisplay, + compositor->drmdev->selected_mode->vdisplay, + i + min_zpos + ); + if (ok != 0) { + fprintf(stderr, "[compositor] Could not present backing store. rendertarget->present: %s\n", strerror(ok)); + } + } else { + ok = target->present_legacy( + target, + drmdev, + plane->plane->plane_id, + 0, + 0, + compositor->drmdev->selected_mode->hdisplay, + compositor->drmdev->selected_mode->vdisplay, + i + min_zpos, + legacy_rendertarget_set_mode && (plane->type == DRM_PLANE_TYPE_PRIMARY) + ); } } else if (layers[i]->type == kFlutterLayerContentTypePlatformView) { cb_data = get_cbs_for_view_id_locked(layers[i]->platform_view->identifier); @@ -1054,33 +1338,40 @@ static bool on_present_layers( } } - for_each_unreserved_plane_in_atomic_req(req, plane) { - if ((plane->type == DRM_PLANE_TYPE_PRIMARY) || (plane->type == DRM_PLANE_TYPE_OVERLAY)) { - drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "FB_ID", 0); - drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "CRTC_ID", 0); + if (compositor->use_atomic_modesetting) { + for_each_unreserved_plane_in_atomic_req(req, plane) { + if ((plane->type == DRM_PLANE_TYPE_PRIMARY) || (plane->type == DRM_PLANE_TYPE_OVERLAY)) { + drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "FB_ID", 0); + drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "CRTC_ID", 0); + } } } eglMakeCurrent(flutterpi.egl.display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); - do_commit: - if (compositor->do_blocking_atomic_commits) { - req_flags &= ~(DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT); - } else { - req_flags |= DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT; - } - - ok = drmdev_atomic_req_commit(req, req_flags, NULL); - if ((compositor->do_blocking_atomic_commits == false) && (ok < 0) && (errno == EBUSY)) { - printf("[compositor] Non-blocking drmModeAtomicCommit failed with EBUSY.\n" - " Future drmModeAtomicCommits will be executed blockingly.\n" - " This may have have an impact on performance.\n"); + if (compositor->use_atomic_modesetting) { + do_commit: + if (compositor->do_blocking_atomic_commits) { + req_flags &= ~(DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT); + } else { + req_flags |= DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT; + } + + ok = drmdev_atomic_req_commit(req, req_flags, NULL); + if ((compositor->do_blocking_atomic_commits == false) && (ok < 0) && (errno == EBUSY)) { + printf("[compositor] Non-blocking drmModeAtomicCommit failed with EBUSY.\n" + " Future drmModeAtomicCommits will be executed blockingly.\n" + " This may have have an impact on performance.\n"); + + compositor->do_blocking_atomic_commits = true; + schedule_fake_page_flip_event = true; + goto do_commit; + } - compositor->do_blocking_atomic_commits = true; - goto do_commit; + drmdev_destroy_atomic_req(req); } - if (compositor->do_blocking_atomic_commits) { + if (schedule_fake_page_flip_event) { uint64_t time = flutterpi.flutter.libflutter_engine.FlutterEngineGetCurrentTime(); struct simulated_page_flip_event_data *data = malloc(sizeof(struct simulated_page_flip_event_data)); @@ -1094,7 +1385,6 @@ static bool on_present_layers( flutterpi_post_platform_task(execute_simulate_page_flip_event, data); } - drmdev_destroy_atomic_req(req); cpset_unlock(&compositor->cbs); return true; diff --git a/src/flutter-pi.c b/src/flutter-pi.c index dfb3e969..17eb8c95 100644 --- a/src/flutter-pi.c +++ b/src/flutter-pi.c @@ -1035,6 +1035,7 @@ void on_pageflip_event( flutterpi.flutter.libflutter_engine.FlutterEngineTraceEventInstant("pageflip"); + printf("on page flip event\n"); cqueue_lock(&flutterpi.frame_queue); diff --git a/src/modesetting.c b/src/modesetting.c index 397b9e46..6e8481e8 100644 --- a/src/modesetting.c +++ b/src/modesetting.c @@ -598,6 +598,18 @@ static int get_plane_property_index_by_name( return prop_index; } +int drmdev_plane_get_type( + struct drmdev *drmdev, + uint32_t plane_id +) { + struct drm_plane *plane = get_plane_by_id(drmdev, plane_id); + if (plane == NULL) { + return -1; + } + + return plane->type; +} + int drmdev_plane_supports_setting_rotation_value( struct drmdev *drmdev, uint32_t plane_id, @@ -1025,4 +1037,222 @@ int drmdev_atomic_req_commit( drmdev_unlock(req->drmdev); return 0; +} + +int drmdev_legacy_set_mode_and_fb( + struct drmdev *drmdev, + uint32_t fb_id +) { + int ok; + + drmdev_lock(drmdev); + + ok = drmModeSetCrtc( + drmdev->fd, + drmdev->selected_crtc->crtc->crtc_id, + fb_id, + 0, + 0, + &drmdev->selected_connector->connector->connector_id, + 1, + (drmModeModeInfoPtr) drmdev->selected_mode + ); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not set CRTC mode and framebuffer. drmModeSetCrtc"); + drmdev_unlock(drmdev); + return ok; + } + + drmdev_unlock(drmdev); + + return 0; +} + +int drmdev_legacy_primary_plane_pageflip( + struct drmdev *drmdev, + uint32_t fb_id, + void *userdata +) { + int ok; + + drmdev_lock(drmdev); + + ok = drmModePageFlip( + drmdev->fd, + drmdev->selected_crtc->crtc->crtc_id, + fb_id, + DRM_MODE_PAGE_FLIP_EVENT, + userdata + ); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not schedule pageflip on primary plane. drmModePageFlip"); + drmdev_unlock(drmdev); + return ok; + } + + drmdev_unlock(drmdev); + + return 0; +} + +int drmdev_legacy_overlay_plane_pageflip( + struct drmdev *drmdev, + uint32_t plane_id, + uint32_t fb_id, + int32_t crtc_x, + int32_t crtc_y, + int32_t crtc_w, + int32_t crtc_h, + uint32_t src_x, + uint32_t src_y, + uint32_t src_w, + uint32_t src_h +) { + int ok; + + drmdev_lock(drmdev); + + ok = drmModeSetPlane( + drmdev->fd, + plane_id, + drmdev->selected_crtc->crtc->crtc_id, + fb_id, + 0, + crtc_x, crtc_y, crtc_w, crtc_h, + src_x, src_y, src_w, src_h + ); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not do blocking pageflip on overlay plane. drmModeSetPlane"); + drmdev_unlock(drmdev); + return ok; + } + + drmdev_unlock(drmdev); + + return 0; +} + +int drmdev_legacy_set_connector_property( + struct drmdev *drmdev, + const char *name, + uint64_t value +) { + int ok; + + drmdev_lock(drmdev); + + for (int i = 0; i < drmdev->selected_connector->props->count_props; i++) { + drmModePropertyRes *prop = drmdev->selected_connector->props_info[i]; + if (strcmp(prop->name, name) == 0) { + ok = drmModeConnectorSetProperty( + drmdev->fd, + drmdev->selected_connector->connector->connector_id, + prop->prop_id, + value + ); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not set connector property. drmModeConnectorSetProperty"); + drmdev_unlock(drmdev); + return ok; + } + + drmdev_unlock(drmdev); + return 0; + } + } + + drmdev_unlock(drmdev); + return EINVAL; +} + +int drmdev_legacy_set_crtc_property( + struct drmdev *drmdev, + const char *name, + uint64_t value +) { + int ok; + + drmdev_lock(drmdev); + + for (int i = 0; i < drmdev->selected_crtc->props->count_props; i++) { + drmModePropertyRes *prop = drmdev->selected_crtc->props_info[i]; + if (strcmp(prop->name, name) == 0) { + ok = drmModeObjectSetProperty( + drmdev->fd, + drmdev->selected_crtc->crtc->crtc_id, + DRM_MODE_OBJECT_CRTC, + prop->prop_id, + value + ); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not set CRTC property. drmModeObjectSetProperty"); + drmdev_unlock(drmdev); + return ok; + } + + drmdev_unlock(drmdev); + return 0; + } + } + + drmdev_unlock(drmdev); + return EINVAL; +} + +int drmdev_legacy_set_plane_property( + struct drmdev *drmdev, + uint32_t plane_id, + const char *name, + uint64_t value +) { + struct drm_plane *plane; + int ok; + + drmdev_lock(drmdev); + + plane = NULL; + for (int i = 0; i < drmdev->n_planes; i++) { + if (drmdev->planes[i].plane->plane_id == plane_id) { + plane = drmdev->planes + i; + break; + } + } + + if (plane == NULL) { + drmdev_unlock(drmdev); + return EINVAL; + } + + for (int i = 0; i < plane->props->count_props; i++) { + drmModePropertyRes *prop; + + prop = plane->props_info[i]; + + if (strcmp(prop->name, name) == 0) { + ok = drmModeObjectSetProperty( + drmdev->fd, + plane_id, + DRM_MODE_OBJECT_PLANE, + prop->prop_id, + value + ); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not set plane property. drmModeObjectSetProperty"); + drmdev_unlock(drmdev); + return ok; + } + + drmdev_unlock(drmdev); + return 0; + } + } + + drmdev_unlock(drmdev); + return EINVAL; } \ No newline at end of file From 1bf90bfe3cb0cb33ad6c0260417959ff9168d161 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Mon, 12 Oct 2020 01:27:11 +0200 Subject: [PATCH 2/5] improve speed of legacy modesetting fix drmdev_new_from_fd failing for legacy modeset devices --- include/compositor.h | 6 ----- include/modesetting.h | 1 + src/compositor.c | 56 +++++++++---------------------------------- src/flutter-pi.c | 2 -- src/modesetting.c | 10 +++++++- 5 files changed, 21 insertions(+), 54 deletions(-) diff --git a/include/compositor.h b/include/compositor.h index 84aa40c8..37cdbd6b 100644 --- a/include/compositor.h +++ b/include/compositor.h @@ -126,12 +126,6 @@ struct compositor { * like usual. */ bool do_blocking_atomic_commits; - - /** - * @brief Whether atomic modesetting should be used. - * If `false`, legacy modesetting is used. - */ - bool use_atomic_modesetting; }; /* diff --git a/include/modesetting.h b/include/modesetting.h index 272e0e9a..d97576c2 100644 --- a/include/modesetting.h +++ b/include/modesetting.h @@ -38,6 +38,7 @@ struct drmdev { int fd; pthread_mutex_t mutex; + bool supports_atomic_modesetting; size_t n_connectors; struct drm_connector *connectors; diff --git a/src/compositor.c b/src/compositor.c index 69228f5a..9a22b585 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -50,8 +50,7 @@ struct compositor compositor = { .has_applied_modeset = false, .should_create_window_surface_backing_store = true, .stale_rendertargets = CPSET_INITIALIZER(CPSET_DEFAULT_MAX_SIZE), - .do_blocking_atomic_commits = false, - .use_atomic_modesetting = false + .do_blocking_atomic_commits = false }; static struct view_cb_data *get_cbs_for_view_id_locked(int64_t view_id) { @@ -471,41 +470,6 @@ static int rendertarget_gbm_present_legacy( ); } - ok = drmdev_plane_supports_setting_rotation_value(drmdev, drm_plane_id, DRM_MODE_ROTATE_0, &supported); - if (ok != 0) return ok; - - if (supported) { - drmdev_legacy_set_plane_property(drmdev, drm_plane_id, "rotation", DRM_MODE_ROTATE_0); - } else { - static bool printed = false; - - if (!printed) { - fprintf(stderr, - "[compositor] GPU does not support reflecting the screen in Y-direction.\n" - " This is required for rendering into hardware overlay planes though.\n" - " Any UI that is drawn in overlay planes will look upside down.\n" - ); - printed = true; - } - } - - ok = drmdev_plane_supports_setting_zpos_value(drmdev, drm_plane_id, zpos, &supported); - if (ok != 0) return ok; - - if (supported) { - drmdev_legacy_set_plane_property(drmdev, drm_plane_id, "zpos", zpos); - } else { - static bool printed = false; - - if (!printed) { - fprintf(stderr, - "[compositor] GPU does not supported the desired HW plane order.\n" - " Some UI layers may be invisible.\n" - ); - printed = true; - } - } - // TODO: move this to the page flip handler. // We can only be sure the buffer can be released when the buffer swap // ocurred. @@ -987,13 +951,15 @@ static bool on_present_layers( void *planes_storage[32] = {0}; bool legacy_rendertarget_set_mode = false; bool schedule_fake_page_flip_event; + bool use_atomic_modesetting; int ok; compositor = userdata; drmdev = compositor->drmdev; schedule_fake_page_flip_event = compositor->do_blocking_atomic_commits; + use_atomic_modesetting = drmdev->supports_atomic_modesetting; - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { drmdev_new_atomic_req(compositor->drmdev, &req); } else { planes = PSET_INITIALIZER_STATIC(planes_storage, 32); @@ -1011,7 +977,7 @@ static bool on_present_layers( req_flags = 0 /* DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK*/; if (compositor->has_applied_modeset == false) { - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { ok = drmdev_atomic_req_put_modeset_props(req, &req_flags); if (ok != 0) return false; } else { @@ -1021,7 +987,7 @@ static bool on_present_layers( int64_t max_zpos = 0; - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { for_each_unreserved_plane_in_atomic_req(req, plane) { if (plane->type == DRM_PLANE_TYPE_CURSOR) { // make sure the cursor is in front of everything @@ -1228,7 +1194,7 @@ static bool on_present_layers( } int64_t min_zpos; - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { for_each_unreserved_plane_in_atomic_req(req, plane) { if (plane->type == DRM_PLANE_TYPE_PRIMARY) { ok = drmdev_plane_get_min_zpos_value(req->drmdev, plane->plane->plane_id, &min_zpos); @@ -1252,7 +1218,7 @@ static bool on_present_layers( for (int i = 0; i < layers_count; i++) { if (layers[i]->type == kFlutterLayerContentTypeBackingStore) { - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { for_each_unreserved_plane_in_atomic_req(req, plane) { // choose a plane which has an "intrinsic" zpos that matches // the zpos we want the plane to have. @@ -1288,7 +1254,7 @@ static bool on_present_layers( struct flutterpi_backing_store *store = layers[i]->backing_store->user_data; struct rendertarget *target = store->target; - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { ok = target->present( target, req, @@ -1338,7 +1304,7 @@ static bool on_present_layers( } } - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { for_each_unreserved_plane_in_atomic_req(req, plane) { if ((plane->type == DRM_PLANE_TYPE_PRIMARY) || (plane->type == DRM_PLANE_TYPE_OVERLAY)) { drmdev_atomic_req_put_plane_property(req, plane->plane->plane_id, "FB_ID", 0); @@ -1349,7 +1315,7 @@ static bool on_present_layers( eglMakeCurrent(flutterpi.egl.display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); - if (compositor->use_atomic_modesetting) { + if (use_atomic_modesetting) { do_commit: if (compositor->do_blocking_atomic_commits) { req_flags &= ~(DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT); diff --git a/src/flutter-pi.c b/src/flutter-pi.c index 17eb8c95..751e02e3 100644 --- a/src/flutter-pi.c +++ b/src/flutter-pi.c @@ -1035,8 +1035,6 @@ void on_pageflip_event( flutterpi.flutter.libflutter_engine.FlutterEngineTraceEventInstant("pageflip"); - printf("on page flip event\n"); - cqueue_lock(&flutterpi.frame_queue); ok = cqueue_try_dequeue_locked(&flutterpi.frame_queue, &presented_frame); diff --git a/src/modesetting.c b/src/modesetting.c index 6e8481e8..785c314b 100644 --- a/src/modesetting.c +++ b/src/modesetting.c @@ -396,10 +396,14 @@ int drmdev_new_from_fd( } ok = drmSetClientCap(drmdev->fd, DRM_CLIENT_CAP_ATOMIC, 1); - if (ok < 0) { + if ((ok < 0) && (errno == EOPNOTSUPP)) { + drmdev->supports_atomic_modesetting = false; + } else if (ok < 0) { ok = errno; perror("[modesetting] Could not set DRM client atomic capable. drmSetClientCap"); goto fail_free_drmdev; + } else { + drmdev->supports_atomic_modesetting = true; } drmdev->res = drmModeGetResources(drmdev->fd); @@ -819,6 +823,10 @@ int drmdev_new_atomic_req( struct drm_plane *plane; int ok; + if (drmdev->supports_atomic_modesetting == false) { + return EOPNOTSUPP; + } + req = calloc(1, sizeof *req); if (req == NULL) { return ENOMEM; From 0aca30d6be8bf3a4fbdfc973467c5aa8d72ddfec Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Mon, 12 Oct 2020 13:53:39 +0200 Subject: [PATCH 3/5] add some logging to initialization --- include/modesetting.h | 2 +- src/flutter-pi.c | 80 ++++++++++++++++++++++++++++++++++++++++++- src/modesetting.c | 4 +++ 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/include/modesetting.h b/include/modesetting.h index d97576c2..f744bd30 100644 --- a/include/modesetting.h +++ b/include/modesetting.h @@ -225,7 +225,7 @@ int drmdev_legacy_set_plane_property( uint64_t value ); - +float mode_get_vrefresh(const drmModeModeInfo *mode); inline static struct drm_connector *__next_connector(const struct drmdev *drmdev, const struct drm_connector *connector) { bool found = connector == NULL; diff --git a/src/flutter-pi.c b/src/flutter-pi.c index 751e02e3..d6a3e042 100644 --- a/src/flutter-pi.c +++ b/src/flutter-pi.c @@ -1278,11 +1278,89 @@ static int init_display(void) { fprintf(stderr, "[flutter-pi] Could not find a connected connector!\n"); return EINVAL; } - + + printf(" modes: \n" + " name refresh (calculated) hdisp hss hse htot vdisp vss vse vtot clock\n"); // Find the preferred mode (GPU drivers _should_ always supply a preferred mode, but of course, they don't) // Alternatively, find the mode with the highest width*height. If there are multiple modes with the same w*h, // prefer higher refresh rates. After that, prefer progressive scanout modes. for_each_mode_in_connector(connector, mode_iter) { + printf(" %s %d (%.2f) %d %d %d %d %d %d %d %d", + mode_iter->name, + mode_iter->vrefresh, + mode_get_vrefresh(mode_iter), + mode_iter->hdisplay, + mode_iter->hsync_start, + mode_iter->hsync_end, + mode_iter->htotal, + mode_iter->vdisplay, + mode_iter->vsync_start, + mode_iter->vsync_end, + mode_iter->vtotal + ); + printf(" %d flags: ", mode_iter->clock); + + { + const char *sep = ""; + for (uint32_t i = 1; i & 0x3FFF; i<<=1) { + if (mode_iter->flags & i) { + const char *name; + switch (i) { + case DRM_MODE_FLAG_PHSYNC: name = "phsync"; break; + case DRM_MODE_FLAG_NHSYNC: name = "nhsync"; break; + case DRM_MODE_FLAG_PVSYNC: name = "pvsync"; break; + case DRM_MODE_FLAG_NVSYNC: name = "nvsync"; break; + case DRM_MODE_FLAG_INTERLACE: name = "interlace"; break; + case DRM_MODE_FLAG_DBLSCAN: name = "dblscan"; break; + case DRM_MODE_FLAG_CSYNC: name = "csync"; break; + case DRM_MODE_FLAG_PCSYNC: name = "pcsync"; break; + case DRM_MODE_FLAG_NCSYNC: name = "ncsync"; break; + case DRM_MODE_FLAG_HSKEW: name = "hskew"; break; + case DRM_MODE_FLAG_BCAST: name = "bcast"; break; + case DRM_MODE_FLAG_PIXMUX: name = "pixmux"; break; + case DRM_MODE_FLAG_DBLCLK: name = "dblclk"; break; + case DRM_MODE_FLAG_CLKDIV2: name = "clkdiv2"; break; + default: name = "?"; break; + } + printf( + "%s%s", + sep, + name + ); + sep = ", "; + } + } + } + + printf("; type: "); + + { + const char *sep = ""; + for (uint32_t i = 1; i & 0x7F; i<<=1) { + if (mode_iter->type & i) { + const char *name; + switch (i) { + case DRM_MODE_TYPE_BUILTIN: name = "builtin"; break; + case DRM_MODE_TYPE_CLOCK_C: name = "clock_c"; break; + case DRM_MODE_TYPE_CRTC_C: name = "crtc_c"; break; + case DRM_MODE_TYPE_PREFERRED: name = "preferred"; break; + case DRM_MODE_TYPE_DEFAULT: name = "default"; break; + case DRM_MODE_TYPE_USERDEF: name = "userdef"; break; + case DRM_MODE_TYPE_DRIVER: name = "driver"; break; + default: name = "?"; break; + } + printf( + "%s%s", + sep, + name + ); + sep = ", "; + } + } + } + + printf("\n"); + if (mode_iter->type & DRM_MODE_TYPE_PREFERRED) { mode = mode_iter; break; diff --git a/src/modesetting.c b/src/modesetting.c index 785c314b..1b9a3a7c 100644 --- a/src/modesetting.c +++ b/src/modesetting.c @@ -374,6 +374,10 @@ static int free_planes(struct drm_plane *planes, size_t n_planes) { } +float mode_get_vrefresh(const drmModeModeInfo *mode) { + return mode->clock * 1000.0 / (mode->htotal * mode->vtotal); +} + int drmdev_new_from_fd( struct drmdev **drmdev_out, int fd From a6dfe38d83a3944f0224f61f77b8a82848a68bb8 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Mon, 12 Oct 2020 23:00:25 +0200 Subject: [PATCH 4/5] fix mode being uninitialized dont create display mode property blob if not using atomic modesetting --- src/flutter-pi.c | 1 + src/modesetting.c | 29 ++++++++++++++++------------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/flutter-pi.c b/src/flutter-pi.c index d6a3e042..43c0940a 100644 --- a/src/flutter-pi.c +++ b/src/flutter-pi.c @@ -1284,6 +1284,7 @@ static int init_display(void) { // Find the preferred mode (GPU drivers _should_ always supply a preferred mode, but of course, they don't) // Alternatively, find the mode with the highest width*height. If there are multiple modes with the same w*h, // prefer higher refresh rates. After that, prefer progressive scanout modes. + mode = NULL; for_each_mode_in_connector(connector, mode_iter) { printf(" %s %d (%.2f) %d %d %d %d %d %d %d %d", mode_iter->name, diff --git a/src/modesetting.c b/src/modesetting.c index 1b9a3a7c..aa28ae8a 100644 --- a/src/modesetting.c +++ b/src/modesetting.c @@ -539,21 +539,24 @@ int drmdev_configure( return EINVAL; } - ok = drmModeCreatePropertyBlob(drmdev->fd, mode, sizeof(*mode), &mode_id); - if (ok < 0) { - perror("[modesetting] Could not create property blob for DRM mode. drmModeCreatePropertyBlob"); - drmdev_unlock(drmdev); - return errno; - } - - if (drmdev->selected_mode != NULL) { - ok = drmModeDestroyPropertyBlob(drmdev->fd, drmdev->selected_mode_blob_id); + mode_id = 0; + if (drmdev->supports_atomic_modesetting) { + ok = drmModeCreatePropertyBlob(drmdev->fd, mode, sizeof(*mode), &mode_id); if (ok < 0) { - ok = errno; - perror("[modesetting] Could not destroy old DRM mode property blob. drmModeDestroyPropertyBlob"); - drmModeDestroyPropertyBlob(drmdev->fd, mode_id); + perror("[modesetting] Could not create property blob for DRM mode. drmModeCreatePropertyBlob"); drmdev_unlock(drmdev); - return ok; + return errno; + } + + if (drmdev->selected_mode != NULL) { + ok = drmModeDestroyPropertyBlob(drmdev->fd, drmdev->selected_mode_blob_id); + if (ok < 0) { + ok = errno; + perror("[modesetting] Could not destroy old DRM mode property blob. drmModeDestroyPropertyBlob"); + drmModeDestroyPropertyBlob(drmdev->fd, mode_id); + drmdev_unlock(drmdev); + return ok; + } } } From 20d60e72eb4bea8d77dbe96d9c12be4743b281e5 Mon Sep 17 00:00:00 2001 From: Hannes Winkler Date: Tue, 13 Oct 2020 14:24:09 +0200 Subject: [PATCH 5/5] remove logging --- src/flutter-pi.c | 78 ------------------------------------------------ 1 file changed, 78 deletions(-) diff --git a/src/flutter-pi.c b/src/flutter-pi.c index 43c0940a..bbe99a51 100644 --- a/src/flutter-pi.c +++ b/src/flutter-pi.c @@ -1279,89 +1279,11 @@ static int init_display(void) { return EINVAL; } - printf(" modes: \n" - " name refresh (calculated) hdisp hss hse htot vdisp vss vse vtot clock\n"); // Find the preferred mode (GPU drivers _should_ always supply a preferred mode, but of course, they don't) // Alternatively, find the mode with the highest width*height. If there are multiple modes with the same w*h, // prefer higher refresh rates. After that, prefer progressive scanout modes. mode = NULL; for_each_mode_in_connector(connector, mode_iter) { - printf(" %s %d (%.2f) %d %d %d %d %d %d %d %d", - mode_iter->name, - mode_iter->vrefresh, - mode_get_vrefresh(mode_iter), - mode_iter->hdisplay, - mode_iter->hsync_start, - mode_iter->hsync_end, - mode_iter->htotal, - mode_iter->vdisplay, - mode_iter->vsync_start, - mode_iter->vsync_end, - mode_iter->vtotal - ); - printf(" %d flags: ", mode_iter->clock); - - { - const char *sep = ""; - for (uint32_t i = 1; i & 0x3FFF; i<<=1) { - if (mode_iter->flags & i) { - const char *name; - switch (i) { - case DRM_MODE_FLAG_PHSYNC: name = "phsync"; break; - case DRM_MODE_FLAG_NHSYNC: name = "nhsync"; break; - case DRM_MODE_FLAG_PVSYNC: name = "pvsync"; break; - case DRM_MODE_FLAG_NVSYNC: name = "nvsync"; break; - case DRM_MODE_FLAG_INTERLACE: name = "interlace"; break; - case DRM_MODE_FLAG_DBLSCAN: name = "dblscan"; break; - case DRM_MODE_FLAG_CSYNC: name = "csync"; break; - case DRM_MODE_FLAG_PCSYNC: name = "pcsync"; break; - case DRM_MODE_FLAG_NCSYNC: name = "ncsync"; break; - case DRM_MODE_FLAG_HSKEW: name = "hskew"; break; - case DRM_MODE_FLAG_BCAST: name = "bcast"; break; - case DRM_MODE_FLAG_PIXMUX: name = "pixmux"; break; - case DRM_MODE_FLAG_DBLCLK: name = "dblclk"; break; - case DRM_MODE_FLAG_CLKDIV2: name = "clkdiv2"; break; - default: name = "?"; break; - } - printf( - "%s%s", - sep, - name - ); - sep = ", "; - } - } - } - - printf("; type: "); - - { - const char *sep = ""; - for (uint32_t i = 1; i & 0x7F; i<<=1) { - if (mode_iter->type & i) { - const char *name; - switch (i) { - case DRM_MODE_TYPE_BUILTIN: name = "builtin"; break; - case DRM_MODE_TYPE_CLOCK_C: name = "clock_c"; break; - case DRM_MODE_TYPE_CRTC_C: name = "crtc_c"; break; - case DRM_MODE_TYPE_PREFERRED: name = "preferred"; break; - case DRM_MODE_TYPE_DEFAULT: name = "default"; break; - case DRM_MODE_TYPE_USERDEF: name = "userdef"; break; - case DRM_MODE_TYPE_DRIVER: name = "driver"; break; - default: name = "?"; break; - } - printf( - "%s%s", - sep, - name - ); - sep = ", "; - } - } - } - - printf("\n"); - if (mode_iter->type & DRM_MODE_TYPE_PREFERRED) { mode = mode_iter; break;