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

[Marek Olšák]

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.

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