[Mesa-dev] [PATCH v5 04/14] anv/cmd_buffer: Add a padding to the vertex buffer

[Jose Maria Casanova Crespo]

From: Alejandro Piñeiro <apinheiro at igalia.com>

As we are using 32-bit surface formats with 16-bit elements we can be
on a situation where a vertex element can poke over the buffer by 2
bytes. To avoid that we add a padding when flushing the state.

This is similar to what the i965 drivers prior to Haswell do, as they
use 4-component formats to fake 3-component formats, and add a padding
there too. See commit:
   7c8dfa78b98a12c1c5f74d11433c8554d4c90657

v2: (Jason Ekstrand)
    Increase by 2 the size returned by GetBufferMemoryRequirements
    when robust buffer access is enabled in a vertex buffer.
    Renamed half_inputs_read to inputs_read_16bit.
v3: Rebase minor changes (Chema Casanova)

Signed-off-by: Jose Maria Casanova Crespo <jmcasanova at igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
---
 src/intel/vulkan/anv_device.c      |  9 +++++++++
 src/intel/vulkan/genX_cmd_buffer.c | 20 ++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 7c8b768c58..1756cf5324 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -2115,6 +2115,15 @@ void anv_GetBufferMemoryRequirements(
         buffer->usage & VK_BUFFER_USAGE_STORAGE_BUFFER_BIT))
       pMemoryRequirements->size = align_u64(buffer->size, 4);
 
+   /* Vertex buffers with 16-bit values need a 2 bytes padding in some cases
+    * because they are read as 32-bit components. By adding 2 bytes to memory
+    * requirements size when robust buffer accesss is enabled the paddings we
+    * read would be outside of the VkBuffer but would not be outside "the
+    * memory range(s) bound to the buffer".
+    */
+   if (device->robust_buffer_access && (buffer->usage & VK_BUFFER_USAGE_VERTEX_BUFFER_BIT))
+      pMemoryRequirements->size += 2;
+
    pMemoryRequirements->memoryTypeBits = memory_types;
 }
 
diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
index ce546249b3..a6aeb1daf1 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2378,6 +2378,11 @@ genX(cmd_buffer_flush_state)(struct anv_cmd_buffer *cmd_buffer)
 {
    struct anv_pipeline *pipeline = cmd_buffer->state.gfx.base.pipeline;
    uint32_t *p;
+#if GEN_GEN >= 8
+   const struct brw_vs_prog_data *vs_prog_data = get_vs_prog_data(pipeline);
+   const uint64_t inputs_read_16bit = vs_prog_data->inputs_read_16bit;
+   const uint32_t elements_16bit = inputs_read_16bit >> VERT_ATTRIB_GENERIC0;
+#endif
 
    uint32_t vb_emit = cmd_buffer->state.gfx.vb_dirty & pipeline->vb_used;
 
@@ -2390,6 +2395,17 @@ genX(cmd_buffer_flush_state)(struct anv_cmd_buffer *cmd_buffer)
    if (vb_emit) {
       const uint32_t num_buffers = __builtin_popcount(vb_emit);
       const uint32_t num_dwords = 1 + num_buffers * 4;
+      /* ISL 16-bit formats do a 16-bit to 32-bit float conversion, so we need
+       * to use ISL 32-bit formats to avoid such conversion in order to support
+       * properly 16-bit formats. This means that the vertex element may poke
+       * over the end of the buffer by 2 bytes.
+       */
+      const unsigned padding =
+#if GEN_GEN >= 8
+         (elements_16bit > 0) * 2;
+#else
+      0;
+#endif
 
       p = anv_batch_emitn(&cmd_buffer->batch, num_dwords,
                           GENX(3DSTATE_VERTEX_BUFFERS));
@@ -2419,9 +2435,9 @@ genX(cmd_buffer_flush_state)(struct anv_cmd_buffer *cmd_buffer)
             .BufferStartingAddress = { buffer->bo, buffer->offset + offset },
 
 #if GEN_GEN >= 8
-            .BufferSize = buffer->size - offset
+            .BufferSize = buffer->size - offset + padding,
 #else
-            .EndAddress = { buffer->bo, buffer->offset + buffer->size - 1},
+            .EndAddress = { buffer->bo, buffer->offset + buffer->size + padding - 1},
 #endif
          };
 
-- 
2.14.3