[PATCH xf86-video-amdgpu] DRI2: Keep MSC monotonic when moving window between CRTCs

Alex Deucher alexdeucher at gmail.com
Fri Aug 14 12:50:11 PDT 2015


On Fri, Aug 14, 2015 at 5:44 AM, Michel Dänzer <michel at daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> This mirrors the DRI3 implementation in xserver. Fixes VDPAU video
> playback hanging when moving the window between CRTCs.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66384
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  src/amdgpu_dri2.c  | 202 +++++++++++++++++++++++++++++++++++------------------
>  src/amdgpu_video.c |   6 --
>  src/amdgpu_video.h |   1 -
>  3 files changed, 134 insertions(+), 75 deletions(-)
>
> diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c
> index bbf286c..cdef3f2 100644
> --- a/src/amdgpu_dri2.c
> +++ b/src/amdgpu_dri2.c
> @@ -65,6 +65,28 @@ struct dri2_buffer_priv {
>         unsigned int refcnt;
>  };
>
> +struct dri2_window_priv {
> +       xf86CrtcPtr crtc;
> +       int vblank_delta;
> +};
> +
> +#if HAS_DEVPRIVATEKEYREC
> +
> +static DevPrivateKeyRec dri2_window_private_key_rec;
> +#define dri2_window_private_key (&dri2_window_private_key_rec)
> +
> +#else
> +
> +static int dri2_window_private_key_index;
> +DevPrivateKey dri2_window_private_key = &dri2_window_private_key_index;
> +
> +#endif /* HAS_DEVPRIVATEKEYREC */
> +
> +#define get_dri2_window_priv(window) \
> +       ((struct dri2_window_priv*) \
> +        dixLookupPrivate(&(window)->devPrivates, dri2_window_private_key))
> +
> +
>  static PixmapPtr get_drawable_pixmap(DrawablePtr drawable)
>  {
>         if (drawable->type == DRAWABLE_PIXMAP)
> @@ -420,16 +442,80 @@ amdgpu_dri2_client_state_changed(CallbackListPtr * ClientStateCallback,
>         }
>  }
>
> +/*
> + * Get current frame count delta for the specified drawable and CRTC
> + */
> +static uint32_t amdgpu_get_msc_delta(DrawablePtr pDraw, xf86CrtcPtr crtc)
> +{
> +       drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +
> +       if (pDraw && pDraw->type == DRAWABLE_WINDOW)
> +               return drmmode_crtc->interpolated_vblanks +
> +                       get_dri2_window_priv((WindowPtr)pDraw)->vblank_delta;
> +
> +       return drmmode_crtc->interpolated_vblanks;
> +}
> +
> +/*
> + * Get current frame count and timestamp of the specified CRTC
> + */
> +static Bool amdgpu_dri2_get_crtc_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
> +{
> +       if (!amdgpu_crtc_is_enabled(crtc) ||
> +           drmmode_crtc_get_ust_msc(crtc, ust, msc) != Success) {
> +               /* CRTC is not running, extrapolate MSC and timestamp */
> +               drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +               ScrnInfoPtr scrn = crtc->scrn;
> +               AMDGPUInfoPtr info = AMDGPUPTR(scrn);
> +               CARD64 now, delta_t, delta_seq;
> +
> +               if (!drmmode_crtc->dpms_last_ust)
> +                       return FALSE;
> +
> +               if (drmmode_get_current_ust(info->dri2.drm_fd, &now) != 0) {
> +                       xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> +                                  "%s cannot get current time\n", __func__);
> +                       return FALSE;
> +               }
> +
> +               delta_t = now - drmmode_crtc->dpms_last_ust;
> +               delta_seq = delta_t * drmmode_crtc->dpms_last_fps;
> +               delta_seq /= 1000000;
> +               *ust = drmmode_crtc->dpms_last_ust;
> +               delta_t = delta_seq * 1000000;
> +               delta_t /= drmmode_crtc->dpms_last_fps;
> +               *ust += delta_t;
> +               *msc = drmmode_crtc->dpms_last_seq;
> +               *msc += delta_seq;
> +       }
> +
> +       return TRUE;
> +}
> +
>  static
>  xf86CrtcPtr amdgpu_dri2_drawable_crtc(DrawablePtr pDraw, Bool consider_disabled)
>  {
>         ScreenPtr pScreen = pDraw->pScreen;
>         ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
> +       xf86CrtcPtr crtc = amdgpu_pick_best_crtc(pScrn, consider_disabled,
> +                                                pDraw->x, pDraw->x + pDraw->width,
> +                                                pDraw->y, pDraw->y + pDraw->height);
> +
> +       if (crtc && pDraw->type == DRAWABLE_WINDOW) {
> +               struct dri2_window_priv *priv = get_dri2_window_priv((WindowPtr)pDraw);
> +
> +               if (priv->crtc && priv->crtc != crtc) {
> +                       CARD64 ust, mscold, mscnew;
>
> -       return amdgpu_pick_best_crtc(pScrn, consider_disabled,
> -                                    pDraw->x,
> -                                    pDraw->x + pDraw->width,
> -                                    pDraw->y, pDraw->y + pDraw->height);
> +                       amdgpu_dri2_get_crtc_msc(priv->crtc, &ust, &mscold);
> +                       amdgpu_dri2_get_crtc_msc(crtc, &ust, &mscnew);
> +                       priv->vblank_delta += mscold - mscnew;
> +               }
> +
> +               priv->crtc = crtc;
> +       }
> +
> +       return crtc;
>  }
>
>  static void
> @@ -456,7 +542,7 @@ amdgpu_dri2_flip_event_handler(ScrnInfoPtr scrn, uint32_t frame, uint64_t usec,
>
>         if (!flip->crtc)
>                 goto abort;
> -       frame += amdgpu_get_interpolated_vblanks(flip->crtc);
> +       frame += amdgpu_get_msc_delta(drawable, flip->crtc);
>
>         screen = scrn->pScreen;
>         pixmap = screen->GetScreenPixmap(screen);
> @@ -692,7 +778,7 @@ static void amdgpu_dri2_frame_event_handler(ScrnInfoPtr scrn, uint32_t seq,
>         if (status != Success)
>                 goto cleanup;
>
> -       seq += amdgpu_get_interpolated_vblanks(event->crtc);
> +       seq += amdgpu_get_msc_delta(drawable, event->crtc);
>
>         switch (event->type) {
>         case DRI2_FLIP:
> @@ -782,8 +868,6 @@ CARD32 amdgpu_dri2_extrapolate_msc_delay(xf86CrtcPtr crtc, CARD64 * target_msc,
>         int nominal_frame_rate = drmmode_crtc->dpms_last_fps;
>         CARD64 last_vblank_ust = drmmode_crtc->dpms_last_ust;
>         uint32_t last_vblank_seq = drmmode_crtc->dpms_last_seq;
> -       int interpolated_vblanks = drmmode_crtc->interpolated_vblanks;
> -       int target_seq;
>         CARD64 now, target_time, delta_t;
>         int64_t d, delta_seq;
>         int ret;
> @@ -800,16 +884,15 @@ CARD32 amdgpu_dri2_extrapolate_msc_delay(xf86CrtcPtr crtc, CARD64 * target_msc,
>                 *target_msc = 0;
>                 return FALLBACK_SWAP_DELAY;
>         }
> -       target_seq = (int)*target_msc - interpolated_vblanks;
> -       delta_seq = (int64_t) target_seq - (int64_t) last_vblank_seq;
> +       delta_seq = *target_msc - last_vblank_seq;
>         delta_seq *= 1000000;
>         target_time = last_vblank_ust;
>         target_time += delta_seq / nominal_frame_rate;
>         d = target_time - now;
>         if (d < 0) {
>                 /* we missed the event, adjust target_msc, do the divisor magic */
> -               CARD64 current_msc;
> -               current_msc = last_vblank_seq + interpolated_vblanks;
> +               CARD64 current_msc = last_vblank_seq;
> +
>                 delta_t = now - last_vblank_ust;
>                 delta_seq = delta_t * nominal_frame_rate;
>                 current_msc += delta_seq / 1000000;
> @@ -823,9 +906,7 @@ CARD32 amdgpu_dri2_extrapolate_msc_delay(xf86CrtcPtr crtc, CARD64 * target_msc,
>                         if ((current_msc % divisor) >= remainder)
>                                 *target_msc += divisor;
>                         *target_msc &= 0xffffffff;
> -                       target_seq = (int)*target_msc - interpolated_vblanks;
> -                       delta_seq =
> -                           (int64_t) target_seq - (int64_t) last_vblank_seq;
> +                       delta_seq = *target_msc - last_vblank_seq;
>                         delta_seq *= 1000000;
>                         target_time = last_vblank_ust;
>                         target_time += delta_seq / nominal_frame_rate;
> @@ -852,7 +933,6 @@ CARD32 amdgpu_dri2_extrapolate_msc_delay(xf86CrtcPtr crtc, CARD64 * target_msc,
>  static int amdgpu_dri2_get_msc(DrawablePtr draw, CARD64 * ust, CARD64 * msc)
>  {
>         xf86CrtcPtr crtc = amdgpu_dri2_drawable_crtc(draw, TRUE);
> -       int ret;
>
>         /* Drawable not displayed, make up a value */
>         if (crtc == NULL) {
> @@ -861,43 +941,12 @@ static int amdgpu_dri2_get_msc(DrawablePtr draw, CARD64 * ust, CARD64 * msc)
>                 return TRUE;
>         }
>
> -       if (amdgpu_crtc_is_enabled(crtc)) {
> -               /* CRTC is running, read vblank counter and timestamp */
> -               ret = drmmode_crtc_get_ust_msc(crtc, ust, msc);
> -               if (ret != Success)
> -                       return FALSE;
> -
> -               *msc += amdgpu_get_interpolated_vblanks(crtc);
> -               *msc &= 0xffffffff;
> -       } else {
> -               /* CRTC is not running, extrapolate MSC and timestamp */
> -               drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> -               ScrnInfoPtr scrn = crtc->scrn;
> -               AMDGPUInfoPtr info = AMDGPUPTR(scrn);
> -               CARD64 now, delta_t, delta_seq;
> -
> -               if (!drmmode_crtc->dpms_last_ust)
> -                       return FALSE;
> -               ret = drmmode_get_current_ust(info->dri2.drm_fd, &now);
> -               if (ret) {
> -                       xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> -                                  "%s cannot get current time\n", __func__);
> -                       return FALSE;
> -               }
> -               delta_t = now - drmmode_crtc->dpms_last_ust;
> -               delta_seq = delta_t * drmmode_crtc->dpms_last_fps;
> -               delta_seq /= 1000000;
> -               *ust = drmmode_crtc->dpms_last_ust;
> -               delta_t = delta_seq * 1000000;
> -               delta_t /= drmmode_crtc->dpms_last_fps;
> -               *ust += delta_t;
> -               *msc = drmmode_crtc->dpms_last_seq;
> -               *msc += drmmode_crtc->interpolated_vblanks;
> -               *msc += delta_seq;
> -               *msc &= 0xffffffff;
> -       }
> +       if (!amdgpu_dri2_get_crtc_msc(crtc, ust, msc))
> +               return FALSE;
>
> -       return ret == Success;
> +       *msc += amdgpu_get_msc_delta(draw, crtc);
> +       *msc &= 0xffffffff;
> +       return TRUE;
>  }
>
>  static
> @@ -985,6 +1034,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>         DRI2FrameEventPtr wait_info = NULL;
>         struct amdgpu_drm_queue_entry *wait = NULL;
>         xf86CrtcPtr crtc = amdgpu_dri2_drawable_crtc(draw, TRUE);
> +       uint32_t msc_delta;
>         drmVBlank vbl;
>         int ret;
>         CARD64 current_msc;
> @@ -999,6 +1049,8 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>         if (crtc == NULL)
>                 goto out_complete;
>
> +       msc_delta = amdgpu_get_msc_delta(draw, crtc);
> +
>         wait_info = calloc(1, sizeof(DRI2FrameEventRec));
>         if (!wait_info)
>                 goto out_complete;
> @@ -1014,6 +1066,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>          */
>         if (!amdgpu_crtc_is_enabled(crtc)) {
>                 CARD32 delay;
> +               target_msc -= msc_delta;
>                 delay = amdgpu_dri2_extrapolate_msc_delay(crtc, &target_msc,
>                                                           divisor, remainder);
>                 amdgpu_dri2_schedule_event(delay, wait_info);
> @@ -1032,8 +1085,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>                 goto out_complete;
>         }
>
> -       current_msc =
> -           vbl.reply.sequence + amdgpu_get_interpolated_vblanks(crtc);
> +       current_msc = vbl.reply.sequence + msc_delta;
>         current_msc &= 0xffffffff;
>
>         wait = amdgpu_drm_queue_alloc(scrn, client, AMDGPU_DRM_QUEUE_ID_DEFAULT,
> @@ -1062,8 +1114,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>                         target_msc = current_msc;
>                 vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
>                 vbl.request.type |= amdgpu_populate_vbl_request_type(crtc);
> -               vbl.request.sequence = target_msc;
> -               vbl.request.sequence -= amdgpu_get_interpolated_vblanks(crtc);
> +               vbl.request.sequence = target_msc - msc_delta;
>                 vbl.request.signal = (unsigned long)wait;
>                 ret = drmWaitVBlank(info->dri2.drm_fd, &vbl);
>                 if (ret) {
> @@ -1085,7 +1136,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>         vbl.request.type |= amdgpu_populate_vbl_request_type(crtc);
>
>         vbl.request.sequence = current_msc - (current_msc % divisor) +
> -           remainder;
> +           remainder - msc_delta;
>
>         /*
>          * If calculated remainder is larger than requested remainder,
> @@ -1095,7 +1146,6 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
>          */
>         if ((current_msc % divisor) >= remainder)
>                 vbl.request.sequence += divisor;
> -       vbl.request.sequence -= amdgpu_get_interpolated_vblanks(crtc);
>
>         vbl.request.signal = (unsigned long)wait;
>         ret = drmWaitVBlank(info->dri2.drm_fd, &vbl);
> @@ -1145,6 +1195,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>         ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
>         AMDGPUInfoPtr info = AMDGPUPTR(scrn);
>         xf86CrtcPtr crtc = amdgpu_dri2_drawable_crtc(draw, TRUE);
> +       uint32_t msc_delta;
>         drmVBlank vbl;
>         int ret, flip = 0;
>         DRI2FrameEventPtr swap_info = NULL;
> @@ -1170,6 +1221,8 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>         if (crtc == NULL)
>                 goto blit_fallback;
>
> +       msc_delta = amdgpu_get_msc_delta(draw, crtc);
> +
>         swap_info = calloc(1, sizeof(DRI2FrameEventRec));
>         if (!swap_info)
>                 goto blit_fallback;
> @@ -1200,8 +1253,11 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>          */
>         if (!amdgpu_crtc_is_enabled(crtc)) {
>                 CARD32 delay;
> +               *target_msc -= msc_delta;
>                 delay = amdgpu_dri2_extrapolate_msc_delay(crtc, target_msc,
>                                                           divisor, remainder);
> +               *target_msc += msc_delta;
> +               *target_msc &= 0xffffffff;
>                 amdgpu_dri2_schedule_event(delay, swap_info);
>                 return TRUE;
>         }
> @@ -1218,8 +1274,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>                 goto blit_fallback;
>         }
>
> -       current_msc =
> -           vbl.reply.sequence + amdgpu_get_interpolated_vblanks(crtc);
> +       current_msc = vbl.reply.sequence + msc_delta;
>         current_msc &= 0xffffffff;
>
>         /* Flips need to be submitted one frame before */
> @@ -1257,8 +1312,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>                 if (current_msc >= *target_msc)
>                         *target_msc = current_msc;
>
> -               vbl.request.sequence = *target_msc;
> -               vbl.request.sequence -= amdgpu_get_interpolated_vblanks(crtc);
> +               vbl.request.sequence = *target_msc - msc_delta;
>                 vbl.request.signal = (unsigned long)swap;
>                 ret = drmWaitVBlank(info->dri2.drm_fd, &vbl);
>                 if (ret) {
> @@ -1268,8 +1322,8 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>                         goto blit_fallback;
>                 }
>
> -               *target_msc = vbl.reply.sequence + flip;
> -               *target_msc += amdgpu_get_interpolated_vblanks(crtc);
> +               *target_msc = vbl.reply.sequence + flip + msc_delta;
> +               *target_msc &= 0xffffffff;
>                 swap_info->frame = *target_msc;
>
>                 return TRUE;
> @@ -1286,7 +1340,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>         vbl.request.type |= amdgpu_populate_vbl_request_type(crtc);
>
>         vbl.request.sequence = current_msc - (current_msc % divisor) +
> -           remainder;
> +           remainder - msc_delta;
>
>         /*
>          * If the calculated deadline vbl.request.sequence is smaller than
> @@ -1301,7 +1355,6 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>          */
>         if (vbl.request.sequence <= current_msc)
>                 vbl.request.sequence += divisor;
> -       vbl.request.sequence -= amdgpu_get_interpolated_vblanks(crtc);
>
>         /* Account for 1 frame extra pageflip delay if flip > 0 */
>         vbl.request.sequence -= flip;
> @@ -1316,8 +1369,8 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
>         }
>
>         /* Adjust returned value for 1 fame pageflip offset of flip > 0 */
> -       *target_msc = vbl.reply.sequence + flip;
> -       *target_msc += amdgpu_get_interpolated_vblanks(crtc);
> +       *target_msc = vbl.reply.sequence + flip + msc_delta;
> +       *target_msc &= 0xffffffff;
>         swap_info->frame = *target_msc;
>
>         return TRUE;
> @@ -1400,6 +1453,19 @@ Bool amdgpu_dri2_screen_init(ScreenPtr pScreen)
>                 driverNames[0] = driverNames[1] = dri2_info.driverName;
>
>                 if (DRI2InfoCnt == 0) {
> +#if HAS_DIXREGISTERPRIVATEKEY
> +                       if (!dixRegisterPrivateKey(dri2_window_private_key,
> +                                                  PRIVATE_WINDOW,
> +                                                  sizeof(struct dri2_window_priv))) {
> +#else
> +                       if (!dixRequestPrivate(dri2_window_private_key,
> +                                              sizeof(struct dri2_window_priv))) {
> +#endif
> +                               xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
> +                                          "Failed to get DRI2 window private\n");
> +                               return FALSE;
> +                       }
> +
>                         AddCallback(&ClientStateCallback,
>                                     amdgpu_dri2_client_state_changed, 0);
>                 }
> diff --git a/src/amdgpu_video.c b/src/amdgpu_video.c
> index 2a5c3a0..95a75e9 100644
> --- a/src/amdgpu_video.c
> +++ b/src/amdgpu_video.c
> @@ -70,12 +70,6 @@ Bool amdgpu_crtc_is_enabled(xf86CrtcPtr crtc)
>         return drmmode_crtc->dpms_mode == DPMSModeOn;
>  }
>
> -uint32_t amdgpu_get_interpolated_vblanks(xf86CrtcPtr crtc)
> -{
> -       drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> -       return drmmode_crtc->interpolated_vblanks;
> -}
> -
>  xf86CrtcPtr
>  amdgpu_pick_best_crtc(ScrnInfoPtr pScrn, Bool consider_disabled,
>                       int x1, int x2, int y1, int y2)
> diff --git a/src/amdgpu_video.h b/src/amdgpu_video.h
> index 2915e3a..d03b935 100644
> --- a/src/amdgpu_video.h
> +++ b/src/amdgpu_video.h
> @@ -7,6 +7,5 @@
>  #include "xf86Crtc.h"
>
>  Bool amdgpu_crtc_is_enabled(xf86CrtcPtr crtc);
> -uint32_t amdgpu_get_interpolated_vblanks(xf86CrtcPtr crtc);
>
>  #endif /* __AMDGPU_VIDEO_H__ */
> --
> 2.5.0
>
> _______________________________________________
> xorg-driver-ati mailing list
> xorg-driver-ati at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-driver-ati


More information about the xorg-driver-ati mailing list