Keith Packard <keithp at 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 at 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131105/a1e60928/attachment.pgp>