On Thu, Oct 12, 2017 at 08:38:17PM +0200, Jose Maria Casanova Crespo wrote:
> From: Alejandro Piñeiro <apinhe...@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
> ---
> src/intel/vulkan/genX_cmd_buffer.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 43437c8eb0..f3ba0108f4 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -1958,6 +1958,11 @@ genX(cmd_buffer_flush_state)(struct anv_cmd_buffer
> *cmd_buffer)
> {
> struct anv_pipeline *pipeline = cmd_buffer->state.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 half_inputs_read = vs_prog_data->half_inputs_read;
> + const uint32_t elements_half = half_inputs_read >> VERT_ATTRIB_GENERIC0;
> +#endif
>
> uint32_t vb_emit = cmd_buffer->state.vb_dirty & pipeline->vb_used;
>
> @@ -1970,6 +1975,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_half > 0) * 2;
> +#else
> + 0;
> +#endif
>
> p = anv_batch_emitn(&cmd_buffer->batch, num_dwords,
> GENX(3DSTATE_VERTEX_BUFFERS));
> @@ -1999,9 +2015,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,
I can't help thinking that the padding should have been taken into account
by the time of allocation of the vertex buffer and marked there into ::size.
Also I'm thinking if offset needs a treatment as well. We use 32-bit formats
adn therefore the offset needs to be 32-bit aligned, right? Or does that get
guaranteed by something else (and therefore just add an assert here)?
Jason, any thoughts?
> #else
> - .EndAddress = { buffer->bo, buffer->offset + buffer->size - 1},
> + .EndAddress = { buffer->bo, buffer->offset + buffer->size +
> padding - 1},
> #endif
> };
>
> --
> 2.13.6
>
> _______________________________________________
> 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
