From 4b288617ce64d7bb43e187c645118e54dfdf3338 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 9 Jul 2023 16:39:44 +0100 Subject: [PATCH] wip --- src/common.h | 6 +-- src/picom.c | 138 +++++++++++++-------------------------------------- src/vblank.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++ src/vblank.h | 44 ++++++++++++++++ src/x.c | 24 ++++----- src/x.h | 8 +-- 6 files changed, 227 insertions(+), 126 deletions(-) create mode 100644 src/vblank.c create mode 100644 src/vblank.h diff --git a/src/common.h b/src/common.h index 2d9096ad93..965ae443a7 100644 --- a/src/common.h +++ b/src/common.h @@ -156,8 +156,6 @@ typedef struct session { ev_timer fade_timer; /// Use an ev_timer callback for drawing ev_timer draw_timer; - /// Timer for the end of each vblanks. Used for calling schedule_render. - ev_timer vblank_timer; /// Called every time we have timeouts or new data on socket, /// so we can be sure if xcb read from X socket at anytime during event /// handling, we will not left any event unhandled in the queue @@ -225,9 +223,6 @@ typedef struct session { bool first_frame; /// Whether screen has been turned off bool screen_is_off; - /// We asked X server to send us a event for the end of a vblank, and we haven't - /// received one yet. - bool vblank_event_requested; /// Event context for X Present extension. uint32_t present_event_id; xcb_special_event_t *present_event; @@ -242,6 +237,7 @@ typedef struct session { uint64_t next_render; /// Whether we can perform frame pacing. bool frame_pacing; + struct vblank_scheduler *vblank_scheduler; /// Render statistics struct render_statistics render_stats; diff --git a/src/picom.c b/src/picom.c index 44c35f0dca..a0f3089fbd 100644 --- a/src/picom.c +++ b/src/picom.c @@ -64,6 +64,7 @@ #include "options.h" #include "statistics.h" #include "uthash_extra.h" +#include "vblank.h" /// Get session_t pointer from a pointer to a member of session_t #define session_ptr(ptr, member) \ @@ -238,7 +239,11 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { ps->render_in_progress = RENDER_QUEUED; ps->redraw_needed = false; - x_request_vblank_event(ps, ps->last_msc + 1); + if (!vblank_scheduler_schedule(ps->vblank_scheduler, + session_get_target_window(ps), &ps->c)) { + // TODO(yshui): handle error here + abort(); + } assert(!ev_is_active(&ps->draw_timer)); ev_timer_set(&ps->draw_timer, delay_s, 0); @@ -248,13 +253,34 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { /// Called after a vblank has ended /// /// Check if previously queued render has finished, and record the time it took. -void handle_end_of_vblank(session_t *ps) { +void handle_end_of_vblank(struct vblank_event *e, void *ud) { + auto ps = (session_t *)ud; + auto target_window = session_get_target_window(ps); + if (ps->last_msc_instant != 0) { + auto frame_count = e->msc - ps->last_msc; + int frame_time = (int)((e->ust - ps->last_msc_instant) / frame_count); + if (frame_count == 1) { + render_statistics_add_vblank_time_sample(&ps->render_stats, frame_time); + log_trace("Frame count %lu, frame time: %d us, rolling average: " + "%u us, " + "ust: %" PRIu64 "", + frame_count, frame_time, + render_statistics_get_vblank_time(&ps->render_stats), + e->ust); + } else { + log_trace("Frame count %lu, frame time: %d us, msc: %" PRIu64 + ", not adding sample.", + frame_count, frame_time, e->ust); + } + } + ps->last_msc_instant = e->ust; + ps->last_msc = e->msc; if (ps->render_in_progress == RENDER_IDLE) { // We didn't start rendering for this vblank, no render time to record. // But if we don't have a vblank estimate, we will ask for one more vblank // event, so we can collect more data and get an estimate sooner. if (render_statistics_get_vblank_time(&ps->render_stats) == 0) { - x_request_vblank_event(ps, ps->last_msc + 1); + vblank_scheduler_schedule(ps->vblank_scheduler, target_window, &ps->c); } return; } @@ -281,7 +307,7 @@ void handle_end_of_vblank(session_t *ps) { render_statistics_get_vblank_time(&ps->render_stats) == 0); if (need_vblank_events) { - x_request_vblank_event(ps, ps->last_msc + 1); + vblank_scheduler_schedule(ps->vblank_scheduler, target_window, &ps->c); } if (!completed) { @@ -1460,97 +1486,6 @@ static void unredirect(session_t *ps) { log_debug("Screen unredirected."); } -/// Handle PresentCompleteNotify events -/// -/// Record the MSC value and their timestamps, and schedule handle_end_of_vblank() at the -/// correct time. -static void -handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_t *cne) { - if (cne->kind != XCB_PRESENT_COMPLETE_KIND_NOTIFY_MSC) { - return; - } - - assert(ps->frame_pacing); - assert(ps->vblank_event_requested); - ps->vblank_event_requested = false; - - // X sometimes sends duplicate/bogus MSC events, when screen has just been turned - // off. Don't use the msc value in these events. We treat this as not receiving a - // vblank event at all, and try to get a new one. - // - // See: - // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 - bool event_is_invalid = cne->msc <= ps->last_msc || cne->ust == 0; - if (event_is_invalid) { - log_debug("Invalid PresentCompleteNotify event, %" PRIu64 " %" PRIu64, - cne->msc, cne->ust); - x_request_vblank_event(ps, ps->last_msc + 1); - return; - } - - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - auto now_us = (int64_t)(now.tv_sec * 1000000L + now.tv_nsec / 1000); - auto drift = iabs((int64_t)cne->ust - now_us); - - if (ps->last_msc_instant == 0 && drift > 1000000) { - // This is the first MSC event we receive, let's check if the timestamps - // align with the monotonic clock. If not, disable frame pacing because we - // can't schedule frames reliably. - log_error("Temporal anomaly detected, frame pacing disabled. (Are we " - "running inside a time namespace?), %" PRIi64 " %" PRIu64, - now_us, ps->last_msc_instant); - if (ps->render_in_progress == RENDER_STARTED) { - // When frame_pacing is off, render_in_progress can't be - // RENDER_STARTED. See the comment on schedule_render(). - ps->render_in_progress = RENDER_IDLE; - } - ps->frame_pacing = false; - return; - } - - x_check_dpms_status(ps); - - if (ps->last_msc_instant != 0) { - auto frame_count = cne->msc - ps->last_msc; - int frame_time = (int)((cne->ust - ps->last_msc_instant) / frame_count); - if (frame_count == 1 && !ps->screen_is_off) { - render_statistics_add_vblank_time_sample(&ps->render_stats, frame_time); - log_trace("Frame count %lu, frame time: %d us, rolling average: " - "%u us, " - "msc: %" PRIu64 ", offset: %d us", - frame_count, frame_time, - render_statistics_get_vblank_time(&ps->render_stats), - cne->ust, (int)drift); - } else { - log_trace("Frame count %lu, frame time: %d us, msc: %" PRIu64 - ", offset: %d us, not adding sample.", - frame_count, frame_time, cne->ust, (int)drift); - } - } - ps->last_msc_instant = cne->ust; - ps->last_msc = cne->msc; - // Note we can't update ps->render_in_progress here because of this present - // complete notify, as we don't know if the render finished before the end of - // vblank or not. We schedule a call to handle_end_of_vblank() to figure out if we - // are still rendering, and update ps->render_in_progress accordingly. - if (now_us > (int64_t)cne->ust) { - handle_end_of_vblank(ps); - } else { - // Wait until the end of the current vblank to call - // handle_end_of_vblank. If we call it too early, it can - // mistakenly think the render missed the vblank, and doesn't - // schedule render for the next vblank, causing frame drops. - log_trace("The end of this vblank is %" PRIi64 " us into the " - "future", - (int64_t)cne->ust - now_us); - assert(!ev_is_active(&ps->vblank_timer)); - ev_timer_set(&ps->vblank_timer, - ((double)cne->ust - (double)now_us) / 1000000.0, 0); - ev_timer_start(ps->loop, &ps->vblank_timer); - } -} - static void handle_present_events(session_t *ps) { if (!ps->present_event) { // Screen not redirected @@ -1566,7 +1501,8 @@ static void handle_present_events(session_t *ps) { // We only subscribed to the complete notify event. assert(ev->evtype == XCB_PRESENT_EVENT_COMPLETE_NOTIFY); - handle_present_complete_notify(ps, (void *)ev); + handle_present_complete_notify(ps->vblank_scheduler, ps->loop, &ps->c, + (void *)ev); next: free(ev); } @@ -1836,13 +1772,6 @@ static void draw_callback(EV_P_ ev_timer *w, int revents) { } } -static void vblank_callback(EV_P_ ev_timer *w, int revents attr_unused) { - session_t *ps = session_ptr(w, vblank_timer); - ev_timer_stop(EV_A_ w); - - handle_end_of_vblank(ps); -} - static void x_event_callback(EV_P attr_unused, ev_io *w, int revents attr_unused) { session_t *ps = (session_t *)w; xcb_generic_event_t *ev = xcb_poll_for_event(ps->c.c); @@ -2431,7 +2360,6 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ev_io_start(ps->loop, &ps->xiow); ev_init(&ps->unredir_timer, tmout_unredir_callback); ev_init(&ps->draw_timer, draw_callback); - ev_init(&ps->vblank_timer, vblank_callback); ev_init(&ps->fade_timer, fade_timer_callback); @@ -2461,6 +2389,8 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ev_set_priority(&ps->event_check, EV_MINPRI); ev_prepare_start(ps->loop, &ps->event_check); + ps->vblank_scheduler = vblank_scheduler_new(handle_end_of_vblank, ps); + // Initialize DBus. We need to do this early, because add_win might call dbus // functions if (ps->o.dbus) { diff --git a/src/vblank.c b/src/vblank.c new file mode 100644 index 0000000000..d27d48448a --- /dev/null +++ b/src/vblank.c @@ -0,0 +1,133 @@ +#include + +#include +#include + +#include "compiler.h" +#include "list.h" // for container_of +#include "log.h" +#include "vblank.h" +#include "x.h" + +enum vblank_scheduler_type { + /// X Present extension based vblank events + PRESENT_VBLANK_SCHEDULER, + /// GLX_SGI_video_sync based vblank events + SGI_VIDEO_VSYNC_VBLANK_SCHEDULER, +}; + +struct vblank_scheduler { + enum vblank_scheduler_type type; + void *user_data; + vblank_callback_t callback; +}; + +struct present_vblank_scheduler { + struct vblank_scheduler base; + + uint64_t last_msc; + /// The timestamp for the end of last vblank. + uint64_t last_ust; + ev_timer callback_timer; + bool vblank_event_requested; +}; + +static void present_vblank_callback(EV_P_ ev_timer *w, int revents) { + auto sched = container_of(w, struct present_vblank_scheduler, callback_timer); + sched->base.callback( + &(struct vblank_event){ + .msc = sched->last_msc, + .ust = sched->last_ust, + }, + sched->base.user_data); +} + +static struct present_vblank_scheduler * +present_vblank_scheduler_new(vblank_callback_t callback, void *user_data) { + auto sched = ccalloc(1, struct present_vblank_scheduler); + sched->base.user_data = user_data; + sched->base.callback = callback; + sched->base.type = PRESENT_VBLANK_SCHEDULER; + ev_timer_init(&sched->callback_timer, present_vblank_callback, 0, 0); + return sched; +} + +static bool present_vblank_scheduler_schedule(struct present_vblank_scheduler *sched, + xcb_window_t window, struct x_connection *c) { + x_request_vblank_event(c, window, sched->last_msc + 1); + return true; +} + +struct vblank_scheduler *vblank_scheduler_new(vblank_callback_t callback, void *user_data) { + return &present_vblank_scheduler_new(callback, user_data)->base; +} + +bool vblank_scheduler_schedule(struct vblank_scheduler *sched, xcb_window_t window, + struct x_connection *c) { + switch (sched->type) { + case PRESENT_VBLANK_SCHEDULER: + return present_vblank_scheduler_schedule( + (struct present_vblank_scheduler *)sched, window, c); + case SGI_VIDEO_VSYNC_VBLANK_SCHEDULER: return false; + default: assert(false); + } +} + +/// Handle PresentCompleteNotify events +/// +/// Schedule the registered callback to be called when the current vblank ends. +void handle_present_complete_notify(struct vblank_scheduler *self, struct ev_loop *loop, + struct x_connection *c, + xcb_present_complete_notify_event_t *cne) { + assert(self->type == PRESENT_VBLANK_SCHEDULER); + + auto sched = (struct present_vblank_scheduler *)self; + if (cne->kind != XCB_PRESENT_COMPLETE_KIND_NOTIFY_MSC) { + return; + } + + assert(sched->vblank_event_requested); + + // X sometimes sends duplicate/bogus MSC events, when screen has just been turned + // off. Don't use the msc value in these events. We treat this as not receiving a + // vblank event at all, and try to get a new one. + // + // See: + // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 + bool event_is_invalid = cne->msc <= sched->last_msc || cne->ust == 0; + if (event_is_invalid) { + log_debug("Invalid PresentCompleteNotify event, %" PRIu64 " %" PRIu64, + cne->msc, cne->ust); + x_request_vblank_event(c, cne->window, sched->last_msc + 1); + return; + } + + sched->vblank_event_requested = false; + + sched->last_ust = cne->ust; + sched->last_msc = cne->msc; + + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + auto now_us = now.tv_sec * 1000000UL + now.tv_nsec / 1000; + if (now_us > cne->ust) { + sched->base.callback( + &(struct vblank_event){ + .msc = cne->msc, + .ust = cne->ust, + }, + sched->base.user_data); + } else { + // Wait until the end of the current vblank to call + // handle_end_of_vblank. If we call it too early, it can + // mistakenly think the render missed the vblank, and doesn't + // schedule render for the next vblank, causing frame drops. + log_trace("The end of this vblank is %" PRIi64 " us into the " + "future", + (int64_t)cne->ust - now_us); + assert(!ev_is_active(&sched->callback_timer)); + ev_timer_set(&sched->callback_timer, + (double)(cne->ust - now_us) / 1000000.0, 0); + ev_timer_start(loop, &sched->callback_timer); + } +} diff --git a/src/vblank.h b/src/vblank.h new file mode 100644 index 0000000000..469d4e6ecb --- /dev/null +++ b/src/vblank.h @@ -0,0 +1,44 @@ +#pragma once + +#include +#include +#include +#include + +#include + +#include "x.h" + +/// An object that schedule vblank events. +struct vblank_scheduler; + +struct vblank_event { + uint64_t msc; + uint64_t ust; +}; + +typedef void (*vblank_callback_t)(struct vblank_event *event, void *user_data); + +/// Schedule a vblank event. +/// +/// Schedule for `cb` to be called when the current vblank +/// ends. +/// +/// Returns whether a new event is scheduled. If there is already an event scheduled for +/// the current vblank, this function will do n1othing and return false. +bool vblank_scheduler_schedule(struct vblank_scheduler *self, xcb_window_t window, + struct x_connection *c); +struct vblank_scheduler *vblank_scheduler_new(vblank_callback_t callback, void *user_data); + +/// Handle PresentCompleteNotify events +/// +/// Schedule the registered callback to be called when the current vblank ends. +void handle_present_complete_notify(struct vblank_scheduler *self, struct ev_loop *loop, + struct x_connection *c, + xcb_present_complete_notify_event_t *cne); + +// NOTE(yshui): OK, this vblank scheduler abstraction is a bit leaky. The core has to call +// handle_present_complete_notify() to drive the scheduler, the scheduler doesn't drive +// itself. In theory we can add an API for the scheduler to register callbacks on specific +// X events. But that's a bit overkill for now, as we only need to handle +// PresentCompleteNotify. diff --git a/src/x.c b/src/x.c index 89786e18e3..45abe31197 100644 --- a/src/x.c +++ b/src/x.c @@ -779,15 +779,10 @@ bool x_fence_sync(struct x_connection *c, xcb_sync_fence_t f) { return false; } -void x_request_vblank_event(session_t *ps, uint64_t msc) { - if (ps->vblank_event_requested) { - return; - } - +void x_request_vblank_event(struct x_connection *c, xcb_window_t window, uint64_t msc) { auto cookie = - xcb_present_notify_msc(ps->c.c, session_get_target_window(ps), 0, msc, 0, 0); - set_cant_fail_cookie(&ps->c, cookie); - ps->vblank_event_requested = true; + xcb_present_notify_msc(c->c, window, 0, msc, 0, 0); + set_cant_fail_cookie(c, cookie); } static inline bool dpms_screen_is_off(xcb_dpms_info_reply_t *info) { @@ -795,18 +790,19 @@ static inline bool dpms_screen_is_off(xcb_dpms_info_reply_t *info) { return info->state && (info->power_level != XCB_DPMS_DPMS_MODE_ON); } -void x_check_dpms_status(session_t *ps) { - auto r = xcb_dpms_info_reply(ps->c.c, xcb_dpms_info(ps->c.c), NULL); +bool x_check_dpms_status(struct x_connection *c, bool *screen_is_off) { + auto r = xcb_dpms_info_reply(c->c, xcb_dpms_info(c->c), NULL); if (!r) { - log_fatal("Failed to query DPMS status."); - abort(); + log_error("Failed to query DPMS status."); + return false; } auto now_screen_is_off = dpms_screen_is_off(r); - if (ps->screen_is_off != now_screen_is_off) { + if (*screen_is_off != now_screen_is_off) { log_debug("Screen is now %s", now_screen_is_off ? "off" : "on"); - ps->screen_is_off = now_screen_is_off; + *screen_is_off = now_screen_is_off; } free(r); + return true; } /** diff --git a/src/x.h b/src/x.h index 60bcfef492..df45b5cca7 100644 --- a/src/x.h +++ b/src/x.h @@ -421,7 +421,9 @@ void x_free_monitor_info(struct x_monitors *); uint32_t attr_deprecated xcb_generate_id(xcb_connection_t *c); /// Ask X server to send us a notification for the next end of vblank. -void x_request_vblank_event(session_t *ps, uint64_t msc); +void x_request_vblank_event(struct x_connection *c, xcb_window_t window, uint64_t msc); -/// Update ps->screen_is_off to reflect the current DPMS state. -void x_check_dpms_status(session_t *ps); +/// Update screen_is_off to reflect the current DPMS state. +/// +/// Returns true if the DPMS state was successfully queried, false otherwise. +bool x_check_dpms_status(struct x_connection *c, bool *screen_is_off);