On Tuesday, June 02, 2015 11:41:56 PM Ben Widawsky wrote: > On Tue, 02 Jun 2015 20:32:13 -0700 > Kenneth Graunke <kenn...@whitecape.org> wrote: > > > On Tuesday, June 02, 2015 08:07:50 PM Ben Widawsky wrote: > > > I'm very confused here. It seems pretty clear that since the > > > command has been introduced with support for MOCS, MOCS lives at > > > bit 8 of dword 0 for all constant buffers. The error has existed > > > since forever AFAICT. > > > > > > No piglit regressions or fixes: > > > http://otc-mesa-ci.jf.intel.com/view/dev/job/bwidawsk/143/ > > > > > > I suspect the low bits are discarded by the hardware when using the > > > buffer offset (we were setting 1). I suppose it could potentially > > > impact perf, though I'd imagine the kernel is already doing the > > > right thing by default anyway. This would explain why the patch > > > doesn't really do anything behaviorally. > > > > > > NOTE: gen8+ should have no change at all since MOCS was always 0 > > > anyway. > > > > > > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > > > --- > > > src/mesa/drivers/dri/i965/gen7_vs_state.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c > > > b/src/mesa/drivers/dri/i965/gen7_vs_state.c index 278b3ec..f60dcb1 > > > 100644 --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c > > > +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c > > > @@ -42,13 +42,13 @@ gen7_upload_constant_state(struct brw_context > > > *brw, > > > int dwords = brw->gen >= 8 ? 11 : 7; > > > BEGIN_BATCH(dwords); > > > - OUT_BATCH(opcode << 16 | (dwords - 2)); > > > + OUT_BATCH(opcode << 16 | (mocs << 8) | (dwords - 2)); > > > OUT_BATCH(active ? stage_state->push_const_size : 0); > > > OUT_BATCH(0); > > > /* Pointer to the constant buffer. Covered by the set of state > > > flags > > > * from gen6_prepare_wm_contants > > > */ > > > - OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0); > > > + OUT_BATCH(active ? stage_state->push_const_offset : 0); > > > OUT_BATCH(0); > > > OUT_BATCH(0); > > > OUT_BATCH(0); > > > > Now I'm confused. > > > > On BDW, DW0 bits 14:8 are MOCS, but "must always be programmed to > > zero." On pre-BDW, DW0 bits 14:8 are clearly marked as "Reserved MBZ". > > > > The documentation for 3DSTATE_CONSTANT(Body) lists DWord 2 (of the > > body, aka DWord 3 of the 3DSTATE_CONSTANT_VS packet as a whole) bits > > 4:0 as "Constant Buffer Object Control State", for all pre-DevBDW > > platforms, which is MOCS for all four buffers. > > > > So the code seems correct as it was, based on the versions of > > documentation I've read. You must be looking at something different > > than I am... > > > > It did seem really strange to me. Everything I looked at for internal > docs seems to suggest DW0 was always MOCS (I am not looking at it now, > this is just from memory). Do me a favor if you haven't already... go > look at internal docs.
I actually was looking at the internal docs - specifically, 3DSTATE_CONSTANT_VS and 3DSTATE_CONSTANT(Body)[All]. > If you don't have time I can check again > tomorrow, but clearly my eyes must not be trustworthy. I've had been > looking at 3DSTATE_CONSTANT_PS. Earlier when I went to verify and > try to figure out how this could be wrong, I [foolishly] checked SNB > (see below). Anyway, here is everything I found from the PRMs which > suggest you're mostly right (though we agree the code is wrong for > BDW?). The code is fine for BDW. We OR 'mocs' into the wrong place, but we set it to zero so it doesn't actually have any effect. Looks a bit funny but works, and since MOCS is required to be zero...hard to really care... > PRE-SNB - Don't care > SNB DW0 (I didn't think SNB had MOCS...): > https://01.org/sites/default/files/documentation/snb_ihd_os_vol2_part1.pdf#page=286 > IVB (MBZ) > https://01.org/sites/default/files/documentation/ivb_ihd_os_vol2_part1.pdf#page=291 > HSW (MBZ) > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-commandrefrence-instructions_0.pdf#page=771 > BYT (MBZ) > https://01.org/sites/default/files/documentation/intel_os_gfx_prm_vol2_-_cmd_ref_instructions.pdf#page=35 > BDW (DW0 as you mentioned)
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
