From 6afb8a9feccd8aaa9083b1a4bdbe6a924f26a1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Pi=C3=B1eiro?= Date: Mon, 15 Mar 2021 22:27:43 +0100 Subject: [PATCH] v3dv/pipeline: use broadcom_shader_stage as pipeline/variant stage type So we could avoid using gl_shader_stage plus a is_coord boolean, that only applies to VERTEX. Reviewed-by: Iago Toral Quiroga Part-of: --- src/broadcom/vulkan/v3dv_pipeline.c | 52 +++++++++++------------ src/broadcom/vulkan/v3dv_pipeline_cache.c | 8 ++-- src/broadcom/vulkan/v3dv_private.h | 44 +++++++++++++++---- 3 files changed, 62 insertions(+), 42 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 703f58d012a..5c6816c64b3 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -429,7 +429,8 @@ shader_module_compile_to_nir(struct v3dv_device *device, const struct spirv_to_nir_options spirv_options = default_spirv_options; nir = spirv_to_nir(spirv, stage->module->size / 4, spec_entries, num_spec_entries, - stage->stage, stage->entrypoint, + broadcom_shader_stage_to_gl(stage->stage), + stage->entrypoint, &spirv_options, nir_options); nir_validate_shader(nir, "after spirv_to_nir"); free(spec_entries); @@ -443,7 +444,7 @@ shader_module_compile_to_nir(struct v3dv_device *device, nir = nir_shader_clone(NULL, stage->module->nir); nir_validate_shader(nir, "nir module"); } - assert(nir->info.stage == stage->stage); + assert(nir->info.stage == broadcom_shader_stage_to_gl(stage->stage)); if (V3D_DEBUG & (V3D_DEBUG_NIR | v3d_debug_flag_for_shader_stage(stage->stage))) { @@ -1162,8 +1163,8 @@ pipeline_populate_v3d_vs_key(struct v3d_vs_key *key, * PIPE_PRIM_POINTS && v3d->rasterizer->base.point_size_per_vertex */ key->per_vertex_point_size = (topology == PIPE_PRIM_POINTS); - key->is_coord = p_stage->is_coord; - if (p_stage->is_coord) { + key->is_coord = p_stage->stage == BROADCOM_SHADER_VERTEX_BIN; + if (key->is_coord) { /* The only output varying on coord shaders are for transform * feedback. Set to 0 as VK_EXT_transform_feedback is not supported. */ @@ -1212,16 +1213,14 @@ pipeline_stage_create_vs_bin(const struct v3dv_pipeline_stage *src, return NULL; p_stage->pipeline = src->pipeline; - assert(src->stage == MESA_SHADER_VERTEX); - p_stage->stage = src->stage; + assert(src->stage == BROADCOM_SHADER_VERTEX); + p_stage->stage = BROADCOM_SHADER_VERTEX_BIN; p_stage->entrypoint = src->entrypoint; p_stage->module = src->module; p_stage->nir = nir_shader_clone(NULL, src->nir); p_stage->spec_info = src->spec_info; memcpy(p_stage->shader_sha1, src->shader_sha1, 20); - p_stage->is_coord = true; - return p_stage; } @@ -1240,8 +1239,7 @@ pipeline_stage_create_vs_bin(const struct v3dv_pipeline_stage *src, static bool upload_assembly(struct v3dv_device *device, struct v3dv_shader_variant *variant, - gl_shader_stage stage, - bool is_coord, + broadcom_shader_stage stage, const void *data, uint32_t size) { @@ -1252,14 +1250,16 @@ upload_assembly(struct v3dv_device *device, assert(variant->assembly_bo == NULL); switch (stage) { - case MESA_SHADER_VERTEX: - name = (is_coord == true) ? "coord_shader_assembly" : - "vertex_shader_assembly"; + case BROADCOM_SHADER_VERTEX: + name = "vertex_shader_assembly"; break; - case MESA_SHADER_FRAGMENT: + case BROADCOM_SHADER_VERTEX_BIN: + name = "vs_bin_shader_assembly"; + break; + case BROADCOM_SHADER_FRAGMENT: name = "fragment_shader_assembly"; break; - case MESA_SHADER_COMPUTE: + case BROADCOM_SHADER_COMPUTE: name = "compute_shader_assembly"; break; default: @@ -1299,7 +1299,7 @@ pipeline_hash_variant(const struct v3dv_pipeline_stage *p_stage, struct v3dv_pipeline *pipeline = p_stage->pipeline; _mesa_sha1_init(&ctx); - if (p_stage->stage == MESA_SHADER_COMPUTE) { + if (p_stage->stage == BROADCOM_SHADER_COMPUTE) { _mesa_sha1_update(&ctx, p_stage->shader_sha1, sizeof(p_stage->shader_sha1)); } else { /* We need to include both on the sha1 key as one could affect the other @@ -1351,8 +1351,7 @@ pipeline_check_spill_size(struct v3dv_pipeline *pipeline, */ struct v3dv_shader_variant * v3dv_shader_variant_create(struct v3dv_device *device, - gl_shader_stage stage, - bool is_coord, + broadcom_shader_stage stage, const unsigned char *variant_sha1, struct v3d_prog_data *prog_data, uint32_t prog_data_size, @@ -1371,13 +1370,12 @@ v3dv_shader_variant_create(struct v3dv_device *device, variant->ref_cnt = 1; variant->stage = stage; - variant->is_coord = is_coord; memcpy(variant->variant_sha1, variant_sha1, sizeof(variant->variant_sha1)); variant->prog_data_size = prog_data_size; variant->prog_data.base = prog_data; if (qpu_insts) { - if (!upload_assembly(device, variant, stage, is_coord, + if (!upload_assembly(device, variant, stage, qpu_insts, qpu_insts_size)) { ralloc_free(variant->prog_data.base); vk_free(&device->vk.alloc, variant); @@ -1464,7 +1462,7 @@ v3dv_get_shader_variant(struct v3dv_pipeline_stage *p_stage, p_stage->program_id); } - variant = v3dv_shader_variant_create(device, p_stage->stage, p_stage->is_coord, + variant = v3dv_shader_variant_create(device, p_stage->stage, variant_sha1, prog_data, v3d_prog_data_size(p_stage->stage), qpu_insts, qpu_insts_size, @@ -1619,7 +1617,7 @@ pipeline_lower_nir(struct v3dv_pipeline *pipeline, static uint32_t get_ucp_enable_mask(struct v3dv_pipeline_stage *p_stage) { - assert(p_stage->stage == MESA_SHADER_VERTEX); + assert(p_stage->stage == BROADCOM_SHADER_VERTEX); const nir_shader *shader = p_stage->nir; assert(shader); @@ -1644,7 +1642,7 @@ pipeline_stage_get_nir(struct v3dv_pipeline_stage *p_stage, p_stage->shader_sha1); if (nir) { - assert(nir->info.stage == p_stage->stage); + assert(nir->info.stage == broadcom_shader_stage_to_gl(p_stage->stage)); return nir; } @@ -1808,9 +1806,7 @@ pipeline_compile_graphics(struct v3dv_pipeline *pipeline, p_atomic_inc_return(&physical_device->next_program_id); p_stage->pipeline = pipeline; - p_stage->stage = stage; - if (stage == MESA_SHADER_VERTEX) - p_stage->is_coord = false; + p_stage->stage = gl_shader_stage_to_broadcom(stage); p_stage->entrypoint = sinfo->pName; p_stage->module = vk_shader_module_from_handle(sinfo->module); p_stage->spec_info = sinfo->pSpecializationInfo; @@ -1851,7 +1847,7 @@ pipeline_compile_graphics(struct v3dv_pipeline *pipeline, return VK_ERROR_OUT_OF_HOST_MEMORY; p_stage->pipeline = pipeline; - p_stage->stage = MESA_SHADER_FRAGMENT; + p_stage->stage = BROADCOM_SHADER_FRAGMENT; p_stage->entrypoint = "main"; p_stage->module = 0; p_stage->nir = b.shader; @@ -2938,7 +2934,7 @@ pipeline_compile_compute(struct v3dv_pipeline *pipeline, p_stage->program_id = p_atomic_inc_return(&physical_device->next_program_id); p_stage->pipeline = pipeline; - p_stage->stage = stage; + p_stage->stage = gl_shader_stage_to_broadcom(stage); p_stage->entrypoint = sinfo->pName; p_stage->module = vk_shader_module_from_handle(sinfo->module); p_stage->spec_info = sinfo->pSpecializationInfo; diff --git a/src/broadcom/vulkan/v3dv_pipeline_cache.c b/src/broadcom/vulkan/v3dv_pipeline_cache.c index e0514ed994d..ab1f71639af 100644 --- a/src/broadcom/vulkan/v3dv_pipeline_cache.c +++ b/src/broadcom/vulkan/v3dv_pipeline_cache.c @@ -319,14 +319,13 @@ shader_variant_create_from_blob(struct v3dv_device *device, { VkResult result; - gl_shader_stage stage = blob_read_uint32(blob); - bool is_coord = blob_read_uint8(blob); + broadcom_shader_stage stage = blob_read_uint32(blob); const unsigned char *variant_sha1 = blob_read_bytes(blob, 20); uint32_t prog_data_size = blob_read_uint32(blob); /* FIXME: as we include the stage perhaps we can avoid prog_data_size? */ - assert(prog_data_size == v3d_prog_data_size(stage)); + assert(prog_data_size == v3d_prog_data_size(broadcom_shader_stage_to_gl(stage))); const void *prog_data = blob_read_bytes(blob, prog_data_size); if (blob->overrun) @@ -362,7 +361,7 @@ shader_variant_create_from_blob(struct v3dv_device *device, ulist->data = ralloc_array(new_prog_data, uint32_t, ulist->count); memcpy(ulist->data, ulist_data_data, ulist_data_size); - return v3dv_shader_variant_create(device, stage, is_coord, + return v3dv_shader_variant_create(device, stage, variant_sha1, new_prog_data, prog_data_size, qpu_insts, qpu_insts_size, @@ -592,7 +591,6 @@ shader_variant_write_to_blob(const struct v3dv_shader_variant *variant, struct blob *blob) { blob_write_uint32(blob, variant->stage); - blob_write_uint8(blob, variant->is_coord); blob_write_bytes(blob, variant->variant_sha1, sizeof(variant->variant_sha1)); diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index 31fd48fff1d..b6af5a7fc46 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -273,6 +273,38 @@ typedef enum { #define BROADCOM_SHADER_STAGES (BROADCOM_SHADER_COMPUTE + 1) +/* Assumes that coordinate shaders will be custom-handled by the caller */ +static inline broadcom_shader_stage +gl_shader_stage_to_broadcom(gl_shader_stage stage) +{ + switch (stage) { + case MESA_SHADER_VERTEX: + return BROADCOM_SHADER_VERTEX; + case MESA_SHADER_FRAGMENT: + return BROADCOM_SHADER_FRAGMENT; + case MESA_SHADER_COMPUTE: + return BROADCOM_SHADER_COMPUTE; + default: + unreachable("Unknown gl shader stage"); + } +} + +static inline gl_shader_stage +broadcom_shader_stage_to_gl(broadcom_shader_stage stage) +{ + switch (stage) { + case BROADCOM_SHADER_VERTEX: + case BROADCOM_SHADER_VERTEX_BIN: + return MESA_SHADER_VERTEX; + case BROADCOM_SHADER_FRAGMENT: + return MESA_SHADER_FRAGMENT; + case BROADCOM_SHADER_COMPUTE: + return MESA_SHADER_COMPUTE; + default: + unreachable("Unknown broadcom shader stage"); + } +} + struct v3dv_pipeline_cache { struct vk_object_base base; @@ -1310,8 +1342,7 @@ vk_to_mesa_shader_stage(VkShaderStageFlagBits vk_stage) struct v3dv_shader_variant { uint32_t ref_cnt; - gl_shader_stage stage; - bool is_coord; + broadcom_shader_stage stage; /* key for the pipeline cache, it is p_stage shader_sha1 + v3d compiler * sha1 @@ -1349,11 +1380,7 @@ struct v3dv_shader_variant { struct v3dv_pipeline_stage { struct v3dv_pipeline *pipeline; - gl_shader_stage stage; - /* FIXME: is_coord only make sense if stage == MESA_SHADER_VERTEX. Perhaps - * a stage base/vs/fs as keys and prog_data? - */ - bool is_coord; + broadcom_shader_stage stage; const struct vk_shader_module *module; const char *entrypoint; @@ -1836,8 +1863,7 @@ v3dv_get_shader_variant(struct v3dv_pipeline_stage *p_stage, struct v3dv_shader_variant * v3dv_shader_variant_create(struct v3dv_device *device, - gl_shader_stage stage, - bool is_coord, + broadcom_shader_stage stage, const unsigned char *variant_sha1, struct v3d_prog_data *prog_data, uint32_t prog_data_size,