Re: [Mesa-dev] [PATCH 2/2] radeonsi: use guard band clipping

[Marek Olšák]

There is one small issue with this. It uses viewport extents clamped
to [0, 16384], but it should use unclamped signed coordinates of the
viewport. The hw clipper doesn't do any clamping, so it's easy to
generate window coordinates outside of [-32768, 32768] if the viewport
covers [-32768, 1] in one dimension and the guard band is > 1 in
that dimension.

Marek

On Wed, Apr 6, 2016 at 10:15 PM, Grigori Goronzy  wrote:
> With the previous changes to handling of viewport clipping, it is
> almost trivial to add proper support for guard band clipping.  Select a
> suitable integer clipping value to keep inside the rasterizer's guard
> band range of [-32768, 32767] and program the hardware to use guard
> band clipping.
>
> Guard band clipping speeds up rasterization for primitives that are
> partially off-screen.  This change in particular results in small
> framerate improvements in a wide range of games.
> ---
>  src/gallium/drivers/radeonsi/si_state.c | 34 
> ++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> b/src/gallium/drivers/radeonsi/si_state.c
> index 0bb41e5..013fcf1 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -882,9 +882,19 @@ static void si_clip_scissor(struct pipe_scissor_state 
> *out,
> out->maxy = MIN2(out->maxy, clip->maxy);
>  }
>
> +static void si_mix_scissor(struct pipe_scissor_state *out,
> +  struct pipe_scissor_state *clip)
> +{
> +   out->minx = MIN2(out->minx, clip->minx);
> +   out->miny = MIN2(out->miny, clip->miny);
> +   out->maxx = MAX2(out->maxx, clip->maxx);
> +   out->maxy = MAX2(out->maxy, clip->maxy);
> +}
> +
>  static void si_emit_one_scissor(struct radeon_winsys_cs *cs,
> struct pipe_viewport_state *vp,
> -   struct pipe_scissor_state *scissor)
> +   struct pipe_scissor_state *scissor,
> +   struct pipe_scissor_state *mix)
>  {
> struct pipe_scissor_state final;
>
> @@ -895,6 +905,7 @@ static void si_emit_one_scissor(struct radeon_winsys_cs 
> *cs,
>
> if (scissor)
> si_clip_scissor(, scissor);
> +   si_mix_scissor(mix, );
>
> radeon_emit(cs, S_028250_TL_X(final.minx) |
> S_028250_TL_Y(final.miny) |
> @@ -903,19 +914,35 @@ static void si_emit_one_scissor(struct radeon_winsys_cs 
> *cs,
> S_028254_BR_Y(final.maxy));
>  }
>
> +static void si_emit_guardband(struct si_context *sctx,
> + struct pipe_scissor_state *scissor)
> +{
> +   struct radeon_winsys_cs *cs = sctx->b.gfx.cs;
> +
> +   int width = scissor->maxx - scissor->minx;
> +   int height = scissor->maxy - scissor->miny;
> +   float guardband_x = width ? (32768 / width) : 1.0;
> +   float guardband_y = height ? (32768 / height) : 1.0;
> +
> +   radeon_set_context_reg(cs, R_028BF0_PA_CL_GB_HORZ_CLIP_ADJ, 
> fui(guardband_x));
> +   radeon_set_context_reg(cs, R_028BE8_PA_CL_GB_VERT_CLIP_ADJ, 
> fui(guardband_y));
> +}
> +
>  static void si_emit_scissors(struct si_context *sctx, struct r600_atom *atom)
>  {
> struct radeon_winsys_cs *cs = sctx->b.gfx.cs;
> struct pipe_scissor_state *states = sctx->scissors.states;
> unsigned mask = sctx->scissors.dirty_mask;
> bool scissor_enable = sctx->queued.named.rasterizer->scissor_enable;
> +   struct pipe_scissor_state max_clip = {0};
>
> /* The simple case: Only 1 viewport is active. */
> if (mask & 1 &&
> !si_get_vs_info(sctx)->writes_viewport_index) {
> radeon_set_context_reg_seq(cs, 
> R_028250_PA_SC_VPORT_SCISSOR_0_TL, 2);
> si_emit_one_scissor(cs, >viewports.states[0],
> -   scissor_enable ? [0] : NULL);
> +   scissor_enable ? [0] : NULL, 
> _clip);
> +   si_emit_guardband(sctx, _clip);
> sctx->scissors.dirty_mask &= ~1; /* clear one bit */
> return;
> }
> @@ -929,9 +956,10 @@ static void si_emit_scissors(struct si_context *sctx, 
> struct r600_atom *atom)
>start * 4 * 2, count * 2);
> for (i = start; i < start+count; i++) {
> si_emit_one_scissor(cs, >viewports.states[i],
> -   scissor_enable ? [i] : 
> NULL);
> +   scissor_enable ? [i] : 
> NULL, _clip);
> }
> }
> +   si_emit_guardband(sctx, _clip);
> sctx->scissors.dirty_mask = 0;
>  }
>
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> 

[Mesa-dev] [PATCH 2/2] radeonsi: use guard band clipping

[Grigori Goronzy]

With the previous changes to handling of viewport clipping, it is
almost trivial to add proper support for guard band clipping.  Select a
suitable integer clipping value to keep inside the rasterizer's guard
band range of [-32768, 32767] and program the hardware to use guard
band clipping.

Guard band clipping speeds up rasterization for primitives that are
partially off-screen.  This change in particular results in small
framerate improvements in a wide range of games.
---
 src/gallium/drivers/radeonsi/si_state.c | 34 ++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 0bb41e5..013fcf1 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -882,9 +882,19 @@ static void si_clip_scissor(struct pipe_scissor_state *out,
out->maxy = MIN2(out->maxy, clip->maxy);
 }
 
+static void si_mix_scissor(struct pipe_scissor_state *out,
+  struct pipe_scissor_state *clip)
+{
+   out->minx = MIN2(out->minx, clip->minx);
+   out->miny = MIN2(out->miny, clip->miny);
+   out->maxx = MAX2(out->maxx, clip->maxx);
+   out->maxy = MAX2(out->maxy, clip->maxy);
+}
+
 static void si_emit_one_scissor(struct radeon_winsys_cs *cs,
struct pipe_viewport_state *vp,
-   struct pipe_scissor_state *scissor)
+   struct pipe_scissor_state *scissor,
+   struct pipe_scissor_state *mix)
 {
struct pipe_scissor_state final;
 
@@ -895,6 +905,7 @@ static void si_emit_one_scissor(struct radeon_winsys_cs *cs,
 
if (scissor)
si_clip_scissor(, scissor);
+   si_mix_scissor(mix, );
 
radeon_emit(cs, S_028250_TL_X(final.minx) |
S_028250_TL_Y(final.miny) |
@@ -903,19 +914,35 @@ static void si_emit_one_scissor(struct radeon_winsys_cs 
*cs,
S_028254_BR_Y(final.maxy));
 }
 
+static void si_emit_guardband(struct si_context *sctx,
+ struct pipe_scissor_state *scissor)
+{
+   struct radeon_winsys_cs *cs = sctx->b.gfx.cs;
+
+   int width = scissor->maxx - scissor->minx;
+   int height = scissor->maxy - scissor->miny;
+   float guardband_x = width ? (32768 / width) : 1.0;
+   float guardband_y = height ? (32768 / height) : 1.0;
+
+   radeon_set_context_reg(cs, R_028BF0_PA_CL_GB_HORZ_CLIP_ADJ, 
fui(guardband_x));
+   radeon_set_context_reg(cs, R_028BE8_PA_CL_GB_VERT_CLIP_ADJ, 
fui(guardband_y));
+}
+
 static void si_emit_scissors(struct si_context *sctx, struct r600_atom *atom)
 {
struct radeon_winsys_cs *cs = sctx->b.gfx.cs;
struct pipe_scissor_state *states = sctx->scissors.states;
unsigned mask = sctx->scissors.dirty_mask;
bool scissor_enable = sctx->queued.named.rasterizer->scissor_enable;
+   struct pipe_scissor_state max_clip = {0};
 
/* The simple case: Only 1 viewport is active. */
if (mask & 1 &&
!si_get_vs_info(sctx)->writes_viewport_index) {
radeon_set_context_reg_seq(cs, 
R_028250_PA_SC_VPORT_SCISSOR_0_TL, 2);
si_emit_one_scissor(cs, >viewports.states[0],
-   scissor_enable ? [0] : NULL);
+   scissor_enable ? [0] : NULL, 
_clip);
+   si_emit_guardband(sctx, _clip);
sctx->scissors.dirty_mask &= ~1; /* clear one bit */
return;
}
@@ -929,9 +956,10 @@ static void si_emit_scissors(struct si_context *sctx, 
struct r600_atom *atom)
   start * 4 * 2, count * 2);
for (i = start; i < start+count; i++) {
si_emit_one_scissor(cs, >viewports.states[i],
-   scissor_enable ? [i] : NULL);
+   scissor_enable ? [i] : NULL, 
_clip);
}
}
+   si_emit_guardband(sctx, _clip);
sctx->scissors.dirty_mask = 0;
 }
 
-- 
1.9.1

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