[PATCH v2 5/5] DRI2: Allow DDX to validate swap_limit changes

[Jesse Barnes]

On Mon,  1 Nov 2010 16:22:01 +0200
Pauli Nieminen <ext-pauli.nieminen at 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.nieminen at nokia.com>
> CC: Mario Kleiner <mario.kleiner at 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;
>      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.

Other than that,
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center