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

[Roland Scheidegger]

Am 27.02.2018 um 17:39 schrieb 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.
I once fixed all such loops in llvmpipe (because even with 32, they
actually showed up in profiles). I probably missed the one in draw
(since that one doesn't do ref counting just set things to zero it
probably was much lower overhead) and wasn't that worried about
softpipe... But I'll fix this one.

> 
> Other instances found with git grep "for.*PIPE_MAX_SHADER_SAMPLER_VIEWS"
> src/gallium/ seem to be in context init/destroy functions.
The one in cso_context destroy should be fixable easily.
The problem with the ones in draw aaline/pstipple is that they don't
actually store the number of active views (albeit they pretend they do).
Since the interface there with start/num just says which ones are
replaced, but the ones above start+num remain. This is not handled
correctly by these pipeline stages, but at least at stage destruction we
definitely free them. Probably time to fix that...
Those were the only 3 I've found with a quick grep.

Roland


> 
> 
>> 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>
>