On Wed, Feb 15, 2017 at 04:06:33PM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > Use the "*batch++ = " style as in the ring emission for better > readability and also simplify the logic a bit by consolidating > the offset and size calculations and overflow checking. The > latter is a programming error so it is not required to check > for it after each write to the object, but rather do it once the > whole state has been written and fail the driver if something > went wrong. > > v2: Rebase. I did not spot any mistakes in the mechanical translations. > + /* > + * Emit the two workaround batch buffers, recording the offset from the > + * start of the workaround batch buffer object for each and their > + * respective sizes. > + */ > + for (i = 0; i < ARRAY_SIZE(wa_bb_f); i++) { > + wa_bb[i]->offset = ALIGN(batch_ptr - batch, CACHELINE_DWORDS); > + batch_ptr = wa_bb_f[i](engine, batch_ptr); I'm just a bit more familar with using _fn for function pointers. > + wa_bb[i]->size = batch_ptr - &batch[wa_bb[i]->offset]; Surprised me that we store the offset/size in dwords not bytes. And then convert to bytes to store in the registers. > } > > -out: > + BUG_ON(batch_ptr - batch > CTX_WA_BB_OBJ_SIZE / sizeof(u32)); Would it make sense going forward just use void *batch, batch_ptr here and compute bytes? Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx