[PATCH v4] etnaviv: fix resource usage tracking across different pipe_context's
Lucas Stach
l.stach at pengutronix.de
Wed Jan 16 11:14:43 UTC 2019
Am Samstag, den 12.01.2019, 22:22 +0100 schrieb Marek Vasut:
> > From: Christian Gmeiner <christian.gmeiner at gmail.com>
>
> A pipe_resource can be shared by all the pipe_context's hanging off the
> same pipe_screen.
>
> > Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> > Signed-off-by: Marek Vasut <marex at denx.de>
> To: mesa-dev at lists.freedesktop.org
> Cc: etnaviv at lists.freedesktop.org
I would appreciate if this can get a review from someone who didn't
totally mess up the mental model of the locking involved. To me this
change looks good. I'll push this through some internal testing and
push upstream by the end of the week if there aren't any objections.
Regards,
Lucas
> ---
> Changes from v1 -> v2:
> - to remove the resource from the used_resources set when it is destroyed
> Changes from v2 -> v3:
> - add locking with mtx_*() to resource and screen (Marek)
> Changes from v3 -> v4:
> - drop rsc->lock, just use screen->lock for the entire serialization (Marek)
> - simplify etna_resource_used() flush condition, which also prevents
> potentially flushing resources twice (Marek)
> - don't remove resouces from screen->used_resources in
> etna_cmd_stream_reset_notify(), they may still be used in other
> contexts and may need flushing there later on (Marek)
> ---
> src/gallium/drivers/etnaviv/etnaviv_context.c | 26 +++++-----
> src/gallium/drivers/etnaviv/etnaviv_context.h | 3 --
> .../drivers/etnaviv/etnaviv_resource.c | 52 +++++++++++++++----
> .../drivers/etnaviv/etnaviv_resource.h | 8 +--
> src/gallium/drivers/etnaviv/etnaviv_screen.c | 12 +++++
> src/gallium/drivers/etnaviv/etnaviv_screen.h | 6 +++
> 6 files changed, 78 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c b/src/gallium/drivers/etnaviv/etnaviv_context.c
> index 3038d21..2f8cae8 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
> @@ -36,6 +36,7 @@
> #include "etnaviv_query.h"
> #include "etnaviv_query_hw.h"
> #include "etnaviv_rasterizer.h"
> +#include "etnaviv_resource.h"
> #include "etnaviv_screen.h"
> #include "etnaviv_shader.h"
> #include "etnaviv_state.h"
> @@ -329,7 +330,8 @@ static void
> etna_cmd_stream_reset_notify(struct etna_cmd_stream *stream, void *priv)
> {
> struct etna_context *ctx = priv;
> - struct etna_resource *rsc, *rsc_tmp;
> + struct etna_screen *screen = ctx->screen;
> + struct set_entry *entry;
>
> etna_set_state(stream, VIVS_GL_API_MODE, VIVS_GL_API_MODE_OPENGL);
> etna_set_state(stream, VIVS_GL_VERTEX_ELEMENT_CONFIG, 0x00000001);
> @@ -384,16 +386,18 @@ etna_cmd_stream_reset_notify(struct etna_cmd_stream *stream, void *priv)
> ctx->dirty = ~0L;
> ctx->dirty_sampler_views = ~0L;
>
> - /* go through all the used resources and clear their status flag */
> - LIST_FOR_EACH_ENTRY_SAFE(rsc, rsc_tmp, &ctx->used_resources, list)
> - {
> - debug_assert(rsc->status != 0);
> - rsc->status = 0;
> - rsc->pending_ctx = NULL;
> - list_delinit(&rsc->list);
> - }
> + /*
> + * Go through all _resources_ associated with this _screen_, pending
> + * in this _context_ and mark them as not pending in this _context_
> + * anymore, since they were just flushed.
> + */
> + mtx_lock(&screen->lock);
> + set_foreach(screen->used_resources, entry) {
> + struct etna_resource *rsc = (struct etna_resource *)entry->key;
>
> - assert(LIST_IS_EMPTY(&ctx->used_resources));
> + _mesa_set_remove_key(rsc->pending_ctx, ctx);
> + }
> + mtx_unlock(&screen->lock);
> }
>
> static void
> @@ -437,8 +441,6 @@ etna_context_create(struct pipe_screen *pscreen, void *priv, unsigned flags)
> /* need some sane default in case state tracker doesn't set some state: */
> ctx->sample_mask = 0xffff;
>
> - list_inithead(&ctx->used_resources);
> -
> /* Set sensible defaults for state */
> etna_cmd_stream_reset_notify(ctx->stream, ctx);
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.h b/src/gallium/drivers/etnaviv/etnaviv_context.h
> index 584caa7..eff0a23 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.h
> @@ -136,9 +136,6 @@ struct etna_context {
> uint32_t prim_hwsupport;
> struct primconvert_context *primconvert;
>
> - /* list of resources used by currently-unsubmitted renders */
> - struct list_head used_resources;
> -
> struct slab_child_pool transfer_pool;
> struct blitter_context *blitter;
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> index 3808c29..46ab849 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> @@ -33,6 +33,7 @@
> #include "etnaviv_screen.h"
> #include "etnaviv_translate.h"
>
> +#include "util/hash_table.h"
> #include "util/u_inlines.h"
> #include "util/u_memory.h"
>
> @@ -275,7 +276,6 @@ etna_resource_alloc(struct pipe_screen *pscreen, unsigned layout,
> rsc->halign = halign;
>
> pipe_reference_init(&rsc->base.reference, 1);
> - list_inithead(&rsc->list);
>
> size = setup_miptree(rsc, paddingX, paddingY, msaa_xscale, msaa_yscale);
>
> @@ -296,6 +296,11 @@ etna_resource_alloc(struct pipe_screen *pscreen, unsigned layout,
> memset(map, 0, size);
> }
>
> + rsc->pending_ctx = _mesa_set_create(NULL, _mesa_hash_pointer,
> > + _mesa_key_pointer_equal);
> + if (!rsc->pending_ctx)
> + goto free_rsc;
> +
> return &rsc->base;
>
> free_rsc:
> @@ -455,8 +460,14 @@ etna_resource_changed(struct pipe_screen *pscreen, struct pipe_resource *prsc)
> static void
> etna_resource_destroy(struct pipe_screen *pscreen, struct pipe_resource *prsc)
> {
> + struct etna_screen *screen = etna_screen(pscreen);
> struct etna_resource *rsc = etna_resource(prsc);
>
> + mtx_lock(&screen->lock);
> + _mesa_set_remove_key(screen->used_resources, rsc);
> + _mesa_set_destroy(rsc->pending_ctx, NULL);
> + mtx_unlock(&screen->lock);
> +
> if (rsc->bo)
> etna_bo_del(rsc->bo);
>
> @@ -466,8 +477,6 @@ etna_resource_destroy(struct pipe_screen *pscreen, struct pipe_resource *prsc)
> if (rsc->scanout)
> renderonly_scanout_destroy(rsc->scanout, etna_screen(pscreen)->ro);
>
> - list_delinit(&rsc->list);
> -
> pipe_resource_reference(&rsc->texture, NULL);
> pipe_resource_reference(&rsc->external, NULL);
>
> @@ -501,7 +510,6 @@ etna_resource_from_handle(struct pipe_screen *pscreen,
> *prsc = *tmpl;
>
> pipe_reference_init(&prsc->reference, 1);
> - list_inithead(&rsc->list);
> prsc->screen = pscreen;
>
> rsc->bo = etna_screen_bo_from_handle(pscreen, handle, &level->stride);
> @@ -547,6 +555,11 @@ etna_resource_from_handle(struct pipe_screen *pscreen,
> goto fail;
> }
>
> + rsc->pending_ctx = _mesa_set_create(NULL, _mesa_hash_pointer,
> > + _mesa_key_pointer_equal);
> + if (!rsc->pending_ctx)
> + goto fail;
> +
> if (rsc->layout == ETNA_LAYOUT_LINEAR) {
> /*
> * Both sampler and pixel pipes can't handle linear, create a compatible
> @@ -621,20 +634,39 @@ void
> etna_resource_used(struct etna_context *ctx, struct pipe_resource *prsc,
> enum etna_resource_status status)
> {
> + struct etna_screen *screen = ctx->screen;
> + struct set_entry *entry;
> struct etna_resource *rsc;
>
> if (!prsc)
> return;
>
> rsc = etna_resource(prsc);
> +
> + mtx_lock(&screen->lock);
> +
> + /*
> + * if we are pending read or write by any other context or
> + * if reading a resource pending a write, then
> + * flush everything all the contexts to maintain coherency
> + */
> + if (((status & ETNA_PENDING_WRITE) && rsc->status) ||
> + ((status & ETNA_PENDING_READ) && (rsc->status & ETNA_PENDING_WRITE))) {
> + set_foreach(rsc->pending_ctx, entry) {
> + struct etna_context *extctx = (struct etna_context *)entry->key;
> + struct pipe_context *pctx = &extctx->base;
> +
> + pctx->flush(pctx, NULL, 0);
> + }
> + rsc->status = 0;
> + }
> +
> rsc->status |= status;
>
> - /* TODO resources can actually be shared across contexts,
> - * so I'm not sure a single list-head will do the trick? */
> - debug_assert((rsc->pending_ctx == ctx) || !rsc->pending_ctx);
> - list_delinit(&rsc->list);
> - list_addtail(&rsc->list, &ctx->used_resources);
> - rsc->pending_ctx = ctx;
> + _mesa_set_add(screen->used_resources, rsc);
> + _mesa_set_add(rsc->pending_ctx, ctx);
> +
> + mtx_unlock(&screen->lock);
> }
>
> bool
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> index 11ccf8f..3827aff 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> @@ -31,7 +31,10 @@
> #include "etnaviv_tiling.h"
> #include "pipe/p_state.h"
> #include "util/list.h"
> +#include "util/set.h"
> +#include "util/u_helpers.h"
>
> +struct etna_context;
> struct pipe_screen;
>
> struct etna_resource_level {
> @@ -83,10 +86,7 @@ struct etna_resource {
>
> enum etna_resource_status status;
>
> - /* resources accessed by queued but not flushed draws are tracked
> - * in the used_resources list. */
> - struct list_head list;
> - struct etna_context *pending_ctx;
> + struct set *pending_ctx;
> };
>
> /* returns TRUE if a is newer than b */
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index fb51aa5..3e93701 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -38,6 +38,7 @@
> #include "etnaviv_resource.h"
> #include "etnaviv_translate.h"
>
> +#include "util/hash_table.h"
> #include "util/os_time.h"
> #include "util/u_math.h"
> #include "util/u_memory.h"
> @@ -82,6 +83,9 @@ etna_screen_destroy(struct pipe_screen *pscreen)
> {
> struct etna_screen *screen = etna_screen(pscreen);
>
> + _mesa_set_destroy(screen->used_resources, NULL);
> + mtx_destroy(&screen->lock);
> +
> if (screen->perfmon)
> etna_perfmon_del(screen->perfmon);
>
> @@ -1026,8 +1030,16 @@ etna_screen_create(struct etna_device *dev, struct etna_gpu *gpu,
> if (screen->drm_version >= ETNA_DRM_VERSION_PERFMON)
> etna_pm_query_setup(screen);
>
> + mtx_init(&screen->lock, mtx_recursive);
> + screen->used_resources = _mesa_set_create(NULL, _mesa_hash_pointer,
> + _mesa_key_pointer_equal);
> + if (!screen->used_resources)
> + goto fail2;
> +
> return pscreen;
>
> +fail2:
> + mtx_destroy(&screen->lock);
> fail:
> etna_screen_destroy(pscreen);
> return NULL;
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.h b/src/gallium/drivers/etnaviv/etnaviv_screen.h
> index bffd4b6..9757985 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.h
> @@ -34,8 +34,10 @@
> #include "os/os_thread.h"
> #include "pipe/p_screen.h"
> #include "renderonly/renderonly.h"
> +#include "util/set.h"
> #include "util/slab.h"
> #include "util/u_dynarray.h"
> +#include "util/u_helpers.h"
>
> struct etna_bo;
>
> @@ -80,6 +82,10 @@ struct etna_screen {
> struct etna_specs specs;
>
> uint32_t drm_version;
> +
> + /* set of resources used by currently-unsubmitted renders */
> + mtx_t lock;
> + struct set *used_resources;
> };
>
> static inline struct etna_screen *
More information about the etnaviv
mailing list