From ce9896727424621eb641fb48610a70af37960d3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Pi=C3=B1eiro?= Date: Thu, 25 Mar 2021 13:30:55 +0100 Subject: [PATCH] v3dv: define a default attribute values with float type We are providing a BO with the default attribute values for the GL_SHADER_STATE_RECORD, that contains 16 vec4. Such default value for each vec4 is (0, 0, 0, 1). As the attribute format could be int or float, the "1" value needs to take into account the attribute format. But in the practice, the most common case is all floats. So we create one default attribute values BO assuming that all attributes will be floats, and we store it at v3dv_device and only create a new one if a int format type is defined. That allows to reduce the amount of BOs needed. Note that we could still try to reduce the amount of BOs used by the pipelines if we create a bigger BO, and we just play with the offsets. But as mentioned, that's not the usual, and would add an extra complexity,so it is not a priority right now. This makes the following test passing when disabling the pipeline cache support: dEQP-VK.api.object_management.max_concurrent.graphics_pipeline Reviewed-by: Iago Toral Quiroga Part-of: --- src/broadcom/vulkan/v3dv_cmd_buffer.c | 7 +++- src/broadcom/vulkan/v3dv_device.c | 7 ++++ src/broadcom/vulkan/v3dv_pipeline.c | 59 ++++++++++++++++++--------- src/broadcom/vulkan/v3dv_private.h | 19 ++++++++- 4 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index e7effa229ba..1b5c93927dd 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -3822,6 +3822,11 @@ emit_gl_shader_state(struct v3dv_cmd_buffer *cmd_buffer) pipeline->shared_data->variants[BROADCOM_SHADER_FRAGMENT]; struct v3dv_bo *assembly_bo = pipeline->shared_data->assembly_bo; + struct v3dv_bo *default_attribute_values = + pipeline->default_attribute_values != NULL ? + pipeline->default_attribute_values : + pipeline->device->default_attribute_float; + cl_emit_with_prepacked(&job->indirect, GL_SHADER_STATE_RECORD, pipeline->shader_state_record, shader) { @@ -3847,7 +3852,7 @@ emit_gl_shader_state(struct v3dv_cmd_buffer *cmd_buffer) shader.fragment_shader_uniforms_address = cmd_buffer->state.uniforms.fs; shader.address_of_default_attribute_values = - v3dv_cl_address(pipeline->default_attribute_values, 0); + v3dv_cl_address(default_attribute_values, 0); } /* Upload vertex element attributes (SHADER_STATE_ATTRIBUTE_RECORD) */ diff --git a/src/broadcom/vulkan/v3dv_device.c b/src/broadcom/vulkan/v3dv_device.c index 06846a06c29..09980e483ff 100644 --- a/src/broadcom/vulkan/v3dv_device.c +++ b/src/broadcom/vulkan/v3dv_device.c @@ -1462,6 +1462,8 @@ v3dv_CreateDevice(VkPhysicalDevice physicalDevice, v3dv_bo_cache_init(device); v3dv_pipeline_cache_init(&device->default_pipeline_cache, device, device->instance->default_pipeline_cache_enabled); + device->default_attribute_float = + v3dv_pipeline_create_default_attribute_values(device, NULL); *pDevice = v3dv_device_to_handle(device); @@ -1487,6 +1489,11 @@ v3dv_DestroyDevice(VkDevice _device, destroy_device_meta(device); v3dv_pipeline_cache_finish(&device->default_pipeline_cache); + if (device->default_attribute_float) { + v3dv_bo_free(device, device->default_attribute_float); + device->default_attribute_float = NULL; + } + /* Bo cache should be removed the last, as any other object could be * freeing their private bos */ diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 5ec734a8cba..35cf35592a8 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -2773,46 +2773,59 @@ get_attr_type(const struct util_format_description *desc) } static bool -create_default_attribute_values(struct v3dv_pipeline *pipeline, - const VkPipelineVertexInputStateCreateInfo *vi_info) +pipeline_has_integer_vertex_attrib(struct v3dv_pipeline *pipeline) +{ + for (uint8_t i = 0; i < pipeline->va_count; i++) { + if (vk_format_is_int(pipeline->va[i].vk_format)) + return true; + } + return false; +} + +/* @pipeline can be NULL. We assume in that case that all the attributes have + * a float format (we only create an all-float BO once and we reuse it with + * all float pipelines), otherwise we look at the actual type of each + * attribute used with the specific pipeline passed in. + */ +struct v3dv_bo * +v3dv_pipeline_create_default_attribute_values(struct v3dv_device *device, + struct v3dv_pipeline *pipeline) { uint32_t size = MAX_VERTEX_ATTRIBS * sizeof(float) * 4; + struct v3dv_bo *bo; - if (pipeline->default_attribute_values == NULL) { - pipeline->default_attribute_values = v3dv_bo_alloc(pipeline->device, size, - "default_vi_attributes", - true); + bo = v3dv_bo_alloc(device, size, "default_vi_attributes", true); - if (!pipeline->default_attribute_values) { - fprintf(stderr, "failed to allocate memory for the default " - "attribute values\n"); - return false; - } + if (!bo) { + fprintf(stderr, "failed to allocate memory for the default " + "attribute values\n"); + return NULL; } - bool ok = v3dv_bo_map(pipeline->device, - pipeline->default_attribute_values, size); + bool ok = v3dv_bo_map(device, bo, size); if (!ok) { fprintf(stderr, "failed to map default attribute values buffer\n"); return false; } - uint32_t *attrs = pipeline->default_attribute_values->map; - + uint32_t *attrs = bo->map; + uint8_t va_count = pipeline != NULL ? pipeline->va_count : 0; for (int i = 0; i < MAX_VERTEX_ATTRIBS; i++) { attrs[i * 4 + 0] = 0; attrs[i * 4 + 1] = 0; attrs[i * 4 + 2] = 0; - if (i < pipeline->va_count && vk_format_is_int(pipeline->va[i].vk_format)) { + VkFormat attr_format = + pipeline != NULL ? pipeline->va[i].vk_format : VK_FORMAT_UNDEFINED; + if (i < va_count && vk_format_is_int(attr_format)) { attrs[i * 4 + 3] = 1; } else { attrs[i * 4 + 3] = fui(1.0); } } - v3dv_bo_unmap(pipeline->device, pipeline->default_attribute_values); + v3dv_bo_unmap(device, bo); - return true; + return bo; } static void @@ -2986,8 +2999,14 @@ pipeline_init(struct v3dv_pipeline *pipeline, } } - if (!create_default_attribute_values(pipeline, vi_info)) - return VK_ERROR_OUT_OF_DEVICE_MEMORY; + if (pipeline_has_integer_vertex_attrib(pipeline)) { + pipeline->default_attribute_values = + v3dv_pipeline_create_default_attribute_values(pipeline->device, pipeline); + if (!pipeline->default_attribute_values) + return VK_ERROR_OUT_OF_DEVICE_MEMORY; + } else { + pipeline->default_attribute_values = NULL; + } return result; } diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index 334d7aa1f2b..e9140fe0e14 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -395,6 +395,13 @@ struct v3dv_device { struct v3dv_pipeline_cache default_pipeline_cache; + /* GL_SHADER_STATE_RECORD needs to speficy default attribute values. The + * following covers the most common case, that is all attributes format + * being float being float, allowing us to reuse the same BO for all + * pipelines matching this requirement. Pipelines that need integer + * attributes will create their own BO. + */ + struct v3dv_bo *default_attribute_float; VkPhysicalDeviceFeatures features; }; @@ -1706,8 +1713,12 @@ struct v3dv_pipeline { struct v3dv_pipeline_shared_data *shared_data; - /* FIXME: this bo is another candidate to data to be uploaded using a - * resource manager, instead of a individual bo + /* In general we can reuse v3dv_device->default_attribute_float, so note + * that the following can be NULL. + * + * FIXME: the content of this BO will be small, so it could be improved to + * be uploaded to a common BO. But as in most cases it will be NULL, it is + * not a priority. */ struct v3dv_bo *default_attribute_values; @@ -2000,6 +2011,10 @@ void v3dv_pipeline_cache_upload_pipeline(struct v3dv_pipeline *pipeline, struct v3dv_pipeline_cache *cache); +struct v3dv_bo * +v3dv_pipeline_create_default_attribute_values(struct v3dv_device *device, + struct v3dv_pipeline *pipeline); + void v3dv_shader_module_internal_init(struct v3dv_device *device, struct vk_shader_module *module, nir_shader *nir);