panvk: Fix disjoint image memory binding

Right now, we store the last VkDeviceMemory object bound to an image in
panvk_image::mem, but this doesn't work for disjoint images where the mem
object can differ on each plane.

Move panvk_image::mem to panvk_image_plane::mem and prefix the offset
field with mem_, so it's clear the offset refers to the mem object.

Note this should only fix host copy on disjoint images, since the GPU
address was already properly set at bind time.

Fixes: 1cd61ee948 ("panvk: implement VK_EXT_host_image_copy for linear color images")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Olivia Lee <olivia.lee@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36926>
(cherry picked from commit d0126f5ced)
This commit is contained in:
Boris Brezillon 2025-08-22 10:51:40 +02:00 committed by Eric Engestrom
parent 8c04110b17
commit 66fdba6e09
6 changed files with 135 additions and 74 deletions

View file

@ -5134,7 +5134,7 @@
"description": "panvk: Fix disjoint image memory binding",
"nominated": true,
"nomination_type": 2,
"resolution": 0,
"resolution": 1,
"main_sha": null,
"because_sha": "1cd61ee94875efbf546455212e67ccc6bac111c6",
"notes": null

View file

@ -72,7 +72,7 @@ struct panvk_rendering_state {
#if PAN_ARCH < 9
uint32_t bo_count;
struct pan_kmod_bo *bos[MAX_RTS + 2];
struct pan_kmod_bo *bos[(MAX_RTS * PANVK_MAX_PLANES) + 2];
#endif
} fb;

View file

