[Mesa-dev] [PATCH 04/12] i965: Add a mechanism for sending native primitives into the driver

[Kristian Høgsberg]

On Mon, Aug 11, 2014 at 08:21:29PM -0700, Ben Widawsky wrote:
> On Mon, Aug 11, 2014 at 05:29:34PM -0700, Kristian Høgsberg wrote:
> > The brw_draw_prims() function is the draw entry point into the driver,
> > and takes struct _mesa_prim for input.  We want to be able to feed
> > native primitives into the driver, and to that end we introduce
> > BRW_PRIM_OFFSET, which lets use describe geometry using the native
> > GEN primitive types.
> > 
> > Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.h |  2 +-
> >  src/mesa/drivers/dri/i965/brw_defines.h |  7 +++++++
> >  src/mesa/drivers/dri/i965/brw_draw.c    | 15 ++++++++++++---
> >  src/mesa/drivers/dri/i965/brw_vec4_gs.c |  3 ++-
> >  4 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> > index e23ee4e..7de9b64 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -1767,7 +1767,7 @@ gen8_emit_depth_stencil_hiz(struct brw_context *brw,
> >  void gen8_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt,
> >                     unsigned int level, unsigned int layer, enum gen6_hiz_op op);
> >  
> > -extern const GLuint prim_to_hw_prim[GL_TRIANGLE_STRIP_ADJACENCY+1];
> > +uint32_t get_hw_prim_for_gl_prim(int mode);
> >  
> >  void
> >  brw_setup_vec4_key_clip_info(struct brw_context *brw,
> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> > index 3564041..a519629 100644
> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > @@ -77,6 +77,13 @@
> >  #define _3DPRIM_LINESTRIP_CONT_BF 0x14
> >  #define _3DPRIM_TRIFAN_NOSTIPPLE  0x15
> >  
> > +/* We use this offset to be able to pass native primitive types in struct
> > + * _mesa_prim::mode.  Native primitive types are BRW_PRIM_OFFSET +
> > + * native_type, which should be different from all GL types and still fit in
> > + * the 8 bits avialable. */
> > +
> > +#define BRW_PRIM_OFFSET           0x80
> > +
> 
> I'm slightly confused about why you decided 8 bits is the cutoff, and
> why you picked 128 0x80 as the marker for native types.

8 bits is the size of the 'mode' bit field in struct _mesa_prim
(see src/mesa/vbo/vbo.h).

> >  #define BRW_ANISORATIO_2     0
> >  #define BRW_ANISORATIO_4     1
> >  #define BRW_ANISORATIO_6     2
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> > index 412c360..f84a2ad 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > @@ -55,7 +55,7 @@
> >  
> >  #define FILE_DEBUG_FLAG DEBUG_PRIMS
> >  
> > -const GLuint prim_to_hw_prim[GL_TRIANGLE_STRIP_ADJACENCY+1] = {
> > +static const GLuint prim_to_hw_prim[GL_TRIANGLE_STRIP_ADJACENCY+1] = {
> >     _3DPRIM_POINTLIST,
> >     _3DPRIM_LINELIST,
> >     _3DPRIM_LINELOOP,
> > @@ -86,6 +86,15 @@ static const GLenum reduced_prim[GL_POLYGON+1] = {
> >     GL_TRIANGLES
> >  };
> >  
> > +uint32_t
> 
> uint8_t based on what you said above? Could also add an assert for mode
> greater than 255 if you so desired

It doesn't really matter... I went with uint32_t because that's what
the variables the return value will be assigned to are.

> > +get_hw_prim_for_gl_prim(int mode)
> > +{
> > +   if (mode >= BRW_PRIM_OFFSET)
> > +      return mode - BRW_PRIM_OFFSET;
> > +   else
> > +      return prim_to_hw_prim[mode];
> > +}
> > +
> >  
> >  /* When the primitive changes, set a state bit and re-validate.  Not
> >   * the nicest and would rather deal with this by having all the
> > @@ -96,7 +105,7 @@ static void brw_set_prim(struct brw_context *brw,
> >                           const struct _mesa_prim *prim)
> >  {
> >     struct gl_context *ctx = &brw->ctx;
> > -   uint32_t hw_prim = prim_to_hw_prim[prim->mode];
> > +   uint32_t hw_prim = get_hw_prim_for_gl_prim(prim->mode);
> >  
> >     DBG("PRIM: %s\n", _mesa_lookup_enum_by_nr(prim->mode));
> >  
> > @@ -133,7 +142,7 @@ static void gen6_set_prim(struct brw_context *brw,
> >  
> >     DBG("PRIM: %s\n", _mesa_lookup_enum_by_nr(prim->mode));
> >  
> > -   hw_prim = prim_to_hw_prim[prim->mode];
> > +   hw_prim = get_hw_prim_for_gl_prim(prim->mode);
> >  
> >     if (hw_prim != brw->primitive) {
> >        brw->primitive = hw_prim;
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> > index 6428291..12cb943 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
> > @@ -217,7 +217,8 @@ do_gs_prog(struct brw_context *brw,
> >     /* URB entry sizes are stored as a multiple of 64 bytes. */
> >     c.prog_data.base.urb_entry_size = ALIGN(output_size_bytes, 64) / 64;
> >  
> > -   c.prog_data.output_topology = prim_to_hw_prim[gp->program.OutputType];
> > +   c.prog_data.output_topology =
> > +      get_hw_prim_for_gl_prim(gp->program.OutputType);
> >  
> >     brw_compute_vue_map(brw, &c.input_vue_map, c.key.input_varyings);
> >  
> 
> With those comments considered, both 3 & 4 are:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

Thanks for reviewing,

Kristian

> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center