[Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

[Chad Versace]

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 <krh at 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).

> +
> +   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;
> +}