@ -115,7 +115,7 @@ panvk_copy_image_to_from_memory(struct image_params img,
unsigned layer_count =
vk_image_subresource_layer_count(&img.img->vk, &img.subres);
void *img_base_ptr = img.ptr + plane->offset + slice_layout->offset_B;
void *img_base_ptr = img.ptr + plane->mem_offset + slice_layout->offset_B;
for (unsigned layer = 0; layer < layer_count; layer++) {
unsigned img_layer = layer + img.subres.baseArrayLayer;
void *img_layer_ptr = img_base_ptr +
@ -197,28 +197,82 @@ panvk_copy_memory_to_image(struct panvk_image *dst, void *dst_cpu,
dst_params, src_params, region->imageExtent, flags, true);
}
static VkResult
mmap_plane(struct panvk_image *img, uint8_t p, int prot,
void *plane_ptrs[static const PANVK_MAX_PLANES])
{
assert(p < PANVK_MAX_PLANES);
if (plane_ptrs[p])
return VK_SUCCESS;
plane_ptrs[p] = pan_kmod_bo_mmap(img->planes[p].mem->bo, 0,
pan_kmod_bo_size(img->planes[p].mem->bo),
prot, MAP_SHARED, NULL);
if (plane_ptrs[p] == MAP_FAILED) {
plane_ptrs[p] = NULL;
return panvk_errorf(img->vk.base.device, VK_ERROR_MEMORY_MAP_FAILED,
"Failed to CPU map image");
}
/* In case of a multi-planar and !disjoint image (or disjoint but with some
* planes pointing to the same memory object), we propagate the BO mapping to
* all relevant entries, so we don't have to mmap the same BO to different
* location if another plane is copied. */
for (uint8_t i = 0; i < PANVK_MAX_PLANES; i++) {
if (p != i && img->planes[p].mem == img->planes[i].mem)
plane_ptrs[i] = plane_ptrs[p];
}
return VK_SUCCESS;
}
static void
munmap_planes(struct panvk_image *img,
void *plane_ptrs[static const PANVK_MAX_PLANES])
{
for (uint8_t i = 0; i < PANVK_MAX_PLANES; i++) {
if (!plane_ptrs[i])
continue;
ASSERTED int ret =
os_munmap(plane_ptrs[i], pan_kmod_bo_size(img->planes[i].mem->bo));
assert(!ret);
/* Make sure we reset all mapping entries pointing to the same virtual
* address so we don't end up with double-munmap() cases. */
for (uint8_t j = i; j < PANVK_MAX_PLANES; j++) {
if (plane_ptrs[i] == plane_ptrs[j])
plane_ptrs[j] = NULL;
}
plane_ptrs[i] = NULL;
}
}
VKAPI_ATTR VkResult VKAPI_CALL
panvk_CopyMemoryToImage(VkDevice device, const VkCopyMemoryToImageInfo *info)
{
VK_FROM_HANDLE(panvk_device, dev, device);
VK_FROM_HANDLE(panvk_image, dst, info->dstImage);
void *dst_cpu = pan_kmod_bo_mmap(
dst->mem->bo, 0, pan_kmod_bo_size(dst->mem->bo), PROT_WRITE, MAP_SHARED,
NULL);
if (dst_cpu == MAP_FAILED)
return panvk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
"Failed to CPU map image");
void *dst_cpu[PANVK_MAX_PLANES] = {NULL};
VkResult result = VK_SUCCESS;
for (unsigned i = 0; i < info->regionCount; i++) {
panvk_copy_memory_to_image(dst, dst_cpu, &info->pRegions[i],
uint8_t p = panvk_plane_index(
dst->vk.format, info->pRegions[i].imageSubresource.aspectMask);
result = mmap_plane(dst, p, PROT_WRITE, dst_cpu);
if (result != VK_SUCCESS)
goto out_unmap;
panvk_copy_memory_to_image(dst, dst_cpu[p], &info->pRegions[i],
info->flags);
}
ASSERTED int ret = os_munmap(dst_cpu, pan_kmod_bo_size(dst->mem->bo));
assert(!ret);
return VK_SUCCESS;
out_unmap:
munmap_planes(dst, dst_cpu);
return result;
}
static void
@ -244,25 +298,25 @@ panvk_copy_image_to_memory(struct panvk_image *src, void *src_cpu,
VKAPI_ATTR VkResult VKAPI_CALL
panvk_CopyImageToMemory(VkDevice device, const VkCopyImageToMemoryInfo *info)
{
VK_FROM_HANDLE(panvk_device, dev, device);
VK_FROM_HANDLE(panvk_image, src, info->srcImage);
void *src_cpu = pan_kmod_bo_mmap(
src->mem->bo, 0, pan_kmod_bo_size(src->mem->bo), PROT_READ, MAP_SHARED,
NULL);
if (src_cpu == MAP_FAILED)
return panvk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
"Failed to CPU map image");
void *src_cpu[PANVK_MAX_PLANES] = {NULL};
VkResult result = VK_SUCCESS;
for (unsigned i = 0; i < info->regionCount; i++) {
panvk_copy_image_to_memory(src, src_cpu, &info->pRegions[i],
uint8_t p = panvk_plane_index(
src->vk.format, info->pRegions[i].imageSubresource.aspectMask);
result = mmap_plane(src, p, PROT_READ, src_cpu);
if (result != VK_SUCCESS)
goto out_unmap;
panvk_copy_image_to_memory(src, src_cpu[p], &info->pRegions[i],
info->flags);
}
ASSERTED int ret = os_munmap(src_cpu, pan_kmod_bo_size(src->mem->bo));
assert(!ret);
return VK_SUCCESS;
out_unmap:
munmap_planes(src, src_cpu);
return result;
}
static void
@ -337,9 +391,9 @@ panvk_copy_image_to_image(struct panvk_image *dst, void *dst_cpu,
unsigned depth = sample_count > 1 ? sample_count : region->extent.depth;
void *src_base_ptr =
src_cpu + src_plane->offset + src_slice_layout->offset_B;
src_cpu + src_plane->mem_offset + src_slice_layout->offset_B;
void *dst_base_ptr =
dst_cpu + dst_plane->offset + dst_slice_layout->offset_B;
dst_cpu + dst_plane->mem_offset + dst_slice_layout->offset_B;
for (unsigned layer = 0; layer < layer_count; layer++) {
unsigned src_layer = layer + src_subres.baseArrayLayer;
unsigned dst_layer = layer + dst_subres.baseArrayLayer;
@ -421,37 +475,32 @@ panvk_CopyImageToImage(VkDevice device, const VkCopyImageToImageInfo *info)
{
VkResult result = VK_SUCCESS;
VK_FROM_HANDLE(panvk_device, dev, device);
VK_FROM_HANDLE(panvk_image, dst, info->dstImage);
VK_FROM_HANDLE(panvk_image, src, info->srcImage);
void *dst_cpu = pan_kmod_bo_mmap(
dst->mem->bo, 0, pan_kmod_bo_size(dst->mem->bo), PROT_WRITE, MAP_SHARED,
NULL);
if (dst_cpu == MAP_FAILED)
return panvk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
"Failed to CPU map image");
void *src_cpu = pan_kmod_bo_mmap(
src->mem->bo, 0, pan_kmod_bo_size(src->mem->bo), PROT_READ, MAP_SHARED,
NULL);
if (src_cpu == MAP_FAILED) {
result = panvk_errorf(dev, VK_ERROR_MEMORY_MAP_FAILED,
"Failed to CPU map image");
goto unmap_dst;
}
void *src_cpu[PANVK_MAX_PLANES] = {NULL};
void *dst_cpu[PANVK_MAX_PLANES] = {NULL};
for (unsigned i = 0; i < info->regionCount; i++) {
panvk_copy_image_to_image(dst, dst_cpu, src, src_cpu, &info->pRegions[i],
info->flags);
uint8_t src_p = panvk_plane_index(
src->vk.format, info->pRegions[i].srcSubresource.aspectMask);
uint8_t dst_p = panvk_plane_index(
dst->vk.format, info->pRegions[i].dstSubresource.aspectMask);
result = mmap_plane(dst, dst_p, PROT_WRITE, dst_cpu);
if (result != VK_SUCCESS)
goto out_unmap;
result = mmap_plane(src, src_p, PROT_READ, src_cpu);
if (result != VK_SUCCESS)
goto out_unmap;
panvk_copy_image_to_image(dst, dst_cpu[dst_p], src, src_cpu[src_p],
&info->pRegions[i], info->flags);
}
ASSERTED int ret = os_munmap(src_cpu, pan_kmod_bo_size(src->mem->bo));
assert(!ret);
unmap_dst:
ret = os_munmap(dst_cpu, pan_kmod_bo_size(dst->mem->bo));
assert(!ret);
out_unmap:
munmap_planes(src, src_cpu);
munmap_planes(dst, dst_cpu);
return result;
}

View file

@ -425,15 +425,16 @@ panvk_image_init(struct panvk_image *image,
static VkResult
panvk_image_plane_bind(struct panvk_device *dev,
struct panvk_image_plane *plane, struct pan_kmod_bo *bo,
uint64_t base, uint64_t offset)
struct panvk_image_plane *plane,
struct panvk_device_memory *mem, uint64_t offset)
{
plane->plane.base = base + offset;
plane->offset = offset;
plane->plane.base = mem->addr.dev + offset;
plane->mem = mem;
plane->mem_offset = offset;
/* Reset the AFBC headers */
if (drm_is_afbc(plane->image.props.modifier)) {
/* Transient CPU mapping */
void *bo_base = pan_kmod_bo_mmap(bo, 0, pan_kmod_bo_size(bo),
void *bo_base = pan_kmod_bo_mmap(mem->bo, 0, pan_kmod_bo_size(mem->bo),
PROT_WRITE, MAP_SHARED, NULL);
if (bo_base == MAP_FAILED)
@ -459,7 +460,7 @@ panvk_image_plane_bind(struct panvk_device *dev,
}
}
ASSERTED int ret = os_munmap(bo_base, pan_kmod_bo_size(bo));
ASSERTED int ret = os_munmap(bo_base, pan_kmod_bo_size(mem->bo));
assert(!ret);
}
@ -700,19 +701,16 @@ panvk_image_bind(struct panvk_device *dev,
}
assert(mem);
image->mem = mem;
if (is_disjoint(image)) {
const VkBindImagePlaneMemoryInfo *plane_info =
vk_find_struct_const(bind_info->pNext, BIND_IMAGE_PLANE_MEMORY_INFO);
const uint8_t plane =
panvk_plane_index(image->vk.format, plane_info->planeAspect);
return panvk_image_plane_bind(dev, &image->planes[plane], mem->bo,
mem->addr.dev, offset);
return panvk_image_plane_bind(dev, &image->planes[plane], mem, offset);
} else {
for (unsigned plane = 0; plane < image->plane_count; plane++) {
VkResult result =
panvk_image_plane_bind(dev, &image->planes[plane], mem->bo,
mem->addr.dev, offset);
panvk_image_plane_bind(dev, &image->planes[plane], mem, offset);
if (result != VK_SUCCESS)
return result;
}

View file

@ -22,15 +22,15 @@ struct panvk_image_plane {
struct pan_image image;
struct pan_image_plane plane;
/* Plane offset inside the image BO */
uint64_t offset;
struct panvk_device_memory *mem;
/* Plane offset inside the memory object. */
uint64_t mem_offset;
};
struct panvk_image {
struct vk_image vk;
struct panvk_device_memory *mem;
uint8_t plane_count;
struct panvk_image_plane planes[PANVK_MAX_PLANES];
};

View file

@ -67,7 +67,18 @@ render_state_set_color_attachment(struct panvk_cmd_buffer *cmdbuf,
state->render.color_attachments.samples[index] = img->vk.samples;
#if PAN_ARCH < 9
state->render.fb.bos[state->render.fb.bo_count++] = img->mem->bo;
for (uint8_t p = 0; p < ARRAY_SIZE(iview->pview.planes); p++) {
struct pan_image_plane_ref pref =
pan_image_view_get_plane(&iview->pview, p);
if (!pref.image)
continue;
assert(pref.plane_idx < ARRAY_SIZE(img->planes));
assert(img->planes[pref.plane_idx].mem->bo != NULL);
state->render.fb.bos[state->render.fb.bo_count++] =
img->planes[pref.plane_idx].mem->bo;
}
#endif
fbinfo->rts[index].view = &iview->pview;
@ -108,7 +119,8 @@ render_state_set_z_attachment(struct panvk_cmd_buffer *cmdbuf,
container_of(iview->vk.image, struct panvk_image, vk);
#if PAN_ARCH < 9
state->render.fb.bos[state->render.fb.bo_count++] = img->mem->bo;
/* Depth plane always comes first. */
state->render.fb.bos[state->render.fb.bo_count++] = img->planes[0].mem->bo;
#endif
state->render.z_attachment.fmt = iview->vk.format;
@ -172,7 +184,9 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf,
container_of(iview->vk.image, struct panvk_image, vk);
#if PAN_ARCH < 9
state->render.fb.bos[state->render.fb.bo_count++] = img->mem->bo;
/* The stencil plane is always last. */
state->render.fb.bos[state->render.fb.bo_count++] =
img->planes[img->plane_count - 1].mem->bo;
#endif
state->render.s_attachment.fmt = iview->vk.format;