[Mesa-dev] [PATCH 3/4] xa: scissor to help tilers

[Thomas Hellstrom]

Rob,

A minor nitpick below:

On 04/01/2014 05:04 PM, Rob Clark wrote:
> From: Rob Clark <robclark at freedesktop.org>
>
> Keep track of the maximal bounds of all the operations and set scissor
> accordingly.  For tiling GPU's this can be a big win by reducing the
> memory bandwidth spent moving pixels from system memory to tile buffer
> and back.
>
> You could imagine being more sophisticated and splitting up disjoint
> operations.  But this simplistic approach is good enough for the common
> cases.
>
> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
>  src/gallium/state_trackers/xa/xa_composite.c |  2 ++
>  src/gallium/state_trackers/xa/xa_context.c   |  3 +++
>  src/gallium/state_trackers/xa/xa_priv.h      | 29 ++++++++++++++++++++++++++++
>  src/gallium/state_trackers/xa/xa_renderer.c  | 14 ++++++++++++++
>  4 files changed, 48 insertions(+)
>
> diff --git a/src/gallium/state_trackers/xa/xa_composite.c b/src/gallium/state_trackers/xa/xa_composite.c
> index 19e7309..861ec29 100644
> --- a/src/gallium/state_trackers/xa/xa_composite.c
> +++ b/src/gallium/state_trackers/xa/xa_composite.c
> @@ -517,6 +517,8 @@ xa_composite_rect(struct xa_context *ctx,
>  	const float *src_matrix = NULL;
>  	const float *mask_matrix = NULL;
>  
> +	xa_scissor_update(ctx, dstX, dstY, dstX + width, dstY + height);
> +
>  	if (comp->src->has_transform)
>  	    src_matrix = comp->src->transform;
>  	if (comp->mask && comp->mask->has_transform)
> diff --git a/src/gallium/state_trackers/xa/xa_context.c b/src/gallium/state_trackers/xa/xa_context.c
> index ee32676..867999a 100644
> --- a/src/gallium/state_trackers/xa/xa_context.c
> +++ b/src/gallium/state_trackers/xa/xa_context.c
> @@ -238,6 +238,8 @@ xa_copy(struct xa_context *ctx,
>  {
>      struct pipe_box src_box;
>  
> +    xa_scissor_update(ctx, dx, dy, dx + width, dy + height);
> +
>      if (ctx->simple_copy) {
>  	u_box_2d(sx, sy, width, height, &src_box);
>  	ctx->pipe->resource_copy_region(ctx->pipe,
> @@ -324,6 +326,7 @@ xa_solid_prepare(struct xa_context *ctx, struct xa_surface *dst,
>  XA_EXPORT void
>  xa_solid(struct xa_context *ctx, int x, int y, int width, int height)
>  {
> +    xa_scissor_update(ctx, x, y, x + width, y + height);
>      renderer_solid(ctx, x, y, x + width, y + height, ctx->solid_color);
>  }
>  
> diff --git a/src/gallium/state_trackers/xa/xa_priv.h b/src/gallium/state_trackers/xa/xa_priv.h
> index 15aedb5..7a43386 100644
> --- a/src/gallium/state_trackers/xa/xa_priv.h
> +++ b/src/gallium/state_trackers/xa/xa_priv.h
> @@ -38,6 +38,8 @@
>  #include "pipe/p_context.h"
>  #include "pipe/p_state.h"
>  
> +#include "util/u_math.h"
> +
>  #if defined(__GNUC__) && __GNUC__ >= 4
>  #define XA_EXPORT __attribute__ ((visibility("default")))
>  #else
> @@ -105,6 +107,12 @@ struct xa_context {
>      struct xa_surface *dst;
>      struct pipe_surface *srf;
>  
> +    /* destination scissor state.. we scissor out untouched parts
> +     * of the dst for the benefit of tilers:
> +     */
> +    struct pipe_scissor_state scissor;
> +    int scissor_valid;
> +
>      int simple_copy;
>  
>      int has_solid_color;
> @@ -118,6 +126,27 @@ struct xa_context {
>      const struct xa_composite *comp;
>  };
>  
> +static INLINE void
> +xa_scissor_reset(struct xa_context *ctx)
> +{
> +    ctx->scissor.maxx = 0;
> +    ctx->scissor.maxy = 0;
> +    ctx->scissor.minx = ~0;
> +    ctx->scissor.miny = ~0;
> +    ctx->scissor_valid = FALSE;
> +}
> +
> +static INLINE void
> +xa_scissor_update(struct xa_context *ctx, unsigned minx, unsigned miny,
> +		unsigned maxx, unsigned maxy)
> +{
> +	ctx->scissor.maxx = MAX2(ctx->scissor.maxx, maxx);
> +	ctx->scissor.maxy = MAX2(ctx->scissor.maxy, maxy);
> +	ctx->scissor.minx = MIN2(ctx->scissor.minx, minx);
> +	ctx->scissor.miny = MIN2(ctx->scissor.miny, miny);
> +	ctx->scissor_valid = TRUE;
> +}
> +
Indentation.

>  enum xa_vs_traits {
>      VS_COMPOSITE = 1 << 0,
>      VS_MASK = 1 << 1,
> diff --git a/src/gallium/state_trackers/xa/xa_renderer.c b/src/gallium/state_trackers/xa/xa_renderer.c
> index e1da28c..8ebda74 100644
> --- a/src/gallium/state_trackers/xa/xa_renderer.c
> +++ b/src/gallium/state_trackers/xa/xa_renderer.c
> @@ -79,11 +79,22 @@ renderer_draw(struct xa_context *r)
>      if (!r->buffer_size)
>  	return;
>  
> +    if (!r->scissor_valid) {
> +	r->scissor.minx = 0;
> +	r->scissor.miny = 0;
> +	r->scissor.maxx = r->dst->tex->width0;
> +	r->scissor.maxy = r->dst->tex->height0;
> +    }
> +
> +    r->pipe->set_scissor_states(r->pipe, 0, 1, &r->scissor);
> +
>      cso_set_vertex_elements(r->cso, r->attrs_per_vertex, r->velems);
>      util_draw_user_vertex_buffer(r->cso, r->buffer, PIPE_PRIM_QUADS,
>                                   num_verts,	/* verts */
>                                   r->attrs_per_vertex);	/* attribs/vert */
>      r->buffer_size = 0;
> +
> +    xa_scissor_reset(r);
>  }
>  
>  static INLINE void
> @@ -111,6 +122,7 @@ renderer_init_state(struct xa_context *r)
>      raster.half_pixel_center = 1;
>      raster.bottom_edge_rule = 1;
>      raster.depth_clip = 1;
> +    raster.scissor = 1;
>      cso_set_rasterizer(r->cso, &raster);
>  
>      /* vertex elements state */
> @@ -359,6 +371,8 @@ renderer_bind_destination(struct xa_context *r,
>      struct pipe_framebuffer_state fb;
>      struct pipe_viewport_state viewport;
>  
> +    xa_scissor_reset(r);
> +
>      /* Framebuffer uses actual surface width/height
>       */
>      memset(&fb, 0, sizeof fb);

Otherwise,
Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>