Keith Packard <keithp@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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.
Attachment:
pgpfAdZmbaLd2.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
