loader/dri3: Try to make sure we only process our own NotifyMSC events

From: Michel Dänzer <michel.daenzer@amd.com>

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.

Cc: mesa-stable@lists.freedesktop.org
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/loader/loader_dri3_helper.c | 31 +++++++++++++++----------------
 src/loader/loader_dri3_helper.h |  4 ----
 2 files changed, 15 insertions(+), 20 deletions(-)

Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>


On 20.10.2017 18:22, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> 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.
> 
> Cc: mesa-stable@lists.freedesktop.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>   src/loader/loader_dri3_helper.c | 31 +++++++++++++++----------------
>   src/loader/loader_dri3_helper.h |  4 ----
>   2 files changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 19ab5815100..3259dde549b 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -378,7 +378,6 @@ dri3_handle_present_event(struct loader_dri3_drawable *draw,
>            draw->ust = ce->ust;
>            draw->msc = ce->msc;
>         } else {
> -         draw->recv_msc_serial = ce->serial;
>            draw->notify_ust = ce->ust;
>            draw->notify_msc = ce->msc;
>         }
> @@ -432,25 +431,25 @@ 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,
> +                                                     0,
> +                                                     target_msc,
> +                                                     divisor,
> +                                                     remainder);
> +   xcb_generic_event_t *ev;
> +   unsigned full_sequence;
>   
>      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(draw))
> -            return false;
> -      }
> -   }
> +   do {
> +      ev = xcb_wait_for_special_event(draw->conn, draw->special_event);
> +      if (!ev)
> +         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 d3f4b0c00a9..f25ed1231aa 100644
> --- a/src/loader/loader_dri3_helper.h
> +++ b/src/loader/loader_dri3_helper.h
> @@ -136,10 +136,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;
>