From 93c2a59f989ea3bdc2e0d2aae18aa5fedc80661c Mon Sep 17 00:00:00 2001 From: Mario Kleiner Date: Tue, 23 Nov 2010 04:41:53 +0100 Subject: [PATCH 4/4] ddx/ati: Fix reporting of pageflip completion events on multi-head. When a drawable is page-flipped on multiple crtc's (fullscreen drawable on mirror-mode or multi-head x-screen), only one pageflip event is finally delivered, after the last participating crtc signals flip completion, this to avoid visual corruption. Old code returned vblank count and timestamps of flip completion of this last crtc, instead of the values of the "master crtc", the one that was used for initially scheduling/triggering the pagflip via vblank events. (master = radeon_dri2_drawable_crtc(drawable)) This patch makes sure that the pageflip completion values of the "master" crtc are returned, otherwise client applications will get confused by the random (msc, ust) values returned by whichever crtc was the last to complete its flip. Without this, the returned values change randomly and jump forward and backward in time and count. The patch also implements a consistency check on returned vblank count values of pageflip completion. Impossible values are detected, a x-warning is logged and returned (msc,ust) values are marked invalid, so clients could perform error handling. Such a warning would indicate bugs in the pageflip completion routine of future kms drivers or the ddx to aid driver debugging. Signed-off-by: Mario Kleiner --- src/drmmode_display.c | 42 ++++++++++++++++++++++++++++++++--- src/drmmode_display.h | 10 +++++++- src/radeon_dri2.c | 58 +++++++++++++++++++++++++++++++----------------- 3 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index a3981fa..30cda21 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -1278,18 +1278,32 @@ static void drmmode_flip_handler(int fd, unsigned int frame, unsigned int tv_sec, unsigned int tv_usec, void *event_data) { - drmmode_ptr drmmode = event_data; + drmmode_flipevtcarrier_ptr flipcarrier = event_data; + drmmode_ptr drmmode = flipcarrier->drmmode; + + /* Is this the event whose info shall be delivered to higher level? */ + if (flipcarrier->dispatch_me) { + /* Yes: Cache msc, ust for later delivery. */ + drmmode->fe_frame = frame; + drmmode->fe_tv_sec = tv_sec; + drmmode->fe_tv_usec = tv_usec; + } + free(flipcarrier); + /* Last crtc completed flip? */ drmmode->flip_count--; if (drmmode->flip_count > 0) return; + /* Release framebuffer */ drmModeRmFB(drmmode->fd, drmmode->old_fb_id); if (drmmode->event_data == NULL) return; - radeon_dri2_flip_event_handler(frame, tv_sec, tv_usec, drmmode->event_data); + /* Deliver cached msc, ust from reference crtc to flip event handler */ + radeon_dri2_flip_event_handler(drmmode->fe_frame, drmmode->fe_tv_sec, + drmmode->fe_tv_usec, drmmode->event_data); } @@ -1578,7 +1592,7 @@ void drmmode_uevent_fini(ScrnInfoPtr scrn, drmmode_ptr drmmode) #endif } -Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data) +Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data, int ref_crtc_hw_id) { RADEONInfoPtr info = RADEONPTR(scrn); xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn); @@ -1588,6 +1602,7 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *dat int i, old_fb_id; uint32_t tiling_flags = 0; int height; + drmmode_flipevtcarrier_ptr flipcarrier; if (info->allowColorTiling) { if (info->ChipFamily >= CHIP_FAMILY_R600) @@ -1617,6 +1632,10 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *dat * Also, flips queued on disabled or incorrectly configured displays * may never complete; this is a configuration error. */ + drmmode->fe_frame = 0; + drmmode->fe_tv_sec = 0; + drmmode->fe_tv_usec = 0; + for (i = 0; i < config->num_crtc; i++) { if (!config->crtc[i]->enabled) continue; @@ -1624,10 +1643,25 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *dat drmmode->event_data = data; drmmode->flip_count++; drmmode_crtc = config->crtc[i]->driver_private; + + flipcarrier = calloc(1, sizeof(drmmode_flipevtcarrier_rec)); + if (!flipcarrier) { + xf86DrvMsg(scrn->scrnIndex, X_WARNING, + "flip queue: carrier alloc failed.\n"); + goto error_undo; + } + + /* Only the reference crtc will finally deliver its page flip + * completion event. All other crtc's events will be discarded. + */ + flipcarrier->dispatch_me = (drmmode_crtc->hw_id == ref_crtc_hw_id); + flipcarrier->drmmode = drmmode; + if (drmModePageFlip(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, - drmmode->fb_id, DRM_MODE_PAGE_FLIP_EVENT, drmmode)) { + drmmode->fb_id, DRM_MODE_PAGE_FLIP_EVENT, flipcarrier)) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue failed: %s\n", strerror(errno)); + free(flipcarrier); goto error_undo; } } diff --git a/src/drmmode_display.h b/src/drmmode_display.h index c3d3561..548907b 100644 --- a/src/drmmode_display.h +++ b/src/drmmode_display.h @@ -52,9 +52,17 @@ typedef struct { drmEventContext event_context; int flip_count; void *event_data; + unsigned int fe_frame; + unsigned int fe_tv_sec; + unsigned int fe_tv_usec; } drmmode_rec, *drmmode_ptr; typedef struct { + drmmode_ptr drmmode; + Bool dispatch_me; +} drmmode_flipevtcarrier_rec, *drmmode_flipevtcarrier_ptr; + +typedef struct { drmmode_ptr drmmode; drmModeCrtcPtr mode_crtc; int hw_id; @@ -101,7 +109,7 @@ extern int drmmode_get_height_align(ScrnInfoPtr scrn, uint32_t tiling); extern int drmmode_get_pitch_align(ScrnInfoPtr scrn, int bpe, uint32_t tiling); extern int drmmode_get_base_align(ScrnInfoPtr scrn, int bpe, uint32_t tiling); -Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data); +Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data, int ref_crtc_hw_id); #endif diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c index c28017c..8b12872 100644 --- a/src/radeon_dri2.c +++ b/src/radeon_dri2.c @@ -571,6 +571,26 @@ radeon_dri2_unref_buffer(BufferPtr buffer) } } +static int radeon_dri2_drawable_crtc(DrawablePtr pDraw) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + xf86CrtcPtr crtc; + int crtc_id = -1; + + crtc = radeon_pick_best_crtc(pScrn, + pDraw->x, + pDraw->x + pDraw->width, + pDraw->y, + pDraw->y + pDraw->height); + + /* Make sure the CRTC is valid and this is the real front buffer */ + if (crtc != NULL && !crtc->rotatedData) { + crtc_id = drmmode_get_crtc_id(crtc); + } + return crtc_id; +} + static Bool radeon_dri2_schedule_flip(ScrnInfoPtr scrn, ClientPtr client, DrawablePtr draw, DRI2BufferPtr front, @@ -581,6 +601,9 @@ radeon_dri2_schedule_flip(ScrnInfoPtr scrn, ClientPtr client, struct radeon_exa_pixmap_priv *exa_priv; DRI2FrameEventPtr flip_info; + /* Main crtc for this drawable shall finally deliver pageflip event. */ + int ref_crtc_hw_id = radeon_dri2_drawable_crtc(draw); + flip_info = calloc(1, sizeof(DRI2FrameEventRec)); if (!flip_info) return FALSE; @@ -596,7 +619,8 @@ radeon_dri2_schedule_flip(ScrnInfoPtr scrn, ClientPtr client, /* Page flip the full screen buffer */ back_priv = back->driverPrivate; exa_priv = exaGetPixmapDriverPrivate(back_priv->pixmap); - return radeon_do_pageflip(scrn, exa_priv->bo, flip_info); + + return radeon_do_pageflip(scrn, exa_priv->bo, flip_info, ref_crtc_hw_id); } static Bool @@ -735,26 +759,6 @@ cleanup: free(event); } -static int radeon_dri2_drawable_crtc(DrawablePtr pDraw) -{ - ScreenPtr pScreen = pDraw->pScreen; - ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; - xf86CrtcPtr crtc; - int crtc_id = -1; - - crtc = radeon_pick_best_crtc(pScrn, - pDraw->x, - pDraw->x + pDraw->width, - pDraw->y, - pDraw->y + pDraw->height); - - /* Make sure the CRTC is valid and this is the real front buffer */ - if (crtc != NULL && !crtc->rotatedData) { - crtc_id = drmmode_get_crtc_id(crtc); - } - return crtc_id; -} - /* * Get current frame count and frame count timestamp, based on drawable's * crtc. @@ -952,6 +956,18 @@ void radeon_dri2_flip_event_handler(unsigned int frame, unsigned int tv_sec, /* We assume our flips arrive in order, so we don't check the frame */ switch (flip->type) { case DRI2_SWAP: + /* Check for too small vblank count of pageflip completion, taking wraparound + * into account. This usually means some defective kms pageflip completion, + * causing wrong (msc, ust) return values and possible visual corruption. + */ + if ((frame < flip->frame) && (flip->frame - frame < 5)) { + xf86DrvMsg(scrn->scrnIndex, X_WARNING, + "%s: Pageflip completion event has impossible msc %d < target_msc %d\n", + __func__, frame, flip->frame); + /* All-Zero values signal failure of (msc, ust) timestamping to client. */ + frame = tv_sec = tv_usec = 0; + } + DRI2SwapComplete(flip->client, drawable, frame, tv_sec, tv_usec, DRI2_FLIP_COMPLETE, flip->event_complete, flip->event_data); -- 1.7.1.1