From 175287f68decbba220af2afe68e6aabc2e164145 Mon Sep 17 00:00:00 2001 From: Jose Maria Casanova Crespo Date: Thu, 30 May 2024 14:30:23 +0200 Subject: [PATCH] v3dv: really fix CLE MMU errors on 7.1HW Rpi5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Macro values that define values for different HW generations should use the V3DV_X helper instead of being defined under a V3D_VERSION #if condition. Without this change, the original V3D_CLE_READAHEAD and V3D_CLE_BUFFER_MIN_SIZE definitions used were only working for 4.2 HW. For the 7.1 HW (RPi5) the 4.2 definitions were applied. The CLE MMU errors were hidden as they were reported at dmesg as "MMU error from client PTB (1) at 0x1884200, pte invalid" instead of client CLE. So fixes all v3dv dmesg warnings for PTB MMU errors on RPi5. With this change we really don't need different functions per HW generation, so we rename back file v3dvx_cl.c to v3dv_cl.c. As before, we can use only the packets definitions for 4.2 HW as they use the same opcode as 7.1 HW. It fixes also an indentation error introduced with 26c8a5cd721. Fixes: bb77ac983e4 ("v3dv: Increase alignment to 16k on CL BO on RPi5") Fixes: 26c8a5cd721 ("v3dv: fix CLE MMU errors avoiding using last bytes of CL BOs.") Reviewed-by: Alejandro Piñeiro Part-of: (cherry picked from commit 07d3d557832a223c574aa205db3f4acfb04161ad) --- .pick_status.json | 2 +- src/broadcom/vulkan/meson.build | 2 +- src/broadcom/vulkan/{v3dvx_cl.c => v3dv_cl.c} | 32 +++++++++++-------- 3 files changed, 20 insertions(+), 16 deletions(-) rename src/broadcom/vulkan/{v3dvx_cl.c => v3dv_cl.c} (87%) diff --git a/.pick_status.json b/.pick_status.json index 349959759fe..7f19cac84c9 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -54,7 +54,7 @@ "description": "v3dv: really fix CLE MMU errors on 7.1HW Rpi5", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "bb77ac983e4f8a265141855e60ad2a5637c9b74d", "notes": null diff --git a/src/broadcom/vulkan/meson.build b/src/broadcom/vulkan/meson.build index 14132887c78..74495831716 100644 --- a/src/broadcom/vulkan/meson.build +++ b/src/broadcom/vulkan/meson.build @@ -34,6 +34,7 @@ v3dv_entrypoints = custom_target( libv3dv_files = files( 'v3dv_bo.c', + 'v3dv_cl.c', 'v3dv_cmd_buffer.c', 'v3dv_debug.c', 'v3dv_debug.h', @@ -56,7 +57,6 @@ libv3dv_files = files( ) + [v3d_xml_pack, vk_common_entrypoints[0], wsi_entrypoints[0]] files_per_version = files( - 'v3dvx_cl.c', 'v3dvx_cmd_buffer.c', 'v3dvx_descriptor_set.c', 'v3dvx_device.c', diff --git a/src/broadcom/vulkan/v3dvx_cl.c b/src/broadcom/vulkan/v3dv_cl.c similarity index 87% rename from src/broadcom/vulkan/v3dvx_cl.c rename to src/broadcom/vulkan/v3dv_cl.c index a9e55541501..af4066c7849 100644 --- a/src/broadcom/vulkan/v3dvx_cl.c +++ b/src/broadcom/vulkan/v3dv_cl.c @@ -1,4 +1,4 @@ - /* +/* * Copyright © 2019 Raspberry Pi Ltd * * Permission is hereby granted, free of charge, to any person obtaining a @@ -22,6 +22,12 @@ */ #include "v3dv_private.h" + +/* We don't expect that the packets we use in this file change across hw + * versions, so we just explicitly set the V3D_VERSION and include v3dx_pack + * here + */ +#define V3D_VERSION 42 #include "broadcom/common/v3d_macros.h" #include "broadcom/cle/v3dx_pack.h" @@ -30,14 +36,10 @@ * the CLE would pre-fetch the data after the end of the CL buffer, reporting * the kernel "MMU error from client CLE". */ -#if V3D_VERSION == 42 -#define V3D_CLE_READAHEAD 256 -#define V3D_CLE_BUFFER_MIN_SIZE 4096 -#endif -#if V3D_VERSION >= 71 -#define V3D_CLE_READAHEAD 1024 -#define V3D_CLE_BUFFER_MIN_SIZE 16384 -#endif +#define V3D42_CLE_READAHEAD 256u +#define V3D42_CLE_BUFFER_MIN_SIZE 4096u +#define V3D71_CLE_READAHEAD 1024u +#define V3D71_CLE_BUFFER_MIN_SIZE 16384u void v3dv_cl_init(struct v3dv_job *job, struct v3dv_cl *cl) @@ -81,12 +83,14 @@ cl_alloc_bo(struct v3dv_cl *cl, uint32_t space, enum * calling cl_submit to use this reserved space. */ uint32_t unusable_space = 0; + uint32_t cle_readahead = V3DV_X(cl->job->device, CLE_READAHEAD); + uint32_t cle_buffer_min_size = V3DV_X(cl->job->device, CLE_BUFFER_MIN_SIZE); switch (chain_type) { case V3D_CL_BO_CHAIN_WITH_BRANCH: - unusable_space = V3D_CLE_READAHEAD + cl_packet_length(BRANCH); + unusable_space = cle_readahead + cl_packet_length(BRANCH); break; case V3D_CL_BO_CHAIN_WITH_RETURN_FROM_SUB_LIST: - unusable_space = V3D_CLE_READAHEAD + cl_packet_length(RETURN_FROM_SUB_LIST); + unusable_space = cle_readahead + cl_packet_length(RETURN_FROM_SUB_LIST); break; case V3D_CL_BO_CHAIN_NONE: break; @@ -96,7 +100,7 @@ cl_alloc_bo(struct v3dv_cl *cl, uint32_t space, enum * of allocations with large command buffers. This has a very significant * impact on the number of draw calls per second reported by vkoverhead. */ - space = align(space + unusable_space, V3D_CLE_BUFFER_MIN_SIZE); + space = align(space + unusable_space, cle_buffer_min_size); if (cl->bo) space = MAX2(cl->bo->size * 2, space); @@ -121,7 +125,7 @@ cl_alloc_bo(struct v3dv_cl *cl, uint32_t space, enum switch (chain_type) { case V3D_CL_BO_CHAIN_WITH_BRANCH: cl->size += cl_packet_length(BRANCH); - assert(cl->size + V3D_CLE_READAHEAD <= cl->bo->size); + assert(cl->size + cle_readahead <= cl->bo->size); cl_emit(cl, BRANCH, branch) { branch.address = v3dv_cl_address(bo, 0); } @@ -133,7 +137,7 @@ cl_alloc_bo(struct v3dv_cl *cl, uint32_t space, enum * end with a 'return from sub list' command. */ cl->size += cl_packet_length(RETURN_FROM_SUB_LIST); - assert(cl->size + V3D_CLE_READAHEAD <= cl->bo->size); + assert(cl->size + cle_readahead <= cl->bo->size); cl_emit(cl, RETURN_FROM_SUB_LIST, ret); FALLTHROUGH; case V3D_CL_BO_CHAIN_NONE: