Create drmmode_wait_vblank helper

Allows cleaning up the code considerably.

v2:
* Fix "drmWaiVBlank" typo, add blank line for readability (Slava Abramov)
* Rename in/out sequence parameters to "target_seq" and "result_seq",
  hopefully that will be clearer.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com> # v1

6 files changed, 85 insertions, 133 deletions

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 46a579ff..d86f8300 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -243,6 +243,41 @@ drmmode_ConvertToKMode(ScrnInfoPtr	scrn,
 }
 
 /*
+ * Utility helper for drmWaitVBlank
+ */
+Bool
+drmmode_wait_vblank(xf86CrtcPtr crtc, drmVBlankSeqType type,
+		    uint32_t target_seq, unsigned long signal, uint64_t *ust,
+		    uint32_t *result_seq)
+{
+	int crtc_id = drmmode_get_crtc_id(crtc);
+	ScrnInfoPtr scrn = crtc->scrn;
+	RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
+	drmVBlank vbl;
+
+	if (crtc_id == 1)
+		type |= DRM_VBLANK_SECONDARY;
+	else if (crtc_id > 1)
+		type |= (crtc_id << DRM_VBLANK_HIGH_CRTC_SHIFT) &
+			DRM_VBLANK_HIGH_CRTC_MASK;
+
+	vbl.request.type = type;
+	vbl.request.sequence = target_seq;
+	vbl.request.signal = signal;
+
+	if (drmWaitVBlank(pRADEONEnt->fd, &vbl) != 0)
+		return FALSE;
+
+	if (ust)
+		*ust = (uint64_t)vbl.reply.tval_sec * 1000000 +
+			vbl.reply.tval_usec;
+	if (result_seq)
+		*result_seq = vbl.reply.sequence;
+
+	return TRUE;
+}
+
+/*
  * Retrieves present time in microseconds that is compatible
  * with units used by vblank timestamps. Depending on the kernel
  * version and DRM kernel module configuration, the vblank
@@ -272,23 +307,15 @@ int drmmode_get_current_ust(int drm_fd, CARD64 *ust)
 int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
 {
     ScrnInfoPtr scrn = crtc->scrn;
-    RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
-    drmVBlank vbl;
-    int ret;
-
-    vbl.request.type = DRM_VBLANK_RELATIVE;
-    vbl.request.type |= radeon_populate_vbl_request_type(crtc);
-    vbl.request.sequence = 0;
+    uint32_t seq;
 
-    ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-    if (ret) {
+    if (!drmmode_wait_vblank(crtc, DRM_VBLANK_RELATIVE, 0, 0, ust, &seq)) {
 	xf86DrvMsg(scrn->scrnIndex, X_WARNING,
 		   "get vblank counter failed: %s\n", strerror(errno));
-	return ret;
+	return -1;
     }
 
-    *ust = ((CARD64)vbl.reply.tval_sec * 1000000) + vbl.reply.tval_usec;
-    *msc = vbl.reply.sequence;
+    *msc = seq;
 
     return Success;
 }
@@ -305,7 +332,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 	drmmode_crtc->pending_dpms_mode = mode;
 
 	if (drmmode_crtc->dpms_mode == DPMSModeOn && mode != DPMSModeOn) {
-		drmVBlank vbl;
+		uint32_t seq;
 
 		/* Wait for any pending flip to finish */
 		if (drmmode_crtc->flip_pending)
@@ -315,20 +342,14 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 		 * On->Off transition: record the last vblank time,
 		 * sequence number and frame period.
 		 */
-		vbl.request.type = DRM_VBLANK_RELATIVE;
-		vbl.request.type |= radeon_populate_vbl_request_type(crtc);
-		vbl.request.sequence = 0;
-		ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-		if (ret)
+		if (!drmmode_wait_vblank(crtc, DRM_VBLANK_RELATIVE, 0, 0, &ust,
+					 &seq))
 			xf86DrvMsg(scrn->scrnIndex, X_ERROR,
 				   "%s cannot get last vblank counter\n",
 				   __func__);
 		else {
-			CARD64 seq = (CARD64)vbl.reply.sequence;
 			CARD64 nominal_frame_rate, pix_in_frame;
 
-			ust = ((CARD64)vbl.reply.tval_sec * 1000000) +
-				vbl.reply.tval_usec;
 			drmmode_crtc->dpms_last_ust = ust;
 			drmmode_crtc->dpms_last_seq = seq;
 			nominal_frame_rate = crtc->mode.Clock;
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index f2104789..71d1c4a4 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -226,5 +226,10 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc);
 int drmmode_get_current_ust(int drm_fd, CARD64 *ust);
 
+Bool drmmode_wait_vblank(xf86CrtcPtr crtc, drmVBlankSeqType type,
+			 uint32_t target_seq, unsigned long signal,
+			 uint64_t *ust, uint32_t *result_seq);
+
+
 #endif
 
diff --git a/src/radeon.h b/src/radeon.h
index d815d3f0..b93e2816 100644
--- a/src/radeon.h
+++ b/src/radeon.h
@@ -693,8 +693,6 @@ extern void radeon_ddx_cs_start(ScrnInfoPtr pScrn,
 void radeon_kms_update_vram_limit(ScrnInfoPtr pScrn, uint32_t new_fb_size);
 extern RADEONEntPtr RADEONEntPriv(ScrnInfoPtr pScrn);
 
-drmVBlankSeqType radeon_populate_vbl_request_type(xf86CrtcPtr crtc);
-
 static inline struct radeon_surface *radeon_get_pixmap_surface(PixmapPtr pPix)
 {
 #ifdef USE_GLAMOR
diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index 80b17f1f..a0deb6ba 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -889,26 +889,6 @@ cleanup:
     radeon_dri2_frame_event_abort(crtc, event_data);
 }
 
-drmVBlankSeqType radeon_populate_vbl_request_type(xf86CrtcPtr crtc)
-{
-    drmVBlankSeqType type = 0;
-    int crtc_id = drmmode_get_crtc_id(crtc);
-
-    if (crtc_id == 1)
-        type |= DRM_VBLANK_SECONDARY;
-    else if (crtc_id > 1)
-#ifdef DRM_VBLANK_HIGH_CRTC_SHIFT
-	type |= (crtc_id << DRM_VBLANK_HIGH_CRTC_SHIFT) &
-		DRM_VBLANK_HIGH_CRTC_MASK;
-#else
-	ErrorF("radeon driver bug: %s called for CRTC %d > 1, but "
-	       "DRM_VBLANK_HIGH_CRTC_MASK not defined at build time\n",
-	       __func__, crtc_id);
-#endif
-
-    return type; 
-}
-
 /*
  * This function should be called on a disabled CRTC only (i.e., CRTC
  * in DPMS-off state). It will calculate the delay necessary to reach
@@ -1087,13 +1067,11 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
 {
     ScreenPtr screen = draw->pScreen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
     DRI2FrameEventPtr wait_info = NULL;
     uintptr_t drm_queue_seq = 0;
     xf86CrtcPtr crtc = radeon_dri2_drawable_crtc(draw, TRUE);
     uint32_t msc_delta;
-    drmVBlank vbl;
-    int ret;
+    uint32_t seq;
     CARD64 current_msc;
 
     /* Truncate to match kernel interfaces; means occasional overflow
@@ -1132,17 +1110,13 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
     }
 
     /* Get current count */
-    vbl.request.type = DRM_VBLANK_RELATIVE;
-    vbl.request.type |= radeon_populate_vbl_request_type(crtc);
-    vbl.request.sequence = 0;
-    ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-    if (ret) {
+    if (!drmmode_wait_vblank(crtc, DRM_VBLANK_RELATIVE, 0, 0, NULL, &seq)) {
         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                 "get vblank counter failed: %s\n", strerror(errno));
         goto out_complete;
     }
 
-    current_msc = vbl.reply.sequence + msc_delta;
+    current_msc = seq + msc_delta;
     current_msc &= 0xffffffff;
 
     drm_queue_seq = radeon_drm_queue_alloc(crtc, client, RADEON_DRM_QUEUE_ID_DEFAULT,
@@ -1169,12 +1143,9 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
          */
         if (current_msc >= target_msc)
             target_msc = current_msc;
-        vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
-	vbl.request.type |= radeon_populate_vbl_request_type(crtc);
-        vbl.request.sequence = target_msc - msc_delta;
-        vbl.request.signal = drm_queue_seq;
-        ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-        if (ret) {
+        if (!drmmode_wait_vblank(crtc, DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT,
+				 target_msc - msc_delta, drm_queue_seq, NULL,
+				 NULL)) {
             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                     "get vblank counter failed: %s\n", strerror(errno));
             goto out_complete;
@@ -1188,11 +1159,7 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
      * If we get here, target_msc has already passed or we don't have one,
      * so we queue an event that will satisfy the divisor/remainder equation.
      */
-    vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
-    vbl.request.type |= radeon_populate_vbl_request_type(crtc);
-
-    vbl.request.sequence = current_msc - (current_msc % divisor) +
-        remainder - msc_delta;
+    target_msc = current_msc - (current_msc % divisor) + remainder - msc_delta;
 
     /*
      * If calculated remainder is larger than requested remainder,
@@ -1201,11 +1168,10 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw,
      * that will happen.
      */
     if ((current_msc % divisor) >= remainder)
-        vbl.request.sequence += divisor;
+        target_msc += divisor;
 
-    vbl.request.signal = drm_queue_seq;
-    ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-    if (ret) {
+    if (!drmmode_wait_vblank(crtc, DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT,
+			     target_msc, drm_queue_seq, NULL, NULL)) {
         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                 "get vblank counter failed: %s\n", strerror(errno));
         goto out_complete;
@@ -1249,14 +1215,14 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 {
     ScreenPtr screen = draw->pScreen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
     xf86CrtcPtr crtc = radeon_dri2_drawable_crtc(draw, TRUE);
     uint32_t msc_delta;
-    drmVBlank vbl;
-    int ret, flip = 0;
+    drmVBlankSeqType type;
+    uint32_t seq;
+    int flip = 0;
     DRI2FrameEventPtr swap_info = NULL;
     uintptr_t drm_queue_seq;
-    CARD64 current_msc;
+    CARD64 current_msc, event_msc;
     BoxRec box;
     RegionRec region;
 
@@ -1319,18 +1285,14 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
     }
 
     /* Get current count */
-    vbl.request.type = DRM_VBLANK_RELATIVE;
-    vbl.request.type |= radeon_populate_vbl_request_type(crtc);
-    vbl.request.sequence = 0;
-    ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-    if (ret) {
+    if (!drmmode_wait_vblank(crtc, DRM_VBLANK_RELATIVE, 0, 0, NULL, &seq)) {
         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                 "first get vblank counter failed: %s\n",
                 strerror(errno));
 	goto blit_fallback;
     }
 
-    current_msc = vbl.reply.sequence + msc_delta;
+    current_msc = seq + msc_delta;
     current_msc &= 0xffffffff;
 
     /* Flips need to be submitted one frame before */
@@ -1352,14 +1314,13 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
      * the swap.
      */
     if (divisor == 0 || current_msc < *target_msc) {
-        vbl.request.type =  DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
+        type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
         /* If non-pageflipping, but blitting/exchanging, we need to use
          * DRM_VBLANK_NEXTONMISS to avoid unreliable timestamping later
          * on.
          */
         if (flip == 0)
-            vbl.request.type |= DRM_VBLANK_NEXTONMISS;
-	vbl.request.type |= radeon_populate_vbl_request_type(crtc);
+            type |= DRM_VBLANK_NEXTONMISS;
 
         /* If target_msc already reached or passed, set it to
          * current_msc to ensure we return a reasonable value back
@@ -1368,17 +1329,15 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
         if (current_msc >= *target_msc)
             *target_msc = current_msc;
 
-        vbl.request.sequence = *target_msc - msc_delta;
-        vbl.request.signal = drm_queue_seq;
-        ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-        if (ret) {
+        if (!drmmode_wait_vblank(crtc, type, *target_msc - msc_delta,
+				 drm_queue_seq, NULL, &seq)) {
             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                     "divisor 0 get vblank counter failed: %s\n",
                     strerror(errno));
 	    goto blit_fallback;
         }
 
-        *target_msc = vbl.reply.sequence + flip + msc_delta;
+        *target_msc = seq + flip + msc_delta;
         swap_info->frame = *target_msc;
 
         return TRUE;
@@ -1389,13 +1348,11 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
      * and we need to queue an event that will satisfy the divisor/remainder
      * equation.
      */
-    vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
+    type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
     if (flip == 0)
-        vbl.request.type |= DRM_VBLANK_NEXTONMISS;
-    vbl.request.type |= radeon_populate_vbl_request_type(crtc);
+        type |= DRM_VBLANK_NEXTONMISS;
 
-    vbl.request.sequence = current_msc - (current_msc % divisor) +
-        remainder - msc_delta;
+    event_msc = current_msc - (current_msc % divisor) + remainder - msc_delta;
 
     /*
      * If the calculated deadline vbl.request.sequence is smaller than
@@ -1408,15 +1365,13 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
      * into account, as well as a potential DRM_VBLANK_NEXTONMISS delay
      * if we are blitting/exchanging instead of flipping.
      */
-    if (vbl.request.sequence <= current_msc)
-        vbl.request.sequence += divisor;
+    if (event_msc <= current_msc)
+        event_msc += divisor;
 
     /* Account for 1 frame extra pageflip delay if flip > 0 */
-    vbl.request.sequence -= flip;
+    event_msc -= flip;
 
-    vbl.request.signal = drm_queue_seq;
-    ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-    if (ret) {
+    if (!drmmode_wait_vblank(crtc, type, event_msc, drm_queue_seq, NULL, &seq)) {
         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                 "final get vblank counter failed: %s\n",
                 strerror(errno));
@@ -1424,7 +1379,7 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
     }
 
     /* Adjust returned value for 1 fame pageflip offset of flip > 0 */
-    *target_msc = vbl.reply.sequence + flip + msc_delta;
+    *target_msc = seq + flip + msc_delta;
     *target_msc &= 0xffffffff;
     swap_info->frame = *target_msc;
 
diff --git a/src/radeon_kms.c b/src/radeon_kms.c
index 5ae94f5c..423d36cb 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -722,11 +722,9 @@ radeon_prime_scanout_update(PixmapDirtyUpdatePtr dirty)
 {
     ScreenPtr screen = dirty->slave_dst->drawable.pScreen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
     xf86CrtcPtr xf86_crtc = radeon_prime_dirty_to_crtc(dirty);
     drmmode_crtc_private_ptr drmmode_crtc;
     uintptr_t drm_queue_seq;
-    drmVBlank vbl;
 
     if (!xf86_crtc || !xf86_crtc->enabled)
 	return;
@@ -748,13 +746,10 @@ radeon_prime_scanout_update(PixmapDirtyUpdatePtr dirty)
 	return;
     }
 
-    vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
-    vbl.request.type |= radeon_populate_vbl_request_type(xf86_crtc);
-    vbl.request.sequence = 1;
-    vbl.request.signal = drm_queue_seq;
-    if (drmWaitVBlank(pRADEONEnt->fd, &vbl)) {
+    if (!drmmode_wait_vblank(xf86_crtc, DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
+			     1, drm_queue_seq, NULL, NULL)) {
 	xf86DrvMsg(scrn->scrnIndex, X_WARNING,
-		   "drmWaitVBlank failed for PRIME update: %s\n",
+		   "drmmode_wait_vblank failed for PRIME update: %s\n",
 		   strerror(errno));
 	radeon_drm_abort_entry(drm_queue_seq);
 	return;
@@ -1019,10 +1014,8 @@ static void
 radeon_scanout_update(xf86CrtcPtr xf86_crtc)
 {
     drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
-    RADEONEntPtr pRADEONEnt = RADEONEntPriv(xf86_crtc->scrn);
     uintptr_t drm_queue_seq;
     ScrnInfoPtr scrn;
-    drmVBlank vbl;
     DamagePtr pDamage;
     RegionPtr pRegion;
     BoxRec extents;
@@ -1059,13 +1052,10 @@ radeon_scanout_update(xf86CrtcPtr xf86_crtc)
 	return;
     }
 
-    vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
-    vbl.request.type |= radeon_populate_vbl_request_type(xf86_crtc);
-    vbl.request.sequence = 1;
-    vbl.request.signal = drm_queue_seq;
-    if (drmWaitVBlank(pRADEONEnt->fd, &vbl)) {
+    if (!drmmode_wait_vblank(xf86_crtc, DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
+			     1, drm_queue_seq, NULL, NULL)) {
 	xf86DrvMsg(scrn->scrnIndex, X_WARNING,
-		   "drmWaitVBlank failed for scanout update: %s\n",
+		   "drmmode_wait_vblank failed for scanout update: %s\n",
 		   strerror(errno));
 	radeon_drm_abort_entry(drm_queue_seq);
 	return;
diff --git a/src/radeon_present.c b/src/radeon_present.c
index c01ba95f..135a0a97 100644
--- a/src/radeon_present.c
+++ b/src/radeon_present.c
@@ -55,16 +55,6 @@ struct radeon_present_vblank_event {
     Bool unflip;
 };
 
-static uint32_t crtc_select(int crtc_id)
-{
-    if (crtc_id > 1)
-	return crtc_id << DRM_VBLANK_HIGH_CRTC_SHIFT;
-    else if (crtc_id > 0)
-	return DRM_VBLANK_SECONDARY;
-    else
-	return 0;
-}
-
 static RRCrtcPtr
 radeon_present_get_crtc(WindowPtr window)
 {
@@ -155,13 +145,8 @@ radeon_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
 {
     xf86CrtcPtr xf86_crtc = crtc->devPrivate;
     ScreenPtr screen = crtc->pScreen;
-    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
-    int crtc_id = drmmode_get_crtc_id(xf86_crtc);
     struct radeon_present_vblank_event *event;
     uintptr_t drm_queue_seq;
-    drmVBlank vbl;
-    int ret;
 
     event = calloc(sizeof(struct radeon_present_vblank_event), 1);
     if (!event)
@@ -177,12 +162,10 @@ radeon_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
 	return BadAlloc;
     }
 
-    vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | crtc_select(crtc_id);
-    vbl.request.sequence = msc;
-    vbl.request.signal = drm_queue_seq;
     for (;;) {
-	ret = drmWaitVBlank(pRADEONEnt->fd, &vbl);
-	if (!ret)
+	if (drmmode_wait_vblank(xf86_crtc,
+				DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT, msc,
+				drm_queue_seq, NULL, NULL))
 	    break;
 	if (errno != EBUSY || !radeon_present_flush_drm_events(screen)) {
 	    radeon_drm_abort_entry(drm_queue_seq);