On Mon, Nov 11, 2013 at 1:57 PM, Chad Versace <chad.vers...@linux.intel.com> wrote: > On 11/11/2013 01:22 PM, Kristian Høgsberg wrote: >> >> Drivers that only call getBuffers to request color buffers can use these >> generic, __DRIimage based helpers to implement the allocBuffer and >> releaseBuffer functions of __DRIdri2Extension. >> >> For the intel dri driver, this consolidates window system color buffer >> allocation in intel_create_image(). >> >> Signed-off-by: Kristian Høgsberg <k...@bitplanet.net> >> --- >> src/mesa/drivers/dri/common/dri_util.c | 75 >> ++++++++++++++++++++++++++++++++ >> src/mesa/drivers/dri/common/dri_util.h | 10 +++++ >> src/mesa/drivers/dri/i915/intel_screen.c | 56 +----------------------- >> src/mesa/drivers/dri/i965/intel_screen.c | 55 +---------------------- >> 4 files changed, 89 insertions(+), 107 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/common/dri_util.c >> b/src/mesa/drivers/dri/common/dri_util.c >> index 86cf24c..a7328e4 100644 >> --- a/src/mesa/drivers/dri/common/dri_util.c >> +++ b/src/mesa/drivers/dri/common/dri_util.c > > > >> +__DRIbuffer * >> +driAllocateBuffer(__DRIscreen *screen, >> + unsigned attachment, unsigned format, >> + int width, int height) >> +{ >> + struct dri_image_buffer *buffer; >> + __DRIimageExtension *image = screen->image_extension; >> + int dri_format, name, stride; >> + >> + assert(attachment == __DRI_BUFFER_FRONT_LEFT || >> + attachment == __DRI_BUFFER_BACK_LEFT); >> + >> + /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches >> + * format / 8. The image format code is stored in the __DRIimage, but >> the >> + * __DRIbuffer we create from the image, only stores the cpp. */ >> + >> + switch (format) { >> + case 32: >> + dri_format = __DRI_IMAGE_FORMAT_XRGB8888; >> + break; >> + case 16: >> + dri_format = __DRI_IMAGE_FORMAT_RGB565; >> + break; >> + default: >> + return NULL; >> + } >> + >> + buffer = calloc(1, sizeof *buffer); >> + if (buffer == NULL) >> + return NULL; >> + >> + buffer->image = image->createImage(screen, >> + width, height, dri_format, >> + __DRI_IMAGE_USE_SHARE | >> + __DRI_IMAGE_USE_SCANOUT, >> + buffer); > > > It's incorrect to specify __DRI_IMAGE_USE_SCANOUT regardless of > attachment type. GBM, Wayland, and Android use driAllocateBuffer > to allocate more than just the scanout buffer. They use it to > allocate auxillary buffers too. If i965 were to respect the > caching restrictions implied by __DRI_IMAGE_USE_SCANOUT, its > use for aux buffers would hurt performance. (As far as I can > tell, though, intel_create_image() wrongly ignores __DRI_IMAGE_USE_SCANOUT). > > Instead, I think you should set the USE_SCANOUT bit if and only if > attachment is one of __DRI_BUFFER_(BACK|FRONT)_(LEFT|RIGHT).
The commit message says: "Drivers that only call getBuffers to request color buffers can use these generic, __DRIimage based helpers to implement the allocBuffer and releaseBuffer functions of __DRIdri2Extension." which is true for the Intel DRI driver. The DRI2 interface allows the driver to ask for auxillary buffers, but since we stopped supporting multiple processes rendering to the same X window, our driver no longer does that. This is under driver control and thus if a driver knows it will never ask for aux buffers, it can use these helpers. Kristian > >> + >> + if (buffer->image == NULL) { >> + free(buffer); >> + return NULL; >> + } >> + >> + image->queryImage(buffer->image, __DRI_IMAGE_ATTRIB_NAME, &name); >> + image->queryImage(buffer->image, __DRI_IMAGE_ATTRIB_STRIDE, &stride); >> + >> + buffer->base.attachment = attachment; >> + buffer->base.name = name; >> + buffer->base.pitch = stride; >> + buffer->base.cpp = format / 8; >> + >> + return &buffer->base; >> +} > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev