From 342ed4909fe7611525b9029977463ef3291ce7ba Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 5 Oct 2021 19:20:03 -0400 Subject: [PATCH 1/4] panfrost: Workaround ISSUE_TSIX_2033 According to mali_kbase, all Bifrost and Valhall GPUs are affected by issue TSIX_2033. This hardware bug breaks the INTERSECT frame shader mode when forcing clean_tile_writes. What does that mean? The hardware considers a tile "clean" if it has been cleared but not drawn to. Setting clean_tile_write forces the hardware to write back such "clean" tiles to main memory. Bifrost hardware supports frame shaders, which insert a rectangle into every tile according to a configured rule. Frame shaders are used in Panfrost to implement tile reloads (i.e. LOAD_OP_LOAD). Two modes are relevant to the current discussion: ALWAYS, which always inserts a frame shader, and INTERSECT, which tries to only insert where there is geometry. Normally, we use INTERSECT for tile reloads as it is more efficient than ALWAYS-- it allows us to skip reloads of tiles that are discarded and never written back to memory. From a software perspective, Panfrost's current logic is correct: if we clear, we set clean_tile_writes, else we use an INTERSECT frame shader. There is no software interaction between the two. Unfortunately, there is a hardware interaction. The hardware forces clean_tile_writes in certain circumstances when AFBC is used. Ordinarily, this is a hardware implementation detail and invisible to software. Unfortunately, this implicit clean tile write is enough to trigger the hardware bug when using INTERSECT. As such, we need to detect this case and use ALWAYS instead of INTERSECT for correct results. Signed-off-by: Alyssa Rosenzweig Cc: mesa-stable Part-of: --- src/panfrost/lib/pan_cs.c | 70 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/src/panfrost/lib/pan_cs.c b/src/panfrost/lib/pan_cs.c index c5ae54600257..b4375a19db69 100644 --- a/src/panfrost/lib/pan_cs.c +++ b/src/panfrost/lib/pan_cs.c @@ -551,6 +551,68 @@ pan_emit_rt(const struct pan_fb_info *fb, } } +#if PAN_ARCH >= 6 +/* All Bifrost and Valhall GPUs are affected by issue TSIX-2033: + * + * Forcing clean_tile_writes breaks INTERSECT readbacks + * + * To workaround, use the frame shader mode ALWAYS instead of INTERSECT if + * clean tile writes is forced. Since INTERSECT is a hint that the hardware may + * ignore, this cannot affect correctness, only performance */ + +static enum mali_pre_post_frame_shader_mode +pan_fix_frame_shader_mode(enum mali_pre_post_frame_shader_mode mode, bool force_clean_tile) +{ + if (force_clean_tile && mode == MALI_PRE_POST_FRAME_SHADER_MODE_INTERSECT) + return MALI_PRE_POST_FRAME_SHADER_MODE_ALWAYS; + else + return mode; +} + +/* Regardless of clean_tile_write_enable, the hardware writes clean tiles if + * the effective tile size differs from the superblock size of any enabled AFBC + * render target. Check this condition. */ + +static bool +pan_force_clean_write_rt(const struct pan_image_view *rt, unsigned tile_size) +{ + if (!drm_is_afbc(rt->image->layout.modifier)) + return false; + + unsigned superblock = panfrost_block_dim(rt->image->layout.modifier, true, 0); + + assert(superblock >= 16); + assert(tile_size <= 16*16); + + /* Tile size and superblock differ unless they are both 16x16 */ + return !(superblock == 16 && tile_size == 16*16); +} + +static bool +pan_force_clean_write(const struct pan_fb_info *fb, unsigned tile_size) +{ + /* Maximum tile size */ + assert(tile_size <= 16*16); + + for (unsigned i = 0; i < fb->rt_count; ++i) { + if (fb->rts[i].view && !fb->rts[i].discard && + pan_force_clean_write_rt(fb->rts[i].view, tile_size)) + return true; + } + + if (fb->zs.view.zs && !fb->zs.discard.z && + pan_force_clean_write_rt(fb->zs.view.zs, tile_size)) + return true; + + if (fb->zs.view.s && !fb->zs.discard.s && + pan_force_clean_write_rt(fb->zs.view.s, tile_size)) + return true; + + return false; +} + +#endif + static unsigned pan_emit_mfbd(const struct panfrost_device *dev, const struct pan_fb_info *fb, @@ -574,11 +636,13 @@ pan_emit_mfbd(const struct panfrost_device *dev, pan_section_pack(fbd, FRAMEBUFFER, PARAMETERS, cfg) { #if PAN_ARCH >= 6 + bool force_clean_write = pan_force_clean_write(fb, tile_size); + cfg.sample_locations = panfrost_sample_positions(dev, pan_sample_pattern(fb->nr_samples)); - cfg.pre_frame_0 = fb->bifrost.pre_post.modes[0]; - cfg.pre_frame_1 = fb->bifrost.pre_post.modes[1]; - cfg.post_frame = fb->bifrost.pre_post.modes[2]; + cfg.pre_frame_0 = pan_fix_frame_shader_mode(fb->bifrost.pre_post.modes[0], force_clean_write); + cfg.pre_frame_1 = pan_fix_frame_shader_mode(fb->bifrost.pre_post.modes[1], force_clean_write); + cfg.post_frame = pan_fix_frame_shader_mode(fb->bifrost.pre_post.modes[2], force_clean_write); cfg.frame_shader_dcds = fb->bifrost.pre_post.dcds.gpu; cfg.tiler = tiler_ctx->bifrost; #endif -- GitLab From 93c9123c313d688913ed86f77c6841738f94988a Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Sat, 16 Oct 2021 19:45:27 -0400 Subject: [PATCH 2/4] panfrost: Add internal afbc_formats We need to know the internal (physical) formats used for AFBC of a given logical format, in order to check format compatibility and determine if we need to decompress AFBC for conformance. Signed-off-by: Alyssa Rosenzweig Cc: mesa-stable Part-of: --- src/panfrost/lib/pan_afbc.c | 60 ++++++++++++++++++++++++++-------- src/panfrost/lib/pan_texture.h | 3 ++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/panfrost/lib/pan_afbc.c b/src/panfrost/lib/pan_afbc.c index 652819b50539..49949081bf36 100644 --- a/src/panfrost/lib/pan_afbc.c +++ b/src/panfrost/lib/pan_afbc.c @@ -70,37 +70,71 @@ #define AFBC_TILE_HEIGHT 16 #define AFBC_CACHE_ALIGN 64 -/* Is it possible to AFBC compress a particular format? Common formats (and - * YUV) are compressible. Some obscure formats are not and fallback on linear, - * at a performance hit. Also, if you need to disable AFBC entirely in the - * driver for debug/profiling, just always return false here. */ +/* AFBC supports compressing a few canonical formats. Additional formats are + * available by using a canonical internal format. Given a PIPE format, find + * the canonical AFBC internal format if it exists, or NONE if the format + * cannot be compressed. */ -bool -panfrost_format_supports_afbc(const struct panfrost_device *dev, enum pipe_format format) +enum pipe_format +panfrost_afbc_format(const struct panfrost_device *dev, enum pipe_format format) { + /* Don't allow swizzled formats on v7 */ switch (format) { + case PIPE_FORMAT_B8G8R8A8_UNORM: + case PIPE_FORMAT_B8G8R8X8_UNORM: + case PIPE_FORMAT_A8R8G8B8_UNORM: + case PIPE_FORMAT_X8R8G8B8_UNORM: + case PIPE_FORMAT_X8B8G8R8_UNORM: + case PIPE_FORMAT_A8B8G8R8_UNORM: + case PIPE_FORMAT_B8G8R8_UNORM: + case PIPE_FORMAT_B5G6R5_UNORM: + if (dev->arch >= 7) + return PIPE_FORMAT_NONE; + + break; + default: + break; + } + + switch (format) { + case PIPE_FORMAT_Z16_UNORM: + return PIPE_FORMAT_R8G8_UNORM; + + case PIPE_FORMAT_R8G8B8_UNORM: + case PIPE_FORMAT_B8G8R8_UNORM: + return PIPE_FORMAT_R8G8B8_UNORM; + case PIPE_FORMAT_R8G8B8A8_UNORM: case PIPE_FORMAT_R8G8B8X8_UNORM: - case PIPE_FORMAT_R8G8B8_UNORM: - case PIPE_FORMAT_R5G6B5_UNORM: case PIPE_FORMAT_Z24_UNORM_S8_UINT: case PIPE_FORMAT_Z24X8_UNORM: - case PIPE_FORMAT_Z16_UNORM: - return true; + case PIPE_FORMAT_X24S8_UINT: case PIPE_FORMAT_B8G8R8A8_UNORM: case PIPE_FORMAT_B8G8R8X8_UNORM: case PIPE_FORMAT_A8R8G8B8_UNORM: case PIPE_FORMAT_X8R8G8B8_UNORM: case PIPE_FORMAT_X8B8G8R8_UNORM: case PIPE_FORMAT_A8B8G8R8_UNORM: - case PIPE_FORMAT_B8G8R8_UNORM: + return PIPE_FORMAT_R8G8B8A8_UNORM; + + case PIPE_FORMAT_R5G6B5_UNORM: case PIPE_FORMAT_B5G6R5_UNORM: - return (dev->arch < 7); + return PIPE_FORMAT_R5G6B5_UNORM; + + /* TODO: More AFBC formats */ default: - return false; + return PIPE_FORMAT_NONE; } } +/* A format may be compressed as AFBC if it has an AFBC internal format */ + +bool +panfrost_format_supports_afbc(const struct panfrost_device *dev, enum pipe_format format) +{ + return panfrost_afbc_format(dev, format) != PIPE_FORMAT_NONE; +} + unsigned panfrost_afbc_header_size(unsigned width, unsigned height) { diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index 24ecf2e493f1..82571216e272 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -148,6 +148,9 @@ bool panfrost_format_supports_afbc(const struct panfrost_device *dev, enum pipe_format format); +enum pipe_format +panfrost_afbc_format(const struct panfrost_device *dev, enum pipe_format format); + #define AFBC_HEADER_BYTES_PER_TILE 16 unsigned -- GitLab From 789601a189eb8b0ff0b0f2a19ed276d9881877a7 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Sat, 16 Oct 2021 19:44:53 -0400 Subject: [PATCH 3/4] panfrost: Decompress for incompatible AFBC formats AFBC is keyed to the format. Depending on the hardware, we'll get an Invalid Data Fault or a GPU timeout if we attempt to sample from an AFBC-compressed RGBA8 texture as R32F (for example). Fixes Piglit ./bin/arb_texture_view-rendering-formats_gles3 with AFBC. Signed-off-by: Alyssa Rosenzweig Cc: mesa-stable Part-of: --- src/gallium/drivers/panfrost/pan_cmdstream.c | 3 +++ src/gallium/drivers/panfrost/pan_resource.c | 26 ++++++++++++++++++++ src/gallium/drivers/panfrost/pan_resource.h | 5 ++++ 3 files changed, 34 insertions(+) diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c index d295aa15a9d0..ae5e116b7a80 100644 --- a/src/gallium/drivers/panfrost/pan_cmdstream.c +++ b/src/gallium/drivers/panfrost/pan_cmdstream.c @@ -3374,8 +3374,11 @@ panfrost_create_sampler_view( struct pipe_resource *texture, const struct pipe_sampler_view *template) { + struct panfrost_context *ctx = pan_context(pctx); struct panfrost_sampler_view *so = rzalloc(pctx, struct panfrost_sampler_view); + pan_legalize_afbc_format(ctx, pan_resource(texture), template->format); + pipe_reference(NULL, &texture->reference); so->base = *template; diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 1e2453599b6a..18c43ea7e478 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -218,8 +218,11 @@ panfrost_create_surface(struct pipe_context *pipe, struct pipe_resource *pt, const struct pipe_surface *surf_tmpl) { + struct panfrost_context *ctx = pan_context(pipe); struct pipe_surface *ps = NULL; + pan_legalize_afbc_format(ctx, pan_resource(pt), surf_tmpl->format); + ps = CALLOC_STRUCT(pipe_surface); if (ps) { @@ -1067,6 +1070,29 @@ pan_resource_modifier_convert(struct panfrost_context *ctx, pipe_resource_reference(&tmp_prsrc, NULL); } +/* Validate that an AFBC resource may be used as a particular format. If it may + * not, decompress it on the fly. Failure to do so can produce wrong results or + * invalid data faults when sampling or rendering to AFBC */ + +void +pan_legalize_afbc_format(struct panfrost_context *ctx, + struct panfrost_resource *rsrc, + enum pipe_format format) +{ + struct panfrost_device *dev = pan_device(ctx->base.screen); + + if (!drm_is_afbc(rsrc->image.layout.modifier)) + return; + + if (panfrost_afbc_format(dev, pan_blit_format(rsrc->base.format)) == + panfrost_afbc_format(dev, pan_blit_format(format))) + return; + + pan_resource_modifier_convert(ctx, rsrc, + DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED, + "Reinterpreting AFBC surface as incompatible format"); +} + static bool panfrost_should_linear_convert(struct panfrost_device *dev, struct panfrost_resource *prsrc, diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h index cb1260ce7995..1d7bb8178164 100644 --- a/src/gallium/drivers/panfrost/pan_resource.h +++ b/src/gallium/drivers/panfrost/pan_resource.h @@ -156,4 +156,9 @@ pan_resource_modifier_convert(struct panfrost_context *ctx, struct panfrost_resource *rsrc, uint64_t modifier, const char *reason); +void +pan_legalize_afbc_format(struct panfrost_context *ctx, + struct panfrost_resource *rsrc, + enum pipe_format format); + #endif /* PAN_RESOURCE_H */ -- GitLab From 2526f6f229406fa364e4375449d33e2048caaab6 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 5 Oct 2021 16:18:22 -0400 Subject: [PATCH 4/4] panfrost: Enable AFBC on v7 The bugs blocking this have been resolved, so flip on AFBC again and get moar fps. Signed-off-by: Alyssa Rosenzweig Cc: mesa-stable Part-of: --- src/gallium/drivers/panfrost/pan_screen.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c index 72999fe7c905..abc21432751e 100644 --- a/src/gallium/drivers/panfrost/pan_screen.c +++ b/src/gallium/drivers/panfrost/pan_screen.c @@ -841,13 +841,6 @@ panfrost_create_screen(int fd, struct renderonly *ro) if (dev->debug & PAN_DBG_NO_AFBC) dev->has_afbc = false; - /* XXX: AFBC is currently broken on Bifrost - * - * - Preload is broken if the effective tile size is not 16x16 - */ - if (dev->arch == 7) - dev->has_afbc = false; - dev->ro = ro; /* Check if we're loading against a supported GPU model. */ -- GitLab