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

[Chad Versace]

On 11/03/2013 02:50 PM, Kenneth Graunke wrote:
> 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>

Thanks for catching my sed silliness. I'll commit it soon.