v3dv: handle multisample rasterization with empty framebuffers

If the framebuffer has no attachments then multisample rasterization
is enabled based on the rasterizationSamples multisample state of
the pipelines. It should be noted that since we don't support
the variableMultisampleRate feature, all pipelines in the same
subpass must have matching number of samples.

V3D requires that we specifically setup our frames to enable
multisampling or not, and we do this when we create jobs inside
a subpass. Since we create the first job for a subpass as soon as
the subpas starts, this is problematic: if we don't have any
attachments, we don't won't enable MSAA at this point, but later
on we might bind an MSAA pipeline, since pipelines can be bound
at any point in the lifespan of a command buffer.

Here, we fix this by testing if the first draw call in a job uses
an MSAA pipeline but the job the was setup to not use MSAA, and in
that case we re-start the job with MSAA enabled.

We also take care of a corner case that seems to be tested by CTS
where a framebuffer with no attachments doesn't bind any pipelines
with MSAA enabled (so according to the Vulkan spec, multisample
rasterization must be disabled) but the fragment shader in use
reads gl_SampleID (which enables per-sample shading). This would
lead to enabling per-sample shading with single-sample rasterization,
which doesn't make sense and makes the simulator complain, so we just
disable per-sample shading in that case.

Fixes:
dEQP-VK.pipeline.multisample.mixed_count.*

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6766>

src/broadcom/vulkan/v3dv_cmd_buffer.c

@@ -2484,6 +2484,10 @@ cmd_buffer_execute_inside_pass(struct v3dv_cmd_buffer *primary,
    if (primary->state.dirty & V3DV_CMD_DIRTY_OCCLUSION_QUERY)
       emit_occlusion_query(primary);
 
+   /* FIXME: if our primary job tiling doesn't enable MSSA but any of the
+    * pipelines used by the secondaries do, we need to re-start the primary
+    * job to enable MSAA. See cmd_buffer_restart_job_for_msaa_if_needed.
+    */
    for (uint32_t i = 0; i < cmd_buffer_count; i++) {
       V3DV_FROM_HANDLE(v3dv_cmd_buffer, secondary, cmd_buffers[i]);
 
@@ -2994,7 +2998,6 @@ bind_graphics_pipeline(struct v3dv_cmd_buffer *cmd_buffer,
    if (cmd_buffer->state.pipeline == pipeline)
       return;
 
-
    /* Enable always flush if we are blending to sRGB render targets. This
     * fixes test failures in:
     * dEQP-VK.pipeline.blend.format.r8g8b8a8_srgb.*
@@ -4011,16 +4014,8 @@ cmd_buffer_emit_draw(struct v3dv_cmd_buffer *cmd_buffer,
 static struct v3dv_job *
 cmd_buffer_pre_draw_split_job(struct v3dv_cmd_buffer *cmd_buffer)
 {
-   /* If we emitted a pipeline barrier right before this draw we won't have
-    * an active job. In that case, create a new job continuing the current
-    * subpass.
-    */
    struct v3dv_job *job = cmd_buffer->state.job;
-   if (!job) {
-      job = v3dv_cmd_buffer_subpass_resume(cmd_buffer,
-                                           cmd_buffer->state.subpass_idx);
-      return job;
-   }
+   assert(job);
 
    /* If the job has been flagged with 'always_flush' and it has already
     * recorded any draw calls then we need to start a new job for it.
@@ -4047,16 +4042,107 @@ cmd_buffer_pre_draw_split_job(struct v3dv_cmd_buffer *cmd_buffer)
    return job;
 }
 
+/**
+ * The Vulkan spec states:
+ *
+ *   "It is legal for a subpass to use no color or depth/stencil
+ *    attachments (...)  This kind of subpass can use shader side effects such
+ *    as image stores and atomics to produce an output. In this case, the
+ *    subpass continues to use the width, height, and layers of the framebuffer
+ *    to define the dimensions of the rendering area, and the
+ *    rasterizationSamples from each pipeline’s
+ *    VkPipelineMultisampleStateCreateInfo to define the number of samples used
+ *    in rasterization."
+ *
+ * We need to enable MSAA in the TILE_BINNING_MODE_CFG packet, which we
+ * emit when we start a new frame at the begining of a subpass. At that point,
+ * if the framebuffer doesn't have any attachments we won't enable MSAA and
+ * the job won't be valid in the scenario described by the spec.
+ *
+ * This function is intended to be called before a draw call and will test if
+ * we are in that scenario, in which case, it will restart the current job
+ * with MSAA enabled.
+ */
+static void
+cmd_buffer_restart_job_for_msaa_if_needed(struct v3dv_cmd_buffer *cmd_buffer)
+{
+   assert(cmd_buffer->state.job);
+
+   /* We don't support variableMultisampleRate so we know that all pipelines
+    * bound in the same subpass must have matching number of samples, so we
+    * can do this check only on the first draw call.
+    */
+   if (cmd_buffer->state.job->draw_count > 0)
+      return;
+
+   /* We only need to restart the frame if the pipeline requires MSAA but
+    * our frame tiling didn't enable it.
+    */
+   if (!cmd_buffer->state.pipeline->msaa ||
+       cmd_buffer->state.job->frame_tiling.msaa) {
+      return;
+   }
+
+   /* FIXME: Secondary command buffers don't start frames. Instead, they are
+    * recorded into primary jobs that start them. For secondaries, we should
+    * still handle this scenario, but we should do that when we record them
+    * into primaries by testing if any of the secondaries has multisampled
+    * draw calls in them, and then using that info to decide if we need to
+    * restart the primary job into which they are being recorded.
+    */
+   if (cmd_buffer->level != VK_COMMAND_BUFFER_LEVEL_PRIMARY)
+      return;
+
+   /* Drop the current job and restart it with MSAA enabled */
+   struct v3dv_job *old_job = cmd_buffer->state.job;
+   cmd_buffer->state.job = NULL;
+
+   struct v3dv_job *job = vk_zalloc(&cmd_buffer->device->alloc,
+                                    sizeof(struct v3dv_job), 8,
+                                    VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
+   if (!job) {
+      v3dv_flag_oom(cmd_buffer, NULL);
+      return;
+   }
+
+   v3dv_job_init(job, V3DV_JOB_TYPE_GPU_CL, cmd_buffer->device, cmd_buffer,
+                 cmd_buffer->state.subpass_idx);
+   cmd_buffer->state.job = job;
+
+   v3dv_job_start_frame(job,
+                        old_job->frame_tiling.width,
+                        old_job->frame_tiling.height,
+                        old_job->frame_tiling.layers,
+                        old_job->frame_tiling.render_target_count,
+                        old_job->frame_tiling.internal_bpp,
+                        true /* msaa */);
+
+   v3dv_job_destroy(old_job);
+}
+
 static void
 cmd_buffer_emit_pre_draw(struct v3dv_cmd_buffer *cmd_buffer)
 {
    assert(cmd_buffer->state.pipeline);
    assert(!(cmd_buffer->state.pipeline->active_stages & VK_SHADER_STAGE_COMPUTE_BIT));
 
+   /* If we emitted a pipeline barrier right before this draw we won't have
+    * an active job. In that case, create a new job continuing the current
+    * subpass.
+    */
+   struct v3dv_job *job = cmd_buffer->state.job;
+   if (!job) {
+      job = v3dv_cmd_buffer_subpass_resume(cmd_buffer,
+                                           cmd_buffer->state.subpass_idx);
+   }
+
+   /* Restart single sample job for MSAA pipeline if needed */
+   cmd_buffer_restart_job_for_msaa_if_needed(cmd_buffer);
+
    /* If the job is configured to flush on every draw call we need to create
     * a new job now.
     */
-   struct v3dv_job *job = cmd_buffer_pre_draw_split_job(cmd_buffer);
+   job = cmd_buffer_pre_draw_split_job(cmd_buffer);
    job->draw_count++;
 
    /* We may need to compile shader variants based on bound textures */

src/broadcom/vulkan/v3dv_pipeline.c

@@ -2238,6 +2238,9 @@ pack_cfg_bits(struct v3dv_pipeline *pipeline,
 {
    assert(sizeof(pipeline->cfg_bits) == cl_packet_length(CFG_BITS));
 
+   pipeline->msaa =
+      ms_info && ms_info->rasterizationSamples > VK_SAMPLE_COUNT_1_BIT;
+
    v3dv_pack(pipeline->cfg_bits, CFG_BITS, config) {
       config.enable_forward_facing_primitive =
          rs_info ? !(rs_info->cullMode & VK_CULL_MODE_FRONT_BIT) : false;
@@ -2263,8 +2266,7 @@ pack_cfg_bits(struct v3dv_pipeline *pipeline,
             rs_info->polygonMode == VK_POLYGON_MODE_POINT;
       }
 
-      config.rasterizer_oversample_mode =
-         ms_info && ms_info->rasterizationSamples > VK_SAMPLE_COUNT_1_BIT ? 1 : 0;
+      config.rasterizer_oversample_mode = pipeline->msaa ? 1 : 0;
 
       /* From the Vulkan spec:
        *
@@ -2528,8 +2530,24 @@ pack_shader_state_record(struct v3dv_pipeline *pipeline)
       shader.fragment_shader_uses_real_pixel_centre_w_in_addition_to_centroid_w2 =
          prog_data_fs->uses_center_w;
 
-      shader.enable_sample_rate_shading = pipeline->sample_rate_shading ||
-                                          prog_data_fs->force_per_sample_msaa;
+      /* The description for gl_SampleID states that if a fragment shader reads
+       * it, then we should automatically activate per-sample shading. However,
+       * the Vulkan spec also states that if a framebuffer has no attachments:
+       *
+       *    "The subpass continues to use the width, height, and layers of the
+       *     framebuffer to define the dimensions of the rendering area, and the
+       *     rasterizationSamples from each pipeline’s
+       *     VkPipelineMultisampleStateCreateInfo to define the number of
+       *     samples used in rasterization multisample rasterization."
+       *
+       * So in this scenario, if the pipeline doesn't enable multiple samples
+       * but the fragment shader accesses gl_SampleID we would be requested
+       * to do per-sample shading in single sample rasterization mode, which
+       * is pointless, so just disable it in that case.
+       */
+      shader.enable_sample_rate_shading =
+         pipeline->sample_rate_shading ||
+         (pipeline->msaa && prog_data_fs->force_per_sample_msaa);
 
       shader.any_shader_reads_hardware_written_primitive_id = false;
 

src/broadcom/vulkan/v3dv_private.h

@@ -1510,6 +1510,7 @@ struct v3dv_pipeline {
 
    enum v3dv_ez_state ez_state;
 
+   bool msaa;
    bool sample_rate_shading;
    uint32_t sample_mask;