On Fri, Dec 15, 2017 at 05:09:09PM -0800, Jason Ekstrand wrote:
> The Vulkan spec says:
>
> "pipelineBindPoint is a VkPipelineBindPoint indicating whether the
> descriptors will be used by graphics pipelines or compute pipelines.
> There is a separate set of bind points for each of graphics and
> compute, so binding one does not disturb the other."
>
> Up until now, we've been ignoring the pipeline bind point and had just
> one bind point for everything. This commit separates things out into
> separate bind points.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102897
> ---
> src/intel/vulkan/anv_cmd_buffer.c | 65
> ++++++++++++++++++++++++++---------
> src/intel/vulkan/anv_descriptor_set.c | 2 ++
> src/intel/vulkan/anv_private.h | 11 +++---
> src/intel/vulkan/genX_cmd_buffer.c | 24 +++++++------
> 4 files changed, 70 insertions(+), 32 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> b/src/intel/vulkan/anv_cmd_buffer.c
> index 636f515..9720e7e 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -124,12 +124,20 @@ anv_cmd_state_init(struct anv_cmd_buffer *cmd_buffer)
> }
>
> static void
> +anv_cmd_pipeline_state_finish(struct anv_cmd_buffer *cmd_buffer,
> + struct anv_cmd_pipeline_state *pipe_state)
> +{
> + for (uint32_t i = 0; i < ARRAY_SIZE(pipe_state->push_descriptors); i++)
> + vk_free(&cmd_buffer->pool->alloc, pipe_state->push_descriptors[i]);
> +}
> +
> +static void
> anv_cmd_state_finish(struct anv_cmd_buffer *cmd_buffer)
> {
> struct anv_cmd_state *state = &cmd_buffer->state;
>
> - for (uint32_t i = 0; i < ARRAY_SIZE(state->push_descriptors); i++)
> - vk_free(&cmd_buffer->pool->alloc, state->push_descriptors[i]);
> + anv_cmd_pipeline_state_finish(cmd_buffer, &state->gfx.base);
> + anv_cmd_pipeline_state_finish(cmd_buffer, &state->compute.base);
>
> for (uint32_t i = 0; i < MESA_SHADER_STAGES; i++)
> vk_free(&cmd_buffer->pool->alloc, state->push_constants[i]);
> @@ -495,6 +503,7 @@ void anv_CmdSetStencilReference(
>
> static void
> anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer *cmd_buffer,
> + VkPipelineBindPoint bind_point,
> struct anv_pipeline_layout *layout,
> uint32_t set_index,
> struct anv_descriptor_set *set,
> @@ -504,7 +513,14 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer
> *cmd_buffer,
> struct anv_descriptor_set_layout *set_layout =
> layout->set[set_index].layout;
>
> - cmd_buffer->state.descriptors[set_index] = set;
> + struct anv_cmd_pipeline_state *pipe_state;
> + if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> + pipe_state = &cmd_buffer->state.compute.base;
> + } else {
> + assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> + pipe_state = &cmd_buffer->state.gfx.base;
> + }
> + pipe_state->descriptors[set_index] = set;
>
> if (dynamic_offsets) {
> if (set_layout->dynamic_offset_count > 0) {
> @@ -514,9 +530,9 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer
> *cmd_buffer,
> /* Assert that everything is in range */
> assert(set_layout->dynamic_offset_count <= *dynamic_offset_count);
> assert(dynamic_offset_start + set_layout->dynamic_offset_count <=
> - ARRAY_SIZE(cmd_buffer->state.dynamic_offsets));
> + ARRAY_SIZE(pipe_state->dynamic_offsets));
>
> -
> typed_memcpy(&cmd_buffer->state.dynamic_offsets[dynamic_offset_start],
> + typed_memcpy(&pipe_state->dynamic_offsets[dynamic_offset_start],
> *dynamic_offsets, set_layout->dynamic_offset_count);
>
> *dynamic_offsets += set_layout->dynamic_offset_count;
> @@ -524,7 +540,13 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer
> *cmd_buffer,
> }
> }
>
> - cmd_buffer->state.descriptors_dirty |= set_layout->shader_stages;
> + if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> + cmd_buffer->state.descriptors_dirty |= VK_SHADER_STAGE_COMPUTE_BIT;
> + } else {
> + assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> + cmd_buffer->state.descriptors_dirty |=
> + set_layout->shader_stages & VK_SHADER_STAGE_ALL_GRAPHICS;
Should we put () around the right hand side? We seem to be using that
elsewhere.
> + }
> }
>
> void anv_CmdBindDescriptorSets(
> @@ -544,8 +566,8 @@ void anv_CmdBindDescriptorSets(
>
> for (uint32_t i = 0; i < descriptorSetCount; i++) {
> ANV_FROM_HANDLE(anv_descriptor_set, set, pDescriptorSets[i]);
> - anv_cmd_buffer_bind_descriptor_set(cmd_buffer, layout,
> - firstSet + i, set,
> + anv_cmd_buffer_bind_descriptor_set(cmd_buffer, pipelineBindPoint,
> + layout, firstSet + i, set,
> &dynamicOffsetCount,
> &pDynamicOffsets);
> }
> @@ -851,10 +873,19 @@ anv_cmd_buffer_get_depth_stencil_view(const struct
> anv_cmd_buffer *cmd_buffer)
>
> static struct anv_push_descriptor_set *
> anv_cmd_buffer_get_push_descriptor_set(struct anv_cmd_buffer *cmd_buffer,
> + VkPipelineBindPoint bind_point,
> uint32_t set)
> {
> + struct anv_cmd_pipeline_state *pipe_state;
> + if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> + pipe_state = &cmd_buffer->state.compute.base;
> + } else {
> + assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> + pipe_state = &cmd_buffer->state.gfx.base;
> + }
> +
> struct anv_push_descriptor_set **push_set =
> - &cmd_buffer->state.push_descriptors[set];
> + &pipe_state->push_descriptors[set];
>
> if (*push_set == NULL) {
> *push_set = vk_alloc(&cmd_buffer->pool->alloc,
> @@ -880,15 +911,14 @@ void anv_CmdPushDescriptorSetKHR(
> ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
> ANV_FROM_HANDLE(anv_pipeline_layout, layout, _layout);
>
> - assert(pipelineBindPoint == VK_PIPELINE_BIND_POINT_GRAPHICS ||
> - pipelineBindPoint == VK_PIPELINE_BIND_POINT_COMPUTE);
> assert(_set < MAX_SETS);
>
> const struct anv_descriptor_set_layout *set_layout =
> layout->set[_set].layout;
>
> struct anv_push_descriptor_set *push_set =
> - anv_cmd_buffer_get_push_descriptor_set(cmd_buffer, _set);
> + anv_cmd_buffer_get_push_descriptor_set(cmd_buffer,
> + pipelineBindPoint, _set);
> if (!push_set)
> return;
>
> @@ -958,8 +988,8 @@ void anv_CmdPushDescriptorSetKHR(
> }
> }
>
> - anv_cmd_buffer_bind_descriptor_set(cmd_buffer, layout, _set,
> - set, NULL, NULL);
> + anv_cmd_buffer_bind_descriptor_set(cmd_buffer, pipelineBindPoint,
> + layout, _set, set, NULL, NULL);
> }
>
> void anv_CmdPushDescriptorSetWithTemplateKHR(
> @@ -980,7 +1010,8 @@ void anv_CmdPushDescriptorSetWithTemplateKHR(
> layout->set[_set].layout;
>
> struct anv_push_descriptor_set *push_set =
> - anv_cmd_buffer_get_push_descriptor_set(cmd_buffer, _set);
> + anv_cmd_buffer_get_push_descriptor_set(cmd_buffer,
> + template->bind_point, _set);
> if (!push_set)
> return;
>
> @@ -997,6 +1028,6 @@ void anv_CmdPushDescriptorSetWithTemplateKHR(
> template,
> pData);
>
> - anv_cmd_buffer_bind_descriptor_set(cmd_buffer, layout, _set,
> - set, NULL, NULL);
> + anv_cmd_buffer_bind_descriptor_set(cmd_buffer, template->bind_point,
> + layout, _set, set, NULL, NULL);
> }
> diff --git a/src/intel/vulkan/anv_descriptor_set.c
> b/src/intel/vulkan/anv_descriptor_set.c
> index 629c041..63557ab 100644
> --- a/src/intel/vulkan/anv_descriptor_set.c
> +++ b/src/intel/vulkan/anv_descriptor_set.c
> @@ -891,6 +891,8 @@ VkResult anv_CreateDescriptorUpdateTemplateKHR(
> if (template == NULL)
> return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
>
> + template->bind_point = pCreateInfo->pipelineBindPoint;
> +
> if (pCreateInfo->templateType ==
> VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_DESCRIPTOR_SET_KHR)
> template->set = pCreateInfo->set;
>
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index e8d0d27..f1868a3 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1319,6 +1319,8 @@ struct anv_descriptor_template_entry {
> };
>
> struct anv_descriptor_update_template {
> + VkPipelineBindPoint bind_point;
> +
> /* The descriptor set this template corresponds to. This value is only
> * valid if the template was created with the templateType
> * VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_DESCRIPTOR_SET_KHR.
> @@ -1686,6 +1688,11 @@ struct anv_attachment_state {
> */
> struct anv_cmd_pipeline_state {
> struct anv_pipeline *pipeline;
> +
> + struct anv_descriptor_set *descriptors[MAX_SETS];
> + uint32_t dynamic_offsets[MAX_DYNAMIC_BUFFERS];
> +
> + struct anv_push_descriptor_set *push_descriptors[MAX_SETS];
> };
>
> /** State tracking for graphics pipeline
> @@ -1734,16 +1741,12 @@ struct anv_cmd_state {
> VkRect2D render_area;
> uint32_t restart_index;
> struct anv_vertex_binding vertex_bindings[MAX_VBS];
> - struct anv_descriptor_set * descriptors[MAX_SETS];
> - uint32_t
> dynamic_offsets[MAX_DYNAMIC_BUFFERS];
> VkShaderStageFlags push_constant_stages;
> struct anv_push_constants *
> push_constants[MESA_SHADER_STAGES];
> struct anv_state
> binding_tables[MESA_SHADER_STAGES];
> struct anv_state samplers[MESA_SHADER_STAGES];
> struct anv_dynamic_state dynamic;
>
> - struct anv_push_descriptor_set * push_descriptors[MAX_SETS];
> -
> /**
> * Whether or not the gen8 PMA fix is enabled. We ensure that, at the top
> * of any command buffer it is disabled by disabling it in
> EndCommandBuffer
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 994c996..75aa40a 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -1434,32 +1434,32 @@ cmd_buffer_alloc_push_constants(struct anv_cmd_buffer
> *cmd_buffer)
> }
>
> static const struct anv_descriptor *
> -anv_descriptor_for_binding(const struct anv_cmd_buffer *cmd_buffer,
> +anv_descriptor_for_binding(const struct anv_cmd_pipeline_state *pipe_state,
> const struct anv_pipeline_binding *binding)
> {
> assert(binding->set < MAX_SETS);
> const struct anv_descriptor_set *set =
> - cmd_buffer->state.descriptors[binding->set];
> + pipe_state->descriptors[binding->set];
> const uint32_t offset =
> set->layout->binding[binding->binding].descriptor_index;
> return &set->descriptors[offset + binding->index];
> }
>
> static uint32_t
> -dynamic_offset_for_binding(const struct anv_cmd_buffer *cmd_buffer,
> +dynamic_offset_for_binding(const struct anv_cmd_pipeline_state *pipe_state,
> const struct anv_pipeline *pipeline,
> const struct anv_pipeline_binding *binding)
> {
> assert(binding->set < MAX_SETS);
> const struct anv_descriptor_set *set =
> - cmd_buffer->state.descriptors[binding->set];
> + pipe_state->descriptors[binding->set];
>
> uint32_t dynamic_offset_idx =
> pipeline->layout->set[binding->set].dynamic_offset_start +
> set->layout->binding[binding->binding].dynamic_offset_index +
> binding->index;
>
> - return cmd_buffer->state.dynamic_offsets[dynamic_offset_idx];
> + return pipe_state->dynamic_offsets[dynamic_offset_idx];
> }
>
> static VkResult
> @@ -1567,7 +1567,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
> }
>
> const struct anv_descriptor *desc =
> - anv_descriptor_for_binding(cmd_buffer, binding);
> + anv_descriptor_for_binding(pipe_state, binding);
>
> switch (desc->type) {
> case VK_DESCRIPTOR_TYPE_SAMPLER:
> @@ -1643,7 +1643,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
> case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: {
> /* Compute the offset within the buffer */
> uint32_t dynamic_offset =
> - dynamic_offset_for_binding(cmd_buffer, pipeline, binding);
> + dynamic_offset_for_binding(pipe_state, pipeline, binding);
> uint64_t offset = desc->offset + dynamic_offset;
> /* Clamp to the buffer size */
> offset = MIN2(offset, desc->buffer->size);
> @@ -1724,7 +1724,7 @@ emit_samplers(struct anv_cmd_buffer *cmd_buffer,
> for (uint32_t s = 0; s < map->sampler_count; s++) {
> struct anv_pipeline_binding *binding = &map->sampler_to_descriptor[s];
> const struct anv_descriptor *desc =
> - anv_descriptor_for_binding(cmd_buffer, binding);
> + anv_descriptor_for_binding(pipe_state, binding);
>
> if (desc->type != VK_DESCRIPTOR_TYPE_SAMPLER &&
> desc->type != VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER)
> @@ -1848,7 +1848,8 @@ static void
> cmd_buffer_flush_push_constants(struct anv_cmd_buffer *cmd_buffer,
> VkShaderStageFlags dirty_stages)
> {
> - const struct anv_pipeline *pipeline = cmd_buffer->state.gfx.base.pipeline;
> + const struct anv_cmd_graphics_state *gfx_state = &cmd_buffer->state.gfx;
> + const struct anv_pipeline *pipeline = gfx_state->base.pipeline;
>
> static const uint32_t push_constant_opcodes[] = {
> [MESA_SHADER_VERTEX] = 21,
> @@ -1901,7 +1902,7 @@ cmd_buffer_flush_push_constants(struct anv_cmd_buffer
> *cmd_buffer,
> &bind_map->surface_to_descriptor[surface];
>
> const struct anv_descriptor *desc =
> - anv_descriptor_for_binding(cmd_buffer, binding);
> + anv_descriptor_for_binding(&gfx_state->base, binding);
>
> struct anv_address read_addr;
> uint32_t read_len;
> @@ -1917,7 +1918,8 @@ cmd_buffer_flush_push_constants(struct anv_cmd_buffer
> *cmd_buffer,
> assert(desc->type ==
> VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC);
>
> uint32_t dynamic_offset =
> - dynamic_offset_for_binding(cmd_buffer, pipeline,
> binding);
> + dynamic_offset_for_binding(&gfx_state->base,
> + pipeline, binding);
> uint32_t buf_offset =
> MIN2(desc->offset + dynamic_offset, desc->buffer->size);
> uint32_t buf_range =
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
