On Thursday, July 09, 2015 11:00:40 AM Ben Widawsky wrote: > While implementing the workaround in the previous patch I noticed things were > starting to get a bit messy. Since gen8 works differently enough from gen7, I > thought splitting it out with be good.
IMHO this is still a bit messy. What about separating the packet
emission from the decision-making about which of the 4 buffers to use?
Something along these lines (warning: doesn't compile):
#define OUT_RELOC_NULL(x) if (x != 0) { OUT_RELOC(x) } else { OUT_BATCH(x); }
#define OUT_RELOC64_NULL(x) if (x != 0) { OUT_RELOC64(x) } else { OUT_BATCH(x);
}
static inline void
emit_3dstate_constant(struct brw_context *brw,
uint32_t opcode,
uint32_t mocs,
uint16_t read_length_0,
uint16_t read_length_1,
uint16_t read_length_2,
uint16_t read_length_3,
uint64_t ptr_0,
uint64_t ptr_1,
uint64_t ptr_2,
uint64_t ptr_3)
{
// XXX: or in mocs wherever it goes
if (brw->gen >= 8) {
BEGIN_BATCH(11);
OUT_BATCH(opcode << 16 | (11 - 2));
OUT_BATCH(read_length_0 | read_length_1 << 16);
OUT_BATCH(read_length_2 | read_length_3 << 16);
OUT_BATCH64(ptr_0);
OUT_RELOC64_NULL(ptr_1);
OUT_RELOC64_NULL(ptr_2);
OUT_RELOC64_NULL(ptr_3);
ADVANCE_BATCH();
} else if (brw->gen == 7) {
/* XXX: we could even put asserts here about the buffers being enabled
* in order, i.e. if you use 2 you have to use 0 and 1 also
*/
BEGIN_BATCH(7);
OUT_BATCH(opcode << 16 | (11 - 2));
OUT_BATCH(read_length_0 | read_length_1 << 16);
OUT_BATCH(read_length_2 | read_length_3 << 16);
OUT_BATCH(ptr_0);
OUT_RELOC_NULL(ptr_1);
OUT_RELOC_NULL(ptr_2);
OUT_RELOC_NULL(ptr_3);
ADVANCE_BATCH();
} else if (brw->gen == 6) {
/* XXX: could probably do gen6 here too */
} else {
unreachable("unhandled gen in emit_3dstate_constant");
}
}
void
gen7_upload_constant_state(struct brw_context *brw,
const struct brw_stage_state *stage_state,
bool active, unsigned opcode)
{
uint32_t mocs = brw->gen < 8 ? GEN7_MOCS_L3 : 0;
/* Disable if the shader stage is inactive or there are no push constants. */
active = active && stage_state->push_const_size != 0;
if (!active) {
emit_3dstate_constant(brw, opcode, mocs, 0, 0, 0, 0, 0, 0, 0, 0);
} else if (brw->gen >= 9) {
/* Workaround for SKL+ (we use option #2 until we have a need for more
* constant buffers). This comes from the documentation for
3DSTATE_CONSTANT_*
*
* The driver must ensure The following case does not occur without a
flush
* to the 3D engine: 3DSTATE_CONSTANT_* with buffer 3 read length equal to
* zero committed followed by a 3DSTATE_CONSTANT_* with buffer 0 read
length
* not equal to zero committed. Possible ways to avoid this condition
* include:
* 1. always force buffer 3 to have a non zero read length
* 2. always force buffer 0 to a zero read length
*/
emit_3dstate_constant(brw, opcode, mocs,
0, stage_state->push_const_size, 0, 0,
0, stage_state->push_const_offset, 0, 0);
} else {
emit_3dstate_constant(brw, opcode, mocs,
stage_state->push_const_size, 0, 0, 0,
stage_state->push_const_offset, 0, 0, 0);
}
/* On SKL+ the new constants don't take effect until the next corresponding
* 3DSTATE_BINDING_TABLE_POINTER_* command is parsed so we need to ensure
* that is sent
*/
if (brw->gen >= 9)
brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
}
By using a static inline, all the code for unused buffers *should* get
compiled away, seeing as it's all 0. We might have to stuff it in a .h
file, or put all the gen6+ constbuf stuff in a single file, i.e.
gen6_push_constants.c.
Anyway, just an idea...
--Ken
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
