On 11/11/10 18:46 +0100, ext Jesse Barnes wrote: > On Mon, 1 Nov 2010 16:22:01 +0200 > Pauli Nieminen <ext-pauli.niemi...@nokia.com> wrote: > > > DDX can now implement validation for swap_limit changes to prevent > > configurations that are not support in driver. > > > > Signed-off-by: Pauli Nieminen <ext-pauli.niemi...@nokia.com> > > CC: Mario Kleiner <mario.klei...@tuebingen.mpg.de> > > --- > > hw/xfree86/dri2/dri2.c | 18 +++++++++++++++++- > > hw/xfree86/dri2/dri2.h | 14 ++++++++++++++ > > 2 files changed, 31 insertions(+), 1 deletions(-) > > > > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c > > index 255fed0..7c6a0e2 100644 > > --- a/hw/xfree86/dri2/dri2.c > > +++ b/hw/xfree86/dri2/dri2.c > > @@ -102,6 +102,7 @@ typedef struct _DRI2Screen { > > DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC; > > DRI2AuthMagicProcPtr AuthMagic; > > DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify; > > + DRI2SwapLimitValidateProcPtr SwapLimitValidate; > > > > HandleExposuresProcPtr HandleExposures; > > > > @@ -191,13 +192,23 @@ DRI2AllocateDrawable(DrawablePtr pDraw) > > return pPriv; > > } > > > > +static Bool > > +DRI2DefaultSwapLimitValidate(DrawablePtr pDraw, int swap_limit) > > +{ > > + return swap_limit == 1; > > +} > > + > > Bool > > DRI2SwapLimit(DrawablePtr pDraw, int swap_limit) > > { > > DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw); > > + DRI2ScreenPtr ds = pPriv->dri2_screen;
Null dereference ^^ Fixing. > > if (!pPriv) > > return FALSE; > > > > + if (!ds->SwapLimitValidate(pDraw, swap_limit)) > > + return FALSE; > > + > > pPriv->swap_limit = swap_limit; > > > > /* Check throttling */ > > @@ -1134,8 +1145,10 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) > > ds->AuthMagic = info->AuthMagic; > > } > > > > - if (info->version >= 6) > > + if (info->version >= 6) { > > ds->ReuseBufferNotify = info->ReuseBufferNotify; > > + ds->SwapLimitValidate = info->SwapLimitValidate; > > + } > > > > /* > > * if the driver doesn't provide an AuthMagic function or the info > > struct > > @@ -1148,6 +1161,9 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) > > goto err_out; > > #endif > > > > + if (!ds->SwapLimitValidate) > > + ds->SwapLimitValidate = DRI2DefaultSwapLimitValidate; > > + > > /* Initialize minor if needed and set to minimum provied by DDX */ > > if (!dri2_minor || dri2_minor > cur_minor) > > dri2_minor = cur_minor; > > diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h > > index 3d01c55..a4bba02 100644 > > --- a/hw/xfree86/dri2/dri2.h > > +++ b/hw/xfree86/dri2/dri2.h > > @@ -169,6 +169,19 @@ typedef void > > (*DRI2InvalidateProcPtr)(DrawablePtr pDraw, > > void *data); > > > > /** > > + * DRI2 calls this hook when ever swap_limit is going to be changed. > > Default > > + * implementation for the hook only accepts one as swap_limit. If driver > > can > > + * support other swap_limits it has to implement supported limits with this > > + * callback. > > + * > > + * \param pDraw drawable whos swap_limit is going to be changed > > + * \param swap_limit new swap_limit that going to be set > > + * \return TRUE if limit is support, FALSE if not. > > + */ > > +typedef Bool (*DRI2SwapLimitValidateProcPtr)(DrawablePtr > > pDraw, > > + int swap_limit); > > + > > +/** > > * Version of the DRI2InfoRec structure defined in this header > > */ > > #define DRI2INFOREC_VERSION 6 > > @@ -203,6 +216,7 @@ typedef struct { > > /* added in version 6 */ > > > > DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify; > > + DRI2SwapLimitValidateProcPtr SwapLimitValidate; > > } DRI2InfoRec, *DRI2InfoPtr; > > > > extern _X_EXPORT int DRI2EventBase; > > This is to validate N buffering on the driver side, right? I'd really > prefer drivers to just implement all the hooks, even if they're only > stubs, since that avoids some complexity on the server side and makes > the code easier to follow (applies to the GetMSC patch from before as > well). > > But I don't feel strongly enough to nack on those grounds, I'll leave > that up to Keith. Would following change look better to you? if (!ds->SwapLimitValidate || !ds->SwapLimitValidate(pDraw, swap_limit) return FALSE; > > Other than that, > Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org> > > -- > Jesse Barnes, Intel Open Source Technology Center _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel