[Mesa-dev] [PATCH 06/14] i965: Add new SIMD8 VS prog data flag

[Kenneth Graunke]

On Tuesday, October 28, 2014 04:25:05 PM Matt Turner wrote:
> On Tue, Oct 28, 2014 at 3:17 PM, Kristian Høgsberg <krh at bitplanet.net> 
wrote:
> > This flag signals that we have a SIMD8 VS shader so we can set up the
> > corresponding state accordingly.  This boils down to setting
> > the BDW+ SIMD8 enable bit in 3DSTATE_VS and making UBO and pull
> > constant buffers use dword pitch.
> >
> > Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.h          |  5 ++++-
> >  src/mesa/drivers/dri/i965/brw_defines.h          |  2 ++
> >  src/mesa/drivers/dri/i965/brw_gs_surface_state.c |  2 +-
> >  src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 10 ++++++++--
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  7 ++++---
> >  src/mesa/drivers/dri/i965/gen8_vs_state.c        |  2 ++
> >  6 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
> > index eb37e75..e7cd30f 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -543,6 +543,8 @@ struct brw_vec4_prog_data {
> >      * is the size of the URB entry used for output.
> >      */
> >     GLuint urb_entry_size;
> > +
> > +   bool simd8;
> 
> brw_vec4_prog_data is going to be the prog_data struct for SIMD8
> vertex shaders? :\
> 
> brw_gs_prog_data, which inherits brw_vec4_prog_data, has
> 
>    /**
>     * Dispatch mode, can be any of:
>     * GEN7_GS_DISPATCH_MODE_DUAL_OBJECT
>     * GEN7_GS_DISPATCH_MODE_DUAL_INSTANCE
>     * GEN7_GS_DISPATCH_MODE_SINGLE
>     */
>    int dispatch_mode;
> 
> Maybe it shouldn't hold the values of the things in the comment
> directly (since they're things like 2<<11) but shouldn't we pull this
> field out and have an enum or something?

I like that plan.  A bunch of stages can (or will be able to) run in 4x1 
(single), 4x2 dual object, 4x2 dual instance, or SIMD8 mode.

enum shader_dispatch_mode {
   DISPATCH_MODE_4X2_DUAL_OBJECT,
   DISPATCH_MODE_4X2_DUAL_INSTANCE,
   DISPATCH_MODE_4X1_SINGLE,
   DISPATCH_MODE_SIMD8,
};

The state upload code would use these values.

> >  const struct brw_tracked_state brw_gs_ubo_surfaces = {
> > diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > index 1cc96cf..24bc06d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > @@ -112,6 +112,7 @@ static void
> >  brw_upload_vs_pull_constants(struct brw_context *brw)
> >  {
> >     struct brw_stage_state *stage_state = &brw->vs.base;
> > +   bool dword_pitch;
> 
> I can't figure out the name of this variable. In most any context I
> would imagine 'dword_pitch' is an integer.
> 
> What does this mean, and can we name it something more descriptive?

See brw_create_constant_surface in brw_wm_surface_state.c.  For SIMD8 shader 
access, we configure the constant buffer SURFACE_STATE to use a pitch of 4, 
while for SIMD4x2 access, we configure it to use a pitch of 16 (vec4 size).

I believe Eric introduced the name, and it's been around for quite a while 
now.  Feel free to submit a patch to rename it.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141028/e8df35ad/attachment-0001.sig>