[Mesa-dev] [PATCH 08/15] i965: Reserve space for "Vertex Count" in GS outputs.

[Paul Berry]

On 30 November 2013 13:14, Kenneth Graunke <kenneth at whitecape.org> wrote:

> On 11/30/2013 10:29 AM, Paul Berry wrote:
> > On 12 November 2013 17:51, Kenneth Graunke <kenneth at whitecape.org
> > <mailto:kenneth at whitecape.org>> wrote:
> >
> >     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
> >     <mailto:kenneth at whitecape.org>>
> >     ---
> >      src/mesa/drivers/dri/i965/brw_vec4_gs.c | 6 ++++++
> >      1 file changed, 6 insertions(+)
> >
> >     diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> >     b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> >     index b52d646..e802c1e 100644
> >     --- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> >     +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> >     @@ -194,6 +194,12 @@ do_gs_prog(struct brw_context *brw,
> >            c.prog_data.output_vertex_size_hwords * 32 *
> >     gp->program.VerticesOut;
> >         output_size_bytes += 32 *
> >     c.prog_data.control_data_header_size_hwords;
> >
> >     +   /* Broadwell stores "Vertex Count" as a full 8 DWord (32 byte)
> >     URB output,
> >     +    * which comes before the conttrol header.
> >     +    */
> >     +   if (brw->gen >= 8)
> >     +      output_size_bytes += 32;
> >     +
> >         assert(output_size_bytes >= 1);
> >         if (output_size_bytes > GEN7_MAX_GS_URB_ENTRY_SIZE_BYTES)
> >            return false;
> >     --
> >     1.8.3.2
> >
> >
> > It looks like changes also need to be made to
> > vec4_gs_visitor::emit_urb_write_opcode() and
> > vec4_gs_visitor::emit_control_data_bits() to offset the vertex data and
> > control data by 32 bytes, and to vec4_gs_visitor::emit_thread_end() to
> > cause the appropriate data to be written to the vertex count.  Are those
> > changes elsewhere in the series?
>
> I handled that in the gen8_vec4_generator class (introduced in patch 12):
>
> The generate_urb_write() function reserves space:
>
> void
> gen8_vec4_generator::generate_urb_write(vec4_instruction *ir, bool vs)
> {
>    ...
>
>    int global_offset = ir->offset;
>    /* The GS needs to increment Global Offset by 1 to make room for the
> extra
>     * "Vertex Count" payload at the beginning.
>     */
>    if (!vs)
>       ++global_offset;
>
>    ...
> }
>
> and generate_gs_thread_end() does a SEND with EOT that writes Vertex Count
> at offset 0.
>
> It seems to work pretty well.  Sorry for the awkward split in the patch
> series.
>
> --Ken
>

Oh, I see.  I have to admit I'm really not a fan of this approach.  I worry
that future maintainers are going to make the same assumption that I made
(that the offset used by vec4_gs_visitor is the actual offset that's going
to be used by the generated instruction), and they're going to get really
confused.  It seems to me that it's not very much more effort to just use
the correct offset in vec4_gs_visitor in the first place.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131201/e627c373/attachment.html>