From ec101645ff680dc05c31182d453ea0fa0b84b99a Mon Sep 17 00:00:00 2001 From: "Juan A. Suarez Romero" Date: Wed, 25 Mar 2026 16:14:01 +0100 Subject: [PATCH] vc4: fix unwanted buffer release on uploader When converting the index buffer from 4-bytes to 2-bytes, we use the uploader for the job. Since commit b3133e250e1 we do an uploader alloc ref, which releases the uploader buffer if there is no enough space, creating a new one. The problem happens when we also need this buffer because it is the one containing the index buffer to convert. This happens, for instance, if we need to convert the primitives because they are not supported (e.g., converting quads to triangles), as this is done also using the uploader. The solution is to ensure the uploader's buffer has an extra reference so when released, it is not destroyed. This can easily achieved by calling first pipe_buffer_map_range(), which is required to access the buffer, and it increases the references. This fixes `spec@!opengl 1.1@longprim`. Fixes: b3133e250e1 ("gallium: add pipe_context::resource_release to eliminate buffer refcounting") Reviewed-by: Iago Toral Quiroga Signed-off-by: Juan A. Suarez Romero (cherry picked from commit 48c086cb4203d1a8e7458e0d0a85cfffc5b4bfe5) Part-of: --- .pick_status.json | 2 +- src/broadcom/ci/broadcom-rpi3-fails.txt | 3 --- src/gallium/drivers/vc4/vc4_resource.c | 20 ++++++++++++++------ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 848b82bd205..375d78f1752 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -4,7 +4,7 @@ "description": "vc4: fix unwanted buffer release on uploader", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "b3133e250e1c40496d98c4ba52386b7ae423d194", "notes": null diff --git a/src/broadcom/ci/broadcom-rpi3-fails.txt b/src/broadcom/ci/broadcom-rpi3-fails.txt index b2439ad42d1..0f97e6722f2 100644 --- a/src/broadcom/ci/broadcom-rpi3-fails.txt +++ b/src/broadcom/ci/broadcom-rpi3-fails.txt @@ -765,9 +765,6 @@ spec@glsl-1.10@execution@glsl-vs-inline-explosion,Crash # stipple spec@!opengl 1.0@gl-1.0-no-op-paths,Fail -# Bisected to b3133e250e1 ("gallium: add pipe_context::resource_release to eliminate buffer refcounting") -spec@!opengl 1.1@longprim,Crash - # fails on arm64, passes on armhf spec@arb_depth_buffer_float@depthstencil-render-miplevels 1024 s=z24_s8_d=z32f,Fail diff --git a/src/gallium/drivers/vc4/vc4_resource.c b/src/gallium/drivers/vc4/vc4_resource.c index 1e2d9c50701..cb4a908d822 100644 --- a/src/gallium/drivers/vc4/vc4_resource.c +++ b/src/gallium/drivers/vc4/vc4_resource.c @@ -1148,12 +1148,6 @@ vc4_get_shadow_index_buffer(struct pipe_context *pctx, struct vc4_resource *orig = vc4_resource(info->index.resource); perf_debug("Fallback conversion for %d uint indices\n", count); - void *data; - struct pipe_resource *shadow_rsc = NULL; - u_upload_alloc_ref(vc4->uploader, 0, count * 2, 4, - shadow_offset, &shadow_rsc, &data); - uint16_t *dst = data; - struct pipe_transfer *src_transfer = NULL; const uint32_t *src; if (info->has_user_indices) { @@ -1165,6 +1159,20 @@ vc4_get_shadow_index_buffer(struct pipe_context *pctx, PIPE_MAP_READ, &src_transfer); } + /* We need to do the upload alloc ref after the + * pipe_buffer_map_range() because there is a risk that the alloc + * frees and destroy its internal buffer, which might be the original + * base buffer we want to copy. Calling it afterwards, we guarantee + * that pipe_buffer_map_range() increases the reference counter so if + * upload alloc ref needs to unreference it, the buffer doesn't reach + * 0 refcounts and thus it won't be destroyed. + */ + void *data; + struct pipe_resource *shadow_rsc = NULL; + u_upload_alloc_ref(vc4->uploader, 0, count * 2, 4, + shadow_offset, &shadow_rsc, &data); + uint16_t *dst = data; + for (int i = 0; i < count; i++) { uint32_t src_index = src[i]; assert(src_index <= 0xffff);