On 15/02/2017 21:18, Chris Wilson wrote:
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.
Will change it, pattern must have escaped me.
+ 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.
I'll check it out.
}
-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?
Together with this.
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Thanks. So you think it is worth it? I am just annoyed with the BUG_ON.
I don't like to add them, but GEM_BUG_ON seemed to weak. On balance it
felt that it is OK. Overall I think it is easier to follow the flow from
the code with this organisation and it is more compact in all respects.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
