From ca14c2d5bf2b360cecaf7fea4b5d61a0756de802 Mon Sep 17 00:00:00 2001 From: Patrick Lerda Date: Thu, 6 Jul 2023 13:43:27 +0200 Subject: [PATCH] panfrost: fix refcnt imbalance related to blitter This issue is mainly a consequence of a call to util_blitter_clear() with unnecessary blitter states, these states are never freed. This change is inspired from radeonsi and r600. Note: PAN_SAVE_FRAGMENT_STATE is added and always enabled at this stage. For instance, this issue is triggered on Mali-T720 with "piglit/bin/fcc-read-after-clear sample tex -auto -fbo", "piglit/bin/cubemap -auto" and "piglit/bin/fbo-srgb -auto" or on Mali-T820 with "piglit/bin/longprim -auto -fbo" and "piglit/bin/ext_render_snorm-render -auto -fbo" while setting GALLIUM_REFCNT_LOG=refcnt.log. cc: mesa-stable Signed-off-by: Patrick Lerda Part-of: (cherry picked from commit 689f38b2b452a8b8bad5ab9388c7f71ff9074cec) --- .pick_status.json | 2 +- src/gallium/drivers/panfrost/pan_blit.c | 51 +++++++++++++-------- src/gallium/drivers/panfrost/pan_context.c | 2 +- src/gallium/drivers/panfrost/pan_resource.c | 6 ++- src/gallium/drivers/panfrost/pan_resource.h | 23 +++++++++- 5 files changed, 60 insertions(+), 24 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index e60e8dab072..412afe037ea 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -2299,7 +2299,7 @@ "description": "panfrost: fix refcnt imbalance related to blitter", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/gallium/drivers/panfrost/pan_blit.c b/src/gallium/drivers/panfrost/pan_blit.c index 190bab39574..d5205ec636d 100644 --- a/src/gallium/drivers/panfrost/pan_blit.c +++ b/src/gallium/drivers/panfrost/pan_blit.c @@ -32,7 +32,8 @@ #include "pan_util.h" void -panfrost_blitter_save(struct panfrost_context *ctx, bool render_cond) +panfrost_blitter_save(struct panfrost_context *ctx, + const enum panfrost_blitter_op blitter_op) { struct blitter_context *blitter = ctx->blitter; @@ -42,26 +43,36 @@ panfrost_blitter_save(struct panfrost_context *ctx, bool render_cond) ctx->uncompiled[PIPE_SHADER_VERTEX]); util_blitter_save_rasterizer(blitter, ctx->rasterizer); util_blitter_save_viewport(blitter, &ctx->pipe_viewport); - util_blitter_save_scissor(blitter, &ctx->scissor); - util_blitter_save_fragment_shader(blitter, - ctx->uncompiled[PIPE_SHADER_FRAGMENT]); - util_blitter_save_blend(blitter, ctx->blend); - util_blitter_save_depth_stencil_alpha(blitter, ctx->depth_stencil); - util_blitter_save_stencil_ref(blitter, &ctx->stencil_ref); util_blitter_save_so_targets(blitter, 0, NULL); - util_blitter_save_sample_mask(blitter, ctx->sample_mask, ctx->min_samples); - util_blitter_save_framebuffer(blitter, &ctx->pipe_framebuffer); - util_blitter_save_fragment_sampler_states( - blitter, ctx->sampler_count[PIPE_SHADER_FRAGMENT], - (void **)(&ctx->samplers[PIPE_SHADER_FRAGMENT])); - util_blitter_save_fragment_sampler_views( - blitter, ctx->sampler_view_count[PIPE_SHADER_FRAGMENT], - (struct pipe_sampler_view **)&ctx->sampler_views[PIPE_SHADER_FRAGMENT]); - util_blitter_save_fragment_constant_buffer_slot( - blitter, ctx->constant_buffer[PIPE_SHADER_FRAGMENT].cb); + if (blitter_op & PAN_SAVE_FRAGMENT_STATE) { + if (blitter_op & PAN_SAVE_FRAGMENT_CONSTANT) + util_blitter_save_fragment_constant_buffer_slot( + blitter, ctx->constant_buffer[PIPE_SHADER_FRAGMENT].cb); - if (!render_cond) { + util_blitter_save_blend(blitter, ctx->blend); + util_blitter_save_depth_stencil_alpha(blitter, ctx->depth_stencil); + util_blitter_save_stencil_ref(blitter, &ctx->stencil_ref); + util_blitter_save_fragment_shader(blitter, + ctx->uncompiled[PIPE_SHADER_FRAGMENT]); + util_blitter_save_sample_mask(blitter, ctx->sample_mask, + ctx->min_samples); + util_blitter_save_scissor(blitter, &ctx->scissor); + } + + if (blitter_op & PAN_SAVE_FRAMEBUFFER) + util_blitter_save_framebuffer(blitter, &ctx->pipe_framebuffer); + + if (blitter_op & PAN_SAVE_TEXTURES) { + util_blitter_save_fragment_sampler_states( + blitter, ctx->sampler_count[PIPE_SHADER_FRAGMENT], + (void **)(&ctx->samplers[PIPE_SHADER_FRAGMENT])); + util_blitter_save_fragment_sampler_views( + blitter, ctx->sampler_view_count[PIPE_SHADER_FRAGMENT], + (struct pipe_sampler_view **)&ctx->sampler_views[PIPE_SHADER_FRAGMENT]); + } + + if (!(blitter_op & PAN_DISABLE_RENDER_COND)) { util_blitter_save_render_condition(blitter, (struct pipe_query *)ctx->cond_query, ctx->cond_cond, ctx->cond_mode); @@ -79,6 +90,8 @@ panfrost_blit(struct pipe_context *pipe, const struct pipe_blit_info *info) if (!util_blitter_is_blit_supported(ctx->blitter, info)) unreachable("Unsupported blit\n"); - panfrost_blitter_save(ctx, info->render_condition_enable); + panfrost_blitter_save(ctx, info->render_condition_enable + ? PAN_RENDER_BLIT_COND + : PAN_RENDER_BLIT); util_blitter_blit(ctx->blitter, info); } diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index 11dbd1b4c4c..c3df6ee3626 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -76,7 +76,7 @@ panfrost_clear(struct pipe_context *pipe, unsigned buffers, } /* Once there is content, clear with a fullscreen quad */ - panfrost_blitter_save(ctx, false /* render condition */); + panfrost_blitter_save(ctx, PAN_RENDER_CLEAR); perf_debug_ctx(ctx, "Clearing with quad"); util_blitter_clear( diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 727a509aa76..e32d67ddb69 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -63,7 +63,8 @@ panfrost_clear_depth_stencil(struct pipe_context *pipe, if (render_condition_enabled && !panfrost_render_condition_check(ctx)) return; - panfrost_blitter_save(ctx, render_condition_enabled); + panfrost_blitter_save( + ctx, render_condition_enabled ? PAN_RENDER_COND : PAN_RENDER_BASE); util_blitter_clear_depth_stencil(ctx->blitter, dst, clear_flags, depth, stencil, dstx, dsty, width, height); } @@ -80,7 +81,8 @@ panfrost_clear_render_target(struct pipe_context *pipe, if (render_condition_enabled && !panfrost_render_condition_check(ctx)) return; - panfrost_blitter_save(ctx, render_condition_enabled); + panfrost_blitter_save( + ctx, render_condition_enabled ? PAN_RENDER_COND : PAN_RENDER_BASE); util_blitter_clear_render_target(ctx->blitter, dst, color, dstx, dsty, width, height); } diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h index c3d76d75bf3..8a0fd0bcd3d 100644 --- a/src/gallium/drivers/panfrost/pan_resource.h +++ b/src/gallium/drivers/panfrost/pan_resource.h @@ -112,7 +112,28 @@ void panfrost_resource_context_init(struct pipe_context *pctx); /* Blitting */ -void panfrost_blitter_save(struct panfrost_context *ctx, bool render_cond); +enum panfrost_blitter_op /* bitmask */ +{ + PAN_SAVE_TEXTURES = 1, + PAN_SAVE_FRAMEBUFFER = 2, + PAN_SAVE_FRAGMENT_STATE = 4, + PAN_SAVE_FRAGMENT_CONSTANT = 8, + PAN_DISABLE_RENDER_COND = 16, +}; + +enum { + PAN_RENDER_BLIT = + PAN_SAVE_TEXTURES | PAN_SAVE_FRAMEBUFFER | PAN_SAVE_FRAGMENT_STATE, + PAN_RENDER_BLIT_COND = PAN_SAVE_TEXTURES | PAN_SAVE_FRAMEBUFFER | + PAN_SAVE_FRAGMENT_STATE | PAN_DISABLE_RENDER_COND, + PAN_RENDER_BASE = PAN_SAVE_FRAMEBUFFER | PAN_SAVE_FRAGMENT_STATE, + PAN_RENDER_COND = + PAN_SAVE_FRAMEBUFFER | PAN_SAVE_FRAGMENT_STATE | PAN_DISABLE_RENDER_COND, + PAN_RENDER_CLEAR = PAN_SAVE_FRAGMENT_STATE | PAN_SAVE_FRAGMENT_CONSTANT, +}; + +void panfrost_blitter_save(struct panfrost_context *ctx, + const enum panfrost_blitter_op blitter_op); void panfrost_blit(struct pipe_context *pipe, const struct pipe_blit_info *info);