[Mesa-dev] [PATCH 1/6] gallium: clarify the constraints on sampler_view_destroy
Nicolai Hähnle
nicolai.haehnle at amd.com
Fri Oct 6 20:45:20 UTC 2017
FWIW, the drivers that really are affected currently are SVGA and
possibly virgl, because they pass Gallium calls on to a supervisor.
Since we guarantee that the creation context is still alive, those
drivers could fix any issues by adding samplers that are destroyed from
a different context to an "orphan" list which is regularly cleaned up by
the creating context.
Cheers,
Nicolai
On 06.10.2017 22:38, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> r600 expects the context that created the sampler view to still be alive
> (there is a per-context list of sampler views).
>
> svga currently bails when the context of destruction is not the same as
> creation.
>
> The GL state tracker, which is the only one that runs into the
> multi-context subtleties (due to share groups), already guarantees that
> sampler views are destroyed before their context of creation is destroyed.
>
> Most drivers are context-agnostic, so the warning message in
> pipe_sampler_view_release doesn't really make sense.
> ---
> src/gallium/auxiliary/util/u_inlines.h | 16 ++++++++++------
> src/gallium/include/pipe/p_context.h | 10 ++++++++++
> src/mesa/state_tracker/st_sampler_view.c | 1 -
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_inlines.h b/src/gallium/auxiliary/util/u_inlines.h
> index 79f62c32266..790352d7800 100644
> --- a/src/gallium/auxiliary/util/u_inlines.h
> +++ b/src/gallium/auxiliary/util/u_inlines.h
> @@ -142,45 +142,49 @@ pipe_resource_reference(struct pipe_resource **ptr, struct pipe_resource *tex)
> struct pipe_resource *next = old_tex->next;
>
> old_tex->screen->resource_destroy(old_tex->screen, old_tex);
> old_tex = next;
> } while (pipe_reference_described(&old_tex->reference, NULL,
> (debug_reference_descriptor)debug_describe_resource));
> }
> *ptr = tex;
> }
>
> +/**
> + * Set *ptr to \p view with proper reference counting.
> + *
> + * The caller must guarantee that \p view and *ptr must have been created in
> + * the same context (if they exist), and that this must be the current context.
> + */
> static inline void
> pipe_sampler_view_reference(struct pipe_sampler_view **ptr, struct pipe_sampler_view *view)
> {
> struct pipe_sampler_view *old_view = *ptr;
>
> if (pipe_reference_described(&(*ptr)->reference, &view->reference,
> (debug_reference_descriptor)debug_describe_sampler_view))
> old_view->context->sampler_view_destroy(old_view->context, old_view);
> *ptr = view;
> }
>
> /**
> * Similar to pipe_sampler_view_reference() but always set the pointer to
> - * NULL and pass in an explicit context. Passing an explicit context is a
> - * work-around for fixing a dangling context pointer problem when textures
> - * are shared by multiple contexts. XXX fix this someday.
> + * NULL and pass in the current context explicitly.
> + *
> + * If *ptr is non-NULL, it may refer to a view that was created in a different
> + * context (however, that context must still be alive).
> */
> static inline void
> pipe_sampler_view_release(struct pipe_context *ctx,
> struct pipe_sampler_view **ptr)
> {
> struct pipe_sampler_view *old_view = *ptr;
> - if (*ptr && (*ptr)->context != ctx) {
> - debug_printf_once(("context mis-match in pipe_sampler_view_release()\n"));
> - }
> if (pipe_reference_described(&(*ptr)->reference, NULL,
> (debug_reference_descriptor)debug_describe_sampler_view)) {
> ctx->sampler_view_destroy(ctx, old_view);
> }
> *ptr = NULL;
> }
>
> static inline void
> pipe_so_target_reference(struct pipe_stream_output_target **ptr,
> struct pipe_stream_output_target *target)
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index 4609d4dbf23..087836d1c0c 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -501,20 +501,30 @@ struct pipe_context {
> void (*fence_server_sync)(struct pipe_context *pipe,
> struct pipe_fence_handle *fence);
>
> /**
> * Create a view on a texture to be used by a shader stage.
> */
> struct pipe_sampler_view * (*create_sampler_view)(struct pipe_context *ctx,
> struct pipe_resource *texture,
> const struct pipe_sampler_view *templat);
>
> + /**
> + * Destroy a view on a texture.
> + *
> + * \param ctx the current context
> + * \param view the view to be destroyed
> + *
> + * \note The current context may not be the context in which the view was
> + * created (view->context). However, the caller must guarantee that
> + * the context which created the view is still alive.
> + */
> void (*sampler_view_destroy)(struct pipe_context *ctx,
> struct pipe_sampler_view *view);
>
>
> /**
> * Get a surface which is a "view" into a resource, used by
> * render target / depth stencil stages.
> */
> struct pipe_surface *(*create_surface)(struct pipe_context *ctx,
> struct pipe_resource *resource,
> diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c
> index fbf0aaeb03a..d1715a888d9 100644
> --- a/src/mesa/state_tracker/st_sampler_view.c
> +++ b/src/mesa/state_tracker/st_sampler_view.c
> @@ -109,21 +109,20 @@ st_texture_release_sampler_view(struct st_context *st,
> /**
> * Release all sampler views attached to the given texture object, regardless
> * of the context.
> */
> void
> st_texture_release_all_sampler_views(struct st_context *st,
> struct st_texture_object *stObj)
> {
> GLuint i;
>
> - /* XXX This should use sampler_views[i]->pipe, not st->pipe */
> for (i = 0; i < stObj->num_sampler_views; ++i)
> pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i]);
> }
>
>
> void
> st_texture_free_sampler_views(struct st_texture_object *stObj)
> {
> free(stObj->sampler_views);
> stObj->sampler_views = NULL;
>
More information about the mesa-dev
mailing list