Hi Roman,
On Thu, May 3, 2018 at 1:13 AM, Roman Gilg <subd...@gmail.com> wrote:
> For backwards compatibility a driver supporting window flip mode still must
> calculate msc values per CRTC.
>
> It is important that the driver returns the msc for the CRTC that Present
> requests the msc for or return an error if this is not possible.
>
> This way Present can calculate the offset correctly in
> present_wnmd_window_to_crtc_msc.
>
> Signed-off-by: Roman Gilg <subd...@gmail.com>
> ---
> hw/xwayland/xwayland-present.c | 10 +++++++++-
> present/present.h | 2 +-
> present/present_wnmd.c | 14 +++++++++-----
> 3 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-
> present.c
> index 66bfaae..adc3a68 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -258,11 +258,19 @@ xwl_present_get_crtc(WindowPtr present_window)
> }
>
> static int
> -xwl_present_get_ust_msc(WindowPtr present_window, uint64_t *ust,
> uint64_t *msc)
> +xwl_present_get_ust_msc(WindowPtr present_window, RRCrtcPtr crtc,
> uint64_t *ust, uint64_t *msc)
> {
> struct xwl_window *xwl_window = xwl_window_from_window(
> present_window);
> if (!xwl_window)
> return BadAlloc;
> +
> + if (xwl_window->present_crtc_fake != crtc) {
> + /* the crtc changed between the last call and this one,
> + * falls back to using the saved window msc in Present
> + */
> + return BadMatch;
> + }
> +
> *ust = xwl_window->present_ust;
> *msc = xwl_window->present_msc;
>
> diff --git a/present/present.h b/present/present.h
> index 3d0b972..0a3682c 100644
> --- a/present/present.h
> +++ b/present/present.h
> @@ -41,7 +41,7 @@ typedef RRCrtcPtr (*present_get_crtc_ptr) (WindowPtr
> window);
> /* Return the current ust/msc for 'crtc'
> */
> typedef int (*present_get_ust_msc_ptr) (RRCrtcPtr crtc, uint64_t *ust,
> uint64_t *msc);
> -typedef int (*present_wnmd_get_ust_msc_ptr) (WindowPtr window, uint64_t
> *ust, uint64_t *msc);
> +typedef int (*present_wnmd_get_ust_msc_ptr) (WindowPtr window, RRCrtcPtr
> crtc, uint64_t *ust, uint64_t *msc);
>
Unfortunately, this is an API/ABI change, I am not sure this is possible at
this point in time wrt the release even though present_wnmd*() is
presumably used only by Xwayland at the moment if I understand correctly.
I'll leave that to Adam to decide.
> /* Queue callback on 'crtc' for time 'msc'. Call present_event_notify
> with 'event_id'
> * at or after 'msc'. Return false if it didn't happen (which might occur
> if 'crtc'
> diff --git a/present/present_wnmd.c b/present/present_wnmd.c
> index 80ffb01..c95bc92 100644
> --- a/present/present_wnmd.c
> +++ b/present/present_wnmd.c
> @@ -61,10 +61,10 @@ present_wnmd_get_crtc(present_screen_priv_ptr
> screen_priv, WindowPtr window)
> }
>
> static int
> -present_wnmd_get_ust_msc(ScreenPtr screen, WindowPtr window, uint64_t
> *ust, uint64_t *msc)
> +present_wnmd_get_ust_msc(ScreenPtr screen, WindowPtr window, RRCrtcPtr
> crtc, uint64_t *ust, uint64_t *msc)
> {
> present_screen_priv_ptr screen_priv = present_screen_priv(screen);
> - return (*screen_priv->wnmd_info->get_ust_msc)(window, ust, msc);
> + return (*screen_priv->wnmd_info->get_ust_msc)(window, crtc, ust,
> msc);
> }
>
> /*
> @@ -76,7 +76,7 @@ present_wnmd_re_execute(present_vblank_ptr vblank)
> {
> uint64_t ust = 0, crtc_msc = 0;
>
> - (void) present_wnmd_get_ust_msc(vblank->screen, vblank->window,
> &ust, &crtc_msc);
> + (void) present_wnmd_get_ust_msc(vblank->screen, vblank->window,
> vblank->crtc, &ust, &crtc_msc);
> present_wnmd_execute(vblank, ust, crtc_msc);
> }
>
> @@ -518,6 +518,7 @@ present_wnmd_window_to_crtc_msc(WindowPtr window,
> RRCrtcPtr crtc, uint64_t windo
> present_window_priv_ptr window_priv = present_get_window_priv(window,
> TRUE);
>
> if (crtc != window_priv->crtc) {
> + uint64_t old_ust, old_msc;
> if (window_priv->crtc == PresentCrtcNeverSet) {
> window_priv->msc_offset = 0;
> } else {
> @@ -525,7 +526,10 @@ present_wnmd_window_to_crtc_msc(WindowPtr window,
> RRCrtcPtr crtc, uint64_t windo
> * we'll just use whatever previous MSC we'd seen from this
> CRTC
> */
>
> - window_priv->msc_offset += new_msc - window_priv->msc;
> + if (present_wnmd_get_ust_msc(window->drawable.pScreen,
> window, window_priv->crtc, &old_ust, &old_msc) != Success)
> + old_msc = window_priv->msc;
> +
> + window_priv->msc_offset += new_msc - old_msc;
> }
> window_priv->crtc = crtc;
> }
> @@ -565,7 +569,7 @@ present_wnmd_pixmap(WindowPtr window,
>
> target_crtc = present_wnmd_get_crtc(screen_priv, window);
>
> - ret = present_wnmd_get_ust_msc(screen, window, &ust, &crtc_msc);
> + ret = present_wnmd_get_ust_msc(screen, window, target_crtc, &ust,
> &crtc_msc);
>
> target_msc = present_wnmd_window_to_crtc_msc(window, target_crtc,
> window_msc, crtc_msc);
>
> --
> 2.7.4
>
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
Regardless of the API/ABI change, this patch fixes the issue in the
reproducer, so:
Tested-by: Olivier Fourdan <ofour...@redhat.com>
Cheers,
Olivier
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
