v3dv: pipeline initialization fixes for disabled rasterization
When rasterization is disabled there are a number of CreateInfo
structs that should be ignored. We were managing this correctly
for some cases, but not all of them. Specifically, viewport state
must be ignored and we weren't doing that.
Fixes:
dEQP-VK.api.descriptor_set.descriptor_set_layout_lifetime.graphics
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6766>
diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c
index 7c5b626..0a9d790 100644
--- a/src/broadcom/vulkan/v3dv_cmd_buffer.c
+++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c
@@ -2439,13 +2439,8 @@
uint32_t dynamic_mask = src->mask;
uint32_t dirty = 0;
- /* See note on SetViewport. We follow radv approach to only allow to set
- * the number of viewports/scissors at pipeline creation time.
- */
- dest->viewport.count = src->viewport.count;
- dest->scissor.count = src->scissor.count;
-
if (!(dynamic_mask & V3DV_DYNAMIC_VIEWPORT)) {
+ dest->viewport.count = src->viewport.count;
if (memcmp(&dest->viewport.viewports, &src->viewport.viewports,
src->viewport.count * sizeof(VkViewport))) {
typed_memcpy(dest->viewport.viewports,
@@ -2460,6 +2455,7 @@
}
if (!(dynamic_mask & V3DV_DYNAMIC_SCISSOR)) {
+ dest->scissor.count = src->scissor.count;
if (memcmp(&dest->scissor.scissors, &src->scissor.scissors,
src->scissor.count * sizeof(VkRect2D))) {
typed_memcpy(dest->scissor.scissors,
@@ -2879,15 +2875,9 @@
assert(firstViewport < MAX_VIEWPORTS);
assert(total_count >= 1 && total_count <= MAX_VIEWPORTS);
- /* anv allows CmdSetViewPort to change how many viewports are being used,
- * while radv not, using the value set on the pipeline creation. spec
- * doesn't specify, but radv approach makes more sense, as CmdSetViewport
- * is intended to set dynamically a specific viewport, increasing the
- * number of viewport used seems like a non-defined collateral
- * effect. Would make sense to open a spec issue to clarify. For now, as we
- * only support one, it is not really important, but we follow radv
- * approach.
- */
+ if (state->dynamic.viewport.count < total_count)
+ state->dynamic.viewport.count = total_count;
+
if (!memcmp(state->dynamic.viewport.viewports + firstViewport,
pViewports, viewportCount * sizeof(*pViewports))) {
return;
@@ -2918,9 +2908,9 @@
assert(firstScissor + scissorCount >= 1 &&
firstScissor + scissorCount <= MAX_SCISSORS);
- /* See note on CmdSetViewport related to anv/radv differences about setting
- * total viewports used. Also applies to scissor.
- */
+ if (state->dynamic.scissor.count < firstScissor + scissorCount)
+ state->dynamic.scissor.count = firstScissor + scissorCount;
+
if (!memcmp(state->dynamic.scissor.scissors + firstScissor,
pScissors, scissorCount * sizeof(*pScissors))) {
return;
@@ -2935,6 +2925,9 @@
static void
emit_scissor(struct v3dv_cmd_buffer *cmd_buffer)
{
+ if (cmd_buffer->state.dynamic.viewport.count == 0)
+ return;
+
struct v3dv_dynamic_state *dynamic = &cmd_buffer->state.dynamic;
/* FIXME: right now we only support one viewport. viewporst[0] would work
@@ -3803,8 +3796,6 @@
}
if (*dirty & (V3DV_CMD_DIRTY_VIEWPORT | V3DV_CMD_DIRTY_SCISSOR)) {
- assert(cmd_buffer->state.dynamic.scissor.count > 0 ||
- cmd_buffer->state.dynamic.viewport.count > 0);
emit_scissor(cmd_buffer);
}
diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c
index d6d69f6..f1076b2 100644
--- a/src/broadcom/vulkan/v3dv_pipeline.c
+++ b/src/broadcom/vulkan/v3dv_pipeline.c
@@ -1623,91 +1623,84 @@
}
static void
-pipeline_init_dynamic_state(struct v3dv_pipeline *pipeline,
- const VkGraphicsPipelineCreateInfo *pCreateInfo)
+pipeline_init_dynamic_state(
+ struct v3dv_pipeline *pipeline,
+ const VkPipelineDynamicStateCreateInfo *pDynamicState,
+ const VkPipelineViewportStateCreateInfo *pViewportState,
+ const VkPipelineDepthStencilStateCreateInfo *pDepthStencilState,
+ const VkPipelineColorBlendStateCreateInfo *pColorBlendState,
+ const VkPipelineRasterizationStateCreateInfo *pRasterizationState)
{
pipeline->dynamic_state = default_dynamic_state;
struct v3dv_dynamic_state *dynamic = &pipeline->dynamic_state;
/* Create a mask of enabled dynamic states */
uint32_t dynamic_states = 0;
- if (pCreateInfo->pDynamicState) {
- uint32_t count = pCreateInfo->pDynamicState->dynamicStateCount;
+ if (pDynamicState) {
+ uint32_t count = pDynamicState->dynamicStateCount;
for (uint32_t s = 0; s < count; s++) {
dynamic_states |=
- v3dv_dynamic_state_mask(pCreateInfo->pDynamicState->pDynamicStates[s]);
+ v3dv_dynamic_state_mask(pDynamicState->pDynamicStates[s]);
}
}
/* For any pipeline states that are not dynamic, set the dynamic state
* from the static pipeline state.
- *
- * Notice that we don't let the number of viewports and scissort rects to
- * be set dynamically, so these are always copied from the pipeline state.
*/
- dynamic->viewport.count = pCreateInfo->pViewportState->viewportCount;
- if (!(dynamic_states & V3DV_DYNAMIC_VIEWPORT)) {
- assert(pCreateInfo->pViewportState);
+ if (pViewportState) {
+ if (!(dynamic_states & V3DV_DYNAMIC_VIEWPORT)) {
+ dynamic->viewport.count = pViewportState->viewportCount;
+ typed_memcpy(dynamic->viewport.viewports, pViewportState->pViewports,
+ pViewportState->viewportCount);
- typed_memcpy(dynamic->viewport.viewports,
- pCreateInfo->pViewportState->pViewports,
- pCreateInfo->pViewportState->viewportCount);
+ for (uint32_t i = 0; i < dynamic->viewport.count; i++) {
+ v3dv_viewport_compute_xform(&dynamic->viewport.viewports[i],
+ dynamic->viewport.scale[i],
+ dynamic->viewport.translate[i]);
+ }
+ }
- for (uint32_t i = 0; i < dynamic->viewport.count; i++) {
- v3dv_viewport_compute_xform(&dynamic->viewport.viewports[i],
- dynamic->viewport.scale[i],
- dynamic->viewport.translate[i]);
+ if (!(dynamic_states & V3DV_DYNAMIC_SCISSOR)) {
+ dynamic->scissor.count = pViewportState->scissorCount;
+ typed_memcpy(dynamic->scissor.scissors, pViewportState->pScissors,
+ pViewportState->scissorCount);
}
}
- dynamic->scissor.count = pCreateInfo->pViewportState->scissorCount;
- if (!(dynamic_states & V3DV_DYNAMIC_SCISSOR)) {
- typed_memcpy(dynamic->scissor.scissors,
- pCreateInfo->pViewportState->pScissors,
- pCreateInfo->pViewportState->scissorCount);
- }
-
- if (pCreateInfo->pDepthStencilState != NULL) {
+ if (pDepthStencilState) {
if (!(dynamic_states & V3DV_DYNAMIC_STENCIL_COMPARE_MASK)) {
dynamic->stencil_compare_mask.front =
- pCreateInfo->pDepthStencilState->front.compareMask;
+ pDepthStencilState->front.compareMask;
dynamic->stencil_compare_mask.back =
- pCreateInfo->pDepthStencilState->back.compareMask;
+ pDepthStencilState->back.compareMask;
}
if (!(dynamic_states & V3DV_DYNAMIC_STENCIL_WRITE_MASK)) {
- dynamic->stencil_write_mask.front =
- pCreateInfo->pDepthStencilState->front.writeMask;
- dynamic->stencil_write_mask.back =
- pCreateInfo->pDepthStencilState->back.writeMask;
+ dynamic->stencil_write_mask.front = pDepthStencilState->front.writeMask;
+ dynamic->stencil_write_mask.back = pDepthStencilState->back.writeMask;
}
if (!(dynamic_states & V3DV_DYNAMIC_STENCIL_REFERENCE)) {
- dynamic->stencil_reference.front =
- pCreateInfo->pDepthStencilState->front.reference;
- dynamic->stencil_reference.back =
- pCreateInfo->pDepthStencilState->back.reference;
+ dynamic->stencil_reference.front = pDepthStencilState->front.reference;
+ dynamic->stencil_reference.back = pDepthStencilState->back.reference;
}
}
- if (pCreateInfo->pColorBlendState &&
- !(dynamic_states & V3DV_DYNAMIC_BLEND_CONSTANTS)) {
- memcpy(dynamic->blend_constants,
- pCreateInfo->pColorBlendState->blendConstants,
+ if (pColorBlendState && !(dynamic_states & V3DV_DYNAMIC_BLEND_CONSTANTS)) {
+ memcpy(dynamic->blend_constants, pColorBlendState->blendConstants,
sizeof(dynamic->blend_constants));
}
- if (pCreateInfo->pRasterizationState &&
- !pCreateInfo->pRasterizationState->rasterizerDiscardEnable) {
- if (pCreateInfo->pRasterizationState->depthBiasEnable &&
+ if (pRasterizationState) {
+ if (pRasterizationState->depthBiasEnable &&
!(dynamic_states & V3DV_DYNAMIC_DEPTH_BIAS)) {
dynamic->depth_bias.constant_factor =
- pCreateInfo->pRasterizationState->depthBiasConstantFactor;
+ pRasterizationState->depthBiasConstantFactor;
dynamic->depth_bias.slope_factor =
- pCreateInfo->pRasterizationState->depthBiasSlopeFactor;
+ pRasterizationState->depthBiasSlopeFactor;
}
if (!(dynamic_states & V3DV_DYNAMIC_LINE_WIDTH))
- dynamic->line_width = pCreateInfo->pRasterizationState->lineWidth;
+ dynamic->line_width = pRasterizationState->lineWidth;
}
pipeline->dynamic_state.mask = dynamic_states;
@@ -2014,28 +2007,20 @@
static void
enable_depth_bias(struct v3dv_pipeline *pipeline,
- const VkGraphicsPipelineCreateInfo *pInfo)
+ const VkPipelineRasterizationStateCreateInfo *rs_info)
{
- assert(pInfo);
-
pipeline->depth_bias.enabled = false;
pipeline->depth_bias.is_z16 = false;
- if (!pInfo->pRasterizationState ||
- !pInfo->pRasterizationState->depthBiasEnable ||
- pInfo->pRasterizationState->rasterizerDiscardEnable)
+ if (!rs_info || !rs_info->depthBiasEnable)
return;
/* Check the depth/stencil attachment description for the subpass used with
* this pipeline.
*/
- struct v3dv_render_pass *pass =
- v3dv_render_pass_from_handle(pInfo->renderPass);
- assert(pass);
-
- assert(pInfo->subpass < pass->subpass_count);
- struct v3dv_subpass *subpass = &pass->subpasses[pInfo->subpass];
- assert(subpass);
+ assert(pipeline->pass && pipeline->subpass);
+ struct v3dv_render_pass *pass = pipeline->pass;
+ struct v3dv_subpass *subpass = pipeline->subpass;
if (subpass->ds_attachment.attachment == VK_ATTACHMENT_UNUSED)
return;
@@ -2367,14 +2352,15 @@
pipeline->pass = render_pass;
pipeline->subpass = &render_pass->subpasses[pCreateInfo->subpass];
- pipeline_init_dynamic_state(pipeline, pCreateInfo);
-
/* If rasterization is not enabled, various CreateInfo structs must be
* ignored.
*/
const bool raster_enabled =
!pCreateInfo->pRasterizationState->rasterizerDiscardEnable;
+ const VkPipelineViewportStateCreateInfo *vp_info =
+ raster_enabled ? pCreateInfo->pViewportState : NULL;
+
const VkPipelineDepthStencilStateCreateInfo *ds_info =
raster_enabled ? pCreateInfo->pDepthStencilState : NULL;
@@ -2384,6 +2370,10 @@
const VkPipelineColorBlendStateCreateInfo *cb_info =
raster_enabled ? pCreateInfo->pColorBlendState : NULL;
+ pipeline_init_dynamic_state(pipeline,
+ pCreateInfo->pDynamicState,
+ vp_info, ds_info, cb_info, rs_info);
+
/* V3D 4.2 doesn't support depth bounds testing so we don't advertise that
* feature and it shouldn't be used by any pipeline.
*/
@@ -2393,7 +2383,7 @@
pack_cfg_bits(pipeline, ds_info, rs_info);
pack_stencil_cfg(pipeline, ds_info);
pipeline_set_ez_state(pipeline, ds_info);
- enable_depth_bias(pipeline, pCreateInfo);
+ enable_depth_bias(pipeline, rs_info);
pipeline->primitive_restart =
pCreateInfo->pInputAssemblyState->primitiveRestartEnable;
