On Sat, 19 Mar 2011, Alex Deucher wrote:
> On Fri, Mar 18, 2011 at 5:58 PM, Ilija Hadzic > <ihadzic at research.bell-labs.com> wrote: >> >> Hi Alex, >> >> Below is a patch against the master branch of xf86-video-ati that adds >> support for waits on vblank events on CRTCs that are greater than 1 (and >> thus cannot be represented using current primary/secondary flags interface). >> The patch makes use of GET_CAP ioctl to check whether >> vblanks on crtc > 1 are supported by the kernel. If they are not >> falls back to legacy behavior. Otherwise, it uses correct crtc numbers >> when waiting on vblank and thus corrects the problem seen in certain >> multiscreen configurations. >> >> The issue was discussed on the dri-devel list in these two threads >> >> http://lists.freedesktop.org/archives/dri-devel/2011-March/009009.html >> http://lists.freedesktop.org/archives/dri-devel/2011-March/009025.html >> >> regards, >> >> Ilija >> >> Reviewed-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de> >> Acked-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de> > > > Reviewed-by: Alex Deucher <alexdeucher at gmail.com> > Tested-by: Alex Deucher <alexdeucher at gmail.com> > Signed-off-by: Ilija Hadzic <ihadzic at research.bell-labs.com> >> >> diff --git a/src/radeon.h b/src/radeon.h >> index a6d20d7..1a746c7 100644 >> --- a/src/radeon.h >> +++ b/src/radeon.h >> @@ -931,6 +931,9 @@ typedef struct { >> >> ? ? RADEONFBLayout ? ?CurrentLayout; >> >> +#ifdef RADEON_DRI2 >> + ? ?Bool ? ? ? ? ? ? ?high_crtc_works; >> +#endif >> ?#ifdef XF86DRI >> ? ? Bool ? ? ? ? ? ? ?directRenderingEnabled; >> ? ? Bool ? ? ? ? ? ? ?directRenderingInited; >> diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c >> index 66df03c..ed27dad 100644 >> --- a/src/radeon_dri2.c >> +++ b/src/radeon_dri2.c >> @@ -783,6 +783,7 @@ static int radeon_dri2_get_msc(DrawablePtr draw, CARD64 >> *ust, CARD64 *msc) >> ? ? drmVBlank vbl; >> ? ? int ret; >> ? ? int crtc = radeon_dri2_drawable_crtc(draw); >> + ? ?int high_crtc = 0; >> >> ? ? /* Drawable not displayed, make up a value */ >> ? ? if (crtc == -1) { >> @@ -791,8 +792,16 @@ static int radeon_dri2_get_msc(DrawablePtr draw, CARD64 >> *ust, CARD64 *msc) >> ? ? ? ? return TRUE; >> ? ? } >> ? ? vbl.request.type = DRM_VBLANK_RELATIVE; >> - ? ?if (crtc > 0) >> + ? ?if (crtc == 1) >> ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?else if (crtc > 1) { >> + ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? } else >> + ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?} >> + ? ?vbl.request.type |= high_crtc; >> ? ? vbl.request.sequence = 0; >> >> ? ? ret = drmWaitVBlank(info->dri2.drm_fd, &vbl); >> @@ -825,6 +834,7 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr >> client, DrawablePtr draw, >> ? ? drmVBlank vbl; >> ? ? int ret, crtc = radeon_dri2_drawable_crtc(draw); >> ? ? CARD64 current_msc; >> + ? ?int high_crtc = 0; >> >> ? ? /* Truncate to match kernel interfaces; means occasional overflow >> ? ? ?* misses, but that's generally not a big deal */ >> @@ -855,8 +865,16 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr >> client, DrawablePtr draw, >> >> ? ? /* Get current count */ >> ? ? vbl.request.type = DRM_VBLANK_RELATIVE; >> - ? ?if (crtc > 0) >> + ? ?if (crtc == 1) >> ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?else if (crtc > 1) { >> + ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? } else >> + ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?} >> + ? ?vbl.request.type |= high_crtc; >> ? ? vbl.request.sequence = 0; >> ? ? ret = drmWaitVBlank(info->dri2.drm_fd, &vbl); >> ? ? if (ret) { >> @@ -882,8 +900,16 @@ 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; >> - ? ? ? ?if (crtc > 0) >> + ? ? ? ?if (crtc == 1) >> ? ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ? ? else if (crtc > 1) { >> + ? ? ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? ? ? } else >> + ? ? ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ? ? } >> + ? ? ? vbl.request.type |= high_crtc; >> ? ? ? ? vbl.request.sequence = target_msc; >> ? ? ? ? vbl.request.signal = (unsigned long)wait_info; >> ? ? ? ? ret = drmWaitVBlank(info->dri2.drm_fd, &vbl); >> @@ -903,8 +929,16 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr >> client, DrawablePtr draw, >> ? ? ?* so we queue an event that will satisfy the divisor/remainder >> equation. >> ? ? ?*/ >> ? ? vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT; >> - ? ?if (crtc > 0) >> + ? ?if (crtc == 1) >> ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?else if (crtc > 1) { >> + ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? } else >> + ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?} >> + ? ?vbl.request.type |= high_crtc; >> >> ? ? vbl.request.sequence = current_msc - (current_msc % divisor) + >> ? ? ? ? remainder; >> @@ -1029,6 +1063,7 @@ static int radeon_dri2_schedule_swap(ClientPtr client, >> DrawablePtr draw, >> ? ? CARD64 current_msc; >> ? ? BoxRec box; >> ? ? RegionRec region; >> + ? ?int high_crtc = 0; >> >> ? ? /* Truncate to match kernel interfaces; means occasional overflow >> ? ? ?* misses, but that's generally not a big deal */ >> @@ -1068,8 +1103,16 @@ static int radeon_dri2_schedule_swap(ClientPtr >> client, DrawablePtr draw, >> >> ? ? /* Get current count */ >> ? ? vbl.request.type = DRM_VBLANK_RELATIVE; >> - ? ?if (crtc > 0) >> + ? ?if (crtc == 1) >> ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?else if (crtc > 1) { >> + ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? } else >> + ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?} >> + ? ?vbl.request.type |= high_crtc; >> ? ? vbl.request.sequence = 0; >> ? ? ret = drmWaitVBlank(info->dri2.drm_fd, &vbl); >> ? ? if (ret) { >> @@ -1111,8 +1154,16 @@ static int radeon_dri2_schedule_swap(ClientPtr >> client, DrawablePtr draw, >> ? ? ? ? ?*/ >> ? ? ? ? if (flip == 0) >> ? ? ? ? ? ? vbl.request.type |= DRM_VBLANK_NEXTONMISS; >> - ? ? ? ?if (crtc > 0) >> + ? ? ? ?if (crtc == 1) >> ? ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ? ? else if (crtc > 1) { >> + ? ? ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? ? ? } else >> + ? ? ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ? ? } >> + ? ? ? vbl.request.type |= high_crtc; >> >> ? ? ? ? /* If target_msc already reached or passed, set it to >> ? ? ? ? ?* current_msc to ensure we return a reasonable value back >> @@ -1145,8 +1196,16 @@ static int radeon_dri2_schedule_swap(ClientPtr >> client, DrawablePtr draw, >> ? ? vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT; >> ? ? if (flip == 0) >> ? ? ? ? vbl.request.type |= DRM_VBLANK_NEXTONMISS; >> - ? ?if (crtc > 0) >> + ? ?if (crtc == 1) >> ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?else if (crtc > 1) { >> + ? ? ? if (info->high_crtc_works) { >> + ? ? ? ? ? high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) & >> + ? ? ? ? ? ? ? DRM_VBLANK_HIGH_CRTC_MASK; >> + ? ? ? } else >> + ? ? ? ? ? vbl.request.type |= DRM_VBLANK_SECONDARY; >> + ? ?} >> + ? ?vbl.request.type |= high_crtc; >> >> ? ? vbl.request.sequence = current_msc - (current_msc % divisor) + >> ? ? ? ? remainder; >> @@ -1217,6 +1276,7 @@ radeon_dri2_screen_init(ScreenPtr pScreen) >> ? ? DRI2InfoRec dri2_info = { 0 }; >> ?#ifdef USE_DRI2_SCHEDULING >> ? ? const char *driverNames[1]; >> + ? ?uint64_t cap_value; >> ?#endif >> >> ? ? if (!info->useEXA) { >> @@ -1248,6 +1308,7 @@ radeon_dri2_screen_init(ScreenPtr pScreen) >> ?#endif >> ? ? dri2_info.CopyRegion = radeon_dri2_copy_region; >> >> + ? ?info->high_crtc_works = FALSE; >> ?#ifdef USE_DRI2_SCHEDULING >> ? ? if (info->dri->pKernelDRMVersion->version_minor >= 4) { >> ? ? ? ? dri2_info.version = 4; >> @@ -1261,6 +1322,20 @@ radeon_dri2_screen_init(ScreenPtr pScreen) >> ? ? ? ? xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "You need a newer kernel for >> sync extension\n"); >> ? ? } >> >> + ? ?if (info->drmmode.mode_res->count_crtcs > 2) { >> + ? ? ? if (drmGetCap(info->dri2.drm_fd, DRM_CAP_HIGH_CRTC, &cap_value)) { >> + ? ? ? ? ? info->high_crtc_works = FALSE; >> + ? ? ? ? ? xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "You need a newer kernel >> for VBLANKs on CRTC>1\n"); >> + ? ? ? } else { >> + ? ? ? ? ? if (cap_value) { >> + ? ? ? ? ? ? ? info->high_crtc_works = TRUE; >> + ? ? ? ? ? } else { >> + ? ? ? ? ? ? ? xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Your kernel does >> not handle VBLANKs on CRTC>1\n"); >> + ? ? ? ? ? ? ? info->high_crtc_works = FALSE; >> + ? ? ? ? ? } >> + ? ? ? } >> + ? ?} >> + >> ? ? if (pRADEONEnt->dri2_info_cnt == 0) { >> ?#if HAS_DIXREGISTERPRIVATEKEY >> ? ? ? ?if (!dixRegisterPrivateKey(DRI2ClientEventsPrivateKey, >> PRIVATE_CLIENT, sizeof(DRI2ClientEventsRec))) { >> >
