From 92a5ea13fcd79fecbdb92d8f4c6ae64f6004c7fb Mon Sep 17 00:00:00 2001 From: Mike Blumenkrantz Date: Tue, 9 Feb 2021 16:22:33 -0500 Subject: [PATCH] zink: implement a global framebuffer cache this uses the same mechanics as surface caching, but it also requires that surfaces keep refs of the framebuffers they're attached to so that they can invalidate the fb object upon destruction, as, similar to program objects, the fb objects are "owned" by their attachments loosely based on patches by Antonio Caggiano Reviewed-by: Dave Airlie Part-of: --- src/gallium/drivers/zink/zink_context.c | 61 ++++++++++++++++++--- src/gallium/drivers/zink/zink_framebuffer.c | 18 +++--- src/gallium/drivers/zink/zink_screen.c | 27 ++++++++- src/gallium/drivers/zink/zink_screen.h | 2 + src/gallium/drivers/zink/zink_surface.c | 36 ++++++++++++ src/gallium/drivers/zink/zink_surface.h | 1 + 6 files changed, 128 insertions(+), 17 deletions(-) diff --git a/src/gallium/drivers/zink/zink_context.c b/src/gallium/drivers/zink/zink_context.c index 0266215dc94..fc763a37fd9 100644 --- a/src/gallium/drivers/zink/zink_context.c +++ b/src/gallium/drivers/zink/zink_context.c @@ -294,7 +294,6 @@ zink_context_destroy(struct pipe_context *pctx) util_blitter_destroy(ctx->blitter); util_copy_framebuffer_state(&ctx->fb_state, NULL); - zink_framebuffer_reference(screen, &ctx->framebuffer, NULL); pipe_resource_reference(&ctx->dummy_vertex_buffer, NULL); pipe_resource_reference(&ctx->dummy_xfb_buffer, NULL); @@ -317,6 +316,14 @@ zink_context_destroy(struct pipe_context *pctx) } } + if (ctx->framebuffer) { + simple_mtx_lock(&screen->framebuffer_mtx); + struct hash_entry *entry = _mesa_hash_table_search(&screen->framebuffer_cache, &ctx->framebuffer->state); + if (zink_framebuffer_reference(screen, &ctx->framebuffer, NULL)) + _mesa_hash_table_remove(&screen->framebuffer_cache, entry); + simple_mtx_unlock(&screen->framebuffer_mtx); + } + hash_table_foreach(ctx->render_pass_cache, he) zink_destroy_render_pass(screen, he->data); @@ -1133,7 +1140,7 @@ get_render_pass(struct zink_context *ctx) } static struct zink_framebuffer * -create_framebuffer(struct zink_context *ctx) +get_framebuffer(struct zink_context *ctx) { struct zink_screen *screen = zink_screen(ctx->base.screen); struct pipe_surface *attachments[PIPE_MAX_COLOR_BUFS + 1] = {}; @@ -1157,8 +1164,24 @@ create_framebuffer(struct zink_context *ctx) state.layers = MAX2(util_framebuffer_get_num_layers(&ctx->fb_state), 1); state.samples = ctx->fb_state.samples; - struct zink_framebuffer *fb = zink_create_framebuffer(ctx, &state, attachments); - zink_init_framebuffer(screen, fb, get_render_pass(ctx)); + struct zink_framebuffer *fb; + simple_mtx_lock(&screen->framebuffer_mtx); + struct hash_entry *entry = _mesa_hash_table_search(&screen->framebuffer_cache, &state); + if (entry) { + fb = (void*)entry->data; + struct zink_framebuffer *fb_ref = NULL; + /* this gains 1 ref every time we reuse it */ + zink_framebuffer_reference(screen, &fb_ref, fb); + } else { + /* this adds 1 extra ref on creation because all newly-created framebuffers are + * going to be bound; necessary to handle framebuffers which have no "real" attachments + * and are only using null surfaces since the only ref they get is the extra one here + */ + fb = zink_create_framebuffer(ctx, &state, attachments); + _mesa_hash_table_insert(&screen->framebuffer_cache, &fb->state, fb); + } + simple_mtx_unlock(&screen->framebuffer_mtx); + return fb; } @@ -1184,12 +1207,11 @@ static void setup_framebuffer(struct zink_context *ctx) { struct zink_screen *screen = zink_screen(ctx->base.screen); - struct zink_framebuffer *fb = create_framebuffer(ctx); + zink_init_framebuffer(screen, ctx->framebuffer, get_render_pass(ctx)); - zink_framebuffer_reference(screen, &ctx->framebuffer, fb); - if (fb->rp != ctx->gfx_pipeline_state.render_pass) + if (ctx->framebuffer->rp != ctx->gfx_pipeline_state.render_pass) ctx->gfx_pipeline_state.dirty = true; - ctx->gfx_pipeline_state.render_pass = fb->rp; + ctx->gfx_pipeline_state.render_pass = ctx->framebuffer->rp; } void @@ -1354,6 +1376,29 @@ zink_set_framebuffer_state(struct pipe_context *pctx, } util_copy_framebuffer_state(&ctx->fb_state, state); + /* get_framebuffer adds a ref if the fb is reused or created; + * always do get_framebuffer first to avoid deleting the same fb + * we're about to use + */ + struct zink_framebuffer *fb = get_framebuffer(ctx); + if (ctx->framebuffer) { + struct zink_screen *screen = zink_screen(pctx->screen); + simple_mtx_lock(&screen->framebuffer_mtx); + struct hash_entry *he = _mesa_hash_table_search(&screen->framebuffer_cache, &ctx->framebuffer->state); + if (ctx->framebuffer && !ctx->framebuffer->state.num_attachments) { + /* if this has no attachments then its lifetime has ended */ + _mesa_hash_table_remove(&screen->framebuffer_cache, he); + he = NULL; + } + /* a framebuffer loses 1 ref every time we unset it; + * we do NOT add refs here, as the ref has already been added in + * get_framebuffer() + */ + if (zink_framebuffer_reference(screen, &ctx->framebuffer, NULL) && he) + _mesa_hash_table_remove(&screen->framebuffer_cache, he); + simple_mtx_unlock(&screen->framebuffer_mtx); + } + ctx->framebuffer = fb; uint8_t rast_samples = util_framebuffer_get_num_samples(state); /* in vulkan, gl_SampleMask needs to be explicitly ignored for sampleCount == 1 */ diff --git a/src/gallium/drivers/zink/zink_framebuffer.c b/src/gallium/drivers/zink/zink_framebuffer.c index b24ea76f7ec..3ddd2f0bcbf 100644 --- a/src/gallium/drivers/zink/zink_framebuffer.c +++ b/src/gallium/drivers/zink/zink_framebuffer.c @@ -71,8 +71,6 @@ zink_destroy_framebuffer(struct zink_screen *screen, vkDestroyFramebuffer(screen->dev, *ptr, NULL); #endif } - for (int i = 0; i < ARRAY_SIZE(fb->surfaces); ++i) - pipe_surface_reference(fb->surfaces + i, NULL); zink_surface_reference(screen, (struct zink_surface**)&fb->null_surface, NULL); @@ -137,17 +135,23 @@ zink_create_framebuffer(struct zink_context *ctx, if (!fb) return NULL; - pipe_reference_init(&fb->reference, 1); - + unsigned num_attachments = 0; for (int i = 0; i < state->num_attachments; i++) { - if (state->attachments[i]) - pipe_surface_reference(&fb->surfaces[i], attachments[i]); - else { + struct zink_surface *surf; + if (state->attachments[i]) { + surf = zink_surface(attachments[i]); + /* no ref! */ + fb->surfaces[i] = attachments[i]; + num_attachments++; + } else { if (!fb->null_surface) fb->null_surface = framebuffer_null_surface_init(ctx, state); + surf = zink_surface(fb->null_surface); state->attachments[i] = zink_surface(fb->null_surface)->image_view; } + util_dynarray_append(&surf->framebuffer_refs, struct zink_framebuffer*, fb); } + pipe_reference_init(&fb->reference, 1 + num_attachments); if (!_mesa_hash_table_init(&fb->objects, fb, _mesa_hash_pointer, _mesa_key_pointer_equal)) goto fail; diff --git a/src/gallium/drivers/zink/zink_screen.c b/src/gallium/drivers/zink/zink_screen.c index f012c3ee79f..6370501c7dc 100644 --- a/src/gallium/drivers/zink/zink_screen.c +++ b/src/gallium/drivers/zink/zink_screen.c @@ -28,6 +28,7 @@ #include "zink_device_info.h" #include "zink_fence.h" #include "zink_format.h" +#include "zink_framebuffer.h" #include "zink_instance.h" #include "zink_public.h" #include "zink_resource.h" @@ -95,6 +96,20 @@ equals_bvci(const void *a, const void *b) return memcmp(a, b, sizeof(VkBufferViewCreateInfo)) == 0; } +static uint32_t +hash_framebuffer_state(const void *key) +{ + struct zink_framebuffer_state* s = (struct zink_framebuffer_state*)key; + return _mesa_hash_data(key, offsetof(struct zink_framebuffer_state, attachments) + sizeof(s->attachments[0]) * s->num_attachments); +} + +static bool +equals_framebuffer_state(const void *a, const void *b) +{ + struct zink_framebuffer_state *s = (struct zink_framebuffer_state*)a; + return memcmp(a, b, offsetof(struct zink_framebuffer_state, attachments) + sizeof(s->attachments[0]) * s->num_attachments) == 0; +} + static VkDeviceSize get_video_mem(struct zink_screen *screen) { @@ -864,8 +879,8 @@ zink_destroy_screen(struct pipe_screen *pscreen) hash_table_foreach(&screen->surface_cache, entry) { struct pipe_surface *psurf = (struct pipe_surface*)entry->data; - pipe_resource_reference(&psurf->texture, NULL); - pipe_surface_reference(&psurf, NULL); + /* context is already destroyed, so this has to be destroyed directly */ + zink_destroy_surface(screen, psurf); } hash_table_foreach(&screen->bufferview_cache, entry) { @@ -873,8 +888,14 @@ zink_destroy_screen(struct pipe_screen *pscreen) zink_buffer_view_reference(screen, &bv, NULL); } + hash_table_foreach(&screen->framebuffer_cache, entry) { + struct zink_framebuffer* fb = (struct zink_framebuffer*)entry->data; + zink_destroy_framebuffer(screen, fb); + } + simple_mtx_destroy(&screen->surface_mtx); simple_mtx_destroy(&screen->bufferview_mtx); + simple_mtx_destroy(&screen->framebuffer_mtx); u_transfer_helper_destroy(pscreen->transfer_helper); zink_screen_update_pipeline_cache(screen); @@ -1404,7 +1425,9 @@ zink_internal_create_screen(const struct pipe_screen_config *config) simple_mtx_init(&screen->surface_mtx, mtx_plain); simple_mtx_init(&screen->bufferview_mtx, mtx_plain); + simple_mtx_init(&screen->framebuffer_mtx, mtx_plain); + _mesa_hash_table_init(&screen->framebuffer_cache, screen, hash_framebuffer_state, equals_framebuffer_state); _mesa_hash_table_init(&screen->surface_cache, screen, NULL, equals_ivci); _mesa_hash_table_init(&screen->bufferview_cache, screen, NULL, equals_bvci); diff --git a/src/gallium/drivers/zink/zink_screen.h b/src/gallium/drivers/zink/zink_screen.h index 86da45db501..10e83040491 100644 --- a/src/gallium/drivers/zink/zink_screen.h +++ b/src/gallium/drivers/zink/zink_screen.h @@ -54,6 +54,8 @@ struct zink_screen { struct sw_winsys *winsys; + struct hash_table framebuffer_cache; + simple_mtx_t framebuffer_mtx; struct hash_table surface_cache; simple_mtx_t surface_mtx; struct hash_table bufferview_cache; diff --git a/src/gallium/drivers/zink/zink_surface.c b/src/gallium/drivers/zink/zink_surface.c index 2f11bd05e62..2f57285e221 100644 --- a/src/gallium/drivers/zink/zink_surface.c +++ b/src/gallium/drivers/zink/zink_surface.c @@ -22,6 +22,7 @@ */ #include "zink_context.h" +#include "zink_framebuffer.h" #include "zink_resource.h" #include "zink_screen.h" #include "zink_surface.h" @@ -115,6 +116,7 @@ create_surface(struct pipe_context *pctx, surface->base.u.tex.level = level; surface->base.u.tex.first_layer = templ->u.tex.first_layer; surface->base.u.tex.last_layer = templ->u.tex.last_layer; + util_dynarray_init(&surface->framebuffer_refs, NULL); if (vkCreateImageView(screen->dev, ivci, NULL, &surface->image_view) != VK_SUCCESS) { @@ -177,6 +179,38 @@ zink_create_surface(struct pipe_context *pctx, return zink_get_surface(zink_context(pctx), pres, templ, &ivci); } +/* framebuffers are owned by their surfaces, so each time a surface that's part of a cached fb + * is destroyed, it has to unref all the framebuffers it's attached to in order to avoid leaking + * all the framebuffers + * + * surfaces are always batch-tracked, so it is impossible for a framebuffer to be destroyed + * while it is in use + */ +static void +surface_clear_fb_refs(struct zink_screen *screen, struct pipe_surface *psurface) +{ + struct zink_surface *surface = zink_surface(psurface); + util_dynarray_foreach(&surface->framebuffer_refs, struct zink_framebuffer*, fb_ref) { + struct zink_framebuffer *fb = *fb_ref; + for (unsigned i = 0; i < fb->state.num_attachments; i++) { + if (fb->surfaces[i] == psurface) { + simple_mtx_lock(&screen->framebuffer_mtx); + fb->surfaces[i] = NULL; + _mesa_hash_table_remove_key(&screen->framebuffer_cache, &fb->state); + zink_framebuffer_reference(screen, &fb, NULL); + simple_mtx_unlock(&screen->framebuffer_mtx); + break; + } + /* null surface doesn't get a ref but it will double-free + * if the pointer isn't unset + */ + if (fb->null_surface == psurface) + fb->null_surface = NULL; + } + } + util_dynarray_fini(&surface->framebuffer_refs); +} + void zink_destroy_surface(struct zink_screen *screen, struct pipe_surface *psurface) { @@ -186,6 +220,8 @@ zink_destroy_surface(struct zink_screen *screen, struct pipe_surface *psurface) assert(he); _mesa_hash_table_remove(&screen->surface_cache, he); simple_mtx_unlock(&screen->surface_mtx); + surface_clear_fb_refs(screen, psurface); + util_dynarray_fini(&surface->framebuffer_refs); pipe_resource_reference(&psurface->texture, NULL); vkDestroyImageView(screen->dev, surface->image_view, NULL); FREE(surface); diff --git a/src/gallium/drivers/zink/zink_surface.h b/src/gallium/drivers/zink/zink_surface.h index 7a471f9854a..6a88a10350f 100644 --- a/src/gallium/drivers/zink/zink_surface.h +++ b/src/gallium/drivers/zink/zink_surface.h @@ -36,6 +36,7 @@ struct zink_surface { VkImageView image_view; uint32_t hash; struct zink_batch_usage batch_uses; + struct util_dynarray framebuffer_refs; }; static inline struct zink_surface *