On Thu, 22 Dec 2011 19:22:16 -0800, Paul Berry <stereotype...@gmail.com> wrote: > On 22 December 2011 16:54, Eric Anholt <e...@anholt.net> wrote: > > > Fixes almost all of the transform feedback piglit tests. Remaining > > are a few tests related to tesselation for > > quads/trifans/tristrips/polygons with flat shading. > > --- > > src/mesa/drivers/dri/i965/gen7_sol_state.c | 199 > > ++++++++++++++++++++++++++- > > 1 files changed, 191 insertions(+), 8 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/gen7_sol_state.c > > b/src/mesa/drivers/dri/i965/gen7_sol_state.c > > index 650f625..a5e28b6 100644 > > --- a/src/mesa/drivers/dri/i965/gen7_sol_state.c > > +++ b/src/mesa/drivers/dri/i965/gen7_sol_state.c
> > static void
> > -upload_sol_state(struct brw_context *brw)
> > +upload_3dstate_so_buffers(struct brw_context *brw)
> > +{
> > + struct intel_context *intel = &brw->intel;
> > + struct gl_context *ctx = &intel->ctx;
> > + /* BRW_NEW_VERTEX_PROGRAM */
> > + const struct gl_shader_program *vs_prog =
> > + ctx->Shader.CurrentVertexProgram;
> > + const struct gl_transform_feedback_info *linked_xfb_info =
> > + &vs_prog->LinkedTransformFeedback;
> > + struct gl_transform_feedback_object *xfb_obj =
> > + ctx->TransformFeedback.CurrentObject;
> >
>
> Can we have a "/* NEW_TRANSFORM_FEEDBACK */" comment here?
Why, yes!
> It looks like we're not setting "SO Buffer Object Control State". Is that
> ok? I'm not too familiar with memory object control states so I'm not
> sure, but it seemed to me that it might be sensible to mark the stream
> output as L3 cacheable.
0 is the "don't override my page tables" value. We don't set it
anywhere else, either.
> > +/**
> > + * Outputs the 3DSTATE_SO_DECL_LIST command.
> > + *
> > + * The data output is a series of 64-bit entries containing a SO_DECL per
> > + * stream. We only have one stream of rendering coming out of the GS
> > unit, so
> > + * we only emit stream 0 (low 16 bits) SO_DECLs.
> > + */
> > +static void
> > +upload_3dstate_so_decl_list(struct brw_context *brw,
> > + struct brw_vue_map *vue_map)
> > +{
> > + struct intel_context *intel = &brw->intel;
> > + struct gl_context *ctx = &intel->ctx;
> > + /* BRW_NEW_VERTEX_PROGRAM */
> > + const struct gl_shader_program *vs_prog =
> > + ctx->Shader.CurrentVertexProgram;
> > + /* NEW_TRANSFORM_FEEDBACK */
> > + const struct gl_transform_feedback_info *linked_xfb_info =
> > + &vs_prog->LinkedTransformFeedback;
> > + int i;
> > + uint16_t so_decl[128];
> >
>
> Can we add an assertion to verify that there is no danger of overflowing
> this array? I think STATIC_ASSERT(ARRAY_SIZE(so_decl) >=
> MAX_PROGRAM_OUTPUTS) ought to do the trick.
Done.
> > + /* Construct the list of SO_DECLs to be emitted. The formatting of the
> > + * command is feels strange -- each dword pair contains a SO_DECL per
> > stream.
> > + */
> > + for (i = 0; i < linked_xfb_info->NumOutputs; i++) {
> > + int buffer = linked_xfb_info->Outputs[i].OutputBuffer;
> > + uint16_t decl = 0;
> > + int vert_result = linked_xfb_info->Outputs[i].OutputRegister;
> > +
> > + buffer_mask |= 1 << buffer;
> > +
> > + decl |= buffer << SO_DECL_OUTPUT_BUFFER_SLOT_SHIFT;
> > + decl |= vue_map->vert_result_to_slot[vert_result] <<
> > + SO_DECL_REGISTER_INDEX_SHIFT;
> > + decl |= ((1 << linked_xfb_info->Outputs[i].NumComponents) - 1) <<
> > + SO_DECL_COMPONENT_MASK_SHIFT;
> > +
> > + /* FINISHME */
> > + assert(linked_xfb_info->Outputs[i].DstOffset ==
> > next_offset[buffer]);
> >
>
> FYI, this assertion should hold true until we implement
> ARB_transfrom_feedback3 (which allows holes in the transform feedback
> structure). I think Marek has some plans to implement that for Gallium
> (not sure of his timeframe though), so we may want to keep an eye out.
Thanks for the clarification there. I thought that was probably the
case (it was for our tests), but wasn't sure. For the record, I didn't
even look at the extension spec while writing the code, thanks to the
many testcases and your gen6 code to work from.
pgp0n4zBeaJo7.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
