On Saturday, December 12, 2015 12:16:33 PM Jordan Justen wrote:
> On 2015-12-11 13:23:55, Kenneth Graunke wrote:
> > Based on a patch by Chris Forbes, but largely rewritten by Ken.
> >
> > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> > ---
> > src/mesa/drivers/dri/i965/brw_compiler.h | 20 ++++++-
> > src/mesa/drivers/dri/i965/brw_vue_map.c | 98
> > ++++++++++++++++++++++++++++++--
> > 2 files changed, 111 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h
> > b/src/mesa/drivers/dri/i965/brw_compiler.h
> > index 514788c..c9e0317 100644
> > --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> > +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> > @@ -442,7 +442,7 @@ struct brw_vue_map {
> > * additional processing is applied before storing them in the VUE), the
> > * value is -1.
> > */
> > - signed char varying_to_slot[BRW_VARYING_SLOT_COUNT];
> > + signed char varying_to_slot[VARYING_SLOT_TESS_MAX];
> >
> > /**
> > * Map from VUE slot to gl_varying_slot value. For slots that do not
> > @@ -451,12 +451,24 @@ struct brw_vue_map {
> > *
> > * For slots that are not in use, the value is BRW_VARYING_SLOT_PAD.
>
> This comment doesn't apply to tess, right?
>
> > */
> > - signed char slot_to_varying[BRW_VARYING_SLOT_COUNT];
> > + signed char slot_to_varying[VARYING_SLOT_TESS_MAX];
>
> So, for VS, the brw_varying_slot enum is used, and for tess
> VARYING_SLOT_TESS_MAX will be used?
Yeah. gl_varying_slot (0..VARYING_SLOT_MAX) is the core Mesa enum.
i965 extends it as brw_varying_slot by adding a few extra fields,
which other than PAD are only used on Gen4-5 IIRC. Tessellation also
extends the core Mesa enum with VARYING_SLOT_PATCH0...TESS_MAX.
This is really ugly: both enums overlap. But, we don't actually use
any of the brw_varying_slot values for tessellation, so it works out.
Ultimately, I'd like to clean up the core Mesa enums in a future patch.
0..VARYING_SLOT_MAX and VARYING_SLOT_PATCH0..VARYING_SLOT_TESS_MAX are
used to index different bitfields, and aren't really that related. In
fact, there are so many bits that a single GLbitfield64 can't address
them all, which is ugly. I'd like to separate them.
> Looking at VARYING_SLOT_TESS_MAX vs. BRW_VARYING_SLOT_COUNT, it seems
> pretty clear that VARYING_SLOT_TESS_MAX is >= BRW_VARYING_SLOT_COUNT.
> I was going to suggest a STATIC_ASSERT, but it doesn't really seem
> required.
Yep.
> >
> > /**
> > * Total number of VUE slots in use
> > */
> > int num_slots;
> > +
> > + /**
> > + * Number of per-patch VUE slots. Only valid for tessellation control
> > + * shader outputs and tessellation evaluation shader inputs.
> > + */
> > + int num_per_patch_slots;
> > +
> > + /**
> > + * Number of per-vertex VUE slots. Only valid for tessellation control
> > + * shader outputs and tessellation evaluation shader inputs.
> > + */
> > + int num_per_vertex_slots;
> > };
> >
> > void brw_print_vue_map(FILE *fp, const struct brw_vue_map *vue_map);
> > @@ -484,6 +496,10 @@ void brw_compute_vue_map(const struct brw_device_info
> > *devinfo,
> > GLbitfield64 slots_valid,
> > bool separate_shader);
> >
> > +void brw_compute_tess_vue_map(struct brw_vue_map *const vue_map,
> > + const GLbitfield64 slots_valid,
> > + const GLbitfield is_patch);
> > +
> > enum shader_dispatch_mode {
> > DISPATCH_MODE_4X1_SINGLE = 0,
> > DISPATCH_MODE_4X2_DUAL_INSTANCE = 1,
> > diff --git a/src/mesa/drivers/dri/i965/brw_vue_map.c
> > b/src/mesa/drivers/dri/i965/brw_vue_map.c
> > index 6cb3da4..469538b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vue_map.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vue_map.c
> > @@ -176,6 +176,77 @@ brw_compute_vue_map(const struct brw_device_info
> > *devinfo,
> > }
> >
> > vue_map->num_slots = separate ? slot + 1 : slot;
> > + vue_map->num_per_vertex_slots = 0;
> > + vue_map->num_per_patch_slots = 0;
> > +}
> > +
> > +/**
> > + * Compute the VUE map for tessellation control shader outputs and
> > + * tessellation evaluation shader inputs.
> > + */
> > +void
> > +brw_compute_tess_vue_map(struct brw_vue_map *vue_map,
> > + GLbitfield64 vertex_slots,
> > + GLbitfield patch_slots)
> > +{
> > + /* I don't think anything actually uses this... */
> > + vue_map->slots_valid = vertex_slots;
> > +
> > + vertex_slots &= ~(VARYING_BIT_TESS_LEVEL_OUTER |
> > + VARYING_BIT_TESS_LEVEL_INNER);
> > +
> > + /* Make sure that the values we store in vue_map->varying_to_slot and
> > + * vue_map->slot_to_varying won't overflow the signed chars that are
> > used
> > + * to store them. Note that since vue_map->slot_to_varying sometimes
> > holds
> > + * values equal to VARYING_SLOT_TESS_MAX , we need to ensure that
> > + * VARYNIG_SLOT_TESS_MAX is <= 127, not 128.
>
> typo
Fixed, thanks!
> > + */
> > + STATIC_ASSERT(VARYING_SLOT_TESS_MAX <= 127);
> > +
> > + for (int i = 0; i < VARYING_SLOT_TESS_MAX ; ++i) {
> > + vue_map->varying_to_slot[i] = -1;
> > + vue_map->slot_to_varying[i] = BRW_VARYING_SLOT_PAD;
> > + }
> > +
> > + int slot = 0;
> > +
> > + /* The first 8 DWords are reserved for the "Patch Header".
> > + *
> > + * VARYING_SLOT_TESS_LEVEL_OUTER / INNER live here, but the exact layout
> > + * depends on the domain type. They might not be in slots 0 and 1 as
> > + * described here, but pretending they're separate allows us to uniquely
> > + * identify them by distinct slot locations.
> > + */
> > + assign_vue_slot(vue_map, VARYING_SLOT_TESS_LEVEL_INNER, slot++);
> > + assign_vue_slot(vue_map, VARYING_SLOT_TESS_LEVEL_OUTER, slot++);
> > +
> > + /* XXX: SSO! :( What if you mix and match TCS/TES separate shaders, and
> > + * the number of per-patch varyings changes?
> > + */
> > +
> > + /* first assign per-patch varyings */
> > + while (patch_slots != 0) {
> > + const int varying = ffsll(patch_slots) - 1;
> > + if (vue_map->varying_to_slot[varying + VARYING_SLOT_PATCH0] == -1) {
> > + assign_vue_slot(vue_map, varying + VARYING_SLOT_PATCH0, slot++);
> > + }
> > + patch_slots &= ~BITFIELD64_BIT(varying);
> > + }
> > +
> > + /* apparently, including the patch header... */
> > + vue_map->num_per_patch_slots = slot;
> > +
> > + /* then assign per-vertex varyings for each vertex in our patch */
> > + while (vertex_slots != 0) {
> > + const int varying = ffsll(vertex_slots) - 1;
> > + if (vue_map->varying_to_slot[varying] == -1) {
> > + assign_vue_slot(vue_map, varying, slot++);
> > + }
> > + vertex_slots &= ~BITFIELD64_BIT(varying);
> > + }
> > +
> > + vue_map->num_per_vertex_slots = slot - vue_map->num_per_patch_slots;
> > + vue_map->num_slots = slot;
> > }
> >
> > static const char *
> > @@ -196,11 +267,28 @@ varying_name(brw_varying_slot slot)
> > void
> > brw_print_vue_map(FILE *fp, const struct brw_vue_map *vue_map)
> > {
> > - fprintf(fp, "VUE map (%d slots, %s)\n",
> > - vue_map->num_slots, vue_map->separate ? "SSO" : "non-SSO");
> > - for (int i = 0; i < vue_map->num_slots; i++) {
> > - fprintf(fp, " [%d] %s\n", i,
> > - varying_name(vue_map->slot_to_varying[i]));
> > + if (vue_map->num_per_vertex_slots > 0 || vue_map->num_per_patch_slots >
> > 0) {
> > + fprintf(fp, "PUE map (%d slots, %d/patch, %d/vertex, %s)\n",
>
> Patch URB Entry, right?
Yep.
> Should it be brw_compute_tess_pue_map?
I kind of hate the PUE name - I found myself just searching for "UE map"
to find either VUE or PUE maps. I may tidy this up later.
> Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
Thanks!
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
