On Fri, 16 Dec 2011 19:31:20 +0800, "Zhigang Gong" <zhigang.gong at linux.intel.com> wrote: > > -----Original Message----- > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk] > > Sent: Friday, December 16, 2011 5:21 PM > > To: zhigang.gong at linux.intel.com > > Cc: intel-gfx at lists.freedesktop.org; zhigang.gong at gmail.com; Zhigang > > Gong > > Subject: Re: [PATCH] uxa/glamor: Fallback to new glamor pixmap if failed > > to create textured pixmap. > > > > On Fri, 16 Dec 2011 15:39:55 +0800, zhigang.gong at linux.intel.com > > wrote: > > > +fallback_glamor: > > > + /* Create textured pixmap failed means glamor failed to > > > + * create a texture from current BO for some reasons. We turn > > > + * to create a new glamor pixmap and clean up current one. > > > + * One thing need to be noted, this new pixmap doesn't > > > + * has a priv and bo attached to it. It's glamor's responsbility > > > + * to take care it. > > > + */ > > This then fails intel_uxa_is_offscreen() and we can no longer fallback to > > swrast correctly as uxa_prepare_access() becomes a no-op. > Right. IMO, this is not a big problem here, as in glamor this type > of pixmap will be marked as texture only pixmap and will never > fallback to DDX layer for any rendering operation. Can you add that to the comment? And make sure glamor creates the pixmap with a devPrivate.ptr=NULL so that any failure in future can be quickly found rather than cause random memory corruption. With the additional bit of explanation to quell my fears, Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> and please push. > > > + if (usage & INTEL_CREATE_PIXMAP_DRI2) { > > > + xf86DrvMsg(scrn->scrnIndex, X_WARNING, > > > + "Failed to create textured DRI2 pixmap."); > > > + return pixmap; > > At which point we really do need a better means for integrating glamor > > and UXA ops... > Actually, I haven't thought out a good idea for this case , say how to > handle such a BO > only pixmap in glamor. For almost all the other scenarios, we can easily > avoid > BO only pixmap by using a texture only pixmap or an in-memory pixmap. This > case is an exception. Right, though we do need to integrate modesetting with gbm as well. > And if I mesa/gbm can support all DRI format, then this case will also be > eliminated, as > It should never fail at glamor side. And if it failed we can just abort, as > that may indicates > a serious low level problem. s/abort/return NullPixmap/ ;-) I'd still just return a fbPixmap and try to fail gracefully with a loud warning in Xorg.log. > If we have to live with a BO only pixmap in glamor environment, one possible > fallback > method is to create another compatible BO for the same DRI2 pixmap. Then at > prepare > access glamor, we can copy the real BO's content to the compatible BO, and > at > finish access glamor, we can copy back the compatible BO's content to the > real DRI2 > buffer's BO. > > Does this way work? Yes, that would work. And you can make the copy lazy if need be by delaying the copy back to the bo until intel_flush_callback(). Alternatively, if glamor is using gbm to allocate its pixmaps then we retrieve the GEM name required for DRI2 from glamor. > But I really want the mesa/gbm to support all DRI2 formats and then.... And then we create bo-less glamor pixmaps by default. -Chris -- Chris Wilson, Intel Open Source Technology Centre