[Mesa-dev] [PATCH] util: fix blitter/draw module interaction
Marek Olšák
maraeo at gmail.com
Mon Oct 13 05:04:02 PDT 2014
Actually, util_blitter_cache_all_shaders was supposed to prevent all
crashes. Drivers that use the Draw AA stages should call it before the
stages are installed, so that no shaders need to be compiled later. I
guess some shaders are missing there. Just updating
util_blitter_cache_all_shaders would be cleaner.
The fixes for bind_fs_state look OK.
Marek
On Mon, Oct 6, 2014 at 11:51 PM, Brian Paul <brianp at vmware.com> wrote:
> If a driver uses the blitter utility and the draw module's AA point/line/
> etc. helpers we have to very careful when using the pipe_context::
> create_fs_state(), bind_fs_state() and delete_fs_state() functions.
> The draw module can override these driver functions.
>
> This fixes the situation where the blitter's calls to create_fs_state()
> and bind_fs_state() were calling the draw module functions but
> delete_fs_state() was calling the driver's function. That mismatch led
> to memory corruption and crashes.
>
> The blitter code tries to avoid hitting the draw module functions.
> But that's derailed when the util_make_*fragment*() functions are
> called for creating shaders on demand. This patch adds new functions
> to patch in and restore the pipe::create_fs_state() function. We
> also call the ctx->bind_fs_state() function everywhere. Now we should
> never accidentally call any of the draw module functions.
> ---
> src/gallium/auxiliary/util/u_blitter.c | 82 ++++++++++++++++++++++++++++++--
> 1 file changed, 79 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c
> index f3fe949..76fba24 100644
> --- a/src/gallium/auxiliary/util/u_blitter.c
> +++ b/src/gallium/auxiliary/util/u_blitter.c
> @@ -137,10 +137,59 @@ struct blitter_context_priv
>
> /* The Draw module overrides these functions.
> * Always create the blitter before Draw. */
> + void * (*create_fs_state)(struct pipe_context *,
> + const struct pipe_shader_state *);
> void (*bind_fs_state)(struct pipe_context *, void *);
> void (*delete_fs_state)(struct pipe_context *, void *);
> +
> + /* Only used between begin/end_create_fs_state() */
> + void * (*saved_create_fs_state)(struct pipe_context *,
> + const struct pipe_shader_state *);
> };
>
> +
> +
> +/**
> + * Need to call this before calling any of the util_make_*fragment*()
> + * functions.
> + * When we create/bind/delete fragment shaders we want to call the actual
> + * driver functions, not the draw module's functions. The draw modules for
> + * drawing AA points/lines can overide the driver's function so when we
> + * call util_make_*fragment*() we may wind up calling a draw module
> + * create_fs_state() function by mistake.
> + *
> + * This helper plugs in the real/driver create_fs_state() function into the
> + * pipe context.
> + *
> + * The alternative to this madness would be to pass a create_fs_state
> + * function into the util_make_*fragment*() functions to be explicit.
> + * But that's not simple either since the tgsi ureg code would need to be
> + * touched too.
> + */
> +static INLINE void
> +begin_create_fs(struct blitter_context_priv *ctx)
> +{
> + struct pipe_context *pipe = ctx->base.pipe;
> + assert(ctx->saved_create_fs_state == NULL);
> + ctx->saved_create_fs_state = pipe->create_fs_state;
> + pipe->create_fs_state = ctx->create_fs_state;
> +}
> +
> +
> +/**
> + * Need to call this after calling any of the util_make_*fragment*()
> + * functions to restore the pipe's create_fs_state() function.
> + */
> +static INLINE void
> +end_create_fs(struct blitter_context_priv *ctx)
> +{
> + struct pipe_context *pipe = ctx->base.pipe;
> + assert(ctx->saved_create_fs_state);
> + pipe->create_fs_state = ctx->saved_create_fs_state;
> + ctx->saved_create_fs_state = NULL;
> +}
> +
> +
> static struct pipe_surface *
> util_blitter_get_next_surface_layer(struct pipe_context *pipe,
> struct pipe_surface *surf);
> @@ -163,6 +212,7 @@ struct blitter_context *util_blitter_create(struct pipe_context *pipe)
> ctx->base.draw_rectangle = util_blitter_draw_rectangle;
> ctx->base.get_next_surface_layer = util_blitter_get_next_surface_layer;
>
> + ctx->create_fs_state = pipe->create_fs_state;
> ctx->bind_fs_state = pipe->bind_fs_state;
> ctx->delete_fs_state = pipe->delete_fs_state;
>
> @@ -356,10 +406,12 @@ static void bind_fs_empty(struct blitter_context_priv *ctx)
> struct pipe_context *pipe = ctx->base.pipe;
>
> if (!ctx->fs_empty) {
> + begin_create_fs(ctx);
> ctx->fs_empty = util_make_empty_fragment_shader(pipe);
> + end_create_fs(ctx);
> }
>
> - pipe->bind_fs_state(pipe, ctx->fs_empty);
> + ctx->bind_fs_state(pipe, ctx->fs_empty);
> }
>
> static void bind_fs_write_one_cbuf(struct blitter_context_priv *ctx)
> @@ -367,12 +419,14 @@ static void bind_fs_write_one_cbuf(struct blitter_context_priv *ctx)
> struct pipe_context *pipe = ctx->base.pipe;
>
> if (!ctx->fs_write_one_cbuf) {
> + begin_create_fs(ctx);
> ctx->fs_write_one_cbuf =
> util_make_fragment_passthrough_shader(pipe, TGSI_SEMANTIC_GENERIC,
> TGSI_INTERPOLATE_CONSTANT, FALSE);
> + end_create_fs(ctx);
> }
>
> - pipe->bind_fs_state(pipe, ctx->fs_write_one_cbuf);
> + ctx->bind_fs_state(pipe, ctx->fs_write_one_cbuf);
> }
>
> static void bind_fs_write_all_cbufs(struct blitter_context_priv *ctx)
> @@ -380,14 +434,18 @@ static void bind_fs_write_all_cbufs(struct blitter_context_priv *ctx)
> struct pipe_context *pipe = ctx->base.pipe;
>
> if (!ctx->fs_write_all_cbufs) {
> + begin_create_fs(ctx);
> ctx->fs_write_all_cbufs =
> util_make_fragment_passthrough_shader(pipe, TGSI_SEMANTIC_GENERIC,
> TGSI_INTERPOLATE_CONSTANT, TRUE);
> + end_create_fs(ctx);
> }
>
> - pipe->bind_fs_state(pipe, ctx->fs_write_all_cbufs);
> + ctx->bind_fs_state(pipe, ctx->fs_write_all_cbufs);
> }
>
> +void **special_addr = NULL;
> +
> void util_blitter_destroy(struct blitter_context *blitter)
> {
> struct blitter_context_priv *ctx = (struct blitter_context_priv*)blitter;
> @@ -850,6 +908,7 @@ static void *blitter_get_fs_texfetch_col(struct blitter_context_priv *ctx,
> shader = &ctx->fs_resolve[target][index][filter];
>
> if (!*shader) {
> + begin_create_fs(ctx);
> if (filter == PIPE_TEX_FILTER_LINEAR) {
> *shader = util_make_fs_msaa_resolve_bilinear(pipe, tgsi_tex,
> src_nr_samples,
> @@ -860,6 +919,7 @@ static void *blitter_get_fs_texfetch_col(struct blitter_context_priv *ctx,
> src_nr_samples,
> is_uint, is_sint);
> }
> + end_create_fs(ctx);
> }
> }
> else {
> @@ -870,7 +930,9 @@ static void *blitter_get_fs_texfetch_col(struct blitter_context_priv *ctx,
>
> /* Create the fragment shader on-demand. */
> if (!*shader) {
> + begin_create_fs(ctx);
> *shader = util_make_fs_blit_msaa_color(pipe, tgsi_tex);
> + end_create_fs(ctx);
> }
> }
>
> @@ -880,8 +942,10 @@ static void *blitter_get_fs_texfetch_col(struct blitter_context_priv *ctx,
>
> /* Create the fragment shader on-demand. */
> if (!*shader) {
> + begin_create_fs(ctx);
> *shader = util_make_fragment_tex_shader(pipe, tgsi_tex,
> TGSI_INTERPOLATE_LINEAR);
> + end_create_fs(ctx);
> }
>
> return *shader;
> @@ -905,8 +969,10 @@ void *blitter_get_fs_texfetch_depth(struct blitter_context_priv *ctx,
> unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target,
> nr_samples);
>
> + begin_create_fs(ctx);
> *shader =
> util_make_fs_blit_msaa_depth(pipe, tgsi_tex);
> + end_create_fs(ctx);
> }
>
> return *shader;
> @@ -917,9 +983,11 @@ void *blitter_get_fs_texfetch_depth(struct blitter_context_priv *ctx,
> if (!*shader) {
> unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target, 0);
>
> + begin_create_fs(ctx);
> *shader =
> util_make_fragment_tex_shader_writedepth(pipe, tgsi_tex,
> TGSI_INTERPOLATE_LINEAR);
> + end_create_fs(ctx);
> }
>
> return *shader;
> @@ -943,8 +1011,10 @@ void *blitter_get_fs_texfetch_depthstencil(struct blitter_context_priv *ctx,
> unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target,
> nr_samples);
>
> + begin_create_fs(ctx);
> *shader =
> util_make_fs_blit_msaa_depthstencil(pipe, tgsi_tex);
> + end_create_fs(ctx);
> }
>
> return *shader;
> @@ -955,9 +1025,11 @@ void *blitter_get_fs_texfetch_depthstencil(struct blitter_context_priv *ctx,
> if (!*shader) {
> unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target, 0);
>
> + begin_create_fs(ctx);
> *shader =
> util_make_fragment_tex_shader_writedepthstencil(pipe, tgsi_tex,
> TGSI_INTERPOLATE_LINEAR);
> + end_create_fs(ctx);
> }
>
> return *shader;
> @@ -981,8 +1053,10 @@ void *blitter_get_fs_texfetch_stencil(struct blitter_context_priv *ctx,
> unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target,
> nr_samples);
>
> + begin_create_fs(ctx);
> *shader =
> util_make_fs_blit_msaa_stencil(pipe, tgsi_tex);
> + end_create_fs(ctx);
> }
>
> return *shader;
> @@ -993,9 +1067,11 @@ void *blitter_get_fs_texfetch_stencil(struct blitter_context_priv *ctx,
> if (!*shader) {
> unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target, 0);
>
> + begin_create_fs(ctx);
> *shader =
> util_make_fragment_tex_shader_writestencil(pipe, tgsi_tex,
> TGSI_INTERPOLATE_LINEAR);
> + end_create_fs(ctx);
> }
>
> return *shader;
> --
> 1.7.10.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list