Ping (sorry I forgot to mark this as v2 in the subject)
On 2017-11-23 10:26 AM, Michel Dänzer wrote: > From: Michel Dänzer <[email protected]> > > We were using a sequence counter value to wait for a specific NotifyMSC > event. However, we can receive events from other clients as well, which > may already be using higher sequence numbers than us. In that case, we > could stop processing after an event from another client, which could > have been received significantly earlier. This would have multiple > undesirable effects: > > * The computed MSC and UST values would be lower than they should be > * We could leave a growing number of NotifyMSC events from ourselves and > other clients in XCB's special event queue > > I ran into this with Firefox and Thunderbird, whose VSync threads both > seem to use the same window. The result was sluggish screen updates and > growing memory consumption in one of them. > > Fix this by checking the XCB sequence number and MSC value of NotifyMSC > events, instead of using our own sequence number. > > v2: > * Use the Present event ID for the sequence parameter of the > PresentNotifyMSC request, as another safeguard against processing > events from other clients > * Rebase on drawable mutex changes > > Cc: [email protected] > Reviewed-by: Nicolai Hähnle <[email protected]> # v1 > Signed-off-by: Michel Dänzer <[email protected]> > --- > src/loader/loader_dri3_helper.c | 36 ++++++++++++++++++------------------ > src/loader/loader_dri3_helper.h | 4 ---- > 2 files changed, 18 insertions(+), 22 deletions(-) > > diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c > index 7e6b8b2e056..d4cd9737ab2 100644 > --- a/src/loader/loader_dri3_helper.c > +++ b/src/loader/loader_dri3_helper.c > @@ -384,8 +384,7 @@ dri3_handle_present_event(struct loader_dri3_drawable > *draw, > > draw->ust = ce->ust; > draw->msc = ce->msc; > - } else { > - draw->recv_msc_serial = ce->serial; > + } else if (ce->serial == draw->eid) { > draw->notify_ust = ce->ust; > draw->notify_msc = ce->msc; > } > @@ -453,28 +452,29 @@ loader_dri3_wait_for_msc(struct loader_dri3_drawable > *draw, > int64_t divisor, int64_t remainder, > int64_t *ust, int64_t *msc, int64_t *sbc) > { > - uint32_t msc_serial; > - > - msc_serial = ++draw->send_msc_serial; > - xcb_present_notify_msc(draw->conn, > - draw->drawable, > - msc_serial, > - target_msc, > - divisor, > - remainder); > + xcb_void_cookie_t cookie = xcb_present_notify_msc(draw->conn, > + draw->drawable, > + draw->eid, > + target_msc, > + divisor, > + remainder); > + xcb_generic_event_t *ev; > + unsigned full_sequence; > > mtx_lock(&draw->mtx); > xcb_flush(draw->conn); > > /* Wait for the event */ > - if (draw->special_event) { > - while ((int32_t) (msc_serial - draw->recv_msc_serial) > 0) { > - if (!dri3_wait_for_event_locked(draw)) { > - mtx_unlock(&draw->mtx); > - return false; > - } > + do { > + ev = xcb_wait_for_special_event(draw->conn, draw->special_event); > + if (!ev) { > + mtx_unlock(&draw->mtx); > + return false; > } > - } > + > + full_sequence = ev->full_sequence; > + dri3_handle_present_event(draw, (void *) ev); > + } while (full_sequence != cookie.sequence || draw->notify_msc < > target_msc); > > *ust = draw->notify_ust; > *msc = draw->notify_msc; > diff --git a/src/loader/loader_dri3_helper.h b/src/loader/loader_dri3_helper.h > index 0dd37e91717..4ce98b8c59f 100644 > --- a/src/loader/loader_dri3_helper.h > +++ b/src/loader/loader_dri3_helper.h > @@ -137,10 +137,6 @@ struct loader_dri3_drawable { > /* Last received UST/MSC values from present notify msc event */ > uint64_t notify_ust, notify_msc; > > - /* Serial numbers for tracking wait_for_msc events */ > - uint32_t send_msc_serial; > - uint32_t recv_msc_serial; > - > struct loader_dri3_buffer *buffers[LOADER_DRI3_NUM_BUFFERS]; > int cur_back; > int num_back; > -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
