On Mon, Jan 15, 2018 at 7:11 AM, Pohjolainen, Topi < topi.pohjolai...@gmail.com> wrote:
> 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. > Meh. I think it's fairly obvious what's going on. I do sometimes insist on wraping == statements in () because a = b == c is something I find hard to read but a = b & c seems obvious to me. > > + } > > } > > > > 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
