Re: [Mesa-dev] [PATCH 3/3] radeonsi: don't emit DB_STENCIL_CONTROL if it has no effect

2017-06-12 Thread Nicolai Hähnle 

For the series:

Reviewed-by: Nicolai Hähnle 

On 09.06.2017 15:49, Marek Olšák wrote:

From: Marek Olšák 

---
  src/gallium/drivers/radeonsi/si_state.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 53f66ac..a8255f2 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -1078,21 +1078,22 @@ static void *si_create_dsa_state(struct pipe_context 
*ctx,
if (state->alpha.enabled) {
dsa->alpha_func = state->alpha.func;
  
  		si_pm4_set_reg(pm4, R_00B030_SPI_SHADER_USER_DATA_PS_0 +

   SI_SGPR_ALPHA_REF * 4, 
fui(state->alpha.ref_value));
} else {
dsa->alpha_func = PIPE_FUNC_ALWAYS;
}
  
  	si_pm4_set_reg(pm4, R_028800_DB_DEPTH_CONTROL, db_depth_control);

-   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL, db_stencil_control);
+   if (state->stencil[0].enabled)
+   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL, 
db_stencil_control);
if (state->depth.bounds_test) {
si_pm4_set_reg(pm4, R_028020_DB_DEPTH_BOUNDS_MIN, 
fui(state->depth.bounds_min));
si_pm4_set_reg(pm4, R_028024_DB_DEPTH_BOUNDS_MAX, 
fui(state->depth.bounds_max));
}
  
  	return dsa;

  }
  
  static void si_bind_dsa_state(struct pipe_context *ctx, void *state)

  {




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] radeonsi: don't emit DB_STENCIL_CONTROL if it has no effect

2017-06-09 Thread Marek Olšák 
On Fri, Jun 9, 2017 at 4:31 PM, Samuel Pitoiset
 wrote:
>
>
> On 06/09/2017 04:26 PM, Marek Olšák wrote:
>>
>> On Fri, Jun 9, 2017 at 4:00 PM, Samuel Pitoiset
>>  wrote:
>>>
>>>
>>>
>>> On 06/09/2017 03:49 PM, Marek Olšák wrote:


 From: Marek Olšák 

 ---
src/gallium/drivers/radeonsi/si_state.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/src/gallium/drivers/radeonsi/si_state.c
 b/src/gallium/drivers/radeonsi/si_state.c
 index 53f66ac..a8255f2 100644
 --- a/src/gallium/drivers/radeonsi/si_state.c
 +++ b/src/gallium/drivers/radeonsi/si_state.c
 @@ -1078,21 +1078,22 @@ static void *si_create_dsa_state(struct
 pipe_context *ctx,
  if (state->alpha.enabled) {
  dsa->alpha_func = state->alpha.func;
  si_pm4_set_reg(pm4, R_00B030_SPI_SHADER_USER_DATA_PS_0
 +
 SI_SGPR_ALPHA_REF * 4,
 fui(state->alpha.ref_value));
  } else {
  dsa->alpha_func = PIPE_FUNC_ALWAYS;
  }
  si_pm4_set_reg(pm4, R_028800_DB_DEPTH_CONTROL,
 db_depth_control);
 -   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL,
 db_stencil_control);
 +   if (state->stencil[0].enabled)
 +   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL,
 db_stencil_control);
>>>
>>>
>>>
>>> How if stencil is enabled, then disabled? Doesn't this reg has to be set
>>> to
>>> 0?
>>
>>
>> DB_DEPTH_CONTROL enables and disables stencil.
>
>
> Right.
>
> Can't we move this in the same if above?

Well, I'd like to register writes grouped together at the end, but it
mostly doesn't matter.

Marek

>
> Except this nitpick, series is:
>
> Reviewed-by: Samuel Pitoiset 
>
>>
>> Marek
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] radeonsi: don't emit DB_STENCIL_CONTROL if it has no effect

2017-06-09 Thread Samuel Pitoiset 



On 06/09/2017 04:26 PM, Marek Olšák wrote:

On Fri, Jun 9, 2017 at 4:00 PM, Samuel Pitoiset
 wrote:



On 06/09/2017 03:49 PM, Marek Olšák wrote:


From: Marek Olšák 

---
   src/gallium/drivers/radeonsi/si_state.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c
b/src/gallium/drivers/radeonsi/si_state.c
index 53f66ac..a8255f2 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -1078,21 +1078,22 @@ static void *si_create_dsa_state(struct
pipe_context *ctx,
 if (state->alpha.enabled) {
 dsa->alpha_func = state->alpha.func;
 si_pm4_set_reg(pm4, R_00B030_SPI_SHADER_USER_DATA_PS_0 +
SI_SGPR_ALPHA_REF * 4,
fui(state->alpha.ref_value));
 } else {
 dsa->alpha_func = PIPE_FUNC_ALWAYS;
 }
 si_pm4_set_reg(pm4, R_028800_DB_DEPTH_CONTROL, db_depth_control);
-   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL,
db_stencil_control);
+   if (state->stencil[0].enabled)
+   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL,
db_stencil_control);



How if stencil is enabled, then disabled? Doesn't this reg has to be set to
0?


DB_DEPTH_CONTROL enables and disables stencil.


Right.

Can't we move this in the same if above?

Except this nitpick, series is:

Reviewed-by: Samuel Pitoiset 



Marek


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

Re: [Mesa-dev] [PATCH 3/3] radeonsi: don't emit DB_STENCIL_CONTROL if it has no effect

2017-06-09 Thread Marek Olšák 
On Fri, Jun 9, 2017 at 4:00 PM, Samuel Pitoiset
 wrote:
>
>
> On 06/09/2017 03:49 PM, Marek Olšák wrote:
>>
>> From: Marek Olšák 
>>
>> ---
>>   src/gallium/drivers/radeonsi/si_state.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c
>> b/src/gallium/drivers/radeonsi/si_state.c
>> index 53f66ac..a8255f2 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -1078,21 +1078,22 @@ static void *si_create_dsa_state(struct
>> pipe_context *ctx,
>> if (state->alpha.enabled) {
>> dsa->alpha_func = state->alpha.func;
>> si_pm4_set_reg(pm4, R_00B030_SPI_SHADER_USER_DATA_PS_0 +
>>SI_SGPR_ALPHA_REF * 4,
>> fui(state->alpha.ref_value));
>> } else {
>> dsa->alpha_func = PIPE_FUNC_ALWAYS;
>> }
>> si_pm4_set_reg(pm4, R_028800_DB_DEPTH_CONTROL, db_depth_control);
>> -   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL,
>> db_stencil_control);
>> +   if (state->stencil[0].enabled)
>> +   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL,
>> db_stencil_control);
>
>
> How if stencil is enabled, then disabled? Doesn't this reg has to be set to
> 0?

DB_DEPTH_CONTROL enables and disables stencil.

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

Re: [Mesa-dev] [PATCH 3/3] radeonsi: don't emit DB_STENCIL_CONTROL if it has no effect

2017-06-09 Thread Samuel Pitoiset 



On 06/09/2017 03:49 PM, Marek Olšák wrote:

From: Marek Olšák 

---
  src/gallium/drivers/radeonsi/si_state.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 53f66ac..a8255f2 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -1078,21 +1078,22 @@ static void *si_create_dsa_state(struct pipe_context 
*ctx,
if (state->alpha.enabled) {
dsa->alpha_func = state->alpha.func;
  
  		si_pm4_set_reg(pm4, R_00B030_SPI_SHADER_USER_DATA_PS_0 +

   SI_SGPR_ALPHA_REF * 4, 
fui(state->alpha.ref_value));
} else {
dsa->alpha_func = PIPE_FUNC_ALWAYS;
}
  
  	si_pm4_set_reg(pm4, R_028800_DB_DEPTH_CONTROL, db_depth_control);

-   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL, db_stencil_control);
+   if (state->stencil[0].enabled)
+   si_pm4_set_reg(pm4, R_02842C_DB_STENCIL_CONTROL, 
db_stencil_control);


How if stencil is enabled, then disabled? Doesn't this reg has to be set 
to 0?



if (state->depth.bounds_test) {
si_pm4_set_reg(pm4, R_028020_DB_DEPTH_BOUNDS_MIN, 
fui(state->depth.bounds_min));
si_pm4_set_reg(pm4, R_028024_DB_DEPTH_BOUNDS_MAX, 
fui(state->depth.bounds_max));
}
  
  	return dsa;

  }
  
  static void si_bind_dsa_state(struct pipe_context *ctx, void *state)

  {


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