[Mesa-dev] [PATCH 2/2] nv50: allow using inline vertex data submit when gl_VertexID is used
Ilia Mirkin
imirkin at alum.mit.edu
Thu Nov 19 08:54:19 PST 2015
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.
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
More information about the mesa-dev
mailing list