[Mesa-dev] [PATCH] i965: Add driconf option clamp_max_samples (v3)

[Kenneth Graunke]

On 11/03/2013 01:51 PM, Chad Versace wrote:
> The new option clamps GL_MAX_SAMPLES to a hardware-supported MSAA mode.
> If negative, then no clamping occurs.
> 
> v2: (for Paul)
>   - Add option to i965 only, not to all DRI drivers.
>   - Do not realy on int->uint cast to convert negative
>     values to large positive values. Explicitly check for
>     clamp_max_samples < 0.
> v3: (for Ken)
>    - Don't allow clamp_max_samples to alter context version.
>    - Use clearer for-loop and correct comment.
>    - Rename variables.
> 
> CC: Paul Berry <stereotype441 at gmail.com>
> CC: Eric Anholt <eric at anholt.net>
> CC: Kenneth Graunke <kenneth at whitecape.org>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
> 
> 
> This patch lives on my driconf-clamp-max-samples branch.
> 
> 
>  src/mesa/drivers/dri/i965/brw_context.c  | 70 ++++++++++++++++++++++++++++----
>  src/mesa/drivers/dri/i965/intel_screen.c |  8 +++-
>  2 files changed, 69 insertions(+), 9 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 38147e9..f405629 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -268,6 +268,53 @@ brw_init_driver_functions(struct brw_context *brw,
>        functions->GetSamplePosition = gen6_get_sample_position;
>  }
>  
> +/**
> + * Return array of MSAA modes supported by the hardware. The array is
> + * zero-terminated and sorted in decreasing order.
> + */
> +static const int*
> +brw_supported_msaa_modes(const struct brw_context *brw)
> +{
> +   if (brw->gen >= 7) {
> +      return (int[]){8, 4, 0};
> +   } else if (brw->gen == 6) {
> +      return (int[]){4, 0};
> +   } else {
> +      return (int[]){0};
> +   }
> +}
> +
> +/**
> + * Override GL_MAX_SAMPLES and related constants according to value of driconf
> + * option 'clamp_max_samples'.
> + */
> +static void
> +brw_override_max_samples(struct brw_context *brw)
> +{
> +   const int clamp_max_samples = driQueryOptioni(&brw->optionCache,
> +                                                 "clamp_max_samples");
> +   if (clamp_max_samples < 0)
> +      return;
> +
> +   const int *supported_msaa_modes = brw_supported_msaa_modes(brw);
> +   int max_samples = 0;
> +
> +   /* Select the largest supported MSAA mode that does not exceed
> +    * clamp_max_samples.
> +    */
> +   for (int i = 0; supported_msaa_modes[i] != 0; ++i) {
> +      if (supported_msaa_modes[i] <= clamp_max_samples) {
> +         max_samples = supported_msaa_modes[i];
> +         break;
> +      }
> +   }
> +
> +   brw->ctx.Const.MaxSamples = max_samples;
> +   brw->ctx.Const.MaxColorTextureSamples = max_samples;
> +   brw->ctx.Const.MaxDepthTextureSamples = max_samples;
> +   brw->ctx.Const.MaxIntegerSamples = max_samples;
> +}
> +
>  static void
>  brw_initialize_context_constants(struct brw_context *brw)
>  {
> @@ -333,16 +380,17 @@ brw_initialize_context_constants(struct brw_context *brw)
>  
>     ctx->Const.AlwaysUseGetTransformFeedbackVertexCount = true;
>  
> +   const int max_samples = brw_supported_msaa_modes(brw)[0];
>     if (brw->gen == 6) {
> -      ctx->Const.MaxSamples = 4;
> -      ctx->Const.MaxColorTextureSamples = 4;
> -      ctx->Const.MaxDepthTextureSamples = 4;
> -      ctx->Const.MaxIntegerSamples = 4;
> +      ctx->Const.MaxSamples = max_samples;
> +      ctx->Const.MaxColorTextureSamples = max_samples;
> +      ctx->Const.MaxDepthTextureSamples = max_samples;
> +      ctx->Const.MaxIntegerSamples = max_samples;
>     } else if (brw->gen >= 7) {
> -      ctx->Const.MaxSamples = 8;
> -      ctx->Const.MaxColorTextureSamples = 8;
> -      ctx->Const.MaxDepthTextureSamples = 8;
> -      ctx->Const.MaxIntegerSamples = 8;
> +      ctx->Const.MaxSamples = max_samples;
> +      ctx->Const.MaxColorTextureSamples = max_samples;
> +      ctx->Const.MaxDepthTextureSamples = max_samples;
> +      ctx->Const.MaxIntegerSamples = max_samples;
>        ctx->Const.MaxProgramTextureGatherComponents = 4;
>     }

This if-statement is now silly...both halves have the exact same code.

You just want:

   const int max_samples = brw_supported_msaa_modes(brw)[0];
   ctx->Const.MaxSamples = max_samples;
   ctx->Const.MaxColorTextureSamples = max_samples;
   ctx->Const.MaxDepthTextureSamples = max_samples;
   ctx->Const.MaxIntegerSamples = max_samples;

With that fixed, this is:
Reviewed-and-tested-by: Kenneth Graunke <kenneth at whitecape.org>

>  
> @@ -696,6 +744,12 @@ brwCreateContext(gl_api api,
>  
>     _mesa_compute_version(ctx);
>  
> +   /* Here we override context constants. We apply the overrides after
> +    * calculation of the context version because we do not want the overridden
> +    * constants to change the version.
> +    */
> +   brw_override_max_samples(brw);
> +
>     _mesa_initialize_dispatch_tables(ctx);
>     _mesa_initialize_vbo_vtxfmt(ctx);
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index eafafa2..ce8124b 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -63,11 +63,17 @@ DRI_CONF_BEGIN
>        DRI_CONF_OPT_BEGIN_B(disable_derivative_optimization, "false")
>  	 DRI_CONF_DESC(en, "Derivatives with finer granularity by default")
>        DRI_CONF_OPT_END
> -
>     DRI_CONF_SECTION_END
> +
>     DRI_CONF_SECTION_QUALITY
>        DRI_CONF_FORCE_S3TC_ENABLE("false")
> +
> +      DRI_CONF_OPT_BEGIN(clamp_max_samples, int, -1)
> +              DRI_CONF_DESC(en, "Clamp the value of GL_MAX_SAMPLES to the "
> +                            "given integer. If negative, then do not clamp.")
> +      DRI_CONF_OPT_END
>     DRI_CONF_SECTION_END
> +
>     DRI_CONF_SECTION_DEBUG
>        DRI_CONF_NO_RAST("false")
>        DRI_CONF_ALWAYS_FLUSH_BATCH("false")
>