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?
>> >> 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
pgpqUazamhnZI.pgp
Description: PGP signature
_______________________________________________ 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