[PATCH 02/13] glamor: Add glamor_program based copy acceleration
Eric Anholt
eric at anholt.net
Thu May 8 12:02:50 PDT 2014
Keith Packard <keithp at keithp.com> writes:
> Michel Dänzer <michel at daenzer.net> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140508/dd0fdc31/attachment.sig>
More information about the xorg-devel
mailing list