[Mesa-dev] [PATCH 1/6] gallium: clarify the constraints on sampler_view_destroy
Nicolai Hähnle
nhaehnle at gmail.com
Tue Oct 10 21:42:49 UTC 2017
Same as on IRC:
On 10.10.2017 04:06, Marek Olšák wrote:
> Is there any difference with piglit/drawoverhead?
Yes, there is.
> If yes, would this be useful?
> https://patchwork.freedesktop.org/patch/41241/
Surprisingly, not that much. I'm going to think though a couple of other
options, but want to already push patch #3; I sent a version that is
rebased on master and disentangled from the locking stuff.
Cheers,
Nicolai
>
> Marek
>
>
> On Fri, Oct 6, 2017 at 10:38 PM, Nicolai Hähnle <nhaehnle at gmail.com> 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;
>> --
>> 2.11.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list