[Mesa-dev] [PATCH 3/3] softpipe: don't use samplers with prebaked sampler and sampler_view state
Roland Scheidegger
sroland at vmware.com
Fri Mar 8 12:42:18 PST 2013
Am 08.03.2013 21:03, schrieb Jose Fonseca:
> ----- 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?
Because the new softpipe_set_sampler_views call below will do this now
(I didn't change this here first actually, but this resulted in the
sampler view changes not being picked up and hence two more piglit
regressions where tests using polygon stipple crashed).
>
>> -
>> -
>> 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]);
Oh I think you're right. That could explain the textures leaking...
Seems to fix it indeed. And as a bonus, also fix the other bug (the
fbo-srgb-blit one). I'll rerun all tests just to make sure. Thanks!
Roland
>
> 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