From 990d76eae6342bc14df998ec8723162a79a103e4 Mon Sep 17 00:00:00 2001 From: Daivik Bhatia Date: Wed, 15 Apr 2026 23:58:57 +0530 Subject: [PATCH] v3dv: Implement and enable nullDescriptor support Handle null descriptors by emitting zeroed descriptor state. When the nullDescriptor feature is enabled, a dedicated null_bo is allocated. Null image descriptors now pack a TEXTURE_SHADER_STATE whose base address points to this BO, ensuring that the TMU reads from valid memory. Reviewed-by: Iago Toral Quiroga Part-of: --- src/broadcom/vulkan/v3dv_cmd_buffer.c | 19 ++-- src/broadcom/vulkan/v3dv_descriptor_set.c | 102 +++++++++++++++++++--- src/broadcom/vulkan/v3dv_device.c | 20 ++++- src/broadcom/vulkan/v3dv_device.h | 5 ++ src/broadcom/vulkan/v3dv_pipeline.c | 2 + src/broadcom/vulkan/v3dv_uniforms.c | 43 ++++++--- src/broadcom/vulkan/v3dvx_cmd_buffer.c | 16 ++-- src/broadcom/vulkan/v3dvx_image.c | 19 ++++ src/broadcom/vulkan/v3dvx_private.h | 2 + 9 files changed, 193 insertions(+), 35 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index af133525479..90fd1eb025c 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -3482,6 +3482,8 @@ v3dv_CmdBindVertexBuffers2(VkCommandBuffer commandBuffer, for (uint32_t i = 0; i < bindingCount; i++) { struct v3dv_buffer *buffer = v3dv_buffer_from_handle(pBuffers[i]); + assert(buffer || cmd_buffer->device->vk.enabled_features.nullDescriptor); + if (vb[firstBinding + i].buffer != buffer) { vb[firstBinding + i].buffer = v3dv_buffer_from_handle(pBuffers[i]); vb_state_changed = true; @@ -3491,14 +3493,19 @@ v3dv_CmdBindVertexBuffers2(VkCommandBuffer commandBuffer, vb[firstBinding + i].offset = pOffsets[i]; vb_state_changed = true; } - assert(pOffsets[i] <= buffer->size); VkDeviceSize size; - if (!pSizes || pSizes[i] == VK_WHOLE_SIZE) - size = buffer->size - pOffsets[i]; - else - size = pSizes[i]; - assert(pOffsets[i] + size <= buffer->size); + if (!buffer) { + size = 0; + } else { + assert(pOffsets[i] <= buffer->size); + + if (!pSizes || pSizes[i] == VK_WHOLE_SIZE) + size = buffer->size - pOffsets[i]; + else + size = pSizes[i]; + assert(pOffsets[i] + size <= buffer->size); + } if (vb[firstBinding + i].size != size) { vb[firstBinding + i].size = size; diff --git a/src/broadcom/vulkan/v3dv_descriptor_set.c b/src/broadcom/vulkan/v3dv_descriptor_set.c index dab7e9e0c31..5d49012dafe 100644 --- a/src/broadcom/vulkan/v3dv_descriptor_set.c +++ b/src/broadcom/vulkan/v3dv_descriptor_set.c @@ -196,8 +196,6 @@ v3dv_descriptor_map_get_sampler(struct v3dv_descriptor_state *descriptor_state, assert(descriptor->type == VK_DESCRIPTOR_TYPE_SAMPLER || descriptor->type == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER); - assert(descriptor->sampler); - return descriptor->sampler; } @@ -238,13 +236,15 @@ v3dv_descriptor_map_get_texture_bo(struct v3dv_descriptor_state *descriptor_stat switch (descriptor->type) { case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: - assert(descriptor->buffer_view); + if (!descriptor->buffer_view) + return NULL; return descriptor->buffer_view->buffer->mem->bo; case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE: case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: { - assert(descriptor->image_view); + if (!descriptor->image_view) + return NULL; struct v3dv_image *image = (struct v3dv_image *) descriptor->image_view->vk.image; assert(map->plane[index] < image->plane_count); @@ -1059,6 +1059,17 @@ write_buffer_descriptor(struct v3dv_descriptor *descriptor, descriptor->type = desc_type; descriptor->buffer = buffer; + + /* This can happen when nullDescriptor is used. In that + * case the compiler will not emit the buffer access so + * the descriptor won't be accessed at all. + */ + if (!buffer) { + descriptor->offset = 0; + descriptor->range = 0; + return; + } + descriptor->offset = buffer_info->offset; if (buffer_info->range == VK_WHOLE_SIZE) { descriptor->range = buffer->size - buffer_info->offset; @@ -1082,12 +1093,46 @@ write_image_descriptor(struct v3dv_device *device, descriptor->sampler = sampler; descriptor->image_view = iview; - assert(iview || sampler); - uint8_t plane_count = iview ? iview->plane_count : sampler->plane_count; + if (!device->vk.enabled_features.nullDescriptor) + assert(iview || sampler); + + /* When VK_KHR_robustness2 nullDescriptor is enabled, applications are + * allowed to write VK_NULL_HANDLE for the imageView and sampler. + */ + const bool sampler_required = + (desc_type == VK_DESCRIPTOR_TYPE_SAMPLER || + desc_type == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER) && + !binding_layout->immutable_samplers_offset; + + const bool image_required = + desc_type == VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE || + desc_type == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE || + desc_type == VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT || + desc_type == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; void *desc_map = descriptor_bo_map(device, set, binding_layout, array_index); + if ((image_required && !iview) || (sampler_required && !sampler)) { + /* Multiplanar YCbCr descriptors require immutable samplers and a non-null + * imageView (VUID-VkWriteDescriptorSet-descriptorType-02738) both of + * which are not true here so we must never reach the null path + * with plane count > 1 when VK_KHR_robustness2 nullDescriptor is enabled. + */ + const uint32_t size = + v3d_X((&device->devinfo), descriptor_bo_size)(desc_type) * + binding_layout->plane_stride; + memset(desc_map, 0, size); + + v3d_X((&device->devinfo), pack_null_texture_state)(device, desc_map); + + descriptor->sampler = NULL; + descriptor->image_view = NULL; + return; + } + + uint8_t plane_count = iview ? iview->plane_count : sampler->plane_count; + for (uint8_t plane = 0; plane < plane_count; plane++) { if (iview) { uint32_t offset = desc_type == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER ? @@ -1128,12 +1173,19 @@ write_buffer_view_descriptor(struct v3dv_device *device, struct v3dv_buffer_view *bview, uint32_t array_index) { - assert(bview); + assert(bview || device->vk.enabled_features.nullDescriptor); + descriptor->type = desc_type; descriptor->buffer_view = bview; void *desc_map = descriptor_bo_map(device, set, binding_layout, array_index); + if (!bview) { + memset(desc_map, 0, sizeof(bview->texture_shader_state)); + v3d_X((&device->devinfo), pack_null_texture_state)(device, desc_map); + return; + } + memcpy(desc_map, bview->texture_shader_state, sizeof(bview->texture_shader_state)); @@ -1240,7 +1292,7 @@ v3dv_UpdateDescriptorSets(VkDevice _device, * image sampler, but for YCbCr we kwnow that we must use * immutable combined image samplers */ - assert(iview->plane_count == 1); + assert(!iview || iview->plane_count == 1); V3DV_FROM_HANDLE(v3dv_sampler, _sampler, image_info->sampler); sampler = _sampler; } @@ -1417,7 +1469,16 @@ v3dv_UpdateDescriptorSetWithTemplate( break; case VK_DESCRIPTOR_TYPE_SAMPLER: - case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: + for (uint32_t j = 0; j < entry->array_count; j++) { + const VkDescriptorImageInfo *info = + pData + entry->offset + j * entry->stride; + V3DV_FROM_HANDLE(v3dv_sampler, sampler, info->sampler); + write_image_descriptor(device, descriptor + entry->array_element + j, + entry->type, set, binding_layout, NULL, + sampler, entry->array_element + j); + } + break; + case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE: case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: @@ -1425,10 +1486,29 @@ v3dv_UpdateDescriptorSetWithTemplate( const VkDescriptorImageInfo *info = pData + entry->offset + j * entry->stride; V3DV_FROM_HANDLE(v3dv_image_view, iview, info->imageView); - V3DV_FROM_HANDLE(v3dv_sampler, sampler, info->sampler); write_image_descriptor(device, descriptor + entry->array_element + j, entry->type, set, binding_layout, iview, - sampler, entry->array_element + j); + NULL, entry->array_element + j); + } + break; + + case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: + for (uint32_t j = 0; j < entry->array_count; j++) { + const VkDescriptorImageInfo *info = + pData + entry->offset + j * entry->stride; + V3DV_FROM_HANDLE(v3dv_image_view, iview, info->imageView); + struct v3dv_sampler *sampler = NULL; + if (!binding_layout->immutable_samplers_offset) { + /* In general we ignore the sampler when updating a combined + * image sampler, but for YCbCr we know that we must use + * immutable combined image samplers. + */ + assert(!iview || iview->plane_count == 1); + sampler = v3dv_sampler_from_handle(info->sampler); + } + write_image_descriptor(device, descriptor + entry->array_element + j, + entry->type, set, binding_layout, + iview, sampler, entry->array_element + j); } break; diff --git a/src/broadcom/vulkan/v3dv_device.c b/src/broadcom/vulkan/v3dv_device.c index fc2604f8596..16679d25570 100644 --- a/src/broadcom/vulkan/v3dv_device.c +++ b/src/broadcom/vulkan/v3dv_device.c @@ -423,7 +423,7 @@ get_features(const struct v3dv_physical_device *physical_device, .robustImageAccess = true, .robustBufferAccess2 = false, .robustImageAccess2 = true, - .nullDescriptor = false, + .nullDescriptor = true, .shaderIntegerDotProduct = true, /* VK_EXT_4444_formats */ @@ -2030,6 +2030,18 @@ v3dv_CreateDevice(VkPhysicalDevice physicalDevice, if (result != VK_SUCCESS) goto fail; + if (device->vk.enabled_features.nullDescriptor) { + device->null_bo = + v3dv_bo_alloc(device, 4096, "null texture data", true); + if (!device->null_bo || + !v3dv_bo_map(device, device->null_bo, + device->null_bo->size)) { + result = vk_error(device, VK_ERROR_OUT_OF_DEVICE_MEMORY); + goto fail; + } + memset(device->null_bo->map, 0, device->null_bo->size); + } + device->device_address_mem_ctx = ralloc_context(NULL); util_dynarray_init(&device->device_address_bo_list, device->device_address_mem_ctx); @@ -2066,6 +2078,7 @@ fail_queues_alloc: v3dv_pipeline_cache_finish(&device->default_pipeline_cache); v3dv_event_free_resources(device); v3dv_query_free_resources(device); + v3dv_bo_free(device, device->null_bo); vk_device_finish(&device->vk); vk_free(&device->vk.alloc, device); @@ -2099,6 +2112,11 @@ v3dv_DestroyDevice(VkDevice _device, device->default_attribute_float = NULL; } + if (device->null_bo) { + v3dv_bo_free(device, device->null_bo); + device->null_bo = NULL; + } + ralloc_free(device->device_address_mem_ctx); /* Bo cache should be removed the last, as any other object could be diff --git a/src/broadcom/vulkan/v3dv_device.h b/src/broadcom/vulkan/v3dv_device.h index a465fcd2f11..350e20956b4 100644 --- a/src/broadcom/vulkan/v3dv_device.h +++ b/src/broadcom/vulkan/v3dv_device.h @@ -395,6 +395,11 @@ struct v3dv_device { */ struct v3dv_bo *default_attribute_float; + /* When nullDescriptor is enabled, this BO provides valid zeroed memory + * for null descriptor paths. + */ + struct v3dv_bo *null_bo; + void *device_address_mem_ctx; struct util_dynarray device_address_bo_list; /* Array of struct v3dv_bo * */ }; diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 14894811b00..52cdf220425 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -1046,6 +1046,8 @@ pipeline_populate_v3d_key(struct v3d_key *key, p_stage->robustness.images == robust_image_enabled; key->robust_image_access_2 = p_stage->robustness.images == robust_image2_enabled; + key->null_descriptor = + p_stage->pipeline->device->vk.enabled_features.nullDescriptor; } uint32_t diff --git a/src/broadcom/vulkan/v3dv_uniforms.c b/src/broadcom/vulkan/v3dv_uniforms.c index f434170a915..70e1008f1ab 100644 --- a/src/broadcom/vulkan/v3dv_uniforms.c +++ b/src/broadcom/vulkan/v3dv_uniforms.c @@ -152,7 +152,11 @@ write_tmu_p0(struct v3dv_cmd_buffer *cmd_buffer, v3dv_descriptor_map_get_texture_bo(descriptor_state, &pipeline->shared_data->maps[stage]->texture_map, pipeline->layout, texture_idx); - assert(texture_bo); + assert(texture_bo || cmd_buffer->device->vk.enabled_features.nullDescriptor); + + if (!texture_bo) + texture_bo = cmd_buffer->device->null_bo; + assert(texture_idx < V3D_MAX_TEXTURE_SAMPLERS); tex_bos->tex[texture_idx] = texture_bo; @@ -201,11 +205,12 @@ write_tmu_p1(struct v3dv_cmd_buffer *cmd_buffer, v3dv_descriptor_map_get_sampler(descriptor_state, &pipeline->shared_data->maps[stage]->sampler_map, pipeline->layout, sampler_idx); - assert(sampler); + + assert(sampler || cmd_buffer->device->vk.enabled_features.nullDescriptor); /* Set unnormalized coordinates flag from sampler object */ uint32_t p1_packed = v3d_unit_data_get_offset(data); - if (sampler->unnormalized_coordinates) { + if (sampler && sampler->unnormalized_coordinates) { v3d_pack_unnormalized_coordinates(&cmd_buffer->device->devinfo, &p1_packed, sampler->unnormalized_coordinates); } @@ -308,15 +313,25 @@ write_ubo_ssbo_uniforms(struct v3dv_cmd_buffer *cmd_buffer, bo = reloc.bo; addr = reloc.bo->offset + reloc.offset + offset; } else { - assert(descriptor->buffer); - assert(descriptor->buffer->mem); - assert(descriptor->buffer->mem->bo); + /* This can happen when nullDescriptor is used. In that + * case the compiler will not emit the buffer access so + * the descriptor won't be accessed at all. + */ + if (!descriptor->buffer) { + assert(cmd_buffer->device->vk.enabled_features.nullDescriptor); + bo = NULL; + addr = 0; + } else { + assert(descriptor->buffer); + assert(descriptor->buffer->mem); + assert(descriptor->buffer->mem->bo); - bo = descriptor->buffer->mem->bo; - addr = bo->offset + - descriptor->buffer->mem_offset + - descriptor->offset + - offset + dynamic_offset; + bo = descriptor->buffer->mem->bo; + addr = bo->offset + + descriptor->buffer->mem_offset + + descriptor->offset + + offset + dynamic_offset; + } } cl_aligned_u32(uniforms, addr); @@ -364,6 +379,9 @@ get_texture_size_from_image_view(struct v3dv_image_view *image_view, enum quniform_contents contents, uint32_t data) { + if (!image_view) + return 0; + switch(contents) { case QUNIFORM_IMAGE_WIDTH: case QUNIFORM_TEXTURE_WIDTH: @@ -401,6 +419,9 @@ get_texture_size_from_buffer_view(struct v3dv_buffer_view *buffer_view, enum quniform_contents contents, uint32_t data) { + if (!buffer_view) + return 0; + switch(contents) { case QUNIFORM_IMAGE_WIDTH: case QUNIFORM_TEXTURE_WIDTH: diff --git a/src/broadcom/vulkan/v3dvx_cmd_buffer.c b/src/broadcom/vulkan/v3dvx_cmd_buffer.c index cd5dc167738..6200afd5394 100644 --- a/src/broadcom/vulkan/v3dvx_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dvx_cmd_buffer.c @@ -2612,12 +2612,16 @@ v3dX(cmd_buffer_emit_gl_shader_state)(struct v3dv_cmd_buffer *cmd_buffer) cl_emit_with_prepacked(&job->indirect, GL_SHADER_STATE_ATTRIBUTE_RECORD, &pipeline->vertex_attrs[i * packet_length], attr) { - - assert(c_vb->buffer->mem->bo); - attr.address = v3dv_cl_address(c_vb->buffer->mem->bo, - c_vb->buffer->mem_offset + - pipeline->va[i].offset + - c_vb->offset); + if (c_vb->buffer) { + assert(c_vb->buffer->mem->bo); + attr.address = v3dv_cl_address(c_vb->buffer->mem->bo, + c_vb->buffer->mem_offset + + pipeline->va[i].offset + + c_vb->offset); + } else { + assert(cmd_buffer->device->null_bo); + attr.address = v3dv_cl_address(cmd_buffer->device->null_bo, 0); + } attr.number_of_values_read_by_coordinate_shader = prog_data_vs_bin->vattr_sizes[location]; diff --git a/src/broadcom/vulkan/v3dvx_image.c b/src/broadcom/vulkan/v3dvx_image.c index a048e92881e..b411bb2872b 100644 --- a/src/broadcom/vulkan/v3dvx_image.c +++ b/src/broadcom/vulkan/v3dvx_image.c @@ -235,3 +235,22 @@ v3dX(pack_texture_shader_state_from_buffer_view)(struct v3dv_device *device, #endif } } + +void +v3dX(pack_null_texture_state)(struct v3dv_device *device, void *map) +{ + assert(device->null_bo); + const uint32_t base_offset = device->null_bo->offset; + + v3dvx_pack(map, TEXTURE_SHADER_STATE, tex) { + tex.image_width = 1; + tex.image_height = 1; + tex.image_depth = 1; + tex.texture_base_pointer = v3dv_cl_address(NULL, base_offset); +#if V3D_VERSION >= 71 + /* See comment in XML field definition for rationale of the shifts */ + tex.texture_base_pointer_cb = base_offset >> 6; + tex.texture_base_pointer_cr = base_offset >> 6; +#endif + } +} diff --git a/src/broadcom/vulkan/v3dvx_private.h b/src/broadcom/vulkan/v3dvx_private.h index d0cb1bf8f45..d77dd41db20 100644 --- a/src/broadcom/vulkan/v3dvx_private.h +++ b/src/broadcom/vulkan/v3dvx_private.h @@ -306,6 +306,8 @@ uint32_t v3dX(combined_image_sampler_texture_state_offset)(uint8_t plane); uint32_t v3dX(combined_image_sampler_sampler_state_offset)(uint8_t plane); +void v3dX(pack_null_texture_state)(struct v3dv_device *device, void *map); + /* General utils */ void