Re: [Mesa-dev] [PATCH] radeonsi: fix mask checking when emitting scissors and viewports

2016-04-11 Thread Grigori Goronzy 

On 2016-04-08 11:00, Marek Olšák wrote:

From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_state.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c
b/src/gallium/drivers/radeonsi/si_state.c
index 8087d23..3894e1d 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -912,8 +912,10 @@ static void si_emit_scissors(struct si_context
*sctx, struct r600_atom *atom)
bool scissor_enable = sctx->queued.named.rasterizer->scissor_enable;

/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))
+   return;
+


Reviewed-by: Grigori Goronzy 

I also noticed this when I tried to implement the guard band feature. 
E.g. OpenArena will often needlessly emit scissors for all 16 viewports 
without this.


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

Re: [Mesa-dev] [PATCH] radeonsi: fix mask checking when emitting scissors and viewports

2016-04-11 Thread Nicolai Hähnle 

Thanks for doing this.

Reviewed-by: Nicolai Hähnle 

On 08.04.2016 04:00, Marek Olšák wrote:

From: Marek Olšák 

---
  src/gallium/drivers/radeonsi/si_state.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 8087d23..3894e1d 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -912,8 +912,10 @@ static void si_emit_scissors(struct si_context *sctx, 
struct r600_atom *atom)
bool scissor_enable = sctx->queued.named.rasterizer->scissor_enable;

/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))
+   return;
+
radeon_set_context_reg_seq(cs, 
R_028250_PA_SC_VPORT_SCISSOR_0_TL, 2);
si_emit_one_scissor(cs, &sctx->viewports.states[0],
scissor_enable ? &states[0] : NULL);
@@ -960,8 +962,10 @@ static void si_emit_viewports(struct si_context *sctx, 
struct r600_atom *atom)
unsigned mask = sctx->viewports.dirty_mask;

/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))
+   return;
+
radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE, 6);
radeon_emit(cs, fui(states[0].scale[0]));
radeon_emit(cs, fui(states[0].translate[0]));


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

Re: [Mesa-dev] [PATCH] radeonsi: fix mask checking when emitting scissors and viewports

2016-04-08 Thread Marek Olšák 
On Fri, Apr 8, 2016 at 12:16 PM,   wrote:
> On 2016-04-08 19:00, Marek Olšák wrote:
>>
>> From: Marek Olšák 
>>
>> ---
>>  src/gallium/drivers/radeonsi/si_state.c | 12 
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c
>> b/src/gallium/drivers/radeonsi/si_state.c
>> index 8087d23..3894e1d 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -912,8 +912,10 @@ static void si_emit_scissors(struct si_context
>> *sctx, struct r600_atom *atom)
>> bool scissor_enable =
>> sctx->queued.named.rasterizer->scissor_enable;
>>
>> /* The simple case: Only 1 viewport is active. */
>> -   if (mask & 1 &&
>> -   !si_get_vs_info(sctx)->writes_viewport_index) {
>> +   if (!si_get_vs_info(sctx)->writes_viewport_index) {
>> +   if (!(mask & 1))
>
>
> seems a bit tentative.. did you want 1u here or?

1 and 1u are equivalent in this case.

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

Re: [Mesa-dev] [PATCH] radeonsi: fix mask checking when emitting scissors and viewports

2016-04-08 Thread eocallaghan 

On 2016-04-08 19:00, Marek Olšák wrote:

From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_state.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c
b/src/gallium/drivers/radeonsi/si_state.c
index 8087d23..3894e1d 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -912,8 +912,10 @@ static void si_emit_scissors(struct si_context
*sctx, struct r600_atom *atom)
bool scissor_enable = sctx->queued.named.rasterizer->scissor_enable;

/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))


seems a bit tentative.. did you want 1u here or?


+   return;
+
 		radeon_set_context_reg_seq(cs, R_028250_PA_SC_VPORT_SCISSOR_0_TL, 
2);

si_emit_one_scissor(cs, &sctx->viewports.states[0],
scissor_enable ? &states[0] : NULL);
@@ -960,8 +962,10 @@ static void si_emit_viewports(struct si_context
*sctx, struct r600_atom *atom)
unsigned mask = sctx->viewports.dirty_mask;

/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))
+   return;
+
radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE, 6);
radeon_emit(cs, fui(states[0].scale[0]));
radeon_emit(cs, fui(states[0].translate[0]));


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

[Mesa-dev] [PATCH] radeonsi: fix mask checking when emitting scissors and viewports

2016-04-08 Thread Marek Olšák 
From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_state.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 8087d23..3894e1d 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -912,8 +912,10 @@ static void si_emit_scissors(struct si_context *sctx, 
struct r600_atom *atom)
bool scissor_enable = sctx->queued.named.rasterizer->scissor_enable;
 
/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))
+   return;
+
radeon_set_context_reg_seq(cs, 
R_028250_PA_SC_VPORT_SCISSOR_0_TL, 2);
si_emit_one_scissor(cs, &sctx->viewports.states[0],
scissor_enable ? &states[0] : NULL);
@@ -960,8 +962,10 @@ static void si_emit_viewports(struct si_context *sctx, 
struct r600_atom *atom)
unsigned mask = sctx->viewports.dirty_mask;
 
/* The simple case: Only 1 viewport is active. */
-   if (mask & 1 &&
-   !si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!si_get_vs_info(sctx)->writes_viewport_index) {
+   if (!(mask & 1))
+   return;
+
radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE, 6);
radeon_emit(cs, fui(states[0].scale[0]));
radeon_emit(cs, fui(states[0].translate[0]));
-- 
2.5.0

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