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. > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
