[Mesa-dev] [PATCH 02/15] i965: Configure how to store *64*PASSTHRU vertex components

[Antía Puentes]

Hi Kenneth,

thanks for reviewing.

On mié, 2016-05-04 at 03:36 -0700, Kenneth Graunke wrote:
> On Thursday, April 28, 2016 1:40:32 PM PDT Antia Puentes wrote:
> > 
> > From the Broadwell specification, structure VERTEX_ELEMENT_STATE
> > description:
> > 
> >    "When SourceElementFormat is set to one of the *64*_PASSTHRU
> >     formats,  64-bit components are stored in the URB without any
> >     conversion. In this case, vertex elements must be written as 128
> >     or 256 bits, with VFCOMP_STORE_0 being used to pad the output
> >     as required. E.g., if R64_PASSTHRU is used to copy a 64-bit Red 
> component into
> > 
> >     the URB, Component 1 must be specified as VFCOMP_STORE_0 (with
> >     Components 2,3 set to VFCOMP_NOSTORE) in order to output a 128-bit
> >     vertex element, or Components 1-3 must be specified as VFCOMP_STORE_0
> >     in order to output a 256-bit vertex element. Likewise, use of
> >     R64G64B64_PASSTHRU requires Component 3 to be specified as 
> VFCOMP_STORE_0
> > 
> >     in order to output a 256-bit vertex element."
> > 
> > Uses 128-bits to write double and dvec2 vertex elements, and 256-bits for
> > dvec3 and dvec4 vertex elements.
> > 
> > Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> > Signed-off-by: Antia Puentes <apuentes at igalia.com>
> > ---
> >  src/mesa/drivers/dri/i965/gen8_draw_upload.c | 34 +++++++++++++++++++++++++
> +++
> > 
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/gen8_draw_upload.c b/src/mesa/
> drivers/dri/i965/gen8_draw_upload.c
> > 
> > index fe5ed35..14f7091 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_draw_upload.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_draw_upload.c
> > @@ -217,6 +217,40 @@ gen8_emit_vertices(struct brw_context *brw)
> >           break;
> >        }
> >  
> > +      /* From the BDW PRM, Volume 2d, page 586 (VERTEX_ELEMENT_STATE):
> > +       *
> > +       *     "When SourceElementFormat is set to one of the *64*_PASSTHRU
> > +       *     formats, 64-bit components are stored in the URB without any
> > +       *     conversion. In this case, vertex elements must be written as 
> 128
> > 
> > +       *     or 256 bits, with VFCOMP_STORE_0 being used to pad the output
> > +       *     as required. E.g., if R64_PASSTHRU is used to copy a 64-bit 
> Red
> > 
> > +       *     component into the URB, Component 1 must be specified as
> > +       *     VFCOMP_STORE_0 (with Components 2,3 set to VFCOMP_NOSTORE)
> > +       *     in order to output a 128-bit vertex element, or Components 1-3 
> must
> > 
> > +       *     be specified as VFCOMP_STORE_0 in order to output a 256-bit 
> vertex
> > 
> > +       *     element. Likewise, use of R64G64B64_PASSTHRU requires 
> Component 3
> > 
> > +       *     to be specified as VFCOMP_STORE_0 in order to output a 256-bit 
> vertex
> > 
> > +       *     element."
> > +       */
> The above comment seems to indicate that R64 needs component 1 set to
> STORE_0, and that looks to be missing.  I'm guessing you need to add:
> 
> > 
> > +      if (input->glarray->Doubles) {
> > +         switch (input->glarray->Size) {
> > +         case 0:
>                comp0 = BRW_VE1_COMPONENT_STORE_0;
>                /* fallthrough */
> 
> > 
> > +         case 1:
>                comp1 = BRW_VE1_COMPONENT_STORE_0;
>                /* fallthrough */
> 

I have not added them because comp0 and comp1 are initialized in the
code immediately above the switch I added. That already existing code
sets the default values for the components:

switch (input->glarray->Size) {
      case 0: comp0 = BRW_VE1_COMPONENT_STORE_0;
      case 1: comp1 = BRW_VE1_COMPONENT_STORE_0;
      case 2: comp2 = BRW_VE1_COMPONENT_STORE_0;
      case 3: comp3 = input->glarray->Integer ?
                       BRW_VE1_COMPONENT_STORE_1_INT
                     : BRW_VE1_COMPONENT_STORE_1_FLT;
         break;
      }

In my switch I just override the values that are not right for the
double-precision floating point cases.

> > +         case 2:
> > +            /*  Use 128-bits instead of 256-bits to write double and dvec2
> > +             *  vertex elements.
> > +             */
> > +            comp2 = BRW_VE1_COMPONENT_NOSTORE;
> > +            comp3 = BRW_VE1_COMPONENT_NOSTORE;
> > +            break;
> > +         case 3:
> > +            /* Pad the output using VFCOMP_STORE_0 as suggested
> > +             * by the BDW PRM.
> > +             */
> > +            comp3 = BRW_VE1_COMPONENT_STORE_0;
> > +         }
> > +      }
> > +
> >        OUT_BATCH((input->buffer << GEN6_VE0_INDEX_SHIFT) |
> >                  GEN6_VE0_VALID |
> >                  (format << BRW_VE0_FORMAT_SHIFT) |
> >