[Mesa-dev] [PATCH 5/6] st/mesa: optimize pipe_sampler_view validation

[Nicolai Hähnle]

On 06.10.2016 02:42, Brian Paul wrote:
> Before, st_get_texture_sampler_view_from_stobj() did a lot of work to
> check if the texture parameters matched the sampler view (format,
> swizzle, min/max lod, first/last layer, etc).  We did this every time
> we validated the texture state.
>
> Now, we use a ctx->Driver.TexParameter() callback and a couple other
> checks to proactively release texture views when we know that
> view-related parameters have changed.  Then, the validation step is
> simplified:
> - Search the texture's list of sampler views (just match the context).
> - If found, we're done.
> - Else, create a new sampler view.
>
> There will never be old, out-of-date sampler views attached to texture
> objects that we have to test.
>
> Most apps create textures and set the texture parameters once.  This
> make sampler view validation much cheaper for that case.
>
> Note that the old texture/sampler comparison code has been converted
> into a set of assertions to verify that the sampler view is in fact
> consistent with the texture parameters.  This should help to spot any
> potential regressions.
>
> Reviewed-by: Edward O'Callaghan <funfunctor at folklore1984.net>
> ---
>  src/mesa/state_tracker/st_atom_texture.c | 58 ++++++++++++++++++++------------
>  src/mesa/state_tracker/st_cb_texture.c   | 52 ++++++++++++++++++++++++++++
>  src/mesa/state_tracker/st_texture.c      | 16 ++++-----
>  src/mesa/state_tracker/st_texture.h      |  5 +++
>  4 files changed, 101 insertions(+), 30 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
> index bfa16dc..45f1f6b 100644
> --- a/src/mesa/state_tracker/st_atom_texture.c
> +++ b/src/mesa/state_tracker/st_atom_texture.c
> @@ -370,7 +370,7 @@ st_create_texture_sampler_view_from_stobj(struct st_context *st,
>  static struct pipe_sampler_view *
>  st_get_texture_sampler_view_from_stobj(struct st_context *st,
>                                         struct st_texture_object *stObj,
> -				       enum pipe_format format,
> +                                       const struct gl_sampler_object *samp,
>                                         unsigned glsl_version)
>  {
>     struct pipe_sampler_view **sv;
> @@ -381,34 +381,42 @@ st_get_texture_sampler_view_from_stobj(struct st_context *st,
>
>     sv = st_texture_get_sampler_view(st, stObj);
>
> -   /* if sampler view has changed dereference it */
>     if (*sv) {
> -      if (check_sampler_swizzle(st, stObj, *sv, glsl_version) ||
> -	  (format != (*sv)->format) ||
> -          gl_target_to_pipe(stObj->base.Target) != (*sv)->target ||
> -          stObj->base.MinLevel + stObj->base.BaseLevel != (*sv)->u.tex.first_level ||
> -          last_level(stObj) != (*sv)->u.tex.last_level ||
> -          stObj->base.MinLayer != (*sv)->u.tex.first_layer ||
> -          last_layer(stObj) != (*sv)->u.tex.last_layer) {
> -	 pipe_sampler_view_reference(sv, NULL);
> +      /* Debug check: make sure that the sampler view's parameters are
> +       * what they're supposed to be.
> +       */
> +      struct pipe_sampler_view *view = *sv;
> +      assert(!check_sampler_swizzle(st, stObj, view, glsl_version));
> +      assert(get_sampler_view_format(st, stObj, samp) == view->format);
> +      assert(gl_target_to_pipe(stObj->base.Target) == view->target);
> +      if (stObj->base.Target == GL_TEXTURE_BUFFER) {
> +         unsigned base = stObj->base.BufferOffset;
> +         unsigned size = MIN2(stObj->pt->width0 - base,
> +                              (unsigned) stObj->base.BufferSize);
> +         assert(view->u.buf.offset == base);
> +         assert(view->u.buf.size == size);
> +      }
> +      else {

Huh, so is this else-style intentional, or a spill-over from another 
code base? I wasn't aware of this style in st/mesa.

Also, the whole sampler lookup code gives me the race condition chills, 
but it looks like it's always been that way, so... patches 4 & 5:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Patch 6 is Acked-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Cheers,
Nicolai