From b920ace4bc79dcda7487ff98bf237e6bcdeb557e Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 4 Jan 2022 15:31:25 -0500 Subject: [PATCH] lima,panfrost: Correct pixel vs block mismatches Different parts of our codebase disagree on whether spatial coordinates/dimensions are given in pixels or blocks, which differ by a constant factor for block-compressed formats. This disagreement manifests as incorrect results accessing block-compressed formats. To resolve this, define the public tiling routines to take their coordinates in pixels, and align the relevant code in Panfrost accordingly. Fixes rendering glitches in Factorio, as well as a pile of piglits on Panfrost. It should also fix glTexSubImage() with ETC1 on Lima, but there are no tests for this in dEQP/Piglit. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Emma Anholt Tested-by: Vasily Khoruzhick [dEQP/Lima] Tested-by: Erico Nunes [Piglit/Lima] Reported-by: Icecream95 Closes: #5560 Cc: mesa-stable Part-of: --- src/gallium/drivers/panfrost/pan_resource.c | 21 +++++++++++++++------ src/panfrost/ci/panfrost-g52-fails.txt | 19 ------------------- src/panfrost/shared/pan_tiling.c | 20 +++++++++++++++++--- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index b6a6d811387..8802ff7b1cb 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -864,7 +864,8 @@ panfrost_ptr_map(struct pipe_context *pctx, struct panfrost_context *ctx = pan_context(pctx); struct panfrost_device *dev = pan_device(pctx->screen); struct panfrost_resource *rsrc = pan_resource(resource); - int bytes_per_pixel = util_format_get_blocksize(rsrc->image.layout.format); + enum pipe_format format = rsrc->image.layout.format; + int bytes_per_block = util_format_get_blocksize(format); struct panfrost_bo *bo = rsrc->image.data.bo; /* Can't map tiled/compressed directly */ @@ -995,9 +996,17 @@ panfrost_ptr_map(struct pipe_context *pctx, } } + /* For access to compressed textures, we want the (x, y, w, h) + * region-of-interest in blocks, not pixels. Then we compute the stride + * between rows of blocks as the width in blocks times the width per + * block, etc. + */ + struct pipe_box box_blocks; + u_box_pixels_to_blocks(&box_blocks, box, format); + if (rsrc->image.layout.modifier == DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED) { - transfer->base.stride = box->width * bytes_per_pixel; - transfer->base.layer_stride = transfer->base.stride * box->height; + transfer->base.stride = box_blocks.width * bytes_per_block; + transfer->base.layer_stride = transfer->base.stride * box_blocks.height; transfer->map = ralloc_size(transfer, transfer->base.layer_stride * box->depth); assert(box->depth == 1); @@ -1038,9 +1047,9 @@ panfrost_ptr_map(struct pipe_context *pctx, return bo->ptr.cpu + rsrc->image.layout.slices[level].offset - + transfer->base.box.z * transfer->base.layer_stride - + transfer->base.box.y * rsrc->image.layout.slices[level].line_stride - + transfer->base.box.x * bytes_per_pixel; + + box->z * transfer->base.layer_stride + + box_blocks.y * rsrc->image.layout.slices[level].line_stride + + box_blocks.x * bytes_per_block; } } diff --git a/src/panfrost/ci/panfrost-g52-fails.txt b/src/panfrost/ci/panfrost-g52-fails.txt index a55059988ce..bc472e6f604 100644 --- a/src/panfrost/ci/panfrost-g52-fails.txt +++ b/src/panfrost/ci/panfrost-g52-fails.txt @@ -95,14 +95,11 @@ spec@arb_framebuffer_object@arb_framebuffer_object-depth-stencil-blit stencil gl spec@arb_framebuffer_object@fbo-luminance-alpha,Fail spec@arb_framebuffer_srgb@fbo-fast-clear,Fail spec@arb_get_program_binary@restore-sso-program,Fail -spec@arb_get_texture_sub_image@arb_get_texture_sub_image-getcompressed,Fail -spec@arb_get_texture_sub_image@arb_get_texture_sub_image-get,Fail spec@arb_pixel_buffer_object@fbo-pbo-readpixels-small,Fail spec@arb_pixel_buffer_object@fbo-pbo-readpixels-small@GL_DEPTH32F_STENCIL8-GL_DEPTH_STENCIL,Fail spec@arb_pixel_buffer_object@fbo-pbo-readpixels-small@GL_DEPTH32F_STENCIL8-GL_STENCIL_INDEX,Fail spec@arb_pixel_buffer_object@texsubimage array pbo,Fail spec@arb_pixel_buffer_object@texsubimage cube_map_array pbo,Fail -spec@arb_pixel_buffer_object@texsubimage pbo,Fail spec@arb_point_sprite@arb_point_sprite-checkerboard,Fail spec@arb_point_sprite@arb_point_sprite-mipmap,Fail spec@arb_provoking_vertex@arb-provoking-vertex-render,Fail @@ -510,8 +507,6 @@ spec@ext_packed_float@texwrap formats bordercolor@GL_R11F_G11F_B10F- border colo spec@ext_packed_float@texwrap formats bordercolor-swizzled,Fail spec@ext_packed_float@texwrap formats bordercolor-swizzled@GL_R11F_G11F_B10F- swizzled- border color only,Fail spec@ext_texture_array@array-texture,Fail -spec@ext_texture_array@compressed texsubimage,Fail -spec@ext_texture_array@compressed texsubimage pbo,Fail spec@ext_texture_array@copyteximage 1d_array,Fail spec@ext_texture_array@copyteximage 1d_array samples=2,Fail spec@ext_texture_array@copyteximage 1d_array samples=4,Fail @@ -530,10 +525,6 @@ spec@ext_texture_compression_rgtc@texwrap formats bordercolor-swizzled@GL_COMPRE spec@ext_texture_compression_rgtc@texwrap formats bordercolor-swizzled@GL_COMPRESSED_RG_RGTC2- swizzled- border color only,Fail spec@ext_texture_compression_rgtc@texwrap formats bordercolor-swizzled@GL_COMPRESSED_SIGNED_RED_RGTC1- swizzled- border color only,Fail spec@ext_texture_compression_rgtc@texwrap formats bordercolor-swizzled@GL_COMPRESSED_SIGNED_RG_RGTC2- swizzled- border color only,Fail -spec@ext_texture_compression_s3tc@s3tc-errors,Fail -spec@ext_texture_compression_s3tc@s3tc-errors_gles2,Fail -spec@ext_texture_compression_s3tc@s3tc-texsubimage,Fail -spec@ext_texture_compression_s3tc@s3tc-texsubimage_gles2,Fail spec@ext_texture_compression_s3tc@texwrap formats bordercolor,Fail spec@ext_texture_compression_s3tc@texwrap formats bordercolor@GL_COMPRESSED_RGBA_S3TC_DXT1_EXT- border color only,Fail spec@ext_texture_compression_s3tc@texwrap formats bordercolor@GL_COMPRESSED_RGBA_S3TC_DXT3_EXT- border color only,Fail @@ -901,17 +892,9 @@ spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA/Destination spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGB/Destination: GL_RGB,Fail spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RG/Destination: GL_RG,Fail spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_RED_RGTC1/Destination: GL_COMPRESSED_RED_RGTC1,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_RGBA_S3TC_DXT1_EXT/Destination: GL_COMPRESSED_RGBA_S3TC_DXT1_EXT,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_RGBA_S3TC_DXT3_EXT/Destination: GL_COMPRESSED_RGBA_S3TC_DXT3_EXT,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_RGBA_S3TC_DXT5_EXT/Destination: GL_COMPRESSED_RGBA_S3TC_DXT5_EXT,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_RGB_S3TC_DXT1_EXT/Destination: GL_COMPRESSED_RGB_S3TC_DXT1_EXT,Fail spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_RG_RGTC2/Destination: GL_COMPRESSED_RG_RGTC2,Fail spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_SIGNED_RED_RGTC1/Destination: GL_COMPRESSED_SIGNED_RED_RGTC1,Fail spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_SIGNED_RG_RGTC2/Destination: GL_COMPRESSED_SIGNED_RG_RGTC2,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT/Destination: GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT/Destination: GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT/Destination: GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,Fail -spec@nv_copy_image@nv_copy_image-formats@Source: GL_COMPRESSED_SRGB_S3TC_DXT1_EXT/Destination: GL_COMPRESSED_SRGB_S3TC_DXT1_EXT,Fail spec@oes_egl_image_external_essl3@oes_egl_image_external_essl3,Crash spec@oes_point_sprite@arb_point_sprite-checkerboard_gles1,Fail spec@oes_texture_view@sampling-2d-array-as-cubemap,Crash @@ -1176,7 +1159,6 @@ spec@!opengl 1.1@polygon-mode-offset@config 6: Expected white pixel on right edg spec@!opengl 1.1@polygon-mode-offset@config 6: Expected white pixel on top edge,Fail spec@!opengl 1.1@polygon-mode-offset,Fail spec@!opengl 1.1@streaming-texture-leak,Crash -spec@!opengl 1.1@texsubimage,Fail spec@!opengl 1.1@texwrap 1d bordercolor,Fail spec@!opengl 1.1@texwrap 1d bordercolor@GL_RGBA8- border color only,Fail spec@!opengl 1.1@texwrap 1d proj bordercolor,Fail @@ -1265,5 +1247,4 @@ spec@!opengl 3.1@primitive-restart-xfb generated,Fail spec@!opengl 3.1@primitive-restart-xfb written,Fail spec@!opengl 3.1@required-texture-attachment-formats,Fail spec@!opengl 3.2@coord-replace-doesnt-eliminate-frag-tex-coords,Fail -spec@!opengl es 3.0@ext_texture_array-compressed_gles3 texsubimage,Fail spec@!opengl es 3.0@gles-3.0-transform-feedback-uniform-buffer-object,Fail diff --git a/src/panfrost/shared/pan_tiling.c b/src/panfrost/shared/pan_tiling.c index 1275fdcf62a..15a7334df28 100644 --- a/src/panfrost/shared/pan_tiling.c +++ b/src/panfrost/shared/pan_tiling.c @@ -250,6 +250,12 @@ TILED_ACCESS_TYPE(pan_uint128_t, 4); TILED_UNALIGNED_TYPE(pan_uint128_t, store, shift) \ } +/* + * Perform a generic access to a tiled image with a given format. This works + * even for block-compressed images on entire blocks at a time. sx/sy/w/h are + * specified in pixels, not blocks, but our internal routines work in blocks, + * so we divide here. Alignment is assumed. + */ static void panfrost_access_tiled_image_generic(void *dst, void *src, unsigned sx, unsigned sy, @@ -261,10 +267,13 @@ panfrost_access_tiled_image_generic(void *dst, void *src, { unsigned bpp = desc->block.bits; - if (desc->block.width > 1) { - w = DIV_ROUND_UP(w, desc->block.width); - h = DIV_ROUND_UP(h, desc->block.height); + /* Convert units */ + sx /= desc->block.width; + sy /= desc->block.height; + w = DIV_ROUND_UP(w, desc->block.width); + h = DIV_ROUND_UP(h, desc->block.height); + if (desc->block.width > 1) { if (_is_store) TILED_UNALIGNED_TYPES(true, 2) else @@ -371,6 +380,11 @@ panfrost_access_tiled_image(void *dst, void *src, panfrost_access_tiled_image_pan_uint128_t(dst, OFFSET(src, x, y), x, y, w, h, dst_stride, src_stride, is_store); } +/** + * Access a tiled image (load or store). Note: the region of interest (x, y, w, + * h) is specified in pixels, not blocks. It is expected that these quantities + * are aligned to the block size. + */ void panfrost_store_tiled_image(void *dst, const void *src, unsigned x, unsigned y,