[Mesa-dev] [PATCH v3 28/43] anv/cmd_buffer: Add a padding to the vertex buffer

Alejandro Piñeiro apinheiro at igalia.com
Mon Oct 16 07:05:47 UTC 2017



On 15/10/17 12:28, Pohjolainen, Topi wrote:
> On Thu, Oct 12, 2017 at 08:38:17PM +0200, Jose Maria Casanova Crespo wrote:
>> 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
>> ---
>>  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.

Well, as mentioned on the commit message, we used as reference commit
7c8dfa78b98a12c1c5f74d11433c8554d4c90657, for consistency, and this one
added the padding when emitting the vertex buffer state. In any case, I
don't mind too much to try your alternative.

> 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? 

offsets are computed properly as far as we see, even for 16 bit scalars
and vec3 (that are the tricky stuff here).

> 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list