[Mesa-dev] [PATCH 3/3] softpipe: don't use samplers with prebaked sampler and sampler_view state

Jose Fonseca jfonseca at vmware.com
Fri Mar 8 12:03:48 PST 2013


----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> This is needed for handling the dx10-style sample opcodes.
> This also simplifies the logic by getting rid of sampler variants
> completely (sampler_views though OTOH have sort of variants because
> some of their state is different depending on the shader stage they
> are bound to).
> Two piglit regressions I haven't figured out, streaming-texture-leak
> (pass -> fail though it's actually crash due to oom kill, looks like
> leaking textures somewhere), and fbo-srgb-blit (looks like the srgb->rgb
> or vice versa state somewhere gets lost).
> No significant performance difference (openarena run:
> 840 frames in 459.8 seconds vs. 840 frames in 463.9 seconds).
> ---
>  src/gallium/drivers/softpipe/sp_context.h       |    8 +-
>  src/gallium/drivers/softpipe/sp_state.h         |    8 +
>  src/gallium/drivers/softpipe/sp_state_derived.c |   33 +-
>  src/gallium/drivers/softpipe/sp_state_sampler.c |  184 +---
>  src/gallium/drivers/softpipe/sp_tex_sample.c    | 1274
>  ++++++++++++-----------
>  src/gallium/drivers/softpipe/sp_tex_sample.h    |  138 +--
>  6 files changed, 779 insertions(+), 866 deletions(-)
> 
> diff --git a/src/gallium/drivers/softpipe/sp_context.h
> b/src/gallium/drivers/softpipe/sp_context.h
> index dcd29be..ed38bac 100644
> --- a/src/gallium/drivers/softpipe/sp_context.h
> +++ b/src/gallium/drivers/softpipe/sp_context.h
> @@ -80,7 +80,7 @@ struct softpipe_context {
>     struct pipe_framebuffer_state framebuffer;
>     struct pipe_poly_stipple poly_stipple;
>     struct pipe_scissor_state scissor;
> -   struct pipe_sampler_view
> *sampler_views[PIPE_SHADER_TYPES][PIPE_MAX_SAMPLERS];
> +   struct pipe_sampler_view
> *sampler_views[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_SAMPLER_VIEWS];
>  
>     struct pipe_viewport_state viewport;
>     struct pipe_vertex_buffer vertex_buffer[PIPE_MAX_ATTRIBS];
> @@ -184,8 +184,10 @@ struct softpipe_context {
>      * Texture caches for vertex, fragment, geometry stages.
>      * Don't use PIPE_SHADER_TYPES here to avoid allocating unused memory
>      * for compute shaders.
> +    * XXX wouldn't it make more sense for the tile cache to just be part
> +    * of sp_sampler_view?
>      */
> -   struct softpipe_tex_tile_cache
> *tex_cache[PIPE_SHADER_GEOMETRY+1][PIPE_MAX_SAMPLERS];
> +   struct softpipe_tex_tile_cache
> *tex_cache[PIPE_SHADER_GEOMETRY+1][PIPE_MAX_SHADER_SAMPLER_VIEWS];
>  
>     unsigned dump_fs : 1;
>     unsigned dump_gs : 1;
> @@ -199,8 +201,6 @@ softpipe_context( struct pipe_context *pipe )
>     return (struct softpipe_context *)pipe;
>  }
>  
> -void
> -softpipe_reset_sampler_variants(struct softpipe_context *softpipe);
>  
>  struct pipe_context *
>  softpipe_create_context( struct pipe_screen *, void *priv );
> diff --git a/src/gallium/drivers/softpipe/sp_state.h
> b/src/gallium/drivers/softpipe/sp_state.h
> index e2c49d2..bf8370e 100644
> --- a/src/gallium/drivers/softpipe/sp_state.h
> +++ b/src/gallium/drivers/softpipe/sp_state.h
> @@ -156,6 +156,14 @@ void
>  softpipe_update_derived(struct softpipe_context *softpipe, unsigned prim);
>  
>  void
> +softpipe_set_sampler_views(struct pipe_context *pipe,
> +                           unsigned shader,
> +                           unsigned start,
> +                           unsigned num,
> +                           struct pipe_sampler_view **views);
> +
> +
> +void
>  softpipe_draw_vbo(struct pipe_context *pipe,
>                    const struct pipe_draw_info *info);
>  
> diff --git a/src/gallium/drivers/softpipe/sp_state_derived.c
> b/src/gallium/drivers/softpipe/sp_state_derived.c
> index 8fbe27b..85fd47d 100644
> --- a/src/gallium/drivers/softpipe/sp_state_derived.c
> +++ b/src/gallium/drivers/softpipe/sp_state_derived.c
> @@ -36,6 +36,7 @@
>  #include "sp_screen.h"
>  #include "sp_state.h"
>  #include "sp_texture.h"
> +#include "sp_tex_sample.h"
>  #include "sp_tex_tile_cache.h"
>  
>  
> @@ -205,12 +206,32 @@ compute_cliprect(struct softpipe_context *sp)
>  
>  
>  static void
> +set_shader_sampler(struct softpipe_context *softpipe,
> +                   unsigned shader,
> +                   int max_sampler)
> +{
> +   int i;
> +   for (i = 0; i <= max_sampler; i++) {
> +      softpipe->tgsi.sampler[shader]->sp_sampler[i] =
> +         (struct sp_sampler *)(softpipe->samplers[shader][i]);
> +   }
> +}
> +
> +static void
>  update_tgsi_samplers( struct softpipe_context *softpipe )
>  {
>     unsigned i, sh;
>  
> -   softpipe_reset_sampler_variants( softpipe );
> +   set_shader_sampler(softpipe, PIPE_SHADER_VERTEX,
> +                      softpipe->vs->max_sampler);
> +   set_shader_sampler(softpipe, PIPE_SHADER_FRAGMENT,
> +
> softpipe->fs_variant->info.file_max[TGSI_FILE_SAMPLER]);
> +   if (softpipe->gs) {
> +      set_shader_sampler(softpipe, PIPE_SHADER_GEOMETRY,
> +                         softpipe->gs->max_sampler);
> +   }
>  
> +   /* XXX is this really necessary here??? */
>     for (sh = 0; sh < Elements(softpipe->tex_cache); sh++) {
>        for (i = 0; i < PIPE_MAX_SAMPLERS; i++) {
>           struct softpipe_tex_tile_cache *tc = softpipe->tex_cache[sh][i];
> @@ -314,12 +335,9 @@ update_polygon_stipple_enable(struct softpipe_context
> *softpipe, unsigned prim)
>        /* sampler state */
>        softpipe->samplers[PIPE_SHADER_FRAGMENT][unit] =
>        softpipe->pstipple.sampler;
>  
> -      /* sampler view */
> -
> pipe_sampler_view_reference(&softpipe->sampler_views[PIPE_SHADER_FRAGMENT][unit],
> -                                  softpipe->pstipple.sampler_view);

Why is this pipe_sampler_view_reference() call being removed?

> -
> -
> sp_tex_tile_cache_set_sampler_view(softpipe->tex_cache[PIPE_SHADER_FRAGMENT][unit],
> -                                         softpipe->pstipple.sampler_view);
> +      /* sampler view state */
> +      softpipe_set_sampler_views(&softpipe->pipe, PIPE_SHADER_FRAGMENT,
> +                                 unit, 1, &softpipe->pstipple.sampler_view);
>  
>        softpipe->dirty |= SP_NEW_SAMPLER;
>     }
> @@ -358,6 +376,7 @@ softpipe_update_derived(struct softpipe_context
> *softpipe, unsigned prim)
>        update_polygon_stipple_enable(softpipe, prim);
>  #endif
>  
> +   /* TODO: this looks suboptimal */
>     if (softpipe->dirty & (SP_NEW_SAMPLER |
>                            SP_NEW_TEXTURE |
>                            SP_NEW_FS |
> diff --git a/src/gallium/drivers/softpipe/sp_state_sampler.c
> b/src/gallium/drivers/softpipe/sp_state_sampler.c
> index eb02ecb..f031ad2 100644
> --- a/src/gallium/drivers/softpipe/sp_state_sampler.c
> +++ b/src/gallium/drivers/softpipe/sp_state_sampler.c
> @@ -41,31 +41,6 @@
>  #include "sp_tex_tile_cache.h"
>  
>  
> -struct sp_sampler {
> -   struct pipe_sampler_state base;
> -   struct sp_sampler_variant *variants;
> -   struct sp_sampler_variant *current;
> -};
> -
> -static struct sp_sampler *sp_sampler( struct pipe_sampler_state *sampler )
> -{
> -   return (struct sp_sampler *)sampler;
> -}
> -
> -
> -static void *
> -softpipe_create_sampler_state(struct pipe_context *pipe,
> -                              const struct pipe_sampler_state *sampler)
> -{
> -   struct sp_sampler *sp_sampler = CALLOC_STRUCT(sp_sampler);
> -
> -   sp_sampler->base = *sampler;
> -   sp_sampler->variants = NULL;
> -
> -   return (void *)sp_sampler;
> -}
> -
> -
>  /**
>   * Bind a range [start, start+num-1] of samplers for a shader stage.
>   */
> @@ -142,24 +117,6 @@ softpipe_bind_geometry_sampler_states(struct
> pipe_context *pipe,
>  }
>  
>  
> -static struct pipe_sampler_view *
> -softpipe_create_sampler_view(struct pipe_context *pipe,
> -                             struct pipe_resource *resource,
> -                             const struct pipe_sampler_view *templ)
> -{
> -   struct pipe_sampler_view *view = CALLOC_STRUCT(pipe_sampler_view);
> -
> -   if (view) {
> -      *view = *templ;
> -      view->reference.count = 1;
> -      view->texture = NULL;
> -      pipe_resource_reference(&view->texture, resource);
> -      view->context = pipe;
> -   }
> -
> -   return view;
> -}
> -
>  
>  static void
>  softpipe_sampler_view_destroy(struct pipe_context *pipe,
> @@ -170,7 +127,7 @@ softpipe_sampler_view_destroy(struct pipe_context *pipe,
>  }
>  
>  
> -static void
> +void
>  softpipe_set_sampler_views(struct pipe_context *pipe,
>                             unsigned shader,
>                             unsigned start,
> @@ -194,12 +151,29 @@ softpipe_set_sampler_views(struct pipe_context *pipe,
>  
>     /* set the new sampler views */
>     for (i = 0; i < num; i++) {
> -      pipe_sampler_view_reference(&softpipe->sampler_views[shader][start +
> i],
> -                                  views[i]);
> +      struct pipe_sampler_view *view = softpipe->sampler_views[shader][start
> + i];
> +      struct sp_sampler_view *sp_sviewsrc;
> +      struct sp_sampler_view *sp_sviewdst =
> +         &softpipe->tgsi.sampler[shader]->sp_sview[start + i];
> +      pipe_sampler_view_reference(&view, views[i]);

Shouldn't it be:

         struct pipe_sampler_view **pview = &softpipe->sampler_views[shader][start + i];
         pipe_sampler_view_reference(pview, views[i]);

e +       * We don't really have variants, however some bits are different per
> shader,
> +       * so just copy?
> +       */
> +      sp_sviewsrc = (struct sp_sampler_view *)view;
> +      if (sp_sviewsrc) {
> +         memcpy(sp_sviewdst, sp_sviewsrc, sizeof(*sp_sviewsrc));
> +         sp_sviewdst->compute_lambda =
> softpipe_get_lambda_func(&sp_sviewdst->base, shader);
> +         sp_sviewdst->cache = softpipe->tex_cache[shader][start + i];
> +      }
> +      else {
> +         memset(sp_sviewdst, 0,  sizeof(*sp_sviewsrc));
> +      }
>     }
>  
> +
>     /* find highest non-null sampler_views[] entry */
>     {
>        unsigned j = MAX2(softpipe->num_sampler_views[shader], start + num);
> @@ -246,128 +220,10 @@ softpipe_set_geometry_sampler_views(struct
[...]

Jose


More information about the mesa-dev mailing list