Re: [PATCH 7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension

From: Keith Packard <keithp@keithp.com>
To: "Kristian Høgsberg" <hoegsberg@gmail.com>
Cc: mesa3d-dev@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension
Date: Wed, 06 Nov 2013 06:55:11 -0800	[thread overview]
Message-ID: <86ob5x4m28.fsf@miki.keithp.com> (raw)
In-Reply-To: <20131106062554.GA12984@tokamak.local>

[-- Attachment #1.1: Type: text/plain, Size: 2573 bytes --]

Kristian Høgsberg <hoegsberg@gmail.com> writes:

> Having written the GBM and Wayland suport for this, it's pretty clear
> that we just want to use __DRIdri2Extension instead of duplicating
> these functions.  Support for the __DRIimage based getBuffers is a
> small incremental patch to src/egl/drivers/dri2:

That would require drivers to support all of the DRI2 functions, rather
than just the few needed for Image support though.

> The problem is that technically we would have to do:
>
>       if (dri2_dpy->image_driver) {
>
>          /* use dri2_dpy->image_driver->createContextAttribs
>
>       } else if (dri2_dpy->dri2 &&
> 	         dri2_dpy->dri2->base.version >= 3) {
>
>          /* use dri2_dpy->dri2->createContextAttribs
>
> all over the place even though they end up calling the same function.
> The only real purpose of __DRIimageDriverExtension is to let the
> loader know that the DRI driver supports and will use
> __DRIimageLoaderExtension if provided, but we can just expose an empty
> dummy extension to indicate that.

Alternatively, the loader could simply call through the DRI2 version,
acknowledging that they must be the same function? I'd sure like to be
able to create an ImageLoader-only driver, even if that's not practical
today.

A couple of other options:

 * Move the shared functions into a new structure which is embedded
   in both the DRIdri2ExtensionRec and the DRIimageDriverExtensionRec,
   then the loader could set a pointer to whichever it wanted to use
   in it's own structure.

 * Have the loader pull the functions out of the ExtensionRec and stick
   them directly into its own structure. Then it could use those
   directly and avoid version checks while running.

But, if we just want to use the DRI2 driver interface and create an
extension purely to mark that the driver will use an Image loader,
that's easy too.

> I think we should only look at image.front if the
> __DRI_IMAGE_BUFFER_FRONT bit is set.  We never want to set up a front
> buffer that we didn't ask for and it seems wrong that the loader has
> to clear image.front, if the driver didn't ask for front.

I simply duplicated the logic from the DRI2 path which works exactly the
same way (asks for buffers, then looks to see what it got). I didn't dig
into the details further than this, I'm afraid. The only case where asks
!= received is for pixmap targets where you always get a front buffer. I
assumed there was some deep semantic requirement for that...

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel