<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 15, 2018 at 7:11 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=102897" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=102897</a> > --- > 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; </div></div>Should we put () around the right hand side? We seem to be using that elsewhere. <div><div class="h5"></div></div></blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > + } > } > > 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 > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>