[Mesa-dev] [PATCH 06/24] st/mesa: don't unbind sampler states if none are used

[Timothy Arceri]

On 15/06/17 03:17, Marek Olšák wrote:
> On Tue, Jun 13, 2017 at 7:46 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> On 13/06/17 15:32, Timothy Arceri wrote:
>>>
>>>
>>>
>>> On 13/06/17 04:23, Ilia Mirkin wrote:
>>>>
>>>> On Mon, Jun 12, 2017 at 2:18 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>>>>
>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>
>>>>> ---
>>>>>    src/mesa/state_tracker/st_atom_sampler.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/mesa/state_tracker/st_atom_sampler.c
>>>>> b/src/mesa/state_tracker/st_atom_sampler.c
>>>>> index f33e334..11db6e1 100644
>>>>> --- a/src/mesa/state_tracker/st_atom_sampler.c
>>>>> +++ b/src/mesa/state_tracker/st_atom_sampler.c
>>>>> @@ -263,21 +263,21 @@ update_shader_samplers(struct st_context *st,
>>>>>                           struct pipe_sampler_state *samplers,
>>>>>                           unsigned *num_samplers)
>>>>>    {
>>>>>       GLbitfield samplers_used = prog->SamplersUsed;
>>>>>       GLbitfield free_slots = ~prog->SamplersUsed;
>>>>>       GLbitfield external_samplers_used = prog->ExternalSamplersUsed;
>>>>>       GLuint unit;
>>>>>       const GLuint old_max = *num_samplers;
>>>>>       const struct pipe_sampler_state *states[PIPE_MAX_SAMPLERS];
>>>>>
>>>>> -   if (*num_samplers == 0 && samplers_used == 0x0)
>>>>> +   if (samplers_used == 0x0)
>>>>>          return;
>>>>>
>>>>>       *num_samplers = 0;
>>>>
>>>>
>>>> Does this still need to get executed even if samplers_used == 0?
>>>
>>>
>>> It seems correct to skip this, otherwise old_max won't be set correctly in
>>> the above code the next time we get here.
>>
>>
>>
>> Although it seems we ignore old_max in the following patches anyway because
>> cso_set_samplers() will set things to NULL for us.
>>
>> So maybe it would make sense to set it to 0?
> 
> Yes, it should be set to 0, but it's not that important (only
> DrawPixels would be affected), though I think we can just drop
> tracking num_samplers in st_context and simply rely on
> num_sampler_views.
> 
> I'll fix that in a follow-up patch, which I'm gonna send shortly
> ([25/24]]. In the meantime, I'd like an Rb on this one if there are no
> other comments.

You still have my r-b here.

25 is also:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>


> 
> Thanks,
> Marek
> 
>>
>>
>>
>>
>>>
>>> Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
>>>
>>>>
>>>>>
>>>>>       /* loop over sampler units (aka tex image units) */
>>>>>       for (unit = 0; unit < max_units; unit++, samplers_used >>= 1) {
>>>>>          struct pipe_sampler_state *sampler = samplers + unit;
>>>>>
>>>>>          if (samplers_used & 1) {
>>>>>             const GLuint texUnit = prog->SamplerUnits[unit];
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev