On Tue, Nov 05, 2013 at 12:04:32PM -0800, Eric Anholt wrote: > Keith Packard <keithp at keithp.com> writes: > > > Keith Packard <keithp at keithp.com> writes: > > > >> This sequence first adds a a couple of new DRIimage extensions to the > >> dri/common, dri/i915 and dri/i965 directories which define a > >> loader-independent API for managing window system operations. > >> > >> The last patch adds a new DRI3000 loader using those new interfaces. > > > > I've figured out that I can also re-use dri2CreateNewScreen2 for the > > image driver bits, as long as I change that function to also look up the > > image loader. That means there are *no* new dri_util functions needed. > > > > To recap, the changes needed to support using the DRIimageExtension > > interfaces for allocating buffers from the driver in the loader are: > > > > DRIimageDriverExtension > > > > A proper subset of DRIdri2DriverExtension, which uses > > the same five functions involved in creating new objects: > > > > /* Common DRI functions, shared with DRI2 */ > > __DRIcreateNewScreen2 createNewScreen2; > > __DRIcreateNewDrawable createNewDrawable; > > __DRIcreateNewContext createNewContext; > > __DRIcreateContextAttribs createContextAttribs; > > __DRIgetAPIMask getAPIMask; > > It seems like we could just stick these things in __DRI_CORE as opposed > to having another new extension to look up. The downside I see there is > bugs in the server, which have patches at xserver-driinterface-versions > of my tree. (Unfortunately, I'm having a hard time building the server > currently, so no testing yet). Having a new extension whose name has > nothing to do with the functions in it seems really weird.
It may make more sense to just extend the existing interfaces, but when we discussed DRIimageDriverExtension, the idea was that we could phase out DRIdri2Extension. I think that still makes sense but introducing more extensions doesn't make this interface better. The way this was done originally was that we have DRIcoreExtension which provided DRI1 support. The DRIdri2Extension extension replaces some of the core functions (it has a createNewScreen that doesn't take a sarea handle, for example...) and allows a loader to implement DRI2, but you have to use both extensions. DRIswrastExtension works in a similar for swrast. The idea was to share the core functionality, but it's obviously messy to have to mix two extensions to get things working. If we're introducing a new extension, I'd suggest we move the functions from DRIcoreExtension that we still use into this new extension and make it completely replace DRIcoreExtension and DRIdri2Extension. The functions from the core extension we still use are: void (*destroyScreen)(__DRIscreen *screen); const __DRIextension **(*getExtensions)(__DRIscreen *screen); int (*getConfigAttrib)(const __DRIconfig *config, unsigned int attrib, unsigned int *value); int (*indexConfigAttrib)(const __DRIconfig *config, int index, unsigned int *attrib, unsigned int *value); void (*destroyDrawable)(__DRIdrawable *drawable); int (*copyContext)(__DRIcontext *dest, __DRIcontext *src, unsigned long mask); void (*destroyContext)(__DRIcontext *context); int (*bindContext)(__DRIcontext *ctx, __DRIdrawable *pdraw, __DRIdrawable *pread); int (*unbindContext)(__DRIcontext *ctx); and if we add those to DRIimageDriverExtension the loader only needs to look for that and the DRIimage extension. Of course, the implementation is already in dri_util.c, we just need to set the function pointers to the DRIcoreExtension functions we share. Kristian