[PATCH 09/12] DRI2: advertise lowest supported DRI2 protocol version

[Jesse Barnes]

On Mon, 22 Mar 2010 15:34:54 -0700
Ian Romanick <idr at freedesktop.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Jesse Barnes wrote:
> > Update our supported DRI2 protocol version as each driver does
> > DRI2ScreenInit, since depending on available kernel features, each DDX
> > may support different callbacks and therefore protocol.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> >  hw/xfree86/dri2/dri2.c      |   12 ++++++++++++
> >  hw/xfree86/dri2/dri2.h      |    3 +++
> >  hw/xfree86/dri2/dri2ext.c   |    4 ++--
> >  include/protocol-versions.h |    4 ----
> >  4 files changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> > index 9825a55..3fc7f4e 100644
> > --- a/hw/xfree86/dri2/dri2.c
> > +++ b/hw/xfree86/dri2/dri2.c
> > @@ -45,6 +45,9 @@
> >  
> >  #include "xf86.h"
> >  
> > +CARD8 dri2_major; /* version of DRI2 supported by DDX */
> > +CARD8 dri2_minor;
> > +
> >  static int dri2ScreenPrivateKeyIndex;
> >  static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex;
> >  static int dri2WindowPrivateKeyIndex;
> > @@ -848,6 +851,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
> >  	"VDPAU", /* DRI2DriverVDPAU */
> >      };
> >      unsigned int i;
> > +    CARD8 cur_minor;
> >  
> >      if (info->version < 3)
> >  	return FALSE;
> > @@ -864,6 +868,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
> >  
> >      ds->fd	       = info->fd;
> >      ds->deviceName     = info->deviceName;
> > +    dri2_major         = 1;
> >  
> >      ds->CreateBuffer   = info->CreateBuffer;
> >      ds->DestroyBuffer  = info->DestroyBuffer;
> > @@ -873,8 +878,15 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
> >  	ds->ScheduleSwap = info->ScheduleSwap;
> >  	ds->ScheduleWaitMSC = info->ScheduleWaitMSC;
> >  	ds->GetMSC = info->GetMSC;
> > +	cur_minor = 2;
> > +    } else {
> > +	cur_minor = 1;
> >      }
> >  
> > +    /* Initialize minor if needed and set to minimum provied by DDX */
> > +    if (!dri2_minor || dri2_minor > cur_minor)
> > +	dri2_minor = cur_minor;
> > +
> 
> Doesn't this fail if one driver says "DRI2 version 1.0" and the next
> driver says "DRI2 version 1.2"?   If the protocol version cannot be
> queried before the drivers are initialized, it may be better to just set
> it to MAX_INT and 'dri2_minor = MIN(dri2_minor, cur_minor)'.

No I think it'll be right?  First registration (with 1.0) will set
cur_minor = 1 and will initialize dri2_minor to 1.

Second registration (with 1.2) will set cur_minor = 2 and
the (!dri2_minor || dri2_minor > cur_minor) test will fail, so
dri2_minor will stay at 1.

I am assuming we'll never need to use dri2_minor = 0 though.

Things also get messy if drivers try to mix & match interfaces that
support incompatible DRI2 extensions.  For example we can't advertise
DRI2Invalidate without also advertising DRI2SwapBuffers, so there is a
requirement on each DDX to support DRI2 features in compatible ways.
-- 
Jesse Barnes, Intel Open Source Technology Center