On 09.05.2014 10:56, Zhigang Gong wrote: > On Thu, May 08, 2014 at 12:02:50PM -0700, Eric Anholt wrote: >> Keith Packard <[email protected]> writes: >> >>> Michel Dänzer <[email protected]> writes: >>> >>>> On 06.05.2014 07:02, Keith Packard wrote: >>>>> >>>>> +static Bool >>>>> +glamor_copy_gl(DrawablePtr src, >>>>> + DrawablePtr dst, >>>>> + GCPtr gc, >>>>> + BoxPtr box, >>>>> + int nbox, >>>>> + int dx, >>>>> + int dy, >>>>> + Bool reverse, >>>>> + Bool upsidedown, >>>>> + Pixel bitplane, >>>>> + void *closure) >>>>> +{ >>>>> + PixmapPtr src_pixmap = glamor_get_drawable_pixmap(src); >>>>> + PixmapPtr dst_pixmap = glamor_get_drawable_pixmap(dst); >>>>> + glamor_pixmap_private *src_priv = >>>>> glamor_get_pixmap_private(src_pixmap); >>>>> + glamor_pixmap_private *dst_priv = >>>>> glamor_get_pixmap_private(dst_pixmap); >>>>> + >>>>> + if (GLAMOR_PIXMAP_PRIV_HAS_FBO(dst_priv)) { >>>>> + if (GLAMOR_PIXMAP_PRIV_HAS_FBO(src_priv)) { >>>>> + if (glamor_copy_needs_temp(src, dst, box, nbox, dx, dy)) >>>>> + return glamor_copy_fbo_fbo_temp(src, dst, gc, box, nbox, >>>>> dx, dy, >>>>> + reverse, upsidedown, >>>>> bitplane, closure); >>>>> + else >>>>> + return glamor_copy_fbo_fbo_draw(src, dst, gc, box, nbox, >>>>> dx, dy, >>>>> + reverse, upsidedown, >>>>> bitplane, closure); >>>>> + } >>>>> + if (bitplane == 0) >>>>> + return glamor_copy_cpu_fbo(src, dst, gc, box, nbox, dx, dy, >>>>> + reverse, upsidedown, bitplane, >>>>> closure); >>>>> + } >>>>> + return FALSE; >>>>> +} >>>> >>>> This results in a crash / memory corruption when confronted with >>>> GLAMOR_DRM_ONLY pixmaps. glamor_copy_bail calls down to fb, but the >>>> pixmap's devPrivate.ptr does not point to any usable storage. >>> >>> Any more hints here? The only way you can get to fb without a >>> devPrivate.ptr is if there is no FBO for the pixmap, and the pixmap >>> isn't just regular memory (like shm). I don't know how that would happen >>> for a DRM pixmap. >> >> Before, there was this code in copyarea's fallbacks: >> >> if (src_pixmap_priv->type == GLAMOR_DRM_ONLY >> || dst_pixmap_priv->type == GLAMOR_DRM_ONLY) { >> LogMessage(X_WARNING, >> "Access a DRM only pixmap is not allowed within >> glamor.\n"); >> return TRUE; >> } >> >> Which looks like a one-shot workaround. If we're going to be failing to >> render and complaining in the log but claiming success at rendering, we >> might want to be just doing an xnfcalloc with a warning in >> glamor_prepare_access() for DRM_ONLY, so we don't have to sprinkle this >> around every fallback. >> >> That said, why do GLAMOR_DRM_ONLY pixmaps even exist? From a quick tour >> of the glamor_egl.c paths that set_pixmap_type(pixmap, GLAMOR_DRM_ONLY) >> and the associated radeon code, it looks like a DRM_ONLY pixmap should >> be immediately getting thrown away to use a normal pixmap insted. > > IIRIC, the original reason of why GLAMOR_DRM_ONLY pixmap exist is that > sometimes > eglCreateImageKHR fails to create a valid eglImage from the external bo passed > in from the DDX driver. One example is a depth 16 DRI2 buffer request. At that > time, eglCreateImageKHR only support depth 24/32 and will fail with a depth 16 > colore format. Then glamor will fail to create a normal pixmap, and have to > set > it as a GLAMOR_DRM_ONLY pixmap to indicate it could not be accessed within > glamor scope. > > I'm not sure whether the situation is changed in the latest in-tree glamor > repo.
Not at all AFAICT; that's exactly the scenario I'm hitting. The problematic copy is for copying the contents of the original X pixmap to the new BO allocated for sharing via DRI2, in the driver's fixup_glamor() function. As such, allocating storage for the fallback wouldn't really help, as the result wouldn't be visible to clients via DRI2 anyway. However, I wonder if the driver couldn't use GetImage for this; I'll play with that. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
