Michel Dänzer <[email protected]> writes: > From: Michel Dänzer <[email protected]> > > When a window moves from one CRTC to another, present_window_to_crtc_msc > updates window_priv->msc_offset according to the delta between the > current MSC values of the old and new CRTC: > > window_priv->msc_offset += new_msc - old_msc; > > window_priv->msc_offset is initially 0, so if new_msc < old_msc, > window_priv->msc_offset wraps around and becomes a large number. If the > window_msc parameter passed in is small (in particular if it's 0, such as > is the case when the client just wants to know the current window MSC > value), the returned CRTC MSC value may still be a large number. In that > case, the existing MSC comparisons in pixmap_present weren't working as > intended, resulting in scheduling a wait far into the future when the > target MSC had actually already passed. This would result in the client > (e.g. the Chromium browser) hanging when moving its window between CRTCs. > > In order to fix this, introduce msc_is_(equal_or_)after helper functions > which take the wraparound into account for comparing two MSC values. > > Signed-off-by: Michel Dänzer <[email protected]>
> ---
> present/present.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/present/present.c b/present/present.c
> index 66e0f21..8cf3b6f 100644
> --- a/present/present.c
> +++ b/present/present.c
> @@ -717,6 +717,28 @@ present_execute(present_vblank_ptr vblank, uint64_t ust,
> uint64_t crtc_msc)
> present_vblank_destroy(vblank);
> }
>
> +/*
> + * Returns:
> + * TRUE if the first MSC value is after the second one
> + * FALSE if the first MSC value is equal to or before the second one
> + */
> +static Bool
> +msc_is_after(uint64_t test, uint64_t reference)
> +{
> + return (int64_t)(test - reference) > 0;
> +}
> +
> +/*
> + * Returns:
> + * TRUE if the first MSC value is equal to or after the second one
> + * FALSE if the first MSC value is before the second one
> + */
> +static Bool
> +msc_is_equal_or_after(uint64_t test, uint64_t reference)
> +{
> + return (int64_t)(test - reference) >= 0;
> +}
> +
These should probably get declared as inline, although the compiler will
probably just inline them anyways.
I also won't be surprised if we end needing to expose these in a header
for other code to use too. Should we just do that now so that someone
needing to compare values finds them?
In any case; this all lgtm.
Reviewed-by: Keith Packard <[email protected]>
--
-keith
signature.asc
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
