On Tuesday, April 28, 2015 12:04:50 AM Jordan Justen wrote: > On 2015-04-27 19:02:38, Kenneth Graunke wrote: > > On Friday, April 24, 2015 04:33:43 PM Jordan Justen wrote: > > > + BEGIN_BATCH(dwords); > > > + OUT_BATCH(GPGPU_WALKER << 16 | (dwords - 2)); > > > > I was going to suggest splitting this into separate Gen8+ and Gen7 > > blocks, but now that I look at the code...these two are slightly > > different indirect handling, and the later one is just a DWord of MBZ, > > so...it's not really that different. I think what you have is fine :) > > Hmm. Maybe time to press my luck. :) > > In my other 20 patch series > "[PATCH v2 19/20] i965/cs: Upload brw_cs_state" > > We discussed this somewhat ugly code: > > + int dw = 0; > > + desc[dw++] = brw->cs.base.prog_offset; > > + if (brw->gen >= 8) > > + dw++; /* Kernel Start Pointer High */ > > + dw++; > > + dw++; > > + desc[dw++] = stage_state->bind_bo_offset; > > It turns out it eventually doesn't look quite so pointless to use the > dw var. Later, it would look like: > > http://cgit.freedesktop.org/~jljusten/mesa/tree/src/mesa/drivers/dri/i965/brw_cs.cpp?h=cs-27#n392 > > Basically, the structure is pretty similar, but an extra dword appears > for the high address in gen8. > > If it seems cleaner, I wouldn't mind splitting either or both of these > to be initialized in separate paths based on the gen. > > Does the link above change your opinion on the other patch? > > Thanks for your time, > > -Jordan
I guess that's fine. In the OUT_BATCH paradigm, we'd do things like:
if (brw->gen >= 8) {
OUT_RELOC64(...)
} else {
OUT_RELOC(...)
}
which effectively does the same thing. But here you're writing indirect
state, so you have to track dwords yourself.
How about just changing 'dw++' to something like:
desc[dw++] = 0; /* MBZ */
desc[dw++] = 0; /* Kernel Start Pointer High */
I feel like that makes it readily apparent that you're just filling in
DWords in order, and not doing anything fancy.
Thoughts?
--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
