On Tue, Jun 16, 2015 at 08:25:25PM +0100, Arun Siluvery wrote: > v3: Length defined in current definitions of LRM, LRR instructions is not correct. > Correct and use existing definitions and also move them out of command parser > placeholder to appropriate place. Not that it wasn't correct. Common form has to be use 0 for instructions whose length vary between platforms and then to fill in (X - 2) as required. (At least that is what we do in userspace.) > + /* WaRsRestoreWithPerCtxtBb:bdw,chv */ > + cmd[index++] = MI_LOAD_REGISTER_IMM(1); > + cmd[index++] = INSTPM; > + cmd[index++] = _MASKED_BIT_DISABLE(INSTPM_FORCE_ORDERING); > + > + flags = MI_ATOMIC_MEMORY_TYPE_GGTT | > + MI_ATOMIC_INLINE_DATA | > + MI_ATOMIC_CS_STALL | > + MI_ATOMIC_RETURN_DATA_CTL | > + MI_ATOMIC_MOVE; > + > + cmd[index++] = MI_ATOMIC(5) | flags; This is very inconsistent style, and indeed flags doesn't make the code any more readable than simply specifying each in the cmd. > + cmd[index++] = scratch_addr; > + cmd[index++] = 0; > + cmd[index++] = _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING); > + cmd[index++] = _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING); > + > + /* > + * BSpec says MI_LOAD_REGISTER_MEM, MI_LOAD_REGISTER_REG and > + * MI_BATCH_BUFFER_END instructions in this sequence need to be > + * in the same cacheline. To satisfy this case even if more WA are > + * added in future, pad current cacheline and start remaining sequence > + * in new cacheline. > + */ > + while (((unsigned long) (cmd + index) % CACHELINE_BYTES) != 0) > + cmd[index++] = MI_NOOP; > + > + cmd[index++] = MI_LOAD_REGISTER_MEM_GEN8 | > + MI_LRM_USE_GLOBAL_GTT | > + MI_LRM_ASYNC_MODE_ENABLE; Use brackets to force alignment when it's ugly like this. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
