[Mesa-dev] [Nouveau] [PATCH] nv50: avoid using inline vertex data submit when gl_VertexID is used

[Ilia Mirkin]

On Mon, Aug 24, 2015 at 11:57 AM, Tobias Klausmann
<tobias.johannes.klausmann at mni.thm.de> wrote:
>
>
> On 24.08.2015 17:51, Ilia Mirkin wrote:
>>
>> The hardware only generates vertexid when vertices come from a VBO. This
>> fixes:
>>
>>    vertexid-drawelements
>>    vertexid-drawarrays
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: "11.0" <mesa-stable at lists.freedesktop.org>
>> ---
>>   src/gallium/drivers/nouveau/nv50/nv50_program.c        | 1 +
>>   src/gallium/drivers/nouveau/nv50/nv50_program.h        | 1 +
>>   src/gallium/drivers/nouveau/nv50/nv50_state_validate.c | 3 ++-
>>   src/gallium/drivers/nouveau/nv50/nv50_vbo.c            | 8 ++++++++
>>   4 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> index 02dc367..eff4477 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> @@ -66,6 +66,7 @@ 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;
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> b/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> index 5d3ff56..f4e8e94 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_program.h
>> @@ -76,6 +76,7 @@ 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;
>>      } vp;
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> index b304a17..66dcf43 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> @@ -503,7 +503,8 @@ 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_vertex_arrays_validate, NV50_NEW_VERTEX | NV50_NEW_ARRAYS |
>> +                                   NV50_NEW_VERTPROG },
>>       { 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 600b973..fb4305f 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> @@ -301,6 +301,14 @@ 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
>
> LGTM!

Thanks. I was a little torn on whether to do it this way (which
penalizes someone switching vertex programs while keeping the vertex
setup the same), as opposed to do it the other way, which would
penalize every program that uses vertexid (by calling
nv50_vertex_arrays_validate twice for those).

Ideally I'd do it in a way that penalized neither, but that's just not
going to happen :)