Re: [PATCH 0/8] Add DRIimage-based DRI3/Present loader

["Kristian Høgsberg" <hoegsberg@gmail.com>]

On Tue, Nov 05, 2013 at 12:04:32PM -0800, Eric Anholt wrote:
> Keith Packard <keithp@keithp.com> writes:
> 
> > Keith Packard <keithp@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