On 9 May 2012 20:07, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On Mon, 7 May 2012 14:32:18 -0700, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote: > +/** > + * Generate a set of aub file annotations for the current batch buffer, and > + * deliver them to DRM. > + * > + * The "used" section of the batch buffer (the portion containing batch > + * commands) is annotated with AUB_TRACE_TYPE_BATCH. The remainder of the > + * batch buffer (which contains data structures pointed to by batch commands) > + * is annotated according to the type of each data structure. > + */ > +void > +brw_annotate_aub(struct intel_context *intel) > +{ > + struct brw_context *brw = brw_context(&intel->ctx); > + > + unsigned annotation_count = 2 * brw->state_batch_count + 1; > + drm_intel_aub_annotation annotations[annotation_count]; > + int a = 0; > + make_annotation(&annotations[a++], AUB_TRACE_TYPE_BATCH, 0, > + 4*intel->batch.used); > + for (int i = brw->state_batch_count; i-- > 0; ) { > + uint32_t type = brw->state_batch_list[i].type; </div>Isn't the for loop start here off by one? </blockquote><div> I don't think so. Note the postdecrement in the loop condition, so the first time through the loop i == brw->state_batch_count - 1. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Other than that, Reviewed-by: Eric Anholt feel free to just squash the <a href="http://configure.ac" target="_blank">configure.ac</a> bump into this. </blockquote></div> Ok, will do. I'll hold off on pushing this patch until after drm is released so that nobody's build gets broken.