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

2017-06-14 Thread Timothy Arceri 



On 15/06/17 03:17, Marek Olšák wrote:

On Tue, Jun 13, 2017 at 7:46 AM, Timothy Arceri  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  wrote:


From: Marek Olšák 

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




Thanks,
Marek








Reviewed-by: Timothy Arceri 





  /* 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-14 Thread Marek Olšák 
On Tue, Jun 13, 2017 at 7:46 AM, Timothy Arceri  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  wrote:

 From: Marek Olšák 

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

  /* 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@lists.freedesktop.org
 https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-12 Thread Timothy Arceri 

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

From: Marek Olšák 

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





Reviewed-by: Timothy Arceri 





 /* 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-12 Thread Timothy Arceri 



On 13/06/17 04:23, Ilia Mirkin wrote:

On Mon, Jun 12, 2017 at 2:18 PM, Marek Olšák  wrote:

From: Marek Olšák 

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


Reviewed-by: Timothy Arceri 





 /* 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-12 Thread Ilia Mirkin 
On Mon, Jun 12, 2017 at 2:18 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  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?

>
> /* 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-12 Thread Marek Olšák 
From: Marek Olšák 

---
 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;
 
/* 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev