From: Emma Anholt Date: Thu, 17 Jun 2021 16:50:15 +0000 (-0700) Subject: freedreno: Handle full blit discards by invalidating the resource. X-Git-Tag: upstream/21.2.3~1581 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=58f5605124a74eeac6a6156b093c9f098bb99c78;p=platform%2Fupstream%2Fmesa.git freedreno: Handle full blit discards by invalidating the resource. The previous implementation had several issues: - It wasn't checking all the conditions necessary for "this blit updates the whole surface", like PIPE_MASK_Z but not S on a depth/stencil buffer. - It would reset the previous batchbuffer, even if that batch had side effects on other buffers. - The layering was painful to follow and made any recursion extra dangerous. Now, we use a more conservative test (enough for the resource shadowing case) and just invalidate the buffer up front, which should have the right logic for discarding drawing to that resource. I found I had to add fd_bc_flush_writer() to the end of fd_blitter_blit() -- a flush was happening at fb state restore time when the discard flag was set, and losing that flush breaks dEQP-GLES31.functional.stencil_texturing.format.stencil_index8_cube. Part-of: --- diff --git a/src/freedreno/ci/deqp-freedreno-a530-fails.txt b/src/freedreno/ci/deqp-freedreno-a530-fails.txt index 0c6ca6e..d901ff7 100644 --- a/src/freedreno/ci/deqp-freedreno-a530-fails.txt +++ b/src/freedreno/ci/deqp-freedreno-a530-fails.txt @@ -3,10 +3,6 @@ dEQP-GLES2.functional.clipping.line.wide_line_clip_viewport_corner,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_center,Fail dEQP-GLES2.functional.clipping.point.wide_point_clip_viewport_corner,Fail -dEQP-GLES2.functional.texture.specification.basic_copytexsubimage2d.2d_alpha,Fail -dEQP-GLES2.functional.texture.specification.basic_copytexsubimage2d.2d_luminance,Fail -dEQP-GLES2.functional.texture.specification.basic_copytexsubimage2d.2d_rgba,Fail -dEQP-GLES2.functional.texture.specification.basic_copytexsubimage2d.2d_rgb,Fail dEQP-GLES31.functional.image_load_store.early_fragment_tests.early_fragment_tests_stencil,Crash dEQP-GLES31.functional.image_load_store.early_fragment_tests.early_fragment_tests_stencil_fbo,Crash dEQP-GLES31.functional.image_load_store.early_fragment_tests.early_fragment_tests_stencil_fbo_with_no_stencil,Crash @@ -141,8 +137,6 @@ dEQP-GLES3.functional.shaders.texture_functions.textureprojgradoffset.sampler2ds dEQP-GLES3.functional.shaders.texture_functions.textureprojgradoffset.sampler3d_fixed_vertex,Fail dEQP-GLES3.functional.shaders.texture_functions.textureprojgradoffset.sampler3d_float_vertex,Fail dEQP-GLES3.functional.shaders.texture_functions.textureprojgrad.sampler2dshadow_vertex,Fail -dEQP-GLES3.functional.texture.specification.basic_copytexsubimage2d.2d_alpha,Fail -dEQP-GLES3.functional.texture.specification.basic_copytexsubimage2d.2d_rgba,Fail dEQP-GLES3.functional.transform_feedback.array_element.interleaved.lines.highp_float,Fail dEQP-GLES3.functional.transform_feedback.array_element.interleaved.lines.highp_ivec2,Fail dEQP-GLES3.functional.transform_feedback.array_element.interleaved.lines.highp_ivec4,Fail diff --git a/src/gallium/drivers/freedreno/freedreno_autotune.c b/src/gallium/drivers/freedreno/freedreno_autotune.c index 9f25307..d4ab28b 100644 --- a/src/gallium/drivers/freedreno/freedreno_autotune.c +++ b/src/gallium/drivers/freedreno/freedreno_autotune.c @@ -157,7 +157,7 @@ fallback_use_bypass(struct fd_batch *batch) /* Fallback logic if we have no historical data about the rendertarget: */ if (batch->cleared || batch->gmem_reason || - ((batch->num_draws > 5) && !batch->blit) || (pfb->samples > 1)) { + (batch->num_draws > 5) || (pfb->samples > 1)) { return false; } diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h b/src/gallium/drivers/freedreno/freedreno_batch.h index 8f69a9d..842537f 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.h +++ b/src/gallium/drivers/freedreno/freedreno_batch.h @@ -94,8 +94,6 @@ struct fd_batch { bool nondraw : 1; bool needs_flush : 1; bool flushed : 1; - bool blit : 1; - bool back_blit : 1; /* only blit so far is resource shadowing back-blit */ bool tessellation : 1; /* tessellation used in batch */ /* Keep track if WAIT_FOR_IDLE is needed for registers we need diff --git a/src/gallium/drivers/freedreno/freedreno_blitter.c b/src/gallium/drivers/freedreno/freedreno_blitter.c index 15bf9e3..8a4b0a1 100644 --- a/src/gallium/drivers/freedreno/freedreno_blitter.c +++ b/src/gallium/drivers/freedreno/freedreno_blitter.c @@ -77,8 +77,7 @@ default_src_texture(struct pipe_sampler_view *src_templ, } static void -fd_blitter_pipe_begin(struct fd_context *ctx, bool render_cond, - bool discard) assert_dt +fd_blitter_pipe_begin(struct fd_context *ctx, bool render_cond) assert_dt { util_blitter_save_vertex_buffer_slot(ctx->blitter, ctx->vtx.vertexbuf.vb); util_blitter_save_vertex_elements(ctx->blitter, ctx->vtx.vtx); @@ -109,25 +108,29 @@ fd_blitter_pipe_begin(struct fd_context *ctx, bool render_cond, if (ctx->batch) fd_batch_update_queries(ctx->batch); - - ctx->in_discard_blit = discard; } static void fd_blitter_pipe_end(struct fd_context *ctx) assert_dt { - ctx->in_discard_blit = false; } bool fd_blitter_blit(struct fd_context *ctx, const struct pipe_blit_info *info) { + struct pipe_context *pctx = &ctx->base; struct pipe_resource *dst = info->dst.resource; struct pipe_resource *src = info->src.resource; struct pipe_context *pipe = &ctx->base; struct pipe_surface *dst_view, dst_templ; struct pipe_sampler_view src_templ, *src_view; - bool discard = false; + + /* If the blit is updating the whole contents of the resource, + * invalidate it so we don't trigger any unnecessary tile loads in the 3D + * path. + */ + if (util_blit_covers_whole_resource(info)) + pctx->invalidate_resource(pctx, info->dst.resource); /* The blit format may not match the resource format in this path, so * we need to validate that we can use the src/dst resource with the @@ -145,16 +148,9 @@ fd_blitter_blit(struct fd_context *ctx, const struct pipe_blit_info *info) if (src == dst) pipe->flush(pipe, NULL, 0); - if (!info->scissor_enable && !info->alpha_blend) { - discard = util_texrange_covers_whole_level( - info->dst.resource, info->dst.level, info->dst.box.x, info->dst.box.y, - info->dst.box.z, info->dst.box.width, info->dst.box.height, - info->dst.box.depth); - } - DBG_BLIT(info, NULL); - fd_blitter_pipe_begin(ctx, info->render_condition_enable, discard); + fd_blitter_pipe_begin(ctx, info->render_condition_enable); /* Initialize the surface. */ default_dst_texture(&dst_templ, dst, info->dst.level, info->dst.box.z); @@ -177,6 +173,12 @@ fd_blitter_blit(struct fd_context *ctx, const struct pipe_blit_info *info) fd_blitter_pipe_end(ctx); + /* While this shouldn't technically be necessary, it is required for + * dEQP-GLES31.functional.stencil_texturing.format.stencil_index8_cube and + * 2d_array to pass. + */ + fd_bc_flush_writer(ctx, fd_resource(info->dst.resource)); + /* The fallback blitter must never fail: */ return true; } @@ -194,7 +196,7 @@ fd_blitter_clear(struct pipe_context *pctx, unsigned buffers, /* Note: don't use discard=true, if there was something to * discard, that would have been already handled in fd_clear(). */ - fd_blitter_pipe_begin(ctx, false, false); + fd_blitter_pipe_begin(ctx, false); util_blitter_save_fragment_constant_buffer_slot( ctx->blitter, ctx->constbuf[PIPE_SHADER_FRAGMENT].cb); @@ -330,8 +332,8 @@ fd_blitter_pipe_copy_region(struct fd_context *ctx, struct pipe_resource *dst, pctx->flush(pctx, NULL, 0); } - /* TODO we could discard if dst box covers dst level fully.. */ - fd_blitter_pipe_begin(ctx, false, false); + /* TODO we could invalidate if dst box covers dst level fully. */ + fd_blitter_pipe_begin(ctx, false); util_blitter_copy_texture(ctx->blitter, dst, dst_level, dstx, dsty, dstz, src, src_level, src_box); fd_blitter_pipe_end(ctx); diff --git a/src/gallium/drivers/freedreno/freedreno_context.h b/src/gallium/drivers/freedreno/freedreno_context.h index 5438da4..ac0fce4 100644 --- a/src/gallium/drivers/freedreno/freedreno_context.h +++ b/src/gallium/drivers/freedreno/freedreno_context.h @@ -344,12 +344,6 @@ struct fd_context { */ bool in_shadow : 1 dt; - /* Ie. in blit situation where we no longer care about previous framebuffer - * contents. Main point is to eliminate blits from fd_try_shadow_resource(). - * For example, in case of texture upload + gen-mipmaps. - */ - bool in_discard_blit : 1 dt; - /* For catching recursion problems with blit fallback: */ bool in_blit : 1 dt; diff --git a/src/gallium/drivers/freedreno/freedreno_draw.c b/src/gallium/drivers/freedreno/freedreno_draw.c index 9065b03..e22da55 100644 --- a/src/gallium/drivers/freedreno/freedreno_draw.c +++ b/src/gallium/drivers/freedreno/freedreno_draw.c @@ -325,11 +325,6 @@ fd_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info, struct fd_batch *batch = fd_context_batch(ctx); - if (ctx->in_discard_blit) { - fd_batch_reset(batch); - fd_context_all_dirty(ctx); - } - batch_draw_tracking(batch, info, indirect); while (unlikely(!fd_batch_lock_submit(batch))) { @@ -343,12 +338,8 @@ fd_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info, assert(ctx->batch == batch); } - batch->blit = ctx->in_discard_blit; - batch->back_blit = ctx->in_shadow; batch->num_draws++; - ctx->in_discard_blit = false; - /* Marking the batch as needing flush must come after the batch * dependency tracking (resource_read()/resource_write()), as that * can trigger a flush @@ -452,11 +443,6 @@ fd_clear(struct pipe_context *pctx, unsigned buffers, struct fd_batch *batch = fd_context_batch(ctx); - if (ctx->in_discard_blit) { - fd_batch_reset(batch); - fd_context_all_dirty(ctx); - } - batch_clear_tracking(batch, buffers); while (unlikely(!fd_batch_lock_submit(batch))) { diff --git a/src/gallium/drivers/freedreno/freedreno_state.c b/src/gallium/drivers/freedreno/freedreno_state.c index 92a4827..46e8a7c 100644 --- a/src/gallium/drivers/freedreno/freedreno_state.c +++ b/src/gallium/drivers/freedreno/freedreno_state.c @@ -287,15 +287,6 @@ fd_set_framebuffer_state(struct pipe_context *pctx, fd_context_all_dirty(ctx); ctx->update_active_queries = true; - if (old_batch && old_batch->blit && !old_batch->back_blit) { - /* for blits, there is not really much point in hanging on - * to the uncommitted batch (ie. you probably don't blit - * multiple times to the same surface), so we might as - * well go ahead and flush this one: - */ - fd_batch_flush(old_batch); - } - fd_batch_reference(&old_batch, NULL); } else if (ctx->batch) { DBG("%d: cbufs[0]=%p, zsbuf=%p", ctx->batch->needs_flush,