On Fri, 12 Nov 2010 10:03:22 +0200
Pauli Nieminen <ext-pauli.niemi...@nokia.com> wrote:
> 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;
Yeah at least that fits in a little better with the other functions.
Thanks,
--
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
