[PATCH xserver] present: in get_ust_msc hook of wnmd provide CRTC as argument

[Olivier Fourdan]

Hi Roman,


On Thu, May 3, 2018 at 1:13 AM, Roman Gilg <subdiff at 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 <subdiff at 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 at 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 <ofourdan at redhat.com>

Cheers,
Olivier
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20180503/a8e7cc22/attachment.html>