On 12/11/10 18:06 +0100, ext Francisco Jerez wrote: > Pauli Nieminen <ext-pauli.niemi...@nokia.com> writes: > > > 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; > > > I don't get the purpose of this change. If I understood correctly, > DRI2SwapLimit() is supposed to be initiated by the DDX itself based on > driver-specific policy (e.g. a configuration file option or some > suitable constant value). What's the point of calling back into the > driver again to validate its own decision? Am I missing something? >
I tried to argue same but the argument was never taken seriously. > >> > >> 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 _______________________________________________ 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