[Mesa-dev] [PATCH] RFC: gallium: increase PIPE_MAX_SHADER_SAMPLER_VIEWS to 128

[Brian Paul]

On 02/26/2018 07:45 PM, Roland Scheidegger wrote:
> Am 27.02.2018 um 03:38 schrieb sroland at vmware.com:
>> From: Roland Scheidegger <sroland at vmware.com>
>>
>> Some state trackers require 128.
>> (There are no plans to increase PIPE_MAX_SAMPLERS too, since with gl
>> state tracker it's unlikely more than 32 will be needed, if you need
>> more use bindless.)
>> ---
>>   src/gallium/include/pipe/p_state.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>> index 2b56d60..cddb3b4 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -64,7 +64,7 @@ extern "C" {
>>   #define PIPE_MAX_SAMPLERS         32
>>   #define PIPE_MAX_SHADER_INPUTS    80 /* 32 GENERIC + 32 PATCH + 16 others */
>>   #define PIPE_MAX_SHADER_OUTPUTS   80 /* 32 GENERIC + 32 PATCH + 16 others */
>> -#define PIPE_MAX_SHADER_SAMPLER_VIEWS 32
>> +#define PIPE_MAX_SHADER_SAMPLER_VIEWS 128
>>   #define PIPE_MAX_SHADER_BUFFERS   32
>>   #define PIPE_MAX_SHADER_IMAGES    32
>>   #define PIPE_MAX_TEXTURE_LEVELS   16
>>
> 
> 
> FWIW if drivers are doing the right thing (so, don't blindly announce
> support for PIPE_MAX_SHADER_SAMPLER_VIEWS assuming it's 32 if you can't
> handle more etc.), this should just work. It will, however, make some
> allocations bigger (cso context comes to mind, but there's probably
> more), so I think it warrants some discussion. Apart from that I don't
> think there should really be any performance degradation, since code
> should not blindly iterate over max views typically (rather, only the
> views up to the max number which has changed and so on).

Looks like there's such a loop at sp_state_sampler.c:184 that should be 
fixed.

Other instances found with git grep "for.*PIPE_MAX_SHADER_SAMPLER_VIEWS" 
src/gallium/ seem to be in context init/destroy functions.


> If not everybody is happy with it, I suppose it could be made into a
> define coming from the build system(s) - we'd rather avoid doing this in
> just a private branch instead, to not getting it broken due to funny
> ideas (e.g. 32 bit bitfields representing active units or similar
> things) in shared code.
> Opinions?

Looks OK to me.

Reviewed-by: Brian Paul <brianp at vmware.com>