diff mbox

[7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension

Message ID 1383618208-21310-8-git-send-email-keithp@keithp.com
State Not Applicable
Headers show

Commit Message

Keith Packard Nov. 5, 2013, 2:23 a.m. UTC
These provide an interface between the driver and the loader to allocate
color buffers through the DRIimage extension interface rather than through a
loader-specific extension (as is used by DRI2, for instance).

The driver uses the loader 'getBuffers' interface to allocate color buffers.

The loader uses the createNewScreen2, createNewDrawable, createNewContext,
getAPIMask and createContextAttribs APIS (mostly shared with DRI2).

This interface will work with the DRI3 loader, and should also work with GBM
and other loaders so that drivers need not be customized for each new loader
interface, as long as they provide this image interface.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 include/GL/internal/dri_interface.h           | 112 +++++++++++++++++++++++++
 src/mesa/drivers/dri/common/dri_util.c        | 113 +++++++++++++++++++++++++
 src/mesa/drivers/dri/common/dri_util.h        |   6 ++
 src/mesa/drivers/dri/i915/intel_context.c     | 111 ++++++++++++++++++++++++-
 src/mesa/drivers/dri/i915/intel_mipmap_tree.c |  33 ++++++++
 src/mesa/drivers/dri/i915/intel_mipmap_tree.h |   8 ++
 src/mesa/drivers/dri/i915/intel_screen.c      |   1 +
 src/mesa/drivers/dri/i965/brw_context.c       | 114 ++++++++++++++++++++++++--
 src/mesa/drivers/dri/i965/brw_context.h       |  16 ++--
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  61 ++++++++++++++
 src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   8 ++
 src/mesa/drivers/dri/i965/intel_screen.c      |   5 +-
 12 files changed, 568 insertions(+), 20 deletions(-)

Comments

Eric Anholt Nov. 5, 2013, 8:05 p.m. UTC | #1
Keith Packard <keithp@keithp.com> writes:

> These provide an interface between the driver and the loader to allocate
> color buffers through the DRIimage extension interface rather than through a
> loader-specific extension (as is used by DRI2, for instance).
>
> The driver uses the loader 'getBuffers' interface to allocate color buffers.
>
> The loader uses the createNewScreen2, createNewDrawable, createNewContext,
> getAPIMask and createContextAttribs APIS (mostly shared with DRI2).
>
> This interface will work with the DRI3 loader, and should also work with GBM
> and other loaders so that drivers need not be customized for each new loader
> interface, as long as they provide this image interface.

Most of my review was going to be whining about yet another (broken)
copy of dri2CreateNewScreen2.  Sounds like you've fixed that.

> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  include/GL/internal/dri_interface.h           | 112 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/common/dri_util.c        | 113 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/common/dri_util.h        |   6 ++
>  src/mesa/drivers/dri/i915/intel_context.c     | 111 ++++++++++++++++++++++++-
>  src/mesa/drivers/dri/i915/intel_mipmap_tree.c |  33 ++++++++
>  src/mesa/drivers/dri/i915/intel_mipmap_tree.h |   8 ++
>  src/mesa/drivers/dri/i915/intel_screen.c      |   1 +
>  src/mesa/drivers/dri/i965/brw_context.c       | 114 ++++++++++++++++++++++++--
>  src/mesa/drivers/dri/i965/brw_context.h       |  16 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  61 ++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   8 ++
>  src/mesa/drivers/dri/i965/intel_screen.c      |   5 +-
>  12 files changed, 568 insertions(+), 20 deletions(-)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 907aeca..8fc1fa6 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -86,6 +86,10 @@ typedef struct __DRIdri2LoaderExtensionRec	__DRIdri2LoaderExtension;
>  typedef struct __DRI2flushExtensionRec	__DRI2flushExtension;
>  typedef struct __DRI2throttleExtensionRec	__DRI2throttleExtension;
>  
> +
> +typedef struct __DRIimageLoaderExtensionRec     __DRIimageLoaderExtension;
> +typedef struct __DRIimageDriverExtensionRec     __DRIimageDriverExtension;
> +
>  /*@}*/
>  
>  
> @@ -1288,4 +1292,112 @@ typedef struct __DRIDriverVtableExtensionRec {
>      const struct __DriverAPIRec *vtable;
>  } __DRIDriverVtableExtension;
>  
> +/**
> + * Image Loader extension. Drivers use this to allocate color buffers
> + */


> +
> +#define __DRI_DRIVER_EXTENSIONS "__driDriverExtensions"

This looks like rebase fail

> +#define __DRI_IMAGE_LOADER "DRI_IMAGE_LOADER"
> +#define __DRI_IMAGE_LOADER_VERSION 1
> +
> +struct __DRIimageLoaderExtensionRec {
> +    __DRIextension base;
> +
> +   /**
> +    * Allocate color buffers.
> +    *
> +    * \param driDrawable
> +    * \param width              Width of allocated buffers
> +    * \param height             Height of allocated buffers
> +    * \param format             one of __DRI_IMAGE_FORMAT_*
> +    * \param stamp              Address of variable to be updated when
> +    *                           getBuffers must be called again
> +    * \param loaderPrivate      The loaderPrivate for driDrawable
> +    * \param buffer_mask        Set of buffers to allocate
> +    * \param buffers            Returned buffers
> +    */
> +   int (*getBuffers)(__DRIdrawable *driDrawable,
> +                     int *width, int *height,
> +                     unsigned int format,
> +                     uint32_t *stamp,
> +                     void *loaderPrivate,
> +                     uint32_t buffer_mask,
> +                     struct __DRIimageList *buffers);
> +
> +    /**
> +     * Flush pending front-buffer rendering
> +     *
> +     * Any rendering that has been performed to the
> +     * fake front will be flushed to the front
> +     *
> +     * \param driDrawable    Drawable whose front-buffer is to be flushed
> +     * \param loaderPrivate  Loader's private data that was previously passed
> +     *                       into __DRIdri2ExtensionRec::createNewDrawable
> +     */
> +    void (*flushFrontBuffer)(__DRIdrawable *driDrawable, void *loaderPrivate);
> +};
> +
> +/**
> + * DRI extension.
> + */
> +
> +//struct gl_context;
> +//struct dd_function_table;

Looks like development leftovers.

> +typedef __DRIscreen *
> +(*__DRIcreateNewScreen2)(int screen, int fd,
> +                         const __DRIextension **extensions,
> +                         const __DRIextension **driver_extensions,
> +                         const __DRIconfig ***driver_configs,
> +                         void *loaderPrivate);
> +
> +typedef __DRIdrawable *
> +(*__DRIcreateNewDrawable)(__DRIscreen *screen,
> +                          const __DRIconfig *config,
> +                          void *loaderPrivate);
> +
> +typedef __DRIcontext *
> +(*__DRIcreateNewContext)(__DRIscreen *screen,
> +                         const __DRIconfig *config,
> +                         __DRIcontext *shared,
> +                         void *loaderPrivate);
> +
> +typedef __DRIcontext *
> +(*__DRIcreateContextAttribs)(__DRIscreen *screen,
> +                             int api,
> +                             const __DRIconfig *config,
> +                             __DRIcontext *shared,
> +                             unsigned num_attribs,
> +                             const uint32_t *attribs,
> +                             unsigned *error,
> +                             void *loaderPrivate);
> +typedef unsigned int
> +(*__DRIgetAPIMask)(__DRIscreen *screen);

Maybe append "Func" to the typedefs so they don't look like just another
struct in the declarations?  And since they're supposed to be the same
function pointers as in the __DRIswrastExtensionRec and
__DRIdri2ExtensionRec, change them to this typedef, too?


> +static void
> +intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable)
> +{
> +   struct gl_framebuffer *fb = drawable->driverPrivate;
> +   __DRIscreen *screen = intel->intelScreen->driScrnPriv;
> +   struct intel_renderbuffer *front_rb;
> +   struct intel_renderbuffer *back_rb;
> +   struct __DRIimageList images;
> +   unsigned int format;
> +   uint32_t buffer_mask = 0;
> +
> +   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
> +   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> +
> +   if (back_rb)
> +      format = intel_rb_format(back_rb);
> +   else if (front_rb)
> +      format = intel_rb_format(front_rb);
> +   else
> +      return;
> +
> +   if ((intel->is_front_buffer_rendering || intel->is_front_buffer_reading || !back_rb) && front_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> +   if (back_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
> +
> +   (*screen->image.loader->getBuffers) (drawable,
> +                                        &drawable->w,
> +                                        &drawable->h,
> +                                        driGLFormatToImageFormat(format),
> +                                        &drawable->dri2.stamp,
> +                                        drawable->loaderPrivate,
> +                                        buffer_mask,
> +                                        &images);
> +
> +   if (images.front) {
> +      assert(front_rb);
> +      intel_update_image_buffer(intel,
> +                                drawable,
> +                                front_rb,
> +                                images.front,
> +                                __DRI_IMAGE_BUFFER_FRONT);
> +   }
> +   if (images.back)
> +      intel_update_image_buffer(intel,
> +                                drawable,
> +                                back_rb,
> +                                images.back,
> +                                __DRI_IMAGE_BUFFER_BACK);
> +}

It looks like getBuffers could just be two getBuffer calls, except for
the updating of width and height.  Have you looked into doing things
that way at all?

> @@ -549,7 +549,7 @@ brw_process_driconf_options(struct brw_context *brw)
>        driQueryOptionb(options, "disable_glsl_line_continuations");
>  }
>  
> -bool
> +GLboolean
>  brwCreateContext(gl_api api,
>  	         const struct gl_config *mesaVis,
>  		 __DRIcontext *driContextPriv,

Unrelated change?

> +static void
> +intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
> +{
> +   struct gl_framebuffer *fb = drawable->driverPrivate;
> +   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
> +   struct intel_renderbuffer *front_rb;
> +   struct intel_renderbuffer *back_rb;
> +   struct __DRIimageList images;
> +   unsigned int format;
> +   uint32_t buffer_mask = 0;
> +
> +   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
> +   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> +
> +   if (back_rb)
> +      format = intel_rb_format(back_rb);
> +   else if (front_rb)
> +      format = intel_rb_format(front_rb);
> +   else
> +      return;
> +
> +   if ((brw->is_front_buffer_rendering || brw->is_front_buffer_reading || !back_rb) && front_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> +   if (back_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
> +
> +   (*screen->image.loader->getBuffers) (drawable,
> +                                        &drawable->w,
> +                                        &drawable->h,
> +                                        driGLFormatToImageFormat(format),
> +                                        &drawable->dri2.stamp,
> +                                        drawable->loaderPrivate,
> +                                        buffer_mask,
> +                                        &images);
> +
> +   if (images.front) {
> +      assert(front_rb);
> +      intel_update_image_buffer(brw,
> +                                drawable,
> +                                front_rb,
> +                                images.front,
> +                                __DRI_IMAGE_BUFFER_FRONT);
> +   }
> +   if (images.back)
> +      intel_update_image_buffer(brw,
> +                                drawable,
> +                                back_rb,
> +                                images.back,
> +                                __DRI_IMAGE_BUFFER_BACK);
> +}

Style nit: we try and put braces around multi-line things like this,
even if they are a single statement.

> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index bec4d6b..1ecbfb7 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1477,14 +1477,14 @@ void intel_prepare_render(struct brw_context *brw);
>  void intel_resolve_for_dri2_flush(struct brw_context *brw,
>                                    __DRIdrawable *drawable);
>  
> -bool brwCreateContext(gl_api api,
> -		      const struct gl_config *mesaVis,
> -		      __DRIcontext *driContextPriv,
> -                      unsigned major_version,
> -                      unsigned minor_version,
> -                      uint32_t flags,
> -                      unsigned *error,
> -		      void *sharedContextPrivate);
> +GLboolean brwCreateContext(gl_api api,
> +                           const struct gl_config *mesaVis,
> +                           __DRIcontext *driContextPriv,
> +                           unsigned major_version,
> +                           unsigned minor_version,
> +                           uint32_t flags,
> +                           unsigned *error,
> +                           void *sharedContextPrivate);

Unrelated change.
Kristian Høgsberg Nov. 5, 2013, 10:59 p.m. UTC | #2
On Mon, Nov 04, 2013 at 06:23:27PM -0800, Keith Packard wrote:
> These provide an interface between the driver and the loader to allocate
> color buffers through the DRIimage extension interface rather than through a
> loader-specific extension (as is used by DRI2, for instance).
> 
> The driver uses the loader 'getBuffers' interface to allocate color buffers.
> 
> The loader uses the createNewScreen2, createNewDrawable, createNewContext,
> getAPIMask and createContextAttribs APIS (mostly shared with DRI2).
> 
> This interface will work with the DRI3 loader, and should also work with GBM
> and other loaders so that drivers need not be customized for each new loader
> interface, as long as they provide this image interface.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  include/GL/internal/dri_interface.h           | 112 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/common/dri_util.c        | 113 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/common/dri_util.h        |   6 ++
>  src/mesa/drivers/dri/i915/intel_context.c     | 111 ++++++++++++++++++++++++-
>  src/mesa/drivers/dri/i915/intel_mipmap_tree.c |  33 ++++++++
>  src/mesa/drivers/dri/i915/intel_mipmap_tree.h |   8 ++
>  src/mesa/drivers/dri/i915/intel_screen.c      |   1 +
>  src/mesa/drivers/dri/i965/brw_context.c       | 114 ++++++++++++++++++++++++--
>  src/mesa/drivers/dri/i965/brw_context.h       |  16 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  61 ++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   8 ++
>  src/mesa/drivers/dri/i965/intel_screen.c      |   5 +-
>  12 files changed, 568 insertions(+), 20 deletions(-)
> 
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 907aeca..8fc1fa6 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -86,6 +86,10 @@ typedef struct __DRIdri2LoaderExtensionRec	__DRIdri2LoaderExtension;
>  typedef struct __DRI2flushExtensionRec	__DRI2flushExtension;
>  typedef struct __DRI2throttleExtensionRec	__DRI2throttleExtension;
>  
> +
> +typedef struct __DRIimageLoaderExtensionRec     __DRIimageLoaderExtension;
> +typedef struct __DRIimageDriverExtensionRec     __DRIimageDriverExtension;
> +
>  /*@}*/
>  
>  
> @@ -1288,4 +1292,112 @@ typedef struct __DRIDriverVtableExtensionRec {
>      const struct __DriverAPIRec *vtable;
>  } __DRIDriverVtableExtension;
>  
> +/**
> + * Image Loader extension. Drivers use this to allocate color buffers
> + */
> +
> +#define __DRI_DRIVER_EXTENSIONS "__driDriverExtensions"
> +
> +enum __DRIimageBufferMask {
> +   __DRI_IMAGE_BUFFER_BACK = (1 << 0),
> +   __DRI_IMAGE_BUFFER_FRONT = (1 << 1)
> +};
> +
> +struct __DRIimageList {
> +   __DRIimage *back;
> +   __DRIimage *front;
> +};
> +
> +#define __DRI_IMAGE_LOADER "DRI_IMAGE_LOADER"
> +#define __DRI_IMAGE_LOADER_VERSION 1
> +
> +struct __DRIimageLoaderExtensionRec {
> +    __DRIextension base;
> +
> +   /**
> +    * Allocate color buffers.
> +    *
> +    * \param driDrawable
> +    * \param width              Width of allocated buffers
> +    * \param height             Height of allocated buffers
> +    * \param format             one of __DRI_IMAGE_FORMAT_*
> +    * \param stamp              Address of variable to be updated when
> +    *                           getBuffers must be called again
> +    * \param loaderPrivate      The loaderPrivate for driDrawable
> +    * \param buffer_mask        Set of buffers to allocate
> +    * \param buffers            Returned buffers
> +    */
> +   int (*getBuffers)(__DRIdrawable *driDrawable,
> +                     int *width, int *height,

We can drop width and height now and just get it from either of the
returned images.  Format is a function of the __DRIconfig and doesn't
change, so we could make that something you can query from the config
in the interest of further reducing the number of arguments.

I'll give this a try with the GBM and Wayland EGL backends now.

Kristian

> +                     unsigned int format,
> +                     uint32_t *stamp,
> +                     void *loaderPrivate,
> +                     uint32_t buffer_mask,
> +                     struct __DRIimageList *buffers);
> +
> +    /**
> +     * Flush pending front-buffer rendering
> +     *
> +     * Any rendering that has been performed to the
> +     * fake front will be flushed to the front
> +     *
> +     * \param driDrawable    Drawable whose front-buffer is to be flushed
> +     * \param loaderPrivate  Loader's private data that was previously passed
> +     *                       into __DRIdri2ExtensionRec::createNewDrawable
> +     */
> +    void (*flushFrontBuffer)(__DRIdrawable *driDrawable, void *loaderPrivate);
> +};
> +
> +/**
> + * DRI extension.
> + */
> +
> +//struct gl_context;
> +//struct dd_function_table;
> +
> +typedef __DRIscreen *
> +(*__DRIcreateNewScreen2)(int screen, int fd,
> +                         const __DRIextension **extensions,
> +                         const __DRIextension **driver_extensions,
> +                         const __DRIconfig ***driver_configs,
> +                         void *loaderPrivate);
> +
> +typedef __DRIdrawable *
> +(*__DRIcreateNewDrawable)(__DRIscreen *screen,
> +                          const __DRIconfig *config,
> +                          void *loaderPrivate);
> +
> +typedef __DRIcontext *
> +(*__DRIcreateNewContext)(__DRIscreen *screen,
> +                         const __DRIconfig *config,
> +                         __DRIcontext *shared,
> +                         void *loaderPrivate);
> +
> +typedef __DRIcontext *
> +(*__DRIcreateContextAttribs)(__DRIscreen *screen,
> +                             int api,
> +                             const __DRIconfig *config,
> +                             __DRIcontext *shared,
> +                             unsigned num_attribs,
> +                             const uint32_t *attribs,
> +                             unsigned *error,
> +                             void *loaderPrivate);
> +
> +typedef unsigned int
> +(*__DRIgetAPIMask)(__DRIscreen *screen);
> +
> +#define __DRI_IMAGE_DRIVER           "DRI_IMAGE_DRIVER"
> +#define __DRI_IMAGE_DRIVER_VERSION   1
> +
> +struct __DRIimageDriverExtensionRec {
> +   __DRIextension               base;
> +
> +   /* Common DRI functions, shared with DRI2 */
> +   __DRIcreateNewScreen2        createNewScreen2;
> +   __DRIcreateNewDrawable       createNewDrawable;
> +   __DRIcreateNewContext        createNewContext;
> +   __DRIcreateContextAttribs    createContextAttribs;
> +   __DRIgetAPIMask              getAPIMask;
> +};
> +
>  #endif
> diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
> index 76c8ae5..d8cb7f6 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -854,3 +854,116 @@ driImageFormatToGLFormat(uint32_t image_format)
>        return MESA_FORMAT_NONE;
>     }
>  }
> +
> +/*
> + * Copyright © 2013 Keith Packard
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +PUBLIC const char __imageConfigOptions[] =
> +   DRI_CONF_BEGIN
> +      DRI_CONF_SECTION_PERFORMANCE
> +         DRI_CONF_VBLANK_MODE(DRI_CONF_VBLANK_DEF_INTERVAL_1)
> +      DRI_CONF_SECTION_END
> +   DRI_CONF_END;
> +
> +static void
> +setup_image_loader_extensions(__DRIscreen *psp,
> +                              const __DRIextension *const*extensions)
> +{
> +    int i;
> +
> +    for (i = 0; extensions[i]; i++) {
> +        if (strcmp(extensions[i]->name, __DRI_IMAGE_LOADER) == 0)
> +           psp->image.loader = (__DRIimageLoaderExtension *) extensions[i];
> +    }
> +}
> +
> +static __DRIscreen *
> +image_create_new_screen_2(int scrn, int fd,
> +                          const __DRIextension **extensions,
> +                          const __DRIextension **driver_extensions,
> +                          const __DRIconfig ***driver_configs, void *data)
> +{
> +    static const __DRIextension *emptyExtensionList[] = { NULL };
> +    __DRIscreen *psp;
> +    drmVersionPtr version;
> +
> +    psp = calloc(1, sizeof(*psp));
> +    if (!psp)
> +	return NULL;
> +
> +    /* By default, use the global driDriverAPI symbol (non-megadrivers). */
> +    psp->driver = globalDriverAPI;
> +
> +    /* If the driver exposes its vtable through its extensions list
> +     * (megadrivers), use that instead.
> +     */
> +    if (driver_extensions) {
> +       for (int i = 0; driver_extensions[i]; i++) {
> +          if (strcmp(driver_extensions[i]->name, __DRI_DRIVER_VTABLE) == 0) {
> +             psp->driver =
> +                ((__DRIDriverVtableExtension *)driver_extensions[i])->vtable;
> +          }
> +       }
> +    }
> +
> +    setupLoaderExtensions(psp, extensions);
> +    setup_image_loader_extensions(psp, extensions);
> +
> +    version = drmGetVersion(fd);
> +    if (version) {
> +	psp->drm_version.major = version->version_major;
> +	psp->drm_version.minor = version->version_minor;
> +	psp->drm_version.patch = version->version_patchlevel;
> +	drmFreeVersion(version);
> +    }
> +
> +    psp->loaderPrivate = data;
> +
> +    psp->extensions = emptyExtensionList;
> +    psp->fd = fd;
> +    psp->myNum = scrn;
> +
> +    psp->api_mask = (1 << __DRI_API_OPENGL);
> +
> +    *driver_configs = psp->driver->InitScreen(psp);
> +    if (*driver_configs == NULL) {
> +	free(psp);
> +	return NULL;
> +    }
> +
> +    driParseOptionInfo(&psp->optionInfo, __imageConfigOptions);
> +    driParseConfigFiles(&psp->optionCache, &psp->optionInfo, psp->myNum, "image");
> +
> +    return psp;
> +}
> +
> +
> +/** Image driver interface */
> +const __DRIimageDriverExtension driImageDriverExtension = {
> +    .base = { __DRI_IMAGE_DRIVER, __DRI_IMAGE_DRIVER_VERSION },
> +
> +    .createNewScreen2           = image_create_new_screen_2,
> +    .createNewDrawable          = driCreateNewDrawable,
> +    .createNewContext           = driCreateNewContext,
> +    .getAPIMask                 = driGetAPIMask,
> +    .createContextAttribs       = driCreateContextAttribs,
> +};
> diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h
> index fd40769..6f5cd87 100644
> --- a/src/mesa/drivers/dri/common/dri_util.h
> +++ b/src/mesa/drivers/dri/common/dri_util.h
> @@ -174,6 +174,10 @@ struct __DRIscreenRec {
>  	__DRIuseInvalidateExtension *useInvalidate;
>      } dri2;
>  
> +    struct {
> +        __DRIimageLoaderExtension *loader;
> +    } image;
> +
>      driOptionCache optionInfo;
>      driOptionCache optionCache;
>  
> @@ -283,4 +287,6 @@ dri2InvalidateDrawable(__DRIdrawable *drawable);
>  extern void
>  driUpdateFramebufferSize(struct gl_context *ctx, const __DRIdrawable *dPriv);
>  
> +extern const __DRIimageDriverExtension driImageDriverExtension;
> +
>  #endif /* _DRI_UTIL_H_ */
> diff --git a/src/mesa/drivers/dri/i915/intel_context.c b/src/mesa/drivers/dri/i915/intel_context.c
> index 1798bc7..8d369ff 100644
> --- a/src/mesa/drivers/dri/i915/intel_context.c
> +++ b/src/mesa/drivers/dri/i915/intel_context.c
> @@ -91,6 +91,8 @@ intelGetString(struct gl_context * ctx, GLenum name)
>     }
>  }
>  
> +#define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
> +
>  static void
>  intel_flush_front(struct gl_context *ctx)
>  {
> @@ -100,11 +102,10 @@ intel_flush_front(struct gl_context *ctx)
>      __DRIscreen *const screen = intel->intelScreen->driScrnPriv;
>  
>      if (intel->front_buffer_dirty && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
> -      if (screen->dri2.loader->flushFrontBuffer != NULL &&
> +      if (flushFront(screen) && 
>            driDrawable &&
>            driDrawable->loaderPrivate) {
> -         screen->dri2.loader->flushFrontBuffer(driDrawable,
> -                                               driDrawable->loaderPrivate);
> +         flushFront(screen)(driDrawable, driDrawable->loaderPrivate);
>  
>  	 /* We set the dirty bit in intel_prepare_render() if we're
>  	  * front buffer rendering once we get there.
> @@ -114,6 +115,9 @@ intel_flush_front(struct gl_context *ctx)
>     }
>  }
>  
> +static void
> +intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable);
> +
>  static unsigned
>  intel_bits_per_pixel(const struct intel_renderbuffer *rb)
>  {
> @@ -194,7 +198,10 @@ intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
>     if (unlikely(INTEL_DEBUG & DEBUG_DRI))
>        fprintf(stderr, "enter %s, drawable %p\n", __func__, drawable);
>  
> -   intel_update_dri2_buffers(intel, drawable);
> +   if (screen->image.loader)
> +      intel_update_image_buffers(intel, drawable);
> +   else
> +      intel_update_dri2_buffers(intel, drawable);
>  
>     driUpdateFramebufferSize(&intel->ctx, drawable);
>  }
> @@ -787,3 +794,99 @@ intel_process_dri2_buffer(struct intel_context *intel,
>                                                   region);
>     intel_region_release(&region);
>  }
> +
> +/**
> + * \brief Query DRI Image loader to obtain a DRIdrawable's buffers.
> + *
> + * To determine which DRI buffers to request, examine the renderbuffers
> + * attached to the drawable's framebuffer. Then request the buffers with
> + * dri3
> + *
> + * This is called from intel_update_renderbuffers().
> + *
> + * \param drawable      Drawable whose buffers are queried.
> + * \param buffers       [out] List of buffers returned by DRI2 query.
> + * \param buffer_count  [out] Number of buffers returned.
> + *
> + * \see intel_update_renderbuffers()
> + */
> +
> +static void
> +intel_update_image_buffer(struct intel_context *intel,
> +                          __DRIdrawable *drawable,
> +                          struct intel_renderbuffer *rb,
> +                          __DRIimage *buffer,
> +                          enum __DRIimageBufferMask buffer_type)
> +{
> +   struct intel_region *region = buffer->region;
> +
> +   if (!rb || !region)
> +      return;
> +
> +   unsigned num_samples = rb->Base.Base.NumSamples;
> +
> +   if (rb->mt &&
> +       rb->mt->region &&
> +       rb->mt->region == region)
> +      return;
> +
> +   intel_miptree_release(&rb->mt);
> +   rb->mt = intel_miptree_create_for_image_buffer(intel,
> +                                                  buffer_type,
> +                                                  intel_rb_format(rb),
> +                                                  num_samples,
> +                                                  region);
> +}
> +
> +
> +static void
> +intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable)
> +{
> +   struct gl_framebuffer *fb = drawable->driverPrivate;
> +   __DRIscreen *screen = intel->intelScreen->driScrnPriv;
> +   struct intel_renderbuffer *front_rb;
> +   struct intel_renderbuffer *back_rb;
> +   struct __DRIimageList images;
> +   unsigned int format;
> +   uint32_t buffer_mask = 0;
> +
> +   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
> +   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> +
> +   if (back_rb)
> +      format = intel_rb_format(back_rb);
> +   else if (front_rb)
> +      format = intel_rb_format(front_rb);
> +   else
> +      return;
> +
> +   if ((intel->is_front_buffer_rendering || intel->is_front_buffer_reading || !back_rb) && front_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> +   if (back_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
> +
> +   (*screen->image.loader->getBuffers) (drawable,
> +                                        &drawable->w,
> +                                        &drawable->h,
> +                                        driGLFormatToImageFormat(format),
> +                                        &drawable->dri2.stamp,
> +                                        drawable->loaderPrivate,
> +                                        buffer_mask,
> +                                        &images);
> +
> +   if (images.front) {
> +      assert(front_rb);
> +      intel_update_image_buffer(intel,
> +                                drawable,
> +                                front_rb,
> +                                images.front,
> +                                __DRI_IMAGE_BUFFER_FRONT);
> +   }
> +   if (images.back)
> +      intel_update_image_buffer(intel,
> +                                drawable,
> +                                back_rb,
> +                                images.back,
> +                                __DRI_IMAGE_BUFFER_BACK);
> +}
> diff --git a/src/mesa/drivers/dri/i915/intel_mipmap_tree.c b/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
> index 8432b6d..66a7a92 100644
> --- a/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
> @@ -322,6 +322,39 @@ intel_miptree_create_for_dri2_buffer(struct intel_context *intel,
>     return mt;
>  }
>  
> +/**
> + * For a singlesample image buffer, this simply wraps the given region with a miptree.
> + *
> + * For a multisample image buffer, this wraps the given region with
> + * a singlesample miptree, then creates a multisample miptree into which the
> + * singlesample miptree is embedded as a child.
> + */
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct intel_context *intel,
> +                                      enum __DRIimageBufferMask buffer_type,
> +                                      gl_format format,
> +                                      uint32_t num_samples,
> +                                      struct intel_region *region)
> +{
> +   struct intel_mipmap_tree *mt = NULL;
> +
> +   /* Only the front and back buffers, which are color buffers, are allocated
> +    * through the image loader.
> +    */
> +   assert(_mesa_get_format_base_format(format) == GL_RGB ||
> +          _mesa_get_format_base_format(format) == GL_RGBA);
> +
> +   mt = intel_miptree_create_for_bo(intel,
> +                                    region->bo,
> +                                    format,
> +                                    0,
> +                                    region->width,
> +                                    region->height,
> +                                    region->pitch,
> +                                    region->tiling);
> +   return mt;
> +}
> +
>  struct intel_mipmap_tree*
>  intel_miptree_create_for_renderbuffer(struct intel_context *intel,
>                                        gl_format format,
> diff --git a/src/mesa/drivers/dri/i915/intel_mipmap_tree.h b/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
> index 1142af6..35cad6d 100644
> --- a/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
> @@ -32,6 +32,7 @@
>  
>  #include "intel_screen.h"
>  #include "intel_regions.h"
> +#include "GL/internal/dri_interface.h"
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -258,6 +259,13 @@ intel_miptree_create_for_dri2_buffer(struct intel_context *intel,
>                                       gl_format format,
>                                       struct intel_region *region);
>  
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct intel_context *intel,
> +                                      enum __DRIimageBufferMask buffer_type,
> +                                      gl_format format,
> +                                      uint32_t num_samples,
> +                                      struct intel_region *region);
> +
>  /**
>   * Create a miptree appropriate as the storage for a non-texture renderbuffer.
>   * The miptree has the following properties:
> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> index 12113c7..38badec 100644
> --- a/src/mesa/drivers/dri/i915/intel_screen.c
> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> @@ -1166,6 +1166,7 @@ static const struct __DRIDriverVtableExtensionRec i915_vtable = {
>  /* This is the table of extensions that the loader will dlsym() for. */
>  static const __DRIextension *i915_driver_extensions[] = {
>      &driCoreExtension.base,
> +    &driImageDriverExtension.base,
>      &driDRI2Extension.base,
>      &i915_vtable.base,
>      &i915_config_options.base,
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 2557d10..f3bf67f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -151,6 +151,8 @@ intelInvalidateState(struct gl_context * ctx, GLuint new_state)
>     brw->NewGLState |= new_state;
>  }
>  
> +#define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
> +
>  static void
>  intel_flush_front(struct gl_context *ctx)
>  {
> @@ -160,8 +162,7 @@ intel_flush_front(struct gl_context *ctx)
>     __DRIscreen *const screen = brw->intelScreen->driScrnPriv;
>  
>     if (brw->front_buffer_dirty && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
> -      if (screen->dri2.loader->flushFrontBuffer != NULL &&
> -          driDrawable &&
> +      if (flushFront(screen) && driDrawable &&
>            driDrawable->loaderPrivate) {
>  
>           /* Resolve before flushing FAKE_FRONT_LEFT to FRONT_LEFT.
> @@ -174,8 +175,7 @@ intel_flush_front(struct gl_context *ctx)
>           intel_resolve_for_dri2_flush(brw, driDrawable);
>           intel_batchbuffer_flush(brw);
>  
> -         screen->dri2.loader->flushFrontBuffer(driDrawable,
> -                                               driDrawable->loaderPrivate);
> +         flushFront(screen)(driDrawable, driDrawable->loaderPrivate);
>  
>           /* We set the dirty bit in intel_prepare_render() if we're
>            * front buffer rendering once we get there.
> @@ -549,7 +549,7 @@ brw_process_driconf_options(struct brw_context *brw)
>        driQueryOptionb(options, "disable_glsl_line_continuations");
>  }
>  
> -bool
> +GLboolean
>  brwCreateContext(gl_api api,
>  	         const struct gl_config *mesaVis,
>  		 __DRIcontext *driContextPriv,
> @@ -979,6 +979,9 @@ intel_process_dri2_buffer(struct brw_context *brw,
>                            const char *buffer_name);
>  
>  static void
> +intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable);
> +
> +static void
>  intel_update_dri2_buffers(struct brw_context *brw, __DRIdrawable *drawable)
>  {
>     struct gl_framebuffer *fb = drawable->driverPrivate;
> @@ -1038,6 +1041,7 @@ void
>  intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
>  {
>     struct brw_context *brw = context->driverPrivate;
> +   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
>  
>     /* Set this up front, so that in case our buffers get invalidated
>      * while we're getting new buffers, we don't clobber the stamp and
> @@ -1047,7 +1051,10 @@ intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
>     if (unlikely(INTEL_DEBUG & DEBUG_DRI))
>        fprintf(stderr, "enter %s, drawable %p\n", __func__, drawable);
>  
> -   intel_update_dri2_buffers(brw, drawable);
> +   if (screen->image.loader)
> +      intel_update_image_buffers(brw, drawable);
> +   else
> +      intel_update_dri2_buffers(brw, drawable);
>  
>     driUpdateFramebufferSize(&brw->ctx, drawable);
>  }
> @@ -1253,3 +1260,98 @@ intel_process_dri2_buffer(struct brw_context *brw,
>                                                   region);
>     intel_region_release(&region);
>  }
> +
> +/**
> + * \brief Query DRI image loader to obtain a DRIdrawable's buffers.
> + *
> + * To determine which DRI buffers to request, examine the renderbuffers
> + * attached to the drawable's framebuffer. Then request the buffers from
> + * the image loader
> + *
> + * This is called from intel_update_renderbuffers().
> + *
> + * \param drawable      Drawable whose buffers are queried.
> + * \param buffers       [out] List of buffers returned by DRI2 query.
> + * \param buffer_count  [out] Number of buffers returned.
> + *
> + * \see intel_update_renderbuffers()
> + */
> +
> +static void
> +intel_update_image_buffer(struct brw_context *intel,
> +                          __DRIdrawable *drawable,
> +                          struct intel_renderbuffer *rb,
> +                          __DRIimage *buffer,
> +                          enum __DRIimageBufferMask buffer_type)
> +{
> +   struct intel_region *region = buffer->region;
> +
> +   if (!rb || !region)
> +      return;
> +
> +   unsigned num_samples = rb->Base.Base.NumSamples;
> +
> +   if (rb->mt &&
> +       rb->mt->region &&
> +       rb->mt->region == region)
> +      return;
> +
> +   intel_miptree_release(&rb->mt);
> +   rb->mt = intel_miptree_create_for_image_buffer(intel,
> +                                                  buffer_type,
> +                                                  intel_rb_format(rb),
> +                                                  num_samples,
> +                                                  region);
> +}
> +
> +static void
> +intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
> +{
> +   struct gl_framebuffer *fb = drawable->driverPrivate;
> +   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
> +   struct intel_renderbuffer *front_rb;
> +   struct intel_renderbuffer *back_rb;
> +   struct __DRIimageList images;
> +   unsigned int format;
> +   uint32_t buffer_mask = 0;
> +
> +   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
> +   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> +
> +   if (back_rb)
> +      format = intel_rb_format(back_rb);
> +   else if (front_rb)
> +      format = intel_rb_format(front_rb);
> +   else
> +      return;
> +
> +   if ((brw->is_front_buffer_rendering || brw->is_front_buffer_reading || !back_rb) && front_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> +   if (back_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
> +
> +   (*screen->image.loader->getBuffers) (drawable,
> +                                        &drawable->w,
> +                                        &drawable->h,
> +                                        driGLFormatToImageFormat(format),
> +                                        &drawable->dri2.stamp,
> +                                        drawable->loaderPrivate,
> +                                        buffer_mask,
> +                                        &images);
> +
> +   if (images.front) {
> +      assert(front_rb);
> +      intel_update_image_buffer(brw,
> +                                drawable,
> +                                front_rb,
> +                                images.front,
> +                                __DRI_IMAGE_BUFFER_FRONT);
> +   }
> +   if (images.back)
> +      intel_update_image_buffer(brw,
> +                                drawable,
> +                                back_rb,
> +                                images.back,
> +                                __DRI_IMAGE_BUFFER_BACK);
> +}
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index bec4d6b..1ecbfb7 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1477,14 +1477,14 @@ void intel_prepare_render(struct brw_context *brw);
>  void intel_resolve_for_dri2_flush(struct brw_context *brw,
>                                    __DRIdrawable *drawable);
>  
> -bool brwCreateContext(gl_api api,
> -		      const struct gl_config *mesaVis,
> -		      __DRIcontext *driContextPriv,
> -                      unsigned major_version,
> -                      unsigned minor_version,
> -                      uint32_t flags,
> -                      unsigned *error,
> -		      void *sharedContextPrivate);
> +GLboolean brwCreateContext(gl_api api,
> +                           const struct gl_config *mesaVis,
> +                           __DRIcontext *driContextPriv,
> +                           unsigned major_version,
> +                           unsigned minor_version,
> +                           uint32_t flags,
> +                           unsigned *error,
> +                           void *sharedContextPrivate);
>  
>  /*======================================================================
>   * brw_misc_state.c
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 2f5e04f..08dbe88 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -725,6 +725,67 @@ intel_miptree_create_for_dri2_buffer(struct brw_context *brw,
>     return multisample_mt;
>  }
>  
> +/**
> + * For a singlesample image buffer, this simply wraps the given region with a miptree.
> + *
> + * For a multisample image buffer, this wraps the given region with
> + * a singlesample miptree, then creates a multisample miptree into which the
> + * singlesample miptree is embedded as a child.
> + */
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct brw_context *intel,
> +                                      enum __DRIimageBufferMask buffer_type,
> +                                      gl_format format,
> +                                      uint32_t num_samples,
> +                                      struct intel_region *region)
> +{
> +   struct intel_mipmap_tree *singlesample_mt = NULL;
> +   struct intel_mipmap_tree *multisample_mt = NULL;
> +
> +   /* Only the front and back buffers, which are color buffers, are allocated
> +    * through the image loader.
> +    */
> +   assert(_mesa_get_format_base_format(format) == GL_RGB ||
> +          _mesa_get_format_base_format(format) == GL_RGBA);
> +
> +   singlesample_mt = intel_miptree_create_for_bo(intel,
> +                                                 region->bo,
> +                                                 format,
> +                                                 0,
> +                                                 region->width,
> +                                                 region->height,
> +                                                 region->pitch,
> +                                                 region->tiling);
> +   if (!singlesample_mt)
> +      return NULL;
> +
> +   intel_region_reference(&singlesample_mt->region, region);
> +
> +   if (num_samples == 0)
> +      return singlesample_mt;
> +
> +   multisample_mt = intel_miptree_create_for_renderbuffer(intel,
> +                                                          format,
> +                                                          region->width,
> +                                                          region->height,
> +                                                          num_samples);
> +   if (!multisample_mt) {
> +      intel_miptree_release(&singlesample_mt);
> +      return NULL;
> +   }
> +
> +   multisample_mt->singlesample_mt = singlesample_mt;
> +   multisample_mt->need_downsample = false;
> +
> +   intel_region_reference(&multisample_mt->region, region);
> +
> +   if (intel->is_front_buffer_rendering && buffer_type == __DRI_IMAGE_BUFFER_FRONT) {
> +      intel_miptree_upsample(intel, multisample_mt);
> +   }
> +
> +   return multisample_mt;
> +}
> +
>  struct intel_mipmap_tree*
>  intel_miptree_create_for_renderbuffer(struct brw_context *brw,
>                                        gl_format format,
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index d718125..8777a8c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -32,6 +32,7 @@
>  
>  #include "intel_regions.h"
>  #include "intel_resolve_map.h"
> +#include <GL/internal/dri_interface.h>
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -529,6 +530,13 @@ intel_miptree_create_for_dri2_buffer(struct brw_context *brw,
>                                       uint32_t num_samples,
>                                       struct intel_region *region);
>  
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct brw_context *intel,
> +                                     enum __DRIimageBufferMask buffer_type,
> +                                     gl_format format,
> +                                     uint32_t num_samples,
> +                                     struct intel_region *region);
> +
>  /**
>   * Create a miptree appropriate as the storage for a non-texture renderbuffer.
>   * The miptree has the following properties:
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index f9339c1..7571921 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1176,7 +1176,8 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>  {
>     struct intel_screen *intelScreen;
>  
> -   if (psp->dri2.loader->base.version <= 2 ||
> +   if (psp->image.loader) {
> +   } else if (psp->dri2.loader->base.version <= 2 ||
>         psp->dri2.loader->getBuffersWithFormat == NULL) {
>        fprintf(stderr,
>  	      "\nERROR!  DRI2 loader with getBuffersWithFormat() "
> @@ -1264,7 +1265,6 @@ intelReleaseBuffer(__DRIscreen *screen, __DRIbuffer *buffer)
>     free(intelBuffer);
>  }
>  
> -
>  static const struct __DriverAPIRec brw_driver_api = {
>     .InitScreen		 = intelInitScreen2,
>     .DestroyScreen	 = intelDestroyScreen,
> @@ -1285,6 +1285,7 @@ static const struct __DRIDriverVtableExtensionRec brw_vtable = {
>  
>  static const __DRIextension *brw_driver_extensions[] = {
>      &driCoreExtension.base,
> +    &driImageDriverExtension.base,
>      &driDRI2Extension.base,
>      &brw_vtable.base,
>      &brw_config_options.base,
> -- 
> 1.8.4.2
>
Keith Packard Nov. 5, 2013, 11:47 p.m. UTC | #3
Eric Anholt <eric@anholt.net> writes:

> Most of my review was going to be whining about yet another (broken)
> copy of dri2CreateNewScreen2.  Sounds like you've fixed that.

Yup, figured that out all on my own after I re-read the code -- the only
difference was that I need to look for the DRIimageLoader hooks, which I
now just do in the shared function.

>> +
>> +#define __DRI_DRIVER_EXTENSIONS "__driDriverExtensions"
>
> This looks like rebase fail

Removed.

>> +//struct gl_context;
>> +//struct dd_function_table;
>
> Looks like development leftovers.

Removed.

> Maybe append "Func" to the typedefs so they don't look like just another
> struct in the declarations?  And since they're supposed to be the same
> function pointers as in the __DRIswrastExtensionRec and
> __DRIdri2ExtensionRec, change them to this typedef, too?

I've moved the typedefs, renamed them and stuck that part of the patch
in the first patch of the series that renames the functions.

> It looks like getBuffers could just be two getBuffer calls, except for
> the updating of width and height.  Have you looked into doing things
> that way at all?

Yes, that was the way I started doing it. There are two reasons for
doing it in a single call:

 1) Pixmaps always need a front buffer, and the driver doesn't know
    which drawables are pixmaps.

 2) Deleting buffers when changing modes. I'd need to add another
    function to let the driver delete unused buffers.
  
Overall, I liked moving more buffer management logic to the loader and
out of the driver, so I followed the DRI2 API here.

> Style nit: we try and put braces around multi-line things like this,
> even if they are a single statement.

Fixed.

>> -bool
>> +GLboolean
>>  brwCreateContext(gl_api api,
>>  	         const struct gl_config *mesaVis,
>>  		 __DRIcontext *driContextPriv,
...
>>                                    __DRIdrawable *drawable);
>>  
>> -bool brwCreateContext(gl_api api,
>> -		      const struct gl_config *mesaVis,
>> -		      __DRIcontext *driContextPriv,
>> -                      unsigned major_version,
>> -                      unsigned minor_version,
>> -                      uint32_t flags,
>> -                      unsigned *error,
>> -		      void *sharedContextPrivate);
>> +GLboolean brwCreateContext(gl_api api,
>> +                           const struct gl_config *mesaVis,
>> +                           __DRIcontext *driContextPriv,
>> +                           unsigned major_version,
>> +                           unsigned minor_version,
>> +                           uint32_t flags,
>> +                           unsigned *error,
>> +                           void *sharedContextPrivate);
...
>
> Unrelated change?

Pulled out to a separate patch -- the return type for createContext is
GLboolean, not bool, and my compiler was whinging.

I've pushed these changes, along with a bunch of new comments in
dri3_glx.c to my dri3 branch. I can send the new series to the list if
that would be helpful.
Keith Packard Nov. 6, 2013, 12:59 a.m. UTC | #4
Kristian Høgsberg <hoegsberg@gmail.com> writes:


> We can drop width and height now and just get it from either of the
> returned images.  Format is a function of the __DRIconfig and doesn't
> change, so we could make that something you can query from the config
> in the interest of further reducing the number of arguments.

Sure would be nice if there were simpler ways than calling the
getAttribs function though; I'm tempted to just leave these params in as
every single driver will need those values and passing them back here
will make the driver code shorter.

> I'll give this a try with the GBM and Wayland EGL backends now.

Cool. Having two backends using this same interface would really help
out.
Kristian Høgsberg Nov. 6, 2013, 2:48 a.m. UTC | #5
On Tue, Nov 5, 2013 at 4:59 PM, Keith Packard <keithp@keithp.com> wrote:
> Kristian Høgsberg <hoegsberg@gmail.com> writes:
>
>
>> We can drop width and height now and just get it from either of the
>> returned images.  Format is a function of the __DRIconfig and doesn't
>> change, so we could make that something you can query from the config
>> in the interest of further reducing the number of arguments.
>
> Sure would be nice if there were simpler ways than calling the
> getAttribs function though; I'm tempted to just leave these params in as
> every single driver will need those values and passing them back here
> will make the driver code shorter.

I would say that it's more important to keep the interfaces between
components as simple as possible, even if it incurs a little
inconvenience inside the driver.

But it's a non-issue, since inside the driver you can just access them
as image->width/height.

>> I'll give this a try with the GBM and Wayland EGL backends now.
>
> Cool. Having two backends using this same interface would really help
> out.

I got this running now and I found a few other things.  I'll reply to
the patch again.

Kristian
Kristian Høgsberg Nov. 6, 2013, 6:25 a.m. UTC | #6
On Mon, Nov 04, 2013 at 06:23:27PM -0800, Keith Packard wrote:
> These provide an interface between the driver and the loader to allocate
> color buffers through the DRIimage extension interface rather than through a
> loader-specific extension (as is used by DRI2, for instance).
> 
> The driver uses the loader 'getBuffers' interface to allocate color buffers.
> 
> The loader uses the createNewScreen2, createNewDrawable, createNewContext,
> getAPIMask and createContextAttribs APIS (mostly shared with DRI2).
> 
> This interface will work with the DRI3 loader, and should also work with GBM
> and other loaders so that drivers need not be customized for each new loader
> interface, as long as they provide this image interface.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  include/GL/internal/dri_interface.h           | 112 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/common/dri_util.c        | 113 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/common/dri_util.h        |   6 ++
>  src/mesa/drivers/dri/i915/intel_context.c     | 111 ++++++++++++++++++++++++-
>  src/mesa/drivers/dri/i915/intel_mipmap_tree.c |  33 ++++++++
>  src/mesa/drivers/dri/i915/intel_mipmap_tree.h |   8 ++
>  src/mesa/drivers/dri/i915/intel_screen.c      |   1 +
>  src/mesa/drivers/dri/i965/brw_context.c       | 114 ++++++++++++++++++++++++--
>  src/mesa/drivers/dri/i965/brw_context.h       |  16 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  61 ++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   8 ++
>  src/mesa/drivers/dri/i965/intel_screen.c      |   5 +-
>  12 files changed, 568 insertions(+), 20 deletions(-)
> 
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 907aeca..8fc1fa6 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -86,6 +86,10 @@ typedef struct __DRIdri2LoaderExtensionRec	__DRIdri2LoaderExtension;
>  typedef struct __DRI2flushExtensionRec	__DRI2flushExtension;
>  typedef struct __DRI2throttleExtensionRec	__DRI2throttleExtension;
>  
> +
> +typedef struct __DRIimageLoaderExtensionRec     __DRIimageLoaderExtension;
> +typedef struct __DRIimageDriverExtensionRec     __DRIimageDriverExtension;
> +
>  /*@}*/
>  
>  
> @@ -1288,4 +1292,112 @@ typedef struct __DRIDriverVtableExtensionRec {
>      const struct __DriverAPIRec *vtable;
>  } __DRIDriverVtableExtension;
>  
> +/**
> + * Image Loader extension. Drivers use this to allocate color buffers
> + */
> +
> +#define __DRI_DRIVER_EXTENSIONS "__driDriverExtensions"
> +
> +enum __DRIimageBufferMask {
> +   __DRI_IMAGE_BUFFER_BACK = (1 << 0),
> +   __DRI_IMAGE_BUFFER_FRONT = (1 << 1)
> +};
> +
> +struct __DRIimageList {
> +   __DRIimage *back;
> +   __DRIimage *front;
> +};
> +
> +#define __DRI_IMAGE_LOADER "DRI_IMAGE_LOADER"
> +#define __DRI_IMAGE_LOADER_VERSION 1
> +
> +struct __DRIimageLoaderExtensionRec {
> +    __DRIextension base;
> +
> +   /**
> +    * Allocate color buffers.
> +    *
> +    * \param driDrawable
> +    * \param width              Width of allocated buffers
> +    * \param height             Height of allocated buffers
> +    * \param format             one of __DRI_IMAGE_FORMAT_*
> +    * \param stamp              Address of variable to be updated when
> +    *                           getBuffers must be called again
> +    * \param loaderPrivate      The loaderPrivate for driDrawable
> +    * \param buffer_mask        Set of buffers to allocate
> +    * \param buffers            Returned buffers
> +    */
> +   int (*getBuffers)(__DRIdrawable *driDrawable,
> +                     int *width, int *height,
> +                     unsigned int format,
> +                     uint32_t *stamp,
> +                     void *loaderPrivate,
> +                     uint32_t buffer_mask,
> +                     struct __DRIimageList *buffers);
> +
> +    /**
> +     * Flush pending front-buffer rendering
> +     *
> +     * Any rendering that has been performed to the
> +     * fake front will be flushed to the front
> +     *
> +     * \param driDrawable    Drawable whose front-buffer is to be flushed
> +     * \param loaderPrivate  Loader's private data that was previously passed
> +     *                       into __DRIdri2ExtensionRec::createNewDrawable
> +     */
> +    void (*flushFrontBuffer)(__DRIdrawable *driDrawable, void *loaderPrivate);
> +};
> +
> +/**
> + * DRI extension.
> + */
> +
> +//struct gl_context;
> +//struct dd_function_table;
> +
> +typedef __DRIscreen *
> +(*__DRIcreateNewScreen2)(int screen, int fd,
> +                         const __DRIextension **extensions,
> +                         const __DRIextension **driver_extensions,
> +                         const __DRIconfig ***driver_configs,
> +                         void *loaderPrivate);
> +
> +typedef __DRIdrawable *
> +(*__DRIcreateNewDrawable)(__DRIscreen *screen,
> +                          const __DRIconfig *config,
> +                          void *loaderPrivate);
> +
> +typedef __DRIcontext *
> +(*__DRIcreateNewContext)(__DRIscreen *screen,
> +                         const __DRIconfig *config,
> +                         __DRIcontext *shared,
> +                         void *loaderPrivate);
> +
> +typedef __DRIcontext *
> +(*__DRIcreateContextAttribs)(__DRIscreen *screen,
> +                             int api,
> +                             const __DRIconfig *config,
> +                             __DRIcontext *shared,
> +                             unsigned num_attribs,
> +                             const uint32_t *attribs,
> +                             unsigned *error,
> +                             void *loaderPrivate);
> +
> +typedef unsigned int
> +(*__DRIgetAPIMask)(__DRIscreen *screen);
> +
> +#define __DRI_IMAGE_DRIVER           "DRI_IMAGE_DRIVER"
> +#define __DRI_IMAGE_DRIVER_VERSION   1
> +
> +struct __DRIimageDriverExtensionRec {
> +   __DRIextension               base;
> +
> +   /* Common DRI functions, shared with DRI2 */
> +   __DRIcreateNewScreen2        createNewScreen2;
> +   __DRIcreateNewDrawable       createNewDrawable;
> +   __DRIcreateNewContext        createNewContext;
> +   __DRIcreateContextAttribs    createContextAttribs;
> +   __DRIgetAPIMask              getAPIMask;
> +};
> +
>  #endif
> diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
> index 76c8ae5..d8cb7f6 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -854,3 +854,116 @@ driImageFormatToGLFormat(uint32_t image_format)
>        return MESA_FORMAT_NONE;
>     }
>  }
> +
> +/*
> + * Copyright © 2013 Keith Packard
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +PUBLIC const char __imageConfigOptions[] =
> +   DRI_CONF_BEGIN
> +      DRI_CONF_SECTION_PERFORMANCE
> +         DRI_CONF_VBLANK_MODE(DRI_CONF_VBLANK_DEF_INTERVAL_1)
> +      DRI_CONF_SECTION_END
> +   DRI_CONF_END;
> +
> +static void
> +setup_image_loader_extensions(__DRIscreen *psp,
> +                              const __DRIextension *const*extensions)
> +{
> +    int i;
> +
> +    for (i = 0; extensions[i]; i++) {
> +        if (strcmp(extensions[i]->name, __DRI_IMAGE_LOADER) == 0)
> +           psp->image.loader = (__DRIimageLoaderExtension *) extensions[i];
> +    }
> +}
> +
> +static __DRIscreen *
> +image_create_new_screen_2(int scrn, int fd,
> +                          const __DRIextension **extensions,
> +                          const __DRIextension **driver_extensions,
> +                          const __DRIconfig ***driver_configs, void *data)
> +{
> +    static const __DRIextension *emptyExtensionList[] = { NULL };
> +    __DRIscreen *psp;
> +    drmVersionPtr version;
> +
> +    psp = calloc(1, sizeof(*psp));
> +    if (!psp)
> +	return NULL;
> +
> +    /* By default, use the global driDriverAPI symbol (non-megadrivers). */
> +    psp->driver = globalDriverAPI;
> +
> +    /* If the driver exposes its vtable through its extensions list
> +     * (megadrivers), use that instead.
> +     */
> +    if (driver_extensions) {
> +       for (int i = 0; driver_extensions[i]; i++) {
> +          if (strcmp(driver_extensions[i]->name, __DRI_DRIVER_VTABLE) == 0) {
> +             psp->driver =
> +                ((__DRIDriverVtableExtension *)driver_extensions[i])->vtable;
> +          }
> +       }
> +    }
> +
> +    setupLoaderExtensions(psp, extensions);
> +    setup_image_loader_extensions(psp, extensions);
> +
> +    version = drmGetVersion(fd);
> +    if (version) {
> +	psp->drm_version.major = version->version_major;
> +	psp->drm_version.minor = version->version_minor;
> +	psp->drm_version.patch = version->version_patchlevel;
> +	drmFreeVersion(version);
> +    }
> +
> +    psp->loaderPrivate = data;
> +
> +    psp->extensions = emptyExtensionList;
> +    psp->fd = fd;
> +    psp->myNum = scrn;
> +
> +    psp->api_mask = (1 << __DRI_API_OPENGL);

This is another reason this just needs to be dri2CreateNewScreen2.
The above regresses GLES* support.  I tried with your ~keithp dri3
branch and using dri2CreateNewScreen2 brings back the GLESes.

> +    *driver_configs = psp->driver->InitScreen(psp);
> +    if (*driver_configs == NULL) {
> +	free(psp);
> +	return NULL;
> +    }
> +
> +    driParseOptionInfo(&psp->optionInfo, __imageConfigOptions);
> +    driParseConfigFiles(&psp->optionCache, &psp->optionInfo, psp->myNum, "image");
> +
> +    return psp;
> +}
> +
> +
> +/** Image driver interface */
> +const __DRIimageDriverExtension driImageDriverExtension = {
> +    .base = { __DRI_IMAGE_DRIVER, __DRI_IMAGE_DRIVER_VERSION },
> +
> +    .createNewScreen2           = image_create_new_screen_2,
> +    .createNewDrawable          = driCreateNewDrawable,
> +    .createNewContext           = driCreateNewContext,
> +    .getAPIMask                 = driGetAPIMask,
> +    .createContextAttribs       = driCreateContextAttribs,
> +};

Having written the GBM and Wayland suport for this, it's pretty clear
that we just want to use __DRIdri2Extension instead of duplicating
these functions.  Support for the __DRIimage based getBuffers is a
small incremental patch to src/egl/drivers/dri2:

 src/egl/drivers/dri2/egl_dri2.c         |  29 +++++--
 src/egl/drivers/dri2/egl_dri2.h         |   5 +-
 src/egl/drivers/dri2/platform_drm.c     |  50 ++++++++++--
 src/egl/drivers/dri2/platform_wayland.c | 133 ++++++++++++++++++++++----------
 src/gbm/backends/dri/gbm_dri.c          |  32 +++++++-
 src/gbm/backends/dri/gbm_driint.h       |  11 ++-
 6 files changed, 199 insertions(+), 61 deletions(-)

The problem is that technically we would have to do:

      if (dri2_dpy->image_driver) {

         /* use dri2_dpy->image_driver->createContextAttribs

      } else if (dri2_dpy->dri2 &&
	         dri2_dpy->dri2->base.version >= 3) {

         /* use dri2_dpy->dri2->createContextAttribs

all over the place even though they end up calling the same function.
The only real purpose of __DRIimageDriverExtension is to let the
loader know that the DRI driver supports and will use
__DRIimageLoaderExtension if provided, but we can just expose an empty
dummy extension to indicate that.

> diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h
> index fd40769..6f5cd87 100644
> --- a/src/mesa/drivers/dri/common/dri_util.h
> +++ b/src/mesa/drivers/dri/common/dri_util.h
> @@ -174,6 +174,10 @@ struct __DRIscreenRec {
>  	__DRIuseInvalidateExtension *useInvalidate;
>      } dri2;
>  
> +    struct {
> +        __DRIimageLoaderExtension *loader;
> +    } image;
> +
>      driOptionCache optionInfo;
>      driOptionCache optionCache;
>  
> @@ -283,4 +287,6 @@ dri2InvalidateDrawable(__DRIdrawable *drawable);
>  extern void
>  driUpdateFramebufferSize(struct gl_context *ctx, const __DRIdrawable *dPriv);
>  
> +extern const __DRIimageDriverExtension driImageDriverExtension;
> +
>  #endif /* _DRI_UTIL_H_ */
> diff --git a/src/mesa/drivers/dri/i915/intel_context.c b/src/mesa/drivers/dri/i915/intel_context.c
> index 1798bc7..8d369ff 100644
> --- a/src/mesa/drivers/dri/i915/intel_context.c
> +++ b/src/mesa/drivers/dri/i915/intel_context.c
> @@ -91,6 +91,8 @@ intelGetString(struct gl_context * ctx, GLenum name)
>     }
>  }
>  
> +#define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
> +
>  static void
>  intel_flush_front(struct gl_context *ctx)
>  {
> @@ -100,11 +102,10 @@ intel_flush_front(struct gl_context *ctx)
>      __DRIscreen *const screen = intel->intelScreen->driScrnPriv;
>  
>      if (intel->front_buffer_dirty && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
> -      if (screen->dri2.loader->flushFrontBuffer != NULL &&
> +      if (flushFront(screen) && 
>            driDrawable &&
>            driDrawable->loaderPrivate) {
> -         screen->dri2.loader->flushFrontBuffer(driDrawable,
> -                                               driDrawable->loaderPrivate);
> +         flushFront(screen)(driDrawable, driDrawable->loaderPrivate);
>  
>  	 /* We set the dirty bit in intel_prepare_render() if we're
>  	  * front buffer rendering once we get there.
> @@ -114,6 +115,9 @@ intel_flush_front(struct gl_context *ctx)
>     }
>  }
>  
> +static void
> +intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable);
> +
>  static unsigned
>  intel_bits_per_pixel(const struct intel_renderbuffer *rb)
>  {
> @@ -194,7 +198,10 @@ intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
>     if (unlikely(INTEL_DEBUG & DEBUG_DRI))
>        fprintf(stderr, "enter %s, drawable %p\n", __func__, drawable);
>  
> -   intel_update_dri2_buffers(intel, drawable);
> +   if (screen->image.loader)
> +      intel_update_image_buffers(intel, drawable);
> +   else
> +      intel_update_dri2_buffers(intel, drawable);
>  
>     driUpdateFramebufferSize(&intel->ctx, drawable);
>  }
> @@ -787,3 +794,99 @@ intel_process_dri2_buffer(struct intel_context *intel,
>                                                   region);
>     intel_region_release(&region);
>  }
> +
> +/**
> + * \brief Query DRI Image loader to obtain a DRIdrawable's buffers.
> + *
> + * To determine which DRI buffers to request, examine the renderbuffers
> + * attached to the drawable's framebuffer. Then request the buffers with
> + * dri3
> + *
> + * This is called from intel_update_renderbuffers().
> + *
> + * \param drawable      Drawable whose buffers are queried.
> + * \param buffers       [out] List of buffers returned by DRI2 query.
> + * \param buffer_count  [out] Number of buffers returned.
> + *
> + * \see intel_update_renderbuffers()
> + */
> +
> +static void
> +intel_update_image_buffer(struct intel_context *intel,
> +                          __DRIdrawable *drawable,
> +                          struct intel_renderbuffer *rb,
> +                          __DRIimage *buffer,
> +                          enum __DRIimageBufferMask buffer_type)
> +{
> +   struct intel_region *region = buffer->region;
> +
> +   if (!rb || !region)
> +      return;
> +
> +   unsigned num_samples = rb->Base.Base.NumSamples;
> +
> +   if (rb->mt &&
> +       rb->mt->region &&
> +       rb->mt->region == region)
> +      return;
> +
> +   intel_miptree_release(&rb->mt);
> +   rb->mt = intel_miptree_create_for_image_buffer(intel,
> +                                                  buffer_type,
> +                                                  intel_rb_format(rb),
> +                                                  num_samples,
> +                                                  region);
> +}
> +
> +
> +static void
> +intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable)
> +{
> +   struct gl_framebuffer *fb = drawable->driverPrivate;
> +   __DRIscreen *screen = intel->intelScreen->driScrnPriv;
> +   struct intel_renderbuffer *front_rb;
> +   struct intel_renderbuffer *back_rb;
> +   struct __DRIimageList images;
> +   unsigned int format;
> +   uint32_t buffer_mask = 0;
> +
> +   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
> +   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> +
> +   if (back_rb)
> +      format = intel_rb_format(back_rb);
> +   else if (front_rb)
> +      format = intel_rb_format(front_rb);
> +   else
> +      return;
> +
> +   if ((intel->is_front_buffer_rendering || intel->is_front_buffer_reading || !back_rb) && front_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> +   if (back_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
> +
> +   (*screen->image.loader->getBuffers) (drawable,
> +                                        &drawable->w,
> +                                        &drawable->h,
> +                                        driGLFormatToImageFormat(format),
> +                                        &drawable->dri2.stamp,
> +                                        drawable->loaderPrivate,
> +                                        buffer_mask,
> +                                        &images);
> +
> +   if (images.front) {

I think we should only look at image.front if the
__DRI_IMAGE_BUFFER_FRONT bit is set.  We never want to set up a front
buffer that we didn't ask for and it seems wrong that the loader has
to clear image.front, if the driver didn't ask for front.

> +      assert(front_rb);
> +      intel_update_image_buffer(intel,
> +                                drawable,
> +                                front_rb,
> +                                images.front,
> +                                __DRI_IMAGE_BUFFER_FRONT);
> +   }
> +   if (images.back)

Same here.

> +      intel_update_image_buffer(intel,
> +                                drawable,
> +                                back_rb,
> +                                images.back,
> +                                __DRI_IMAGE_BUFFER_BACK);
> +}
> diff --git a/src/mesa/drivers/dri/i915/intel_mipmap_tree.c b/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
> index 8432b6d..66a7a92 100644
> --- a/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
> @@ -322,6 +322,39 @@ intel_miptree_create_for_dri2_buffer(struct intel_context *intel,
>     return mt;
>  }
>  
> +/**
> + * For a singlesample image buffer, this simply wraps the given region with a miptree.
> + *
> + * For a multisample image buffer, this wraps the given region with
> + * a singlesample miptree, then creates a multisample miptree into which the
> + * singlesample miptree is embedded as a child.
> + */
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct intel_context *intel,
> +                                      enum __DRIimageBufferMask buffer_type,
> +                                      gl_format format,
> +                                      uint32_t num_samples,
> +                                      struct intel_region *region)
> +{
> +   struct intel_mipmap_tree *mt = NULL;
> +
> +   /* Only the front and back buffers, which are color buffers, are allocated
> +    * through the image loader.
> +    */
> +   assert(_mesa_get_format_base_format(format) == GL_RGB ||
> +          _mesa_get_format_base_format(format) == GL_RGBA);
> +
> +   mt = intel_miptree_create_for_bo(intel,
> +                                    region->bo,
> +                                    format,
> +                                    0,
> +                                    region->width,
> +                                    region->height,
> +                                    region->pitch,
> +                                    region->tiling);
> +   return mt;
> +}
> +
>  struct intel_mipmap_tree*
>  intel_miptree_create_for_renderbuffer(struct intel_context *intel,
>                                        gl_format format,
> diff --git a/src/mesa/drivers/dri/i915/intel_mipmap_tree.h b/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
> index 1142af6..35cad6d 100644
> --- a/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
> @@ -32,6 +32,7 @@
>  
>  #include "intel_screen.h"
>  #include "intel_regions.h"
> +#include "GL/internal/dri_interface.h"
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -258,6 +259,13 @@ intel_miptree_create_for_dri2_buffer(struct intel_context *intel,
>                                       gl_format format,
>                                       struct intel_region *region);
>  
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct intel_context *intel,
> +                                      enum __DRIimageBufferMask buffer_type,
> +                                      gl_format format,
> +                                      uint32_t num_samples,
> +                                      struct intel_region *region);
> +
>  /**
>   * Create a miptree appropriate as the storage for a non-texture renderbuffer.
>   * The miptree has the following properties:
> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> index 12113c7..38badec 100644
> --- a/src/mesa/drivers/dri/i915/intel_screen.c
> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> @@ -1166,6 +1166,7 @@ static const struct __DRIDriverVtableExtensionRec i915_vtable = {
>  /* This is the table of extensions that the loader will dlsym() for. */
>  static const __DRIextension *i915_driver_extensions[] = {
>      &driCoreExtension.base,
> +    &driImageDriverExtension.base,
>      &driDRI2Extension.base,
>      &i915_vtable.base,
>      &i915_config_options.base,
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 2557d10..f3bf67f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -151,6 +151,8 @@ intelInvalidateState(struct gl_context * ctx, GLuint new_state)
>     brw->NewGLState |= new_state;
>  }
>  
> +#define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
> +
>  static void
>  intel_flush_front(struct gl_context *ctx)
>  {
> @@ -160,8 +162,7 @@ intel_flush_front(struct gl_context *ctx)
>     __DRIscreen *const screen = brw->intelScreen->driScrnPriv;
>  
>     if (brw->front_buffer_dirty && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
> -      if (screen->dri2.loader->flushFrontBuffer != NULL &&
> -          driDrawable &&
> +      if (flushFront(screen) && driDrawable &&
>            driDrawable->loaderPrivate) {
>  
>           /* Resolve before flushing FAKE_FRONT_LEFT to FRONT_LEFT.
> @@ -174,8 +175,7 @@ intel_flush_front(struct gl_context *ctx)
>           intel_resolve_for_dri2_flush(brw, driDrawable);
>           intel_batchbuffer_flush(brw);
>  
> -         screen->dri2.loader->flushFrontBuffer(driDrawable,
> -                                               driDrawable->loaderPrivate);
> +         flushFront(screen)(driDrawable, driDrawable->loaderPrivate);
>  
>           /* We set the dirty bit in intel_prepare_render() if we're
>            * front buffer rendering once we get there.
> @@ -549,7 +549,7 @@ brw_process_driconf_options(struct brw_context *brw)
>        driQueryOptionb(options, "disable_glsl_line_continuations");
>  }
>  
> -bool
> +GLboolean
>  brwCreateContext(gl_api api,
>  	         const struct gl_config *mesaVis,
>  		 __DRIcontext *driContextPriv,
> @@ -979,6 +979,9 @@ intel_process_dri2_buffer(struct brw_context *brw,
>                            const char *buffer_name);
>  
>  static void
> +intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable);
> +
> +static void
>  intel_update_dri2_buffers(struct brw_context *brw, __DRIdrawable *drawable)
>  {
>     struct gl_framebuffer *fb = drawable->driverPrivate;
> @@ -1038,6 +1041,7 @@ void
>  intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
>  {
>     struct brw_context *brw = context->driverPrivate;
> +   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
>  
>     /* Set this up front, so that in case our buffers get invalidated
>      * while we're getting new buffers, we don't clobber the stamp and
> @@ -1047,7 +1051,10 @@ intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
>     if (unlikely(INTEL_DEBUG & DEBUG_DRI))
>        fprintf(stderr, "enter %s, drawable %p\n", __func__, drawable);
>  
> -   intel_update_dri2_buffers(brw, drawable);
> +   if (screen->image.loader)
> +      intel_update_image_buffers(brw, drawable);
> +   else
> +      intel_update_dri2_buffers(brw, drawable);
>  
>     driUpdateFramebufferSize(&brw->ctx, drawable);
>  }
> @@ -1253,3 +1260,98 @@ intel_process_dri2_buffer(struct brw_context *brw,
>                                                   region);
>     intel_region_release(&region);
>  }
> +
> +/**
> + * \brief Query DRI image loader to obtain a DRIdrawable's buffers.
> + *
> + * To determine which DRI buffers to request, examine the renderbuffers
> + * attached to the drawable's framebuffer. Then request the buffers from
> + * the image loader
> + *
> + * This is called from intel_update_renderbuffers().
> + *
> + * \param drawable      Drawable whose buffers are queried.
> + * \param buffers       [out] List of buffers returned by DRI2 query.
> + * \param buffer_count  [out] Number of buffers returned.
> + *
> + * \see intel_update_renderbuffers()
> + */
> +
> +static void
> +intel_update_image_buffer(struct brw_context *intel,
> +                          __DRIdrawable *drawable,
> +                          struct intel_renderbuffer *rb,
> +                          __DRIimage *buffer,
> +                          enum __DRIimageBufferMask buffer_type)
> +{
> +   struct intel_region *region = buffer->region;
> +
> +   if (!rb || !region)
> +      return;
> +
> +   unsigned num_samples = rb->Base.Base.NumSamples;
> +
> +   if (rb->mt &&
> +       rb->mt->region &&
> +       rb->mt->region == region)
> +      return;
> +
> +   intel_miptree_release(&rb->mt);
> +   rb->mt = intel_miptree_create_for_image_buffer(intel,
> +                                                  buffer_type,
> +                                                  intel_rb_format(rb),
> +                                                  num_samples,
> +                                                  region);
> +}
> +
> +static void
> +intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
> +{
> +   struct gl_framebuffer *fb = drawable->driverPrivate;
> +   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
> +   struct intel_renderbuffer *front_rb;
> +   struct intel_renderbuffer *back_rb;
> +   struct __DRIimageList images;
> +   unsigned int format;
> +   uint32_t buffer_mask = 0;
> +
> +   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
> +   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> +
> +   if (back_rb)
> +      format = intel_rb_format(back_rb);
> +   else if (front_rb)
> +      format = intel_rb_format(front_rb);
> +   else
> +      return;
> +
> +   if ((brw->is_front_buffer_rendering || brw->is_front_buffer_reading || !back_rb) && front_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> +   if (back_rb)
> +      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
> +
> +   (*screen->image.loader->getBuffers) (drawable,
> +                                        &drawable->w,
> +                                        &drawable->h,
> +                                        driGLFormatToImageFormat(format),
> +                                        &drawable->dri2.stamp,
> +                                        drawable->loaderPrivate,
> +                                        buffer_mask,
> +                                        &images);
> +
> +   if (images.front) {
> +      assert(front_rb);
> +      intel_update_image_buffer(brw,
> +                                drawable,
> +                                front_rb,
> +                                images.front,
> +                                __DRI_IMAGE_BUFFER_FRONT);
> +   }
> +   if (images.back)
> +      intel_update_image_buffer(brw,
> +                                drawable,
> +                                back_rb,
> +                                images.back,
> +                                __DRI_IMAGE_BUFFER_BACK);
> +}
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index bec4d6b..1ecbfb7 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1477,14 +1477,14 @@ void intel_prepare_render(struct brw_context *brw);
>  void intel_resolve_for_dri2_flush(struct brw_context *brw,
>                                    __DRIdrawable *drawable);
>  
> -bool brwCreateContext(gl_api api,
> -		      const struct gl_config *mesaVis,
> -		      __DRIcontext *driContextPriv,
> -                      unsigned major_version,
> -                      unsigned minor_version,
> -                      uint32_t flags,
> -                      unsigned *error,
> -		      void *sharedContextPrivate);
> +GLboolean brwCreateContext(gl_api api,
> +                           const struct gl_config *mesaVis,
> +                           __DRIcontext *driContextPriv,
> +                           unsigned major_version,
> +                           unsigned minor_version,
> +                           uint32_t flags,
> +                           unsigned *error,
> +                           void *sharedContextPrivate);
>  
>  /*======================================================================
>   * brw_misc_state.c
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 2f5e04f..08dbe88 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -725,6 +725,67 @@ intel_miptree_create_for_dri2_buffer(struct brw_context *brw,
>     return multisample_mt;
>  }
>  
> +/**
> + * For a singlesample image buffer, this simply wraps the given region with a miptree.
> + *
> + * For a multisample image buffer, this wraps the given region with
> + * a singlesample miptree, then creates a multisample miptree into which the
> + * singlesample miptree is embedded as a child.
> + */
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct brw_context *intel,
> +                                      enum __DRIimageBufferMask buffer_type,
> +                                      gl_format format,
> +                                      uint32_t num_samples,
> +                                      struct intel_region *region)
> +{
> +   struct intel_mipmap_tree *singlesample_mt = NULL;
> +   struct intel_mipmap_tree *multisample_mt = NULL;
> +
> +   /* Only the front and back buffers, which are color buffers, are allocated
> +    * through the image loader.
> +    */
> +   assert(_mesa_get_format_base_format(format) == GL_RGB ||
> +          _mesa_get_format_base_format(format) == GL_RGBA);
> +
> +   singlesample_mt = intel_miptree_create_for_bo(intel,
> +                                                 region->bo,
> +                                                 format,
> +                                                 0,
> +                                                 region->width,
> +                                                 region->height,
> +                                                 region->pitch,
> +                                                 region->tiling);
> +   if (!singlesample_mt)
> +      return NULL;
> +
> +   intel_region_reference(&singlesample_mt->region, region);
> +
> +   if (num_samples == 0)
> +      return singlesample_mt;
> +
> +   multisample_mt = intel_miptree_create_for_renderbuffer(intel,
> +                                                          format,
> +                                                          region->width,
> +                                                          region->height,
> +                                                          num_samples);
> +   if (!multisample_mt) {
> +      intel_miptree_release(&singlesample_mt);
> +      return NULL;
> +   }
> +
> +   multisample_mt->singlesample_mt = singlesample_mt;
> +   multisample_mt->need_downsample = false;
> +
> +   intel_region_reference(&multisample_mt->region, region);
> +
> +   if (intel->is_front_buffer_rendering && buffer_type == __DRI_IMAGE_BUFFER_FRONT) {
> +      intel_miptree_upsample(intel, multisample_mt);
> +   }
> +
> +   return multisample_mt;
> +}
> +
>  struct intel_mipmap_tree*
>  intel_miptree_create_for_renderbuffer(struct brw_context *brw,
>                                        gl_format format,
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index d718125..8777a8c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -32,6 +32,7 @@
>  
>  #include "intel_regions.h"
>  #include "intel_resolve_map.h"
> +#include <GL/internal/dri_interface.h>
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -529,6 +530,13 @@ intel_miptree_create_for_dri2_buffer(struct brw_context *brw,
>                                       uint32_t num_samples,
>                                       struct intel_region *region);
>  
> +struct intel_mipmap_tree*
> +intel_miptree_create_for_image_buffer(struct brw_context *intel,
> +                                     enum __DRIimageBufferMask buffer_type,
> +                                     gl_format format,
> +                                     uint32_t num_samples,
> +                                     struct intel_region *region);
> +
>  /**
>   * Create a miptree appropriate as the storage for a non-texture renderbuffer.
>   * The miptree has the following properties:
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index f9339c1..7571921 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1176,7 +1176,8 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>  {
>     struct intel_screen *intelScreen;
>  
> -   if (psp->dri2.loader->base.version <= 2 ||
> +   if (psp->image.loader) {
> +   } else if (psp->dri2.loader->base.version <= 2 ||
>         psp->dri2.loader->getBuffersWithFormat == NULL) {
>        fprintf(stderr,
>  	      "\nERROR!  DRI2 loader with getBuffersWithFormat() "
> @@ -1264,7 +1265,6 @@ intelReleaseBuffer(__DRIscreen *screen, __DRIbuffer *buffer)
>     free(intelBuffer);
>  }
>  
> -
>  static const struct __DriverAPIRec brw_driver_api = {
>     .InitScreen		 = intelInitScreen2,
>     .DestroyScreen	 = intelDestroyScreen,
> @@ -1285,6 +1285,7 @@ static const struct __DRIDriverVtableExtensionRec brw_vtable = {
>  
>  static const __DRIextension *brw_driver_extensions[] = {
>      &driCoreExtension.base,
> +    &driImageDriverExtension.base,
>      &driDRI2Extension.base,
>      &brw_vtable.base,
>      &brw_config_options.base,
> -- 
> 1.8.4.2
>
Keith Packard Nov. 6, 2013, 2:55 p.m. UTC | #7
Kristian Høgsberg <hoegsberg@gmail.com> writes:

> Having written the GBM and Wayland suport for this, it's pretty clear
> that we just want to use __DRIdri2Extension instead of duplicating
> these functions.  Support for the __DRIimage based getBuffers is a
> small incremental patch to src/egl/drivers/dri2:

That would require drivers to support all of the DRI2 functions, rather
than just the few needed for Image support though.

> The problem is that technically we would have to do:
>
>       if (dri2_dpy->image_driver) {
>
>          /* use dri2_dpy->image_driver->createContextAttribs
>
>       } else if (dri2_dpy->dri2 &&
> 	         dri2_dpy->dri2->base.version >= 3) {
>
>          /* use dri2_dpy->dri2->createContextAttribs
>
> all over the place even though they end up calling the same function.
> The only real purpose of __DRIimageDriverExtension is to let the
> loader know that the DRI driver supports and will use
> __DRIimageLoaderExtension if provided, but we can just expose an empty
> dummy extension to indicate that.

Alternatively, the loader could simply call through the DRI2 version,
acknowledging that they must be the same function? I'd sure like to be
able to create an ImageLoader-only driver, even if that's not practical
today.

A couple of other options:

 * Move the shared functions into a new structure which is embedded
   in both the DRIdri2ExtensionRec and the DRIimageDriverExtensionRec,
   then the loader could set a pointer to whichever it wanted to use
   in it's own structure.

 * Have the loader pull the functions out of the ExtensionRec and stick
   them directly into its own structure. Then it could use those
   directly and avoid version checks while running.

But, if we just want to use the DRI2 driver interface and create an
extension purely to mark that the driver will use an Image loader,
that's easy too.

> I think we should only look at image.front if the
> __DRI_IMAGE_BUFFER_FRONT bit is set.  We never want to set up a front
> buffer that we didn't ask for and it seems wrong that the loader has
> to clear image.front, if the driver didn't ask for front.

I simply duplicated the logic from the DRI2 path which works exactly the
same way (asks for buffers, then looks to see what it got). I didn't dig
into the details further than this, I'm afraid. The only case where asks
!= received is for pixmap targets where you always get a front buffer. I
assumed there was some deep semantic requirement for that...
Kristian Høgsberg Nov. 6, 2013, 4:17 p.m. UTC | #8
On Wed, Nov 6, 2013 at 6:55 AM, Keith Packard <keithp@keithp.com> wrote:
> Kristian Høgsberg <hoegsberg@gmail.com> writes:
>
>> Having written the GBM and Wayland suport for this, it's pretty clear
>> that we just want to use __DRIdri2Extension instead of duplicating
>> these functions.  Support for the __DRIimage based getBuffers is a
>> small incremental patch to src/egl/drivers/dri2:
>
> That would require drivers to support all of the DRI2 functions, rather
> than just the few needed for Image support though.

It just the two older create context functions (which fall back to
calling driCreateContextAtribs) and allocateBuffer and releaseBuffer.
The two buffer functions are __DRIbuffer specific of course, but we
can implement them in terms of __DRIimage in dri_util.c now.

>> The problem is that technically we would have to do:
>>
>>       if (dri2_dpy->image_driver) {
>>
>>          /* use dri2_dpy->image_driver->createContextAttribs
>>
>>       } else if (dri2_dpy->dri2 &&
>>                dri2_dpy->dri2->base.version >= 3) {
>>
>>          /* use dri2_dpy->dri2->createContextAttribs
>>
>> all over the place even though they end up calling the same function.
>> The only real purpose of __DRIimageDriverExtension is to let the
>> loader know that the DRI driver supports and will use
>> __DRIimageLoaderExtension if provided, but we can just expose an empty
>> dummy extension to indicate that.
>
> Alternatively, the loader could simply call through the DRI2 version,
> acknowledging that they must be the same function? I'd sure like to be
> able to create an ImageLoader-only driver, even if that's not practical
> today.
>
> A couple of other options:
>
>  * Move the shared functions into a new structure which is embedded
>    in both the DRIdri2ExtensionRec and the DRIimageDriverExtensionRec,
>    then the loader could set a pointer to whichever it wanted to use
>    in it's own structure.
>
>  * Have the loader pull the functions out of the ExtensionRec and stick
>    them directly into its own structure. Then it could use those
>    directly and avoid version checks while running.

There is a third option, which is to pull the functions we need from
__DRIcoreExtension into the __DRIimageDriverExtension, which then is
all you need along with __DRIimageExtension.  This is as painful in
the short term as the current __DRIimageDriverExtension, but it lets
of cut loose of the DRI1 (__DRIcoreExtension has the DRI1
createNewScreen in it) and DRI2 baggage properly later on.  And
pulling out the functions into a loader private struct as you suggest
will make it a lot less painful.  The functions we move from core to
_DRIimageDriverExtension will share the same implementation in
dri_util.c so there's no new code.

> But, if we just want to use the DRI2 driver interface and create an
> extension purely to mark that the driver will use an Image loader,
> that's easy too.

Right - I actually like the clean break idea, but if we're going to
take that pain I want to get rid of all the junk and avoid the awkward
"use some stuff from __DRIcoreExtension and other stuff from
__DRIimageDriverExtension" setup.  So we should either 1) make
__DRIimageDriverExtension completely replace __DRIcoreExtension and
__DRIdri2Extension, or 2) just do a minimal, incremental change (just
the extension to indicate the support for __DRIimage based
getbuffers).

Kristian

>> I think we should only look at image.front if the
>> __DRI_IMAGE_BUFFER_FRONT bit is set.  We never want to set up a front
>> buffer that we didn't ask for and it seems wrong that the loader has
>> to clear image.front, if the driver didn't ask for front.
>
> I simply duplicated the logic from the DRI2 path which works exactly the
> same way (asks for buffers, then looks to see what it got). I didn't dig
> into the details further than this, I'm afraid. The only case where asks
> != received is for pixmap targets where you always get a front buffer. I
> assumed there was some deep semantic requirement for that...

The difference is that there the loader returns a packed array of the
buffers the driver asked for. Now we're using a struct which can be
sparsely populated, so the driver should only look at the fields it
asked for.
diff mbox

Patch

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index 907aeca..8fc1fa6 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -86,6 +86,10 @@  typedef struct __DRIdri2LoaderExtensionRec	__DRIdri2LoaderExtension;
 typedef struct __DRI2flushExtensionRec	__DRI2flushExtension;
 typedef struct __DRI2throttleExtensionRec	__DRI2throttleExtension;
 
+
+typedef struct __DRIimageLoaderExtensionRec     __DRIimageLoaderExtension;
+typedef struct __DRIimageDriverExtensionRec     __DRIimageDriverExtension;
+
 /*@}*/
 
 
@@ -1288,4 +1292,112 @@  typedef struct __DRIDriverVtableExtensionRec {
     const struct __DriverAPIRec *vtable;
 } __DRIDriverVtableExtension;
 
+/**
+ * Image Loader extension. Drivers use this to allocate color buffers
+ */
+
+#define __DRI_DRIVER_EXTENSIONS "__driDriverExtensions"
+
+enum __DRIimageBufferMask {
+   __DRI_IMAGE_BUFFER_BACK = (1 << 0),
+   __DRI_IMAGE_BUFFER_FRONT = (1 << 1)
+};
+
+struct __DRIimageList {
+   __DRIimage *back;
+   __DRIimage *front;
+};
+
+#define __DRI_IMAGE_LOADER "DRI_IMAGE_LOADER"
+#define __DRI_IMAGE_LOADER_VERSION 1
+
+struct __DRIimageLoaderExtensionRec {
+    __DRIextension base;
+
+   /**
+    * Allocate color buffers.
+    *
+    * \param driDrawable
+    * \param width              Width of allocated buffers
+    * \param height             Height of allocated buffers
+    * \param format             one of __DRI_IMAGE_FORMAT_*
+    * \param stamp              Address of variable to be updated when
+    *                           getBuffers must be called again
+    * \param loaderPrivate      The loaderPrivate for driDrawable
+    * \param buffer_mask        Set of buffers to allocate
+    * \param buffers            Returned buffers
+    */
+   int (*getBuffers)(__DRIdrawable *driDrawable,
+                     int *width, int *height,
+                     unsigned int format,
+                     uint32_t *stamp,
+                     void *loaderPrivate,
+                     uint32_t buffer_mask,
+                     struct __DRIimageList *buffers);
+
+    /**
+     * Flush pending front-buffer rendering
+     *
+     * Any rendering that has been performed to the
+     * fake front will be flushed to the front
+     *
+     * \param driDrawable    Drawable whose front-buffer is to be flushed
+     * \param loaderPrivate  Loader's private data that was previously passed
+     *                       into __DRIdri2ExtensionRec::createNewDrawable
+     */
+    void (*flushFrontBuffer)(__DRIdrawable *driDrawable, void *loaderPrivate);
+};
+
+/**
+ * DRI extension.
+ */
+
+//struct gl_context;
+//struct dd_function_table;
+
+typedef __DRIscreen *
+(*__DRIcreateNewScreen2)(int screen, int fd,
+                         const __DRIextension **extensions,
+                         const __DRIextension **driver_extensions,
+                         const __DRIconfig ***driver_configs,
+                         void *loaderPrivate);
+
+typedef __DRIdrawable *
+(*__DRIcreateNewDrawable)(__DRIscreen *screen,
+                          const __DRIconfig *config,
+                          void *loaderPrivate);
+
+typedef __DRIcontext *
+(*__DRIcreateNewContext)(__DRIscreen *screen,
+                         const __DRIconfig *config,
+                         __DRIcontext *shared,
+                         void *loaderPrivate);
+
+typedef __DRIcontext *
+(*__DRIcreateContextAttribs)(__DRIscreen *screen,
+                             int api,
+                             const __DRIconfig *config,
+                             __DRIcontext *shared,
+                             unsigned num_attribs,
+                             const uint32_t *attribs,
+                             unsigned *error,
+                             void *loaderPrivate);
+
+typedef unsigned int
+(*__DRIgetAPIMask)(__DRIscreen *screen);
+
+#define __DRI_IMAGE_DRIVER           "DRI_IMAGE_DRIVER"
+#define __DRI_IMAGE_DRIVER_VERSION   1
+
+struct __DRIimageDriverExtensionRec {
+   __DRIextension               base;
+
+   /* Common DRI functions, shared with DRI2 */
+   __DRIcreateNewScreen2        createNewScreen2;
+   __DRIcreateNewDrawable       createNewDrawable;
+   __DRIcreateNewContext        createNewContext;
+   __DRIcreateContextAttribs    createContextAttribs;
+   __DRIgetAPIMask              getAPIMask;
+};
+
 #endif
diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
index 76c8ae5..d8cb7f6 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -854,3 +854,116 @@  driImageFormatToGLFormat(uint32_t image_format)
       return MESA_FORMAT_NONE;
    }
 }
+
+/*
+ * Copyright © 2013 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+PUBLIC const char __imageConfigOptions[] =
+   DRI_CONF_BEGIN
+      DRI_CONF_SECTION_PERFORMANCE
+         DRI_CONF_VBLANK_MODE(DRI_CONF_VBLANK_DEF_INTERVAL_1)
+      DRI_CONF_SECTION_END
+   DRI_CONF_END;
+
+static void
+setup_image_loader_extensions(__DRIscreen *psp,
+                              const __DRIextension *const*extensions)
+{
+    int i;
+
+    for (i = 0; extensions[i]; i++) {
+        if (strcmp(extensions[i]->name, __DRI_IMAGE_LOADER) == 0)
+           psp->image.loader = (__DRIimageLoaderExtension *) extensions[i];
+    }
+}
+
+static __DRIscreen *
+image_create_new_screen_2(int scrn, int fd,
+                          const __DRIextension **extensions,
+                          const __DRIextension **driver_extensions,
+                          const __DRIconfig ***driver_configs, void *data)
+{
+    static const __DRIextension *emptyExtensionList[] = { NULL };
+    __DRIscreen *psp;
+    drmVersionPtr version;
+
+    psp = calloc(1, sizeof(*psp));
+    if (!psp)
+	return NULL;
+
+    /* By default, use the global driDriverAPI symbol (non-megadrivers). */
+    psp->driver = globalDriverAPI;
+
+    /* If the driver exposes its vtable through its extensions list
+     * (megadrivers), use that instead.
+     */
+    if (driver_extensions) {
+       for (int i = 0; driver_extensions[i]; i++) {
+          if (strcmp(driver_extensions[i]->name, __DRI_DRIVER_VTABLE) == 0) {
+             psp->driver =
+                ((__DRIDriverVtableExtension *)driver_extensions[i])->vtable;
+          }
+       }
+    }
+
+    setupLoaderExtensions(psp, extensions);
+    setup_image_loader_extensions(psp, extensions);
+
+    version = drmGetVersion(fd);
+    if (version) {
+	psp->drm_version.major = version->version_major;
+	psp->drm_version.minor = version->version_minor;
+	psp->drm_version.patch = version->version_patchlevel;
+	drmFreeVersion(version);
+    }
+
+    psp->loaderPrivate = data;
+
+    psp->extensions = emptyExtensionList;
+    psp->fd = fd;
+    psp->myNum = scrn;
+
+    psp->api_mask = (1 << __DRI_API_OPENGL);
+
+    *driver_configs = psp->driver->InitScreen(psp);
+    if (*driver_configs == NULL) {
+	free(psp);
+	return NULL;
+    }
+
+    driParseOptionInfo(&psp->optionInfo, __imageConfigOptions);
+    driParseConfigFiles(&psp->optionCache, &psp->optionInfo, psp->myNum, "image");
+
+    return psp;
+}
+
+
+/** Image driver interface */
+const __DRIimageDriverExtension driImageDriverExtension = {
+    .base = { __DRI_IMAGE_DRIVER, __DRI_IMAGE_DRIVER_VERSION },
+
+    .createNewScreen2           = image_create_new_screen_2,
+    .createNewDrawable          = driCreateNewDrawable,
+    .createNewContext           = driCreateNewContext,
+    .getAPIMask                 = driGetAPIMask,
+    .createContextAttribs       = driCreateContextAttribs,
+};
diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h
index fd40769..6f5cd87 100644
--- a/src/mesa/drivers/dri/common/dri_util.h
+++ b/src/mesa/drivers/dri/common/dri_util.h
@@ -174,6 +174,10 @@  struct __DRIscreenRec {
 	__DRIuseInvalidateExtension *useInvalidate;
     } dri2;
 
+    struct {
+        __DRIimageLoaderExtension *loader;
+    } image;
+
     driOptionCache optionInfo;
     driOptionCache optionCache;
 
@@ -283,4 +287,6 @@  dri2InvalidateDrawable(__DRIdrawable *drawable);
 extern void
 driUpdateFramebufferSize(struct gl_context *ctx, const __DRIdrawable *dPriv);
 
+extern const __DRIimageDriverExtension driImageDriverExtension;
+
 #endif /* _DRI_UTIL_H_ */
diff --git a/src/mesa/drivers/dri/i915/intel_context.c b/src/mesa/drivers/dri/i915/intel_context.c
index 1798bc7..8d369ff 100644
--- a/src/mesa/drivers/dri/i915/intel_context.c
+++ b/src/mesa/drivers/dri/i915/intel_context.c
@@ -91,6 +91,8 @@  intelGetString(struct gl_context * ctx, GLenum name)
    }
 }
 
+#define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
+
 static void
 intel_flush_front(struct gl_context *ctx)
 {
@@ -100,11 +102,10 @@  intel_flush_front(struct gl_context *ctx)
     __DRIscreen *const screen = intel->intelScreen->driScrnPriv;
 
     if (intel->front_buffer_dirty && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
-      if (screen->dri2.loader->flushFrontBuffer != NULL &&
+      if (flushFront(screen) && 
           driDrawable &&
           driDrawable->loaderPrivate) {
-         screen->dri2.loader->flushFrontBuffer(driDrawable,
-                                               driDrawable->loaderPrivate);
+         flushFront(screen)(driDrawable, driDrawable->loaderPrivate);
 
 	 /* We set the dirty bit in intel_prepare_render() if we're
 	  * front buffer rendering once we get there.
@@ -114,6 +115,9 @@  intel_flush_front(struct gl_context *ctx)
    }
 }
 
+static void
+intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable);
+
 static unsigned
 intel_bits_per_pixel(const struct intel_renderbuffer *rb)
 {
@@ -194,7 +198,10 @@  intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
    if (unlikely(INTEL_DEBUG & DEBUG_DRI))
       fprintf(stderr, "enter %s, drawable %p\n", __func__, drawable);
 
-   intel_update_dri2_buffers(intel, drawable);
+   if (screen->image.loader)
+      intel_update_image_buffers(intel, drawable);
+   else
+      intel_update_dri2_buffers(intel, drawable);
 
    driUpdateFramebufferSize(&intel->ctx, drawable);
 }
@@ -787,3 +794,99 @@  intel_process_dri2_buffer(struct intel_context *intel,
                                                  region);
    intel_region_release(&region);
 }
+
+/**
+ * \brief Query DRI Image loader to obtain a DRIdrawable's buffers.
+ *
+ * To determine which DRI buffers to request, examine the renderbuffers
+ * attached to the drawable's framebuffer. Then request the buffers with
+ * dri3
+ *
+ * This is called from intel_update_renderbuffers().
+ *
+ * \param drawable      Drawable whose buffers are queried.
+ * \param buffers       [out] List of buffers returned by DRI2 query.
+ * \param buffer_count  [out] Number of buffers returned.
+ *
+ * \see intel_update_renderbuffers()
+ */
+
+static void
+intel_update_image_buffer(struct intel_context *intel,
+                          __DRIdrawable *drawable,
+                          struct intel_renderbuffer *rb,
+                          __DRIimage *buffer,
+                          enum __DRIimageBufferMask buffer_type)
+{
+   struct intel_region *region = buffer->region;
+
+   if (!rb || !region)
+      return;
+
+   unsigned num_samples = rb->Base.Base.NumSamples;
+
+   if (rb->mt &&
+       rb->mt->region &&
+       rb->mt->region == region)
+      return;
+
+   intel_miptree_release(&rb->mt);
+   rb->mt = intel_miptree_create_for_image_buffer(intel,
+                                                  buffer_type,
+                                                  intel_rb_format(rb),
+                                                  num_samples,
+                                                  region);
+}
+
+
+static void
+intel_update_image_buffers(struct intel_context *intel, __DRIdrawable *drawable)
+{
+   struct gl_framebuffer *fb = drawable->driverPrivate;
+   __DRIscreen *screen = intel->intelScreen->driScrnPriv;
+   struct intel_renderbuffer *front_rb;
+   struct intel_renderbuffer *back_rb;
+   struct __DRIimageList images;
+   unsigned int format;
+   uint32_t buffer_mask = 0;
+
+   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
+   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
+
+   if (back_rb)
+      format = intel_rb_format(back_rb);
+   else if (front_rb)
+      format = intel_rb_format(front_rb);
+   else
+      return;
+
+   if ((intel->is_front_buffer_rendering || intel->is_front_buffer_reading || !back_rb) && front_rb)
+      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
+
+   if (back_rb)
+      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
+
+   (*screen->image.loader->getBuffers) (drawable,
+                                        &drawable->w,
+                                        &drawable->h,
+                                        driGLFormatToImageFormat(format),
+                                        &drawable->dri2.stamp,
+                                        drawable->loaderPrivate,
+                                        buffer_mask,
+                                        &images);
+
+   if (images.front) {
+      assert(front_rb);
+      intel_update_image_buffer(intel,
+                                drawable,
+                                front_rb,
+                                images.front,
+                                __DRI_IMAGE_BUFFER_FRONT);
+   }
+   if (images.back)
+      intel_update_image_buffer(intel,
+                                drawable,
+                                back_rb,
+                                images.back,
+                                __DRI_IMAGE_BUFFER_BACK);
+}
diff --git a/src/mesa/drivers/dri/i915/intel_mipmap_tree.c b/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
index 8432b6d..66a7a92 100644
--- a/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i915/intel_mipmap_tree.c
@@ -322,6 +322,39 @@  intel_miptree_create_for_dri2_buffer(struct intel_context *intel,
    return mt;
 }
 
+/**
+ * For a singlesample image buffer, this simply wraps the given region with a miptree.
+ *
+ * For a multisample image buffer, this wraps the given region with
+ * a singlesample miptree, then creates a multisample miptree into which the
+ * singlesample miptree is embedded as a child.
+ */
+struct intel_mipmap_tree*
+intel_miptree_create_for_image_buffer(struct intel_context *intel,
+                                      enum __DRIimageBufferMask buffer_type,
+                                      gl_format format,
+                                      uint32_t num_samples,
+                                      struct intel_region *region)
+{
+   struct intel_mipmap_tree *mt = NULL;
+
+   /* Only the front and back buffers, which are color buffers, are allocated
+    * through the image loader.
+    */
+   assert(_mesa_get_format_base_format(format) == GL_RGB ||
+          _mesa_get_format_base_format(format) == GL_RGBA);
+
+   mt = intel_miptree_create_for_bo(intel,
+                                    region->bo,
+                                    format,
+                                    0,
+                                    region->width,
+                                    region->height,
+                                    region->pitch,
+                                    region->tiling);
+   return mt;
+}
+
 struct intel_mipmap_tree*
 intel_miptree_create_for_renderbuffer(struct intel_context *intel,
                                       gl_format format,
diff --git a/src/mesa/drivers/dri/i915/intel_mipmap_tree.h b/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
index 1142af6..35cad6d 100644
--- a/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i915/intel_mipmap_tree.h
@@ -32,6 +32,7 @@ 
 
 #include "intel_screen.h"
 #include "intel_regions.h"
+#include "GL/internal/dri_interface.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -258,6 +259,13 @@  intel_miptree_create_for_dri2_buffer(struct intel_context *intel,
                                      gl_format format,
                                      struct intel_region *region);
 
+struct intel_mipmap_tree*
+intel_miptree_create_for_image_buffer(struct intel_context *intel,
+                                      enum __DRIimageBufferMask buffer_type,
+                                      gl_format format,
+                                      uint32_t num_samples,
+                                      struct intel_region *region);
+
 /**
  * Create a miptree appropriate as the storage for a non-texture renderbuffer.
  * The miptree has the following properties:
diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
index 12113c7..38badec 100644
--- a/src/mesa/drivers/dri/i915/intel_screen.c
+++ b/src/mesa/drivers/dri/i915/intel_screen.c
@@ -1166,6 +1166,7 @@  static const struct __DRIDriverVtableExtensionRec i915_vtable = {
 /* This is the table of extensions that the loader will dlsym() for. */
 static const __DRIextension *i915_driver_extensions[] = {
     &driCoreExtension.base,
+    &driImageDriverExtension.base,
     &driDRI2Extension.base,
     &i915_vtable.base,
     &i915_config_options.base,
diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 2557d10..f3bf67f 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -151,6 +151,8 @@  intelInvalidateState(struct gl_context * ctx, GLuint new_state)
    brw->NewGLState |= new_state;
 }
 
+#define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
+
 static void
 intel_flush_front(struct gl_context *ctx)
 {
@@ -160,8 +162,7 @@  intel_flush_front(struct gl_context *ctx)
    __DRIscreen *const screen = brw->intelScreen->driScrnPriv;
 
    if (brw->front_buffer_dirty && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
-      if (screen->dri2.loader->flushFrontBuffer != NULL &&
-          driDrawable &&
+      if (flushFront(screen) && driDrawable &&
           driDrawable->loaderPrivate) {
 
          /* Resolve before flushing FAKE_FRONT_LEFT to FRONT_LEFT.
@@ -174,8 +175,7 @@  intel_flush_front(struct gl_context *ctx)
          intel_resolve_for_dri2_flush(brw, driDrawable);
          intel_batchbuffer_flush(brw);
 
-         screen->dri2.loader->flushFrontBuffer(driDrawable,
-                                               driDrawable->loaderPrivate);
+         flushFront(screen)(driDrawable, driDrawable->loaderPrivate);
 
          /* We set the dirty bit in intel_prepare_render() if we're
           * front buffer rendering once we get there.
@@ -549,7 +549,7 @@  brw_process_driconf_options(struct brw_context *brw)
       driQueryOptionb(options, "disable_glsl_line_continuations");
 }
 
-bool
+GLboolean
 brwCreateContext(gl_api api,
 	         const struct gl_config *mesaVis,
 		 __DRIcontext *driContextPriv,
@@ -979,6 +979,9 @@  intel_process_dri2_buffer(struct brw_context *brw,
                           const char *buffer_name);
 
 static void
+intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable);
+
+static void
 intel_update_dri2_buffers(struct brw_context *brw, __DRIdrawable *drawable)
 {
    struct gl_framebuffer *fb = drawable->driverPrivate;
@@ -1038,6 +1041,7 @@  void
 intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
 {
    struct brw_context *brw = context->driverPrivate;
+   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
 
    /* Set this up front, so that in case our buffers get invalidated
     * while we're getting new buffers, we don't clobber the stamp and
@@ -1047,7 +1051,10 @@  intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
    if (unlikely(INTEL_DEBUG & DEBUG_DRI))
       fprintf(stderr, "enter %s, drawable %p\n", __func__, drawable);
 
-   intel_update_dri2_buffers(brw, drawable);
+   if (screen->image.loader)
+      intel_update_image_buffers(brw, drawable);
+   else
+      intel_update_dri2_buffers(brw, drawable);
 
    driUpdateFramebufferSize(&brw->ctx, drawable);
 }
@@ -1253,3 +1260,98 @@  intel_process_dri2_buffer(struct brw_context *brw,
                                                  region);
    intel_region_release(&region);
 }
+
+/**
+ * \brief Query DRI image loader to obtain a DRIdrawable's buffers.
+ *
+ * To determine which DRI buffers to request, examine the renderbuffers
+ * attached to the drawable's framebuffer. Then request the buffers from
+ * the image loader
+ *
+ * This is called from intel_update_renderbuffers().
+ *
+ * \param drawable      Drawable whose buffers are queried.
+ * \param buffers       [out] List of buffers returned by DRI2 query.
+ * \param buffer_count  [out] Number of buffers returned.
+ *
+ * \see intel_update_renderbuffers()
+ */
+
+static void
+intel_update_image_buffer(struct brw_context *intel,
+                          __DRIdrawable *drawable,
+                          struct intel_renderbuffer *rb,
+                          __DRIimage *buffer,
+                          enum __DRIimageBufferMask buffer_type)
+{
+   struct intel_region *region = buffer->region;
+
+   if (!rb || !region)
+      return;
+
+   unsigned num_samples = rb->Base.Base.NumSamples;
+
+   if (rb->mt &&
+       rb->mt->region &&
+       rb->mt->region == region)
+      return;
+
+   intel_miptree_release(&rb->mt);
+   rb->mt = intel_miptree_create_for_image_buffer(intel,
+                                                  buffer_type,
+                                                  intel_rb_format(rb),
+                                                  num_samples,
+                                                  region);
+}
+
+static void
+intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
+{
+   struct gl_framebuffer *fb = drawable->driverPrivate;
+   __DRIscreen *screen = brw->intelScreen->driScrnPriv;
+   struct intel_renderbuffer *front_rb;
+   struct intel_renderbuffer *back_rb;
+   struct __DRIimageList images;
+   unsigned int format;
+   uint32_t buffer_mask = 0;
+
+   front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
+   back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
+
+   if (back_rb)
+      format = intel_rb_format(back_rb);
+   else if (front_rb)
+      format = intel_rb_format(front_rb);
+   else
+      return;
+
+   if ((brw->is_front_buffer_rendering || brw->is_front_buffer_reading || !back_rb) && front_rb)
+      buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
+
+   if (back_rb)
+      buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
+
+   (*screen->image.loader->getBuffers) (drawable,
+                                        &drawable->w,
+                                        &drawable->h,
+                                        driGLFormatToImageFormat(format),
+                                        &drawable->dri2.stamp,
+                                        drawable->loaderPrivate,
+                                        buffer_mask,
+                                        &images);
+
+   if (images.front) {
+      assert(front_rb);
+      intel_update_image_buffer(brw,
+                                drawable,
+                                front_rb,
+                                images.front,
+                                __DRI_IMAGE_BUFFER_FRONT);
+   }
+   if (images.back)
+      intel_update_image_buffer(brw,
+                                drawable,
+                                back_rb,
+                                images.back,
+                                __DRI_IMAGE_BUFFER_BACK);
+}
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index bec4d6b..1ecbfb7 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1477,14 +1477,14 @@  void intel_prepare_render(struct brw_context *brw);
 void intel_resolve_for_dri2_flush(struct brw_context *brw,
                                   __DRIdrawable *drawable);
 
-bool brwCreateContext(gl_api api,
-		      const struct gl_config *mesaVis,
-		      __DRIcontext *driContextPriv,
-                      unsigned major_version,
-                      unsigned minor_version,
-                      uint32_t flags,
-                      unsigned *error,
-		      void *sharedContextPrivate);
+GLboolean brwCreateContext(gl_api api,
+                           const struct gl_config *mesaVis,
+                           __DRIcontext *driContextPriv,
+                           unsigned major_version,
+                           unsigned minor_version,
+                           uint32_t flags,
+                           unsigned *error,
+                           void *sharedContextPrivate);
 
 /*======================================================================
  * brw_misc_state.c
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 2f5e04f..08dbe88 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -725,6 +725,67 @@  intel_miptree_create_for_dri2_buffer(struct brw_context *brw,
    return multisample_mt;
 }
 
+/**
+ * For a singlesample image buffer, this simply wraps the given region with a miptree.
+ *
+ * For a multisample image buffer, this wraps the given region with
+ * a singlesample miptree, then creates a multisample miptree into which the
+ * singlesample miptree is embedded as a child.
+ */
+struct intel_mipmap_tree*
+intel_miptree_create_for_image_buffer(struct brw_context *intel,
+                                      enum __DRIimageBufferMask buffer_type,
+                                      gl_format format,
+                                      uint32_t num_samples,
+                                      struct intel_region *region)
+{
+   struct intel_mipmap_tree *singlesample_mt = NULL;
+   struct intel_mipmap_tree *multisample_mt = NULL;
+
+   /* Only the front and back buffers, which are color buffers, are allocated
+    * through the image loader.
+    */
+   assert(_mesa_get_format_base_format(format) == GL_RGB ||
+          _mesa_get_format_base_format(format) == GL_RGBA);
+
+   singlesample_mt = intel_miptree_create_for_bo(intel,
+                                                 region->bo,
+                                                 format,
+                                                 0,
+                                                 region->width,
+                                                 region->height,
+                                                 region->pitch,
+                                                 region->tiling);
+   if (!singlesample_mt)
+      return NULL;
+
+   intel_region_reference(&singlesample_mt->region, region);
+
+   if (num_samples == 0)
+      return singlesample_mt;
+
+   multisample_mt = intel_miptree_create_for_renderbuffer(intel,
+                                                          format,
+                                                          region->width,
+                                                          region->height,
+                                                          num_samples);
+   if (!multisample_mt) {
+      intel_miptree_release(&singlesample_mt);
+      return NULL;
+   }
+
+   multisample_mt->singlesample_mt = singlesample_mt;
+   multisample_mt->need_downsample = false;
+
+   intel_region_reference(&multisample_mt->region, region);
+
+   if (intel->is_front_buffer_rendering && buffer_type == __DRI_IMAGE_BUFFER_FRONT) {
+      intel_miptree_upsample(intel, multisample_mt);
+   }
+
+   return multisample_mt;
+}
+
 struct intel_mipmap_tree*
 intel_miptree_create_for_renderbuffer(struct brw_context *brw,
                                       gl_format format,
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index d718125..8777a8c 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -32,6 +32,7 @@ 
 
 #include "intel_regions.h"
 #include "intel_resolve_map.h"
+#include <GL/internal/dri_interface.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -529,6 +530,13 @@  intel_miptree_create_for_dri2_buffer(struct brw_context *brw,
                                      uint32_t num_samples,
                                      struct intel_region *region);
 
+struct intel_mipmap_tree*
+intel_miptree_create_for_image_buffer(struct brw_context *intel,
+                                     enum __DRIimageBufferMask buffer_type,
+                                     gl_format format,
+                                     uint32_t num_samples,
+                                     struct intel_region *region);
+
 /**
  * Create a miptree appropriate as the storage for a non-texture renderbuffer.
  * The miptree has the following properties:
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index f9339c1..7571921 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1176,7 +1176,8 @@  __DRIconfig **intelInitScreen2(__DRIscreen *psp)
 {
    struct intel_screen *intelScreen;
 
-   if (psp->dri2.loader->base.version <= 2 ||
+   if (psp->image.loader) {
+   } else if (psp->dri2.loader->base.version <= 2 ||
        psp->dri2.loader->getBuffersWithFormat == NULL) {
       fprintf(stderr,
 	      "\nERROR!  DRI2 loader with getBuffersWithFormat() "
@@ -1264,7 +1265,6 @@  intelReleaseBuffer(__DRIscreen *screen, __DRIbuffer *buffer)
    free(intelBuffer);
 }
 
-
 static const struct __DriverAPIRec brw_driver_api = {
    .InitScreen		 = intelInitScreen2,
    .DestroyScreen	 = intelDestroyScreen,
@@ -1285,6 +1285,7 @@  static const struct __DRIDriverVtableExtensionRec brw_vtable = {
 
 static const __DRIextension *brw_driver_extensions[] = {
     &driCoreExtension.base,
+    &driImageDriverExtension.base,
     &driDRI2Extension.base,
     &brw_vtable.base,
     &brw_config_options.base,