[PATCH] present: Don't stash the MSC value when present_get_ust_msc fails

[Michel Dänzer]

On 11.09.2015 06:33, Fredrik Höglund wrote:
> Otherwise we stash an uninitalized value, and later use it to compute
> the msc_offset for the window.  Also initialize ust and crtc_msc so we
> never use uninitalized values when present_get_ust_msc fails.
> 
> This fixes clients getting stuck waiting indefinitely for an idle
> event when a CRTC is turned off.
> 
> Signed-off-by: Fredrik Höglund <fredrik at kde.org>
> ---
>  present/present.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/present/present.c b/present/present.c
> index a634601..7ddffbd 100644
> --- a/present/present.c
> +++ b/present/present.c
> @@ -710,9 +710,9 @@ present_pixmap(WindowPtr window,
>                 present_notify_ptr notifies,
>                 int num_notifies)
>  {
> -    uint64_t                    ust;
> +    uint64_t                    ust = 0;
>      uint64_t                    target_msc;
> -    uint64_t                    crtc_msc;
> +    uint64_t                    crtc_msc = 0;
>      int                         ret;
>      present_vblank_ptr          vblank, tmp;
>      ScreenPtr                   screen = window->drawable.pScreen;
> @@ -734,13 +734,15 @@ present_pixmap(WindowPtr window,
>              target_crtc = present_get_crtc(window);
>      }
>  
> -    present_get_ust_msc(screen, target_crtc, &ust, &crtc_msc);
> +    ret = present_get_ust_msc(screen, target_crtc, &ust, &crtc_msc);
>  
>      target_msc = present_window_to_crtc_msc(window, target_crtc, window_msc, crtc_msc);
>  
> -    /* Stash the current MSC away in case we need it later
> -     */
> -    window_priv->msc = crtc_msc;
> +    if (ret == Success) {
> +        /* Stash the current MSC away in case we need it later
> +         */
> +        window_priv->msc = crtc_msc;
> +    }
>  
>      /* Adjust target_msc to match modulus
>       */
> 

My only doubt is about present_window_to_crtc_msc(). If target_msc
doesn't match window_priv->crtc, presumably present_get_ust_msc() will
fail there as well, so window_priv->msc (the last recorded valid MSC
value for the window) will be added to window_priv->msc_offset. I
suspect that's not right and might still cause similar trouble down the
road.

That said, it's probably an unlikely scenario, and your change certainly
looks like an improvement.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer