On Thu, Aug 22, 2013 at 03:11:06PM -0700, Ian Romanick wrote: > On 07/17/2013 04:49 AM, Tomasz Lis wrote: > >From: Tomasz Lis <tomasz....@intel.com> > > > >glx: Creation of dummy X pixmap associated with float buffer. > > > >This change addresses the fact that float configs can be only used for > >pbuffers, > >and that 2D driver may not allow creation of an associated pixmap. > >It wouldn't be needed if 2D driver could always support 128 bpp buffers, > >but even then, there wouldn't be any point in allocating full-size associated > >pixmap which can never be used. > > Okay... I talked to Kristian about this code and your patch. He > confirmed my suspicion that this is not the right way to fix this, > but the bigger picture is quite different than my understanding. > > This code came to life with Mesa commit 5a43dbac. This added > support for direct-rendering pbuffers. We had previously supported > indirect-rendering pbuffers (in Mesa's libGL) for some time. When > Kristian implemented DRI2, we got direct rendering to pixmaps > essentially for free. To get pbuffers, he just allocated a pixmap > and called it a pbuffer. :) All of the pixmap rendering code would > "just work." > > In this mechanism, the pixmap is the storage for the pbuffer. As > such, allocated a 1bpp pixmap to hold 128bpp data is definitely > wrong. > > All of this occurred long before the invention of __DRIimage. > Kristian suggested that we modify the code here and in > src/glx/dri2_glx.c to use __DRIimage for pbuffers instead. There's > conceptually similar code in src/egl/drivers/dri2/platform_drm.c. > I've looked at the existing dri2_glx.c code, and this seems like a > plausible approach. > > Hopefully Kristian will reply with a more fleshed out proposal. :)
Yea, I think we're ready to drop the pixmap as backing for pbuffers. When I wrote the pbuffer code, using a pixmap as backing was an easy way to get pbuffers working and allowed us to share the getbuffer callback implementation between glx windows, pixmaps and pbuffers. Now we have DRI driver extensions to let us allocate buffers from the DRI loader, and with that we can implement buffer allocation for pbuffers in the loader without using X pixmaps. I've sketched out an implementation below, it doesn't compile, but should hopefully flesh out the idea a bit more. We stop creating the pixmap for pbuffers and pass None as the XID for the X drawable when we call into dri2_glx.c to create the DRI drawable. This also avoids creating the DRI2Drawable for the pbuffer. Then when the DRI driver calls out to dri2GetBuffersWithFormat() get color and aux buffers, we have to check if the xdrawable is None to see if it's a pbuffer, and handle buffer allocation locally. Local buffer allocation is something we already do in platform_drm.c and I've copied the relevant function and helpers into dri2_glx.c in the patch. They need to be massaged a bit to work in dri2_glx.c but the overall idea is the same. gbm_bo_create() ends up calling gbm_dri_bo_create() in src/gbm/backends/dri/gbm_dri.c... but I think we may even just be able to use get_aux_bo() for both color and aux buffers, but either way, the DRI driver needs to be modified to handle the new color format in the allocateBuffer functions or the createImage function. Kristian diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c index c54edac..81896b3 100644 --- a/src/glx/dri2_glx.c +++ b/src/glx/dri2_glx.c @@ -368,7 +368,7 @@ dri2DestroyDrawable(__GLXDRIdrawable *base) * knowing when the application is done with it. The server will * destroy the DRI2 drawable when it destroys the X drawable or the * client exits anyway. */ - if (pdraw->base.xDrawable != pdraw->base.drawable) + if (pdraw->base.xDrawable && pdraw->base.xDrawable != pdraw->base.drawable) DRI2DestroyDrawable(psc->base.dpy, pdraw->base.xDrawable); free(pdraw); @@ -413,7 +413,8 @@ dri2CreateDrawable(struct glx_screen *base, XID xDrawable, break; } - DRI2CreateDrawable(psc->base.dpy, xDrawable); + if (xDrawable) + DRI2CreateDrawable(psc->base.dpy, xDrawable); dpyPriv = __glXInitialize(psc->base.dpy); pdp = (struct dri2_display *)dpyPriv->dri2Display;; @@ -428,9 +429,10 @@ dri2CreateDrawable(struct glx_screen *base, XID xDrawable, return NULL; } - if (__glxHashInsert(pdp->dri2Hash, xDrawable, pdraw)) { + if (xDrawable && __glxHashInsert(pdp->dri2Hash, xDrawable, pdraw)) { (*psc->core->destroyDrawable) (pdraw->driDrawable); - DRI2DestroyDrawable(psc->base.dpy, xDrawable); + if (xDrawable) + DRI2DestroyDrawable(psc->base.dpy, xDrawable); free(pdraw); return None; } @@ -893,6 +895,123 @@ dri2GetBuffers(__DRIdrawable * driDrawable, return pdraw->buffers; } +/* get_back_bo(), get_aux_bo() and dri2PbufferGetBuffers() come from + * src/egl/drivers/dri2/platform_drm.c, which allocates color and aux buffers + * itself. I've not edited them to work with a struct dri2_drawable instead + * of a struct dri2_egl_surface, that needs to be done, but the general idea + * is the same. + * + * The 'locked' flag in get_back_bo() indicates that the gbm user is using the + * buffer (scanning it out, typically) and we can't use it. We don't have + * anything like that for pbuffers, we can just round-robin the buffers... are + * pbuffers even double buffered? */ + +static int +get_back_bo(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer) +{ + struct dri2_egl_display *dri2_dpy = + dri2_egl_display(dri2_surf->base.Resource.Display); + struct gbm_dri_bo *bo; + struct gbm_dri_surface *surf = dri2_surf->gbm_surf; + int i, name, pitch; + + if (dri2_surf->back == NULL) { + for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) { + if (!dri2_surf->color_buffers[i].locked) { + dri2_surf->back = &dri2_surf->color_buffers[i]; + break; + } + } + } + + if (dri2_surf->back == NULL) + return -1; + if (dri2_surf->back->bo == NULL) + dri2_surf->back->bo = gbm_bo_create(&dri2_dpy->gbm_dri->base.base, + surf->base.width, surf->base.height, + surf->base.format, surf->base.flags); + if (dri2_surf->back->bo == NULL) + return -1; + + bo = (struct gbm_dri_bo *) dri2_surf->back->bo; + + dri2_dpy->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_NAME, &name); + dri2_dpy->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_STRIDE, &pitch); + + buffer->attachment = __DRI_BUFFER_BACK_LEFT; + buffer->name = name; + buffer->pitch = pitch; + buffer->cpp = 4; + buffer->flags = 0; + + return 0; +} + +static int +get_aux_bo(struct dri2_egl_surface *dri2_surf, + unsigned int attachment, unsigned int format, __DRIbuffer *buffer) +{ + struct dri2_egl_display *dri2_dpy = + dri2_egl_display(dri2_surf->base.Resource.Display); + __DRIbuffer *b = dri2_surf->dri_buffers[attachment]; + + if (b == NULL) { + b = dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen, + attachment, format, + dri2_surf->base.Width, + dri2_surf->base.Height); + dri2_surf->dri_buffers[attachment] = b; + } + if (b == NULL) + return -1; + + memcpy(buffer, b, sizeof *buffer); + + return 0; +} + +static __DRIbuffer * +dri2PbufferGetBuffers(__DRIdrawable * driDrawable, + int *width, int *height, + unsigned int *attachments, int count, + int *out_count, void *loaderPrivate) +{ + struct dri2_drawable *pdraw = loaderPrivate; + DRI2Buffer *buffers; + int i, j; + + dri2_surf->buffer_count = 0; + for (i = 0, j = 0; i < 2 * count; i += 2, j++) { + assert(attachments[i] < __DRI_BUFFER_COUNT); + assert(dri2_surf->buffer_count < 5); + + switch (attachments[i]) { + case __DRI_BUFFER_BACK_LEFT: + if (get_back_bo(dri2_surf, &dri2_surf->buffers[j]) < 0) { + _eglError(EGL_BAD_ALLOC, "failed to allocate color buffer"); + return NULL; + } + break; + default: + if (get_aux_bo(dri2_surf, attachments[i], attachments[i + 1], + &dri2_surf->buffers[j]) < 0) { + _eglError(EGL_BAD_ALLOC, "failed to allocate aux buffer"); + return NULL; + } + break; + } + } + + *out_count = j; + if (j == 0) + return NULL; + + *width = dri2_surf->base.Width; + *height = dri2_surf->base.Height; + + return dri2_surf->buffers; +} + static __DRIbuffer * dri2GetBuffersWithFormat(__DRIdrawable * driDrawable, int *width, int *height, @@ -902,6 +1021,13 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable, struct dri2_drawable *pdraw = loaderPrivate; DRI2Buffer *buffers; + /* If xDrawable is None we're asked for buffers for a pbuffer. */ + if (pdraw->base.xDrawable = None) + return dri2PbufferGetBuffers(driDrawable, width, height, + attachments, count, out_count, + loaderPrivate); + + buffers = DRI2GetBuffersWithFormat(pdraw->base.psc->dpy, pdraw->base.xDrawable, width, height, attachments, diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c index f11305a..52039c9 100644 --- a/src/glx/glx_pbuffer.c +++ b/src/glx/glx_pbuffer.c @@ -554,10 +554,10 @@ CreatePbuffer(Display * dpy, struct glx_config *config, UnlockDisplay(dpy); SyncHandle(); - pixmap = XCreatePixmap(dpy, RootWindow(dpy, config->screen), - width, height, config->rgbBits); + /* Passing None as the X drawable ID indicates that we're createing a + * pbuffer. */ - if (!CreateDRIDrawable(dpy, config, pixmap, id, attrib_list, i)) { + if (!CreateDRIDrawable(dpy, config, None, id, attrib_list, i)) { CARD32 o = glx_1_3 ? X_GLXDestroyPbuffer : X_GLXvop_DestroyGLXPbufferSGIX; XFreePixmap(dpy, pixmap); protocolDestroyDrawable(dpy, id, o); _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev