[Mesa-dev] [PATCH 05/19] etnaviv: Use only DRAW_INSTANCED on GC3000+

[Wladimir J. van der Laan]

On Wed, Nov 01, 2017 at 11:57:39AM +0100, Philipp Zabel wrote:
> On Mon, 2017-10-30 at 17:16 +0100, Wladimir J. van der Laan wrote:
> > The blob does this, as DRAW_INSTANCED can replace fully all the other
> > draw commands - the other path is only there for compatibility and
> > will go away (or at least rot to become buggy due to dis-use) in newer
> > hardware.
> > 
> > Preparation for GC7000 support.
> 
> This also changes behaviour for <= GC2000 in the indexed case, should
> this be mentioned in the commit message?

Yes, it does. I'll add it to the commit message.

I could also keep exactly the same behavior for old hw, but it didn't feel worth
the extra complexity because offsetting INDEX_ADDR does the same thing and we update
that state every time before an indexed draw, so as I see it this should not result
in more state per draw - correct me if wrong!

> > Signed-off-by: Wladimir J. van der Laan <laanwj at gmail.com>
> > ---
> >  src/gallium/drivers/etnaviv/etnaviv_context.c | 16 ++++++++++++----
> >  src/gallium/drivers/etnaviv/etnaviv_emit.h    | 21 +++++++++++++++++++++
> >  2 files changed, 33 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c b/src/gallium/drivers/etnaviv/etnaviv_context.c
> > index 65c20d2..5aa9c66 100644
> > --- a/src/gallium/drivers/etnaviv/etnaviv_context.c
> > +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
> > @@ -188,6 +188,8 @@ etna_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info)
> >           BUG("Index buffer upload failed.");
> >           return;
> >        }
> > +      /* Add start to index offset, when rendering indexed */
> > +      index_offset += info->start * info->index_size;
> >  
> >        ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = etna_resource(indexbuf)->bo;
> >        ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset;
> 
> So adding the start offset here makes up for always emitting a zero
> start offset with DRAW_INDEXED_PRIMITIVES below.

Yes.

> > @@ -273,10 +275,16 @@ etna_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info)
> >     /* First, sync state, then emit DRAW_PRIMITIVES or DRAW_INDEXED_PRIMITIVES */
> >     etna_emit_state(ctx);
> >  
> > -   if (info->index_size)
> > -      etna_draw_indexed_primitives(ctx->stream, draw_mode, info->start, prims, info->index_bias);
> > -   else
> > -      etna_draw_primitives(ctx->stream, draw_mode, info->start, prims);
> > +   if (ctx->specs.halti >= 2) {
> > +      /* On HALTI2+ (GC3000 and higher) only use instanced drawing commands, as the blob does */
> > +      etna_draw_instanced(ctx->stream, info->index_size, draw_mode, 1,
> > +         info->count, info->index_size ? info->index_bias : info->start);
> > +   } else {
> > +      if (info->index_size)
> > +         etna_draw_indexed_primitives(ctx->stream, draw_mode, 0, prims, info->index_bias);
> 
> Since this is the only place where etna_draw_indexed_primitives is
> called, should the unused start parameter be removed from this
> function?

Sounds good to me.

Regards,
Wladimir