The old code saves which vertex element is the first to use a VBO slot.
When VBOs are added to the buffer list, each VBO is added only for such
vertex elements, and not added for others. So the old and new code do
exactly the same thing but differently.
Marek
On Sun, May 20, 2018 at 11:40 AM, Mathias Fröhlich <
mathias.froehl...@gmx.net> wrote:
> Hi Marek,
>
> On Sunday, 20 May 2018 16:42:51 CEST Marek Olšák wrote:
> > Can you explain what the difference is between the old and new code?
>
> The old code used a bit for each vertex attribute that is uploaded, the new
> code uses a bit for each buffer binding. Vertex attributes refer to a
> buffer binding, but several vertex attributes may refer to the same
> buffer binding. Means the number of buffer bindings is <= the number of
> vertex
> elements. If the VAO stems from the vbo module or if the application
> cares to provide VAO's with as few buffer bindings as possible we may
> get an array of vertex attributes that all refer to a much smaller array
> of buffer bindings containing references to really distinct buffer objects.
> That is in the better case the number of buffer bindings is < the number
> of vertex
> elements. Or in the best case the number of buffer bindings is == 1 which
> is
> much less then the number of vertex elements.
> So, instead of calling radeon_add_to_buffer_list/amdgpu_cs_add_buffer
> for each vertex attribute that refers to the same buffer binding
> multiple times with the same buffer object, in effect trying to add the
> same
> buffer object multiple times and calling the mentioned functions multiple
> times making it recognize that this buffer object is already there
> multiple times, the new code already avoids this redundant calls
> as far as the information that the calls are redundant is available
> at this point.
>
> best
>
> Mathias
>
>
>
> >
> > Marek
> >
> > On Sat, May 19, 2018 at 4:30 AM, <mathias.froehl...@gmx.net> wrote:
> >
> > > From: Mathias Fröhlich <mathias.froehl...@web.de>
> > >
> > > Hi,
> > >
> > > Below a patch to radeonsi to get a small bit more out of the recent
> > > VAO changes.
> > > The change did not introduce regressions into piglit and some variant
> > > of deqp availble through piglit on a radeonsi system.
> > > Please review!
> > >
> > > best Mathias
> > >
> > >
> > > Instead of tracking which pipe_vertex_elements backing buffer objects
> > > have already been uploaded, do this for the pipe_vertex_buffers, which
> > > is the data structure that contains the buffer objects.
> > > That should slightly decrease some unnecessary load on calls to
> > > radeon_add_to_buffer_list.
> > >
> > > Signed-off-by: Mathias Fröhlich <mathias.froehl...@web.de>
> > > ---
> > > src/gallium/drivers/radeonsi/si_descriptors.c | 4 +++-
> > > src/gallium/drivers/radeonsi/si_state.c | 11 ++++++-----
> > > 2 files changed, 9 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
> > > b/src/gallium/drivers/radeonsi/si_descriptors.c
> > > index 1d14c9df1e..3e3a3a2164 100644
> > > --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> > > +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> > > @@ -1161,7 +1161,9 @@ bool si_upload_vertex_buffer_descriptors(struct
> > > si_context *sctx)
> > > desc[2] = num_records;
> > > desc[3] = velems->rsrc_word3[i];
> > >
> > > - if (first_vb_use_mask & (1 << i)) {
> > > + const unsigned vbo_bit = 1u << vbo_index;
> > > + if (first_vb_use_mask & vbo_bit) {
> > > + first_vb_use_mask ^= vbo_bit;
> > > radeon_add_to_buffer_list(sctx, sctx->gfx_cs,
> > > r600_resource(vb->buffer.
> > > resource),
> > > RADEON_USAGE_READ,
> > > RADEON_PRIO_VERTEX_BUFFER);
> > > diff --git a/src/gallium/drivers/radeonsi/si_state.c
> > > b/src/gallium/drivers/radeonsi/si_state.c
> > > index 675b1adbe6..4306cf42bf 100644
> > > --- a/src/gallium/drivers/radeonsi/si_state.c
> > > +++ b/src/gallium/drivers/radeonsi/si_state.c
> > > @@ -4323,10 +4323,11 @@ static void *si_create_vertex_elements(struct
> > > pipe_context *ctx,
> > > {
> > > struct si_screen *sscreen = (struct si_screen*)ctx->screen;
> > > struct si_vertex_elements *v = CALLOC_STRUCT(si_vertex_
> elements);
> > > - bool used[SI_NUM_VERTEX_BUFFERS] = {};
> > > + unsigned first_vb_use_mask = 0;
> > > int i;
> > >
> > > assert(count <= SI_MAX_ATTRIBS);
> > > + assert(SI_NUM_VERTEX_BUFFERS <= sizeof(unsigned)*CHAR_BIT);
> > > if (!v)
> > > return NULL;
> > >
> > > @@ -4356,10 +4357,7 @@ static void *si_create_vertex_elements(struct
> > > pipe_context *ctx,
> > > v->instance_divisor_is_fetched |= 1u
> << i;
> > > }
> > >
> > > - if (!used[vbo_index]) {
> > > - v->first_vb_use_mask |= 1 << i;
> > > - used[vbo_index] = true;
> > > - }
> > > + first_vb_use_mask |= (1u << vbo_index);
> > >
> > > desc = util_format_description(
> elements[i].src_format);
> > > first_non_void = util_format_get_first_non_
> > > void_channel(elements[i].src_format);
> > > @@ -4463,6 +4461,9 @@ static void *si_create_vertex_elements(struct
> > > pipe_context *ctx,
> > > S_008F0C_NUM_FORMAT(num_format) |
> > > S_008F0C_DATA_FORMAT(data_format);
> > > }
> > > +
> > > + v->first_vb_use_mask = first_vb_use_mask;
> > > +
> > > return v;
> > > }
> > >
> > > --
> > > 2.14.3
> > >
> > > _______________________________________________
> > > 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
