Re: [PATCH] xf86-video-ati: vblank wait on crtc > 1

[Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx>]

On Sat, 19 Mar 2011, Alex Deucher wrote:

> On Fri, Mar 18, 2011 at 5:58 PM, Ilija Hadzic
> <ihadzic@xxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> Tested-by: Alex Deucher <alexdeucher@xxxxxxxxx>

Signed-off-by: Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx>



> > 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))) {
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel