From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH 7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension Date: Tue, 05 Nov 2013 12:05:32 -0800 Message-ID: <87zjpifwc3.fsf@eliezer.anholt.net> References: <1383618208-21310-1-git-send-email-keithp@keithp.com> <1383618208-21310-8-git-send-email-keithp@keithp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2008555070==" Return-path: In-Reply-To: <1383618208-21310-8-git-send-email-keithp@keithp.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Keith Packard , mesa3d-dev@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============2008555070== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Keith Packard writes: > These provide an interface between the driver and the loader to allocate > color buffers through the DRIimage extension interface rather than throug= h a > loader-specific extension (as is used by DRI2, for instance). > > The driver uses the loader 'getBuffers' interface to allocate color buffe= rs. > > 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 loa= der > 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 > --- > 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/dr= i_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 __DRIdri2Lo= aderExtension; > typedef struct __DRI2flushExtensionRec __DRI2flushExtension; > typedef struct __DRI2throttleExtensionRec __DRI2throttleExtension; >=20=20 > + > +typedef struct __DRIimageLoaderExtensionRec __DRIimageLoaderExtensio= n; > +typedef struct __DRIimageDriverExtensionRec __DRIimageDriverExtensio= n; > + > /*@}*/ >=20=20 >=20=20 > @@ -1288,4 +1292,112 @@ typedef struct __DRIDriverVtableExtensionRec { > const struct __DriverAPIRec *vtable; > } __DRIDriverVtableExtension; >=20=20 > +/** > + * 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 p= assed > + * into __DRIdri2ExtensionRec::createNewDrawab= le > + */ > + void (*flushFrontBuffer)(__DRIdrawable *driDrawable, void *loaderPri= vate); > +}; > + > +/** > + * 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 *d= rawable) > +{ > + struct gl_framebuffer *fb =3D drawable->driverPrivate; > + __DRIscreen *screen =3D intel->intelScreen->driScrnPriv; > + struct intel_renderbuffer *front_rb; > + struct intel_renderbuffer *back_rb; > + struct __DRIimageList images; > + unsigned int format; > + uint32_t buffer_mask =3D 0; > + > + front_rb =3D intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT); > + back_rb =3D intel_get_renderbuffer(fb, BUFFER_BACK_LEFT); > + > + if (back_rb) > + format =3D intel_rb_format(back_rb); > + else if (front_rb) > + format =3D intel_rb_format(front_rb); > + else > + return; > + > + if ((intel->is_front_buffer_rendering || intel->is_front_buffer_readi= ng || !back_rb) && front_rb) > + buffer_mask |=3D __DRI_IMAGE_BUFFER_FRONT; > + > + if (back_rb) > + buffer_mask |=3D __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"); > } >=20=20 > -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 *drawa= ble) > +{ > + struct gl_framebuffer *fb =3D drawable->driverPrivate; > + __DRIscreen *screen =3D brw->intelScreen->driScrnPriv; > + struct intel_renderbuffer *front_rb; > + struct intel_renderbuffer *back_rb; > + struct __DRIimageList images; > + unsigned int format; > + uint32_t buffer_mask =3D 0; > + > + front_rb =3D intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT); > + back_rb =3D intel_get_renderbuffer(fb, BUFFER_BACK_LEFT); > + > + if (back_rb) > + format =3D intel_rb_format(back_rb); > + else if (front_rb) > + format =3D intel_rb_format(front_rb); > + else > + return; > + > + if ((brw->is_front_buffer_rendering || brw->is_front_buffer_reading |= | !back_rb) && front_rb) > + buffer_mask |=3D __DRI_IMAGE_BUFFER_FRONT; > + > + if (back_rb) > + buffer_mask |=3D __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/d= ri/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); >=20=20 > -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. --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) iQIcBAEBCAAGBQJSeU+MAAoJELXWKTbR/J7o1qcP/3CWXPFw+ZmthPnCvaAy/Jw3 lnikNr2+AERMVy7ATVI5caNzDXVq0IH6diocPwCzwMcQ1fhCKSq6bV9Y4gdYaUCF 4wMvLCl18iIqHKPb+uaYEiZ9IJ9a3wjaDCgdSc9X5qukculxQB+PnOQEoT5bo5h4 8y5id0QoPqCD0y56aIqXhhyMbQJvp4pZJNuWZ72GgbbORbUMqv/1tkIgB6EF4aUo 0ZbuAuoM0JWBZiRhmugwbqEsMKEbhGS5felvuFx5RseNepxhxr805jnYgZmvfPIt RsdzeStY6+/gYno3VDghS2XG+QL+Tmg1sUUSnIEZ824vaoSH49MIG+p0KxmVfGWZ Cz2O+ymevtGv7+bvEvWqXZK6tEp7POzb8Bo+xMomOm9NTs9dc6W7HESabVYktx2z pZY793qpve6VlmB/N5qbwr5ksjJwThb3UggoMV0yPKdJB2FrfpU3l2krRfN29eL6 F6WLic8nJX8Um+APIBIuSN7/HXRKOnFRhKsu4LHimuNJYCapOKZGhl4D+Hzzofgg r5KGVWZWcvYCIrebA8CIerVP0ePGa/To9HTxugzX7TQmHLxKIswiocng9WJMlTM7 KKEH2tlie3gQUWl4/GYECq26pJTbAAxm2t63qKzY3GNKaf1yCJsgB3RzIqwX6xrl nV99lexXA9I9NwkLHVUW =+CUZ -----END PGP SIGNATURE----- --=-=-=-- --===============2008555070== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============2008555070==--