Marcin Slusarz <[email protected]> writes: > On Thu, May 03, 2012 at 03:15:51PM +0200, Francisco Jerez wrote: >> Marcin Slusarz <[email protected]> writes: >> >> > Port of commit ae45d7e6d8e6844cd4586c9ee97c21b257fa788f in xf86-video-ati. >> > >> > Fixes https://bugs.freedesktop.org/show_bug.cgi?id=49351 >> > >> > (Additionally, don't try to pageflip if user disabled it in xorg.conf. >> > Currently this change is a no-op, because can_exchange returns true only >> > when >> > page flipping is enabled, but commit >> > 169512fbe91f0671a90dfee5e280357f0a4ef701 - >> > which changed can_exchange behavior - is due to be reverted) >> > --- >> > src/nouveau_dri2.c | 3 ++- >> > 1 files changed, 2 insertions(+), 1 deletions(-) >> > >> > diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c >> > index 588735f..3d8d22f 100644 >> > --- a/src/nouveau_dri2.c >> > +++ b/src/nouveau_dri2.c >> > @@ -328,7 +328,8 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned >> > int frame, >> > type = DRI2_EXCHANGE_COMPLETE; >> > DamageRegionAppend(draw, ®); >> > >> > - if (DRI2CanFlip(draw)) { >> > + if (DRI2CanFlip(draw) && pNv->has_pageflip && >> > + draw->type == DRAWABLE_WINDOW) { >> >> Hey, >> >> How about 'if (nouveau_exa_pixmap_is_onscreen(dst_pix)) {...'? We >> should really never get to that point unless we know for sure that we >> can either flip or exchange, so the 'has_pageflip' check is redundant. > > I see your point, but it's all non obvious. How about this patch instead? > :) > I guess the confusion stems from the meaning of "exchange": Do you exchange as well as you flip or does one exclude the other? This code only (used to) make sense if you read it the first way, i.e. as if "flip" were the subset of "exchange" you use with on-screen windows. :P
That said, I'm OK with changing the name of "can_exchange" to "can_exchange_or_flip" if that makes its meaning clearer. TBH the rest of the changes in this patch don't really look to me like they're making anything easier to understand. That's quite a personal matter though. > --- > From: Marcin Slusarz <[email protected]> > Subject: [PATCH] dri2: don't try to page flip pixmaps > > Based on commit ae45d7e6d8e6844cd4586c9ee97c21b257fa788f in xf86-video-ati. > > Fixes https://bugs.freedesktop.org/show_bug.cgi?id=49351 > --- > src/nouveau_dri2.c | 31 ++++++++++++++++++++----------- > 1 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c > index 588735f..86039ef 100644 > --- a/src/nouveau_dri2.c > +++ b/src/nouveau_dri2.c > @@ -178,12 +178,14 @@ update_front(DrawablePtr draw, DRI2BufferPtr front) > } > > static Bool > -can_exchange(DrawablePtr draw, PixmapPtr dst_pix, PixmapPtr src_pix) > +can_exchange_or_flip(DrawablePtr draw, PixmapPtr dst_pix, PixmapPtr src_pix, > + Bool *flip) > { > ScrnInfoPtr scrn = xf86Screens[draw->pScreen->myNum]; > xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); > NVPtr pNv = NVPTR(scrn); > int i; > + Bool dst_is_screen; > > for (i = 0; i < xf86_config->num_crtc; i++) { > xf86CrtcPtr crtc = xf86_config->crtc[i]; > @@ -192,11 +194,17 @@ can_exchange(DrawablePtr draw, PixmapPtr dst_pix, > PixmapPtr src_pix) > > } > > - return ((DRI2CanFlip(draw) && pNv->has_pageflip)) && > - dst_pix->drawable.width == src_pix->drawable.width && > - dst_pix->drawable.height == src_pix->drawable.height && > - dst_pix->drawable.bitsPerPixel == > src_pix->drawable.bitsPerPixel && > - dst_pix->devKind == src_pix->devKind; > + if (dst_pix->drawable.width != src_pix->drawable.width || > + dst_pix->drawable.height != src_pix->drawable.height || > + dst_pix->drawable.bitsPerPixel != > src_pix->drawable.bitsPerPixel || > + dst_pix->devKind != src_pix->devKind) > + return FALSE; > + > + dst_is_screen = nouveau_exa_pixmap_is_onscreen(dst_pix); > + *flip = dst_is_screen && pNv->has_pageflip && DRI2CanFlip(draw); > + > + /* Exchange path is currently disabled, see fdo bug 35930 */ > + return *flip /*|| !dst_is_screen*/; > } > > static Bool > @@ -280,7 +288,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int > frame, > struct nouveau_pushbuf *push = pNv->pushbuf; > RegionRec reg; > int type, ret; > - Bool front_updated; > + Bool front_updated, flip; > > REGION_INIT(0, ®, (&(BoxRec){ 0, 0, draw->width, draw->height }), 0); > REGION_TRANSLATE(0, ®, draw->x, draw->y); > @@ -324,17 +332,18 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int > frame, > nouveau_pushbuf_kick(push, push->channel); > } > > - if (front_updated && can_exchange(draw, dst_pix, src_pix)) { > - type = DRI2_EXCHANGE_COMPLETE; > + if (front_updated && can_exchange_or_flip(draw, dst_pix, src_pix, > &flip)) { > DamageRegionAppend(draw, ®); > > - if (DRI2CanFlip(draw)) { > + if (flip) { > type = DRI2_FLIP_COMPLETE; > ret = drmmode_page_flip(draw, src_pix, > violate_oml(draw) ? NULL : s, > ref_crtc_hw_id); > if (!ret) > goto out; > + } else { > + type = DRI2_EXCHANGE_COMPLETE; > } > > SWAP(s->dst->name, s->src->name); > @@ -343,7 +352,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int > frame, > DamageRegionProcessPending(draw); > > /* If it is a page flip, finish it in the flip event handler. */ > - if ((type == DRI2_FLIP_COMPLETE) && !violate_oml(draw)) > + if (flip && !violate_oml(draw)) > return; > } else { > type = DRI2_BLIT_COMPLETE;
pgpcLkoFMSCLM.pgp
Description: PGP signature
_______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
