[Mesa-dev] [PATCH] loader/dri3: Try to make sure we only process our own NotifyMSC events

[Nicolai Hähnle]

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


On 20.10.2017 18:22, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at 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 at lists.freedesktop.org
> Signed-off-by: Michel Dänzer <michel.daenzer at 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;
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.