[Mesa-dev] [PATCH 2/2] nv50: allow using inline vertex data submit when gl_VertexID is used

Samuel Pitoiset samuel.pitoiset at gmail.com
Thu Nov 19 09:01:00 PST 2015



On 11/19/2015 05:54 PM, Ilia Mirkin wrote:
> This appears to all be correct. I assume you ran piglit on this and
> tried a few (esp older) games? I don't know of any offhand that hit
> the nv50_push logic, but I'd feel more at ease if you just tried a
> couple.
>

Yes, I did a full piglit run, there are no regressions but I didn't test 
any games. Which ones do you want me to test ?

> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>
> On Thu, Nov 19, 2015 at 3:51 AM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> The hardware can actually generates vertexid when vertices come from
>> a client-side buffer like when glDrawElements is used.
>>
>> This doesn't fix (or break) any piglit tests but it improves the
>> previous attempt of Ilia (c830d19 "nv50: avoid using inline vertex
>> data submit when gl_VertexID is used")
>>
>> The only disadvantage is that only works on G84+, but we don't really
>> care of that weird and old NV50 chipset.
>>
>> Changes from v2:
>>   - use NV84_3D previously introduced
>>   - reset gl_VertexID after the draw is done
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/gallium/drivers/nouveau/nv50/nv50_program.c    |  3 +-
>>   src/gallium/drivers/nouveau/nv50/nv50_program.h    |  2 +-
>>   src/gallium/drivers/nouveau/nv50/nv50_push.c       | 42 +++++++++++++++++++++-
>>   .../drivers/nouveau/nv50/nv50_state_validate.c     |  3 +-
>>   src/gallium/drivers/nouveau/nv50/nv50_vbo.c        | 11 +-----
>>   5 files changed, 46 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_program.c b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> index 48057d2..a4b8ddf 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> @@ -66,7 +66,6 @@ nv50_vertprog_assign_slots(struct nv50_ir_prog_info *info)
>>         case TGSI_SEMANTIC_VERTEXID:
>>            prog->vp.attrs[2] |= NV50_3D_VP_GP_BUILTIN_ATTR_EN_VERTEX_ID;
>>            prog->vp.attrs[2] |= NV50_3D_VP_GP_BUILTIN_ATTR_EN_VERTEX_ID_DRAW_ARRAYS_ADD_START;
>> -         prog->vp.vertexid = 1;
>>            continue;
>>         default:
>>            break;
>> @@ -383,6 +382,8 @@ nv50_program_translate(struct nv50_program *prog, uint16_t chipset,
>>      prog->max_gpr = MAX2(4, (info->bin.maxGPR >> 1) + 1);
>>      prog->tls_space = info->bin.tlsSpace;
>>
>> +   prog->vp.need_vertex_id = info->io.vertexId < PIPE_MAX_SHADER_INPUTS;
>> +
>>      if (prog->type == PIPE_SHADER_FRAGMENT) {
>>         if (info->prop.fp.writesDepth) {
>>            prog->fp.flags[0] |= NV50_3D_FP_CONTROL_EXPORTS_Z;
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_program.h b/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> index f001670..1de5122 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> @@ -76,9 +76,9 @@ struct nv50_program {
>>         ubyte psiz;        /* output slot of point size */
>>         ubyte bfc[2];      /* indices into varying for FFC (FP) or BFC (VP) */
>>         ubyte edgeflag;
>> -      ubyte vertexid;
>>         ubyte clpd[2];     /* output slot of clip distance[i]'s 1st component */
>>         ubyte clpd_nr;
>> +      bool need_vertex_id;
>>      } vp;
>>
>>      struct {
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_push.c b/src/gallium/drivers/nouveau/nv50/nv50_push.c
>> index f31eaa0..cbef95d 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_push.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_push.c
>> @@ -24,6 +24,10 @@ struct push_context {
>>      struct translate *translate;
>>
>>      bool primitive_restart;
>> +
>> +   bool need_vertex_id;
>> +   int32_t index_bias;
>> +
>>      uint32_t prim;
>>      uint32_t restart_index;
>>      uint32_t instance_id;
>> @@ -74,6 +78,11 @@ emit_vertices_i08(struct push_context *ctx, unsigned start, unsigned count)
>>
>>         size = ctx->vertex_words * nr;
>>
>> +      if (unlikely(ctx->need_vertex_id)) {
>> +         BEGIN_NV04(ctx->push, NV84_3D(VERTEX_ID_BASE), 1);
>> +         PUSH_DATA (ctx->push, *elts + ctx->index_bias);
>> +      }
>> +
>>         BEGIN_NI04(ctx->push, NV50_3D(VERTEX_DATA), size);
>>
>>         ctx->translate->run_elts8(ctx->translate, elts, nr, 0, ctx->instance_id,
>> @@ -107,6 +116,11 @@ emit_vertices_i16(struct push_context *ctx, unsigned start, unsigned count)
>>
>>         size = ctx->vertex_words * nr;
>>
>> +      if (unlikely(ctx->need_vertex_id)) {
>> +         BEGIN_NV04(ctx->push, NV84_3D(VERTEX_ID_BASE), 1);
>> +         PUSH_DATA (ctx->push, *elts + ctx->index_bias);
>> +      }
>> +
>>         BEGIN_NI04(ctx->push, NV50_3D(VERTEX_DATA), size);
>>
>>         ctx->translate->run_elts16(ctx->translate, elts, nr, 0, ctx->instance_id,
>> @@ -140,6 +154,11 @@ emit_vertices_i32(struct push_context *ctx, unsigned start, unsigned count)
>>
>>         size = ctx->vertex_words * nr;
>>
>> +      if (unlikely(ctx->need_vertex_id)) {
>> +         BEGIN_NV04(ctx->push, NV84_3D(VERTEX_ID_BASE), 1);
>> +         PUSH_DATA (ctx->push, *elts + ctx->index_bias);
>> +      }
>> +
>>         BEGIN_NI04(ctx->push, NV50_3D(VERTEX_DATA), size);
>>
>>         ctx->translate->run_elts(ctx->translate, elts, nr, 0, ctx->instance_id,
>> @@ -161,10 +180,18 @@ emit_vertices_i32(struct push_context *ctx, unsigned start, unsigned count)
>>   static void
>>   emit_vertices_seq(struct push_context *ctx, unsigned start, unsigned count)
>>   {
>> +   uint32_t elts = 0;
>> +
>>      while (count) {
>>         unsigned push = MIN2(count, ctx->packet_vertex_limit);
>>         unsigned size = ctx->vertex_words * push;
>>
>> +      if (unlikely(ctx->need_vertex_id)) {
>> +         /* For non-indexed draws, gl_VertexID goes up after each vertex. */
>> +         BEGIN_NV04(ctx->push, NV84_3D(VERTEX_ID_BASE), 1);
>> +         PUSH_DATA (ctx->push, elts++);
>> +      }
>> +
>>         BEGIN_NI04(ctx->push, NV50_3D(VERTEX_DATA), size);
>>
>>         ctx->translate->run(ctx->translate, start, push, 0, ctx->instance_id,
>> @@ -216,7 +243,14 @@ nv50_push_vbo(struct nv50_context *nv50, const struct pipe_draw_info *info)
>>
>>      ctx.push = nv50->base.pushbuf;
>>      ctx.translate = nv50->vertex->translate;
>> -   ctx.packet_vertex_limit = nv50->vertex->packet_vertex_limit;
>> +
>> +   ctx.need_vertex_id = nv50->screen->base.class_3d >= NV84_3D_CLASS &&
>> +      nv50->vertprog->vp.need_vertex_id && (nv50->vertex->num_elements < 32);
>> +   ctx.index_bias = info->index_bias;
>> +
>> +   /* For indexed draws, gl_VertexID must be emitted for every vertex. */
>> +   ctx.packet_vertex_limit =
>> +      ctx.need_vertex_id ? 1 : nv50->vertex->packet_vertex_limit;
>>      ctx.vertex_words = nv50->vertex->vertex_size;
>>
>>      assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
>> @@ -307,4 +341,10 @@ nv50_push_vbo(struct nv50_context *nv50, const struct pipe_draw_info *info)
>>         ctx.instance_id++;
>>         ctx.prim |= NV50_3D_VERTEX_BEGIN_GL_INSTANCE_NEXT;
>>      }
>> +
>> +   if (unlikely(ctx.need_vertex_id)) {
>> +      /* Reset gl_VertexID to prevent future indexed draws to be confused. */
>> +      BEGIN_NV04(ctx.push, NV84_3D(VERTEX_ID_BASE), 1);
>> +      PUSH_DATA (ctx.push, nv50->state.index_bias);
>> +   }
>>   }
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> index b6181ed..02a759c 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> @@ -503,8 +503,7 @@ static struct state_validate {
>>       { nv50_validate_samplers,      NV50_NEW_SAMPLERS },
>>       { nv50_stream_output_validate, NV50_NEW_STRMOUT |
>>                                      NV50_NEW_VERTPROG | NV50_NEW_GMTYPROG },
>> -    { nv50_vertex_arrays_validate, NV50_NEW_VERTEX | NV50_NEW_ARRAYS |
>> -                                   NV50_NEW_VERTPROG },
>> +    { nv50_vertex_arrays_validate, NV50_NEW_VERTEX | NV50_NEW_ARRAYS },
>>       { nv50_validate_min_samples,   NV50_NEW_MIN_SAMPLES },
>>   };
>>   #define validate_list_len (sizeof(validate_list) / sizeof(validate_list[0]))
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> index ac0c4d9..85878d5 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> @@ -294,8 +294,7 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
>>      uint64_t addrs[PIPE_MAX_ATTRIBS];
>>      uint32_t limits[PIPE_MAX_ATTRIBS];
>>      struct nouveau_pushbuf *push = nv50->base.pushbuf;
>> -   struct nv50_vertex_stateobj dummy = {};
>> -   struct nv50_vertex_stateobj *vertex = nv50->vertex ? nv50->vertex : &dummy;
>> +   struct nv50_vertex_stateobj *vertex = nv50->vertex;
>>      struct pipe_vertex_buffer *vb;
>>      struct nv50_vertex_element *ve;
>>      uint32_t mask;
>> @@ -303,14 +302,6 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
>>      unsigned i;
>>      const unsigned n = MAX2(vertex->num_elements, nv50->state.num_vtxelts);
>>
>> -   /* A vertexid is not generated for inline data uploads. Have to use a
>> -    * VBO. This check must come after the vertprog has been validated,
>> -    * otherwise vertexid may be unset.
>> -    */
>> -   assert(nv50->vertprog->translated);
>> -   if (nv50->vertprog->vp.vertexid)
>> -      nv50->vbo_push_hint = 0;
>> -
>>      if (unlikely(vertex->need_conversion))
>>         nv50->vbo_fifo = ~0;
>>      else
>> --
>> 2.6.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
-Samuel


More information about the mesa-dev mailing